SHA1 Hash bzw. Salt

[Zero01]

Hallo Doc, du hast vollkommen recht. Ich sollte diese ganzen maroden Sitten wirklich mal ad acta legen. Meine Wissensdefizite sind in vielen Details wirklich prekär aber ich versuche immer wieder die Dinge aufzuarbeiten, ich möchte nichts beschönigen. Ich bin hauptberuflich kein Entwickler und beschäftige mich immer mal wieder beiläufig mit der Materie um auf der Arbeit Probleme schnell lösen zu können. Ich hoffe, dass dich dies nicht weiter abschreckt mir zu helfen. Ich habe mir eben ein Tutoral über die STD reingezogen und was der Unterschied zwischen C-Style Casts und C++ Casts sind. Letzteres macht natürlich mehr sind da wir nunmal C++ verwenden....Gott wie peinlich. Mir war dieser Unterschied wirklich nicht bewusst. In den C++ Cast Erklärungen war neben dem statisch oder dynamisch machen auch zu lesen, dass rein nur reinterpret casts in Frage kommen wenn Pointer im Spiel sind. Daher habe ich folgenden Stand auf die schnelle Entworfen. Deine Umrechnungsfunktionen für die Hexadezimalwerte habe ich in ganz groben Auszügen verstanden und würde dich gerne nochmals näher bohren, warum dies so funktioniert (Bitverschiebung).

Irgendwo passt wohl ein Cast nicht oder die Längenübergabe des std::string password an den api call ist missglückt. Wenn ich password als c_string über cout Ausgebe, steht nur Müll geschrieben. Aber wo ist der Schnitzer?

int SHA1_Class::GetSHA1asString(std::string &password, std::string &salt)
{

              char Buff[256];
			  unsigned long BuffSize;

			  HCRYPTPROV handle_cryptprov = 0;
			  HCRYPTHASH handle_hash = 0;



			  //----Handle zum Cryptoprovider
			  if(!CryptAcquireContext(&handle_cryptprov, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
			  {
				 //CryptAcquireContext failed
				 //call GetLastError für Details
				 return 1;
			  }



			  if(!CryptCreateHash(handle_cryptprov, CALG_SHA1, 0, 0, &handle_hash))
			  {
				 //CryptCreateHash failed
				 //call GetLastError für Details
				 CryptReleaseContext(handle_cryptprov, 0);
				 return 2;
			  }






			  if(CryptHashData(handle_hash, reinterpret_cast <BYTE*>(&password), password.length(), 0))
			  {

				   ZeroMemory(&Buff, sizeof(Buff));
				   BuffSize = sizeof(Buff);

				   CryptGetHashParam(handle_hash, HP_HASHVAL, reinterpret_cast <BYTE*>(&Buff), &BuffSize, 0);

				   password = buffer_to_hex_string(reinterpret_cast <unsigned char*> (&Buff), BuffSize);

			  }
			  else
			  {
			     //CryptHashData failed
				 //call GetLastError für Details
				 return 3;

			  }




              //---Ressourcen freigeben
			  CryptDestroyHash(handle_hash);
			  CryptReleaseContext(handle_cryptprov, 0);





			  return 0;




}
//------------------------------------------------------------------------------
char SHA1_Class::nibble_to_hex( char value )
{
	 if( value > 9 )
	 return 'a' + value - 10;
	 else
	 return '0' + value;
}
//------------------------------------------------------------------------------
std::string SHA1_Class::buffer_to_hex_string( const char* buffer, unsigned long size )
{
	std::string returnvalue;

	returnvalue.resize( size * 2 );

	for( unsigned long i = 0; i < size;  i++ )
	{
	  returnvalue[2 * i    ] = nibble_to_hex( buffer[i] >> 4 );
	  returnvalue[2 * i + 1] = nibble_to_hex( buffer[i] & 0x0f );
    }

	return returnvalue;
}
//------------------------------------------------------------------------------

[DocShoe]

Ob man C++ jetzt hauptberuflich oder hobbymäßig einsetzt ist total egal, ausschlaggebend ist die Bereitschaft Lernen zu wollen. Und leider hat C++ eine steile Lernkurve.

Dass reinterpret_cast immer bei Zeigern eingesetzt werden muss ist schlichtweg falsch. reinterpret_cast wird eigentlich nur dann eingesetzt, wenn man es besser weiß als der Compiler, was nur selten der Fall sein sollte, zB bei Konvertierungen aus void*.

Zu deinem Quelltext:
Du brauchst keine SHA1 Class, das kann man alles in einer freien Funktion implementieren. Und du solltest GetLastError auch aufrufen und auswerten, nicht nur in den Kommentar schreiben Auf den ersten Blick springt mir nur reinterpret_cast<BYTE*>( &password ) in´s Auge, das ist nach wie vor falsch. Du möchtest in Inhalt des strings hashen, nicht das Stringobjekt selbst.
Du gibst nicht in allen Programmpfaden die angeforderten Handles frei, wenn CryptHashData fehlschlägt gibst du einfach den Wert drei zurück. Überhaupt solltest du dir überlegen, wie deine Funktion aussehen sollte und was sie zuückgeben sollte. Wie übergibst du denn den Hash an den Aufrufer, wenn deine Funktion einen intals Rückgabetyp hat?

[Zero01]

Hallo Doc,
die Feinheiten mit GetLastError hätte ich direkt im Anschluss erledigt. Ich habe mir angewöhnt ersteinmal die Grundfunktionalität herzustellen und dann alles weitere auszuschmücken. Deinen Einwand der Objektfreigabe sehe ich und werde es umgehend beheben. Ich hatte bei den Casts auch Bauchweh beim schreiben zugegebenermaßen, aber das ist der Stand wie ich es erstmal interpretieren konnte. Also das was ich da fabriziere bezieht sich auf das Objekt selbst und nicht auf die Daten hinter dem Pointer? Ich hoffe das du mir mit einem Hinweis helfen kannst.

Password und Salt wird ja via Referenz an diese Funktion übergeben und somit ergibt sich die Referenz an die Variablen des Aufrufers ja automatisch.

Ich danke dir sehr, dass du bereit bis mich ans Händchen zu nehmen.

[Th69]

@zero01 sagte in SHA1 Hash bzw. Salt:

Password und Salt wird ja via Referenz an diese Funktion übergeben und somit ergibt sich die Referenz an die Variablen des Aufrufers ja automatisch.

Was soll denn nach dem Aufruf in diesen Variablen stehen (außer den übergebenen Werten)?
M.E. sollten diese beiden Parameter const sein.

Um an den Inhalt eines std::string als nullterminierte Zeichenkette zu kommen, gibt es die .c_str()-Methode.

[Zero01]

Sorry, habe mich falsch ausgedrückt. Vor dem Aufruf der Methode sind 2 Variablen deklariert und übergebe deren Referenz an diese SHA1 Funktion. Über diese Zuweisung gebe ich den Hash via Referenz zurück in die Main:

password = buffer_to_hex_string(reinterpret_cast <unsigned char*> (&Buff), BuffSize);

[DocShoe]

@zero01
Schlechte Idee. Du übergibst ein Passwort und bekommst an dessen Stelle den dazu passenden SHA1 Hash zurück? Guter Code sollte sich selbst dokumentieren, und wenn du dir in 5 Jahren anguckst, was da passiert, wirste das nicht mehr wissen. Vergleich:

string raw_data = "hello world";

// Option 1: Man weiß sofort was passiert
string hash_1 = calculate_sha1_hash( raw_data );

// Option2: WTF?
calculate_sha1_hash( raw_data );

[Zero01]

Genau in dieser Art habe ich es jetzt Ich probiere immer per Pointer oder Referenz zu übergeben (um das ständige kopieren zu vermeiden) wo es nur geht und im ersten Beispiel stand ich mir selbst im Weg. Ich sehe es ein, ist rumgewurschtel.

Für die Errorcodes von GetLastError übergebe ich den Error DWORD an eine void Funktion und haue via FormatMessage den Fehlertext über cout raus -> so ist der Plan.

Lass uns bitte noch einmal auf die Casts der Api Calls zurück kommen, wie macht man das in diesem Fall? Du sagst, ich übergebe das Objekt selbst aber nicht den Inhalt des Objekts. Ich verstehe es nicht so ganz....ich müsste doch die Referenzierung im Cast entfernen um die Daten zu erhalten.

Auszug aus der aufrufenden Funktion:

SHA1_Class sha;
	std::string password;
	std::string salt;
	std::string result;


	cout<<"Please enter password to hash:";
	cin>>password;


	result = sha.GetSHA1asString(password, salt);
	cout<<result.c_str()<<endl;

Der jetzige Stand der Hash-Funktion:

std::string SHA1_Class::GetSHA1asString(std::string password, std::string salt)
{

			  char Buff[256];
			  unsigned long BuffSize;

			  HCRYPTPROV handle_cryptprov = 0;
			  HCRYPTHASH handle_hash = 0;



			   if(CryptAcquireContext(&handle_cryptprov, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
			  {


						if(CryptCreateHash(handle_cryptprov, CALG_SHA1, 0, 0, &handle_hash))
					   {


								  if(CryptHashData(handle_hash, reinterpret_cast <BYTE*>(&password), password.length(), 0))
								  {

										   ZeroMemory(&Buff, sizeof(Buff));
										   BuffSize = sizeof(Buff);


										   if(CryptGetHashParam(handle_hash, HP_HASHVAL, reinterpret_cast <BYTE*>(&Buff), &BuffSize, 0))
										   {

												 //---Ressourcen freigeben
												 CryptDestroyHash(handle_hash);
												 CryptReleaseContext(handle_cryptprov, 0);


												 return buffer_to_hex_string(reinterpret_cast <unsigned char*> (&Buff), BuffSize);

										   }
										   else
										   PrintErrorToScreen(GetLastError());





								  }
								  else
								  PrintErrorToScreen(GetLastError());




					   }
					   else
					   PrintErrorToScreen(GetLastError());



			  }
			  else
			  PrintErrorToScreen(GetLastError());




			  
			  //---Ressourcen freigeben
			  if(handle_hash != 0)
			  CryptDestroyHash(handle_hash);

			  if(handle_cryptprov != 0)
			  CryptReleaseContext(handle_cryptprov, 0);


}

[DocShoe]

@zero01 sagte in SHA1 Hash bzw. Salt:

Formatier´ deinen Code bitte vernünftig, 8 Zeichen Einrückung sind fürchterlich. Und mehrere aufeinanderfolgende Leerzeilen auch.
Du übergibst deine Parameter jetzt per value statt per reference. Das war sicher auch nicht der Plan...
Und was deine Frage nach den Aufrufparametern für die WINAPI angeht: Was gibt &password denn zurück? Die Adresse des Objekts, das den Text speichert, oder einen Zeiger auf den Text selbst?

[Zero01]

Hallo Doc,
sorry für die Formatierung. Die Parameter per Value habe ich umgestellt, da die Rückgabe nun per return realisiert ist und ich sie im nachgang, wie bereits angemerkt, noch const machen kann.

&password Gibt die Adresse des Objekts selbst zurück, da explizit der Referenzierungsoperator davor steht. Der Pointer auf den Text selbst (value) wäre hier schlicht password bzw. password.c_str() Siehe da, die Ausgabe geht nach Umstellung der folgenden Funktion schon mal trotz zusätzlichem Zeichenwirwar in Richtung hex:

CryptHashData(handle_hash, password.c_str(), password.length, 0);

Zugegebenermaßen steige ich bei den Casts von Buff vollständig aus. Sind hier die Casts überhaupt vonnöten? ich vermute nein aber bin mir absolut nicht sicher. Evtl. liegt der Fehler auch noch an der Längenermittlung des std::string password; password.length();

Ich bin jetzt bis kommenden Donnerstag im Aussendienst und werde erst nach meiner Rückkehr weiter machen können.

[firefly]

Schalt mal die compiler warnungen an.

CryptHashData(handle_hash, password.c_str(), password.length, 0);

Da ist ein fehler im 3. Parameter

[Zero01]

Nach dem jetzigen, dank eurer Hilfe, erlangten Verständnis sieht mein Lösungsversuch jetzt wie folgt aus. Allerdings habe ich noch Zeichenwirrwar im hex string. Ist es richtig, dass die Übergabe via &Buff[0] erfolgen muss, da deine Funktion einen char Pointer erwartet und kein Array of char? Ich gebe deiner Funktion ja so die erste Stelle des Arrays via Reference und mit BuffSize die Länge.

Kompilieren tut er es klaglos aber es kommt immer noch Müll herraus.

std::string SHA1_Class::GetSHA1asString(std::string password, std::string salt)
{

	  char Buff[256];
	  unsigned long BuffSize;

	  HCRYPTPROV handle_cryptprov = 0;
	  HCRYPTHASH handle_hash = 0;



	   if(CryptAcquireContext(&handle_cryptprov, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
	  {


				if(CryptCreateHash(handle_cryptprov, CALG_SHA1, 0, 0, &handle_hash))
			   {


						  if(CryptHashData(handle_hash, password.c_str(), password.length(), 0))
						  {

								   ZeroMemory(&Buff, sizeof(Buff));
								   BuffSize = sizeof(Buff);


								   if(CryptGetHashParam(handle_hash, HP_HASHVAL, Buff, &BuffSize, 0))
								   {

										 //---Ressourcen freigeben
										 CryptDestroyHash(handle_hash);
										 CryptReleaseContext(handle_cryptprov, 0);


										 return buffer_to_hex_string(&Buff[0], BuffSize);

								   }
								   else
								   PrintErrorToScreen(GetLastError());
						  }
						  else
						  PrintErrorToScreen(GetLastError());
			   }
			   else
			   PrintErrorToScreen(GetLastError());
	  }
	  else
	  PrintErrorToScreen(GetLastError());





	  //---Ressourcen freigeben
	  if(handle_hash != 0)
	  CryptDestroyHash(handle_hash);

	  if(handle_cryptprov != 0)
	  CryptReleaseContext(handle_cryptprov, 0);


}

[Zero01]

@firefly meinst du weil er unsigned long erwartet und ich ihm unsigned int liefere? Oder müsste ich gar +1 rechnen da der c_str() nullterminiert ist?

Ich muss jetzt leider los, bis nächste Woche....

[firefly]

Nein meine ich nicht.

Was ist der unterschied zwischen

password.length

und

password.length()

[DocShoe]

@zero01 sagte in SHA1 Hash bzw. Salt:

sorry für die Formatierung.

Und warum änderst du sie dann nicht? Du rückst immer noch mit 8 Zeichen ein und bist äußerst großzügig mit Leerzeilen. Es macht das Lesen anstrengend, wenn man schon für 3 Funktionsaufrufe scrollen muss. Bitte formatier´ den Quelltext so, dass man ihn hier im Forum vernünftig lesen kann.

@zero01 sagte in SHA1 Hash bzw. Salt:

Die Parameter per Value habe ich umgestellt, da die Rückgabe nun per return realisiert ist und ich sie im nachgang, wie bereits angemerkt, noch const machen kann.

Wo? Ich seh da keine Übergabe per const-reference.

[Zero01]

Auf dem Smartphone ist´s jetzt noch schlimmer, ich hatte eingerückt aber beim Kopieren ist mir die Formatierung wieder flöten gegangen, ich achte das nächste mal genau drauf.

Wie gesagt, kann ich es gerne als const reference übergeben. Oder besser sogar als unsigned const ref? Ist diese Vorgehensweiße sicherheitsrelevant? Ich stehe gerade etwas auf dem Schlauch was daran schlimm ist ein Value für den ersten erfolgreichen Test einfach so zu übergeben.

@firefly Kurz vor Verlassen des Büros habe ich alle Compiler warnings aktiviert und er meckerte eine Vermischung der char typen an. Der Aufruf password.length ohne Klammern macht er nicht, daher verstehe ich nicht ganz deine Frage. Der "." ist doch eine referenzierter Aufruf eines members und die runden Klammern besagen, dass es sich um einen Funktionsaufruf handelt (ggf. mit returnvalue) und ohne Klammern eine Member Variable gemeint sein muss.

[DocShoe]

Nö, Password.length() ist ein Methodenaufruf, es gibt keine std::string::length Klasse. Du rufst die Methode length() auf dem Objekt Password auf, das hat nix mit Konstruktoren zu tun.
Und was die Übergabe von Parametern angeht hatten wir vor kurzem einen Thread hier im Forum. Als Daumenregeln hilft dir das vllt weiter, die Punkte sind nach Priorität aufgelistet:

1. übergib ihn als Referenz, wenn er in der Funktion beschrieben wird und vom Aufrufer benutzt wird (call by reference)

bool decodate_date( std::time_t timestamp, unsigned short& year, unsigned short& month, unsigned short& day )
{
   year =...
   month = ...
   day = ...

2. übergib Datentypen, die in ein Register passen (aktuell fast immer 64bit) , als Kopie (call by value). Dazu gehören insbesondere all built-in Typen (bool, char, int, float, double, etc).

double mul_div( double factor1, double factor2, double divisor )
{
   return (factor1 * factor 2) / divisor;
}

3. übergib Datentypen, die nicht in ein Register passen und nur lesend benutzt werden, per const-Referenez (call by const reference)

std::string calculate_sha1_hash( const std::string& input_data )
{
   ...
}

[Zero01]

Super Doc! habe vielen herzlichen Dank. Ich melde mich bald

Du hast wohl auf meinen ersten Gedanken geantwortet, den ich kurz drauf wegeditiert hatte

Gibt mir der Aufruf der Memberfunktion Length() nicht die korrekte Länge zurück?

[DocShoe]

@zero01
Genau das tut er.

[Zero01]

Hallo Doc,
folgenden Stand habe ich jetzt mit allen Tips hinbekommen. Ich vermute, dass der noch ausgegebene Datenschrott mit meinem Verständnis des gezeigten char Buff[] zusammen hängt. Mir fällt es gerade wahnsinnig schwer, ob auch die Größenermittlung mit sizeof logisch so richtig ist.

Worin liegt eigentlich der Unterschied zwischen char Buff und char Buff[256]? Kann die Variable Buff ohne Feldlängenangabe nur ein Zeichen aufnehmen?

std::string SHA1_Class::GetSHA1asString(const std::string &password)
{

  unsigned char Buff[256];
  unsigned long BuffSize;

  HCRYPTPROV handle_cryptprov = 0;
  HCRYPTHASH handle_hash = 0;


  ZeroMemory(&Buff, sizeof(Buff));
  BuffSize = sizeof(Buff);


   if(CryptAcquireContext(&handle_cryptprov, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT))
  {
	if(CryptCreateHash(handle_cryptprov, CALG_SHA1, 0, 0, &handle_hash))
	{
	  if(CryptHashData(handle_hash, password.c_str(), password.length(), 0))
	  {
		if(CryptGetHashParam(handle_hash, HP_HASHVAL, Buff, &BuffSize, 0))
		{

		   //---Ressourcen freigeben
		   CryptDestroyHash(handle_hash);
		   CryptReleaseContext(handle_cryptprov, 0);


		   return buffer_to_hex_string(Buff, BuffSize);

		}
		else
		PrintErrorToScreen(GetLastError());
	  }
	  else
	  PrintErrorToScreen(GetLastError());
	}
	else
	PrintErrorToScreen(GetLastError());
  }
  else
  PrintErrorToScreen(GetLastError());





  //---Ressourcen freigeben
  if(handle_hash != 0)
  CryptDestroyHash(handle_hash);

  if(handle_cryptprov != 0)
  CryptReleaseContext(handle_cryptprov, 0);


  return "0";


}
//------------------------------------------------------------------------------
char SHA1_Class::nibble_to_hex( char value )
{
	 if( value > 9 )
	 return 'a' + value - 10;
	 else
	 return '0' + value;
}
//------------------------------------------------------------------------------
std::string SHA1_Class::buffer_to_hex_string( const char* buffer, unsigned long size )
{
	std::string returnvalue;

	returnvalue.resize( size * 2 );

	for( unsigned long i = 0; i < size;  i++ )
	{
	  returnvalue[2 * i    ] = nibble_to_hex( buffer[i] >> 4 );
	  returnvalue[2 * i + 1] = nibble_to_hex( buffer[i] & 0x0f );
    }

	return returnvalue;
}

[DocShoe]

Ist das Thema noch aktuell?