Reihenfolge beim Überprüfen neuer Passwörter

[vapita]

Hallo liebe Gemeinde,

angenommen, ich habe soeben per POST zwei Passwörter erhalten, die ich nun vergleichen und dann speichern möchte. Welche Reihenfolge ist am sinnvollsten? Oder ist es eigentlich egal?

a)

Ich prüfe, ob sie sich gleichen. Wenn ja -> hashen und speichern.

b)

Ich hashe beide Passwörter und prüfe, ob sie sich gleichen. Wenn ja -> speichern.

c)

Ich prüfe erstmal, ob es sich jeweils um einen String handelt. Wenn ja, vergleiche ich sie dann und hashe und speichere ggf.

d)

Ich prüfe, ob es Strings sind. Wenn ja, hashe und vergleiche ich sie und speichere dann ggf.

Oder gibt es gar ein besseres Vorgehen?

Beste Grüße und bleibt gesund

vapita

[Rolf B]

Hallo vapita,

du bekommst als POST oder GET Parameter immer nur Strings. Das musst Du also nicht wirklich prüfen. Prüfen könntest Du, ob die Parameter gefüllt oder leer waren (mit empty($_POST['...']).

Danach vergleichst Du. Das spart Zeit und Strom, denn wenn Du vor dem Vergleich hashen willst, musst Du zweimal hashen. Wenn Du zuerst vergleichst, nur einmal.

Vom Ergebnis her ist es aber egal, denn die Arbeitshypothese beim Hashen ist ja, dass der Raum der möglichen Hashes so gigantisch ist, dass es keine unterschiedlichen User-Passworte gibt, die zum gleichen Hash führen. Und wenn doch, dann ist die Chance dafür astronomisch - äh - atomisch klein.

Edit: Dieser Absatz würde nur gelten, wenn man nicht salzt. Aber password_hash streut Zufallssalz ein und liefert damit für gleiche Passwörter unterschiedliches Hashes. Thanks@tk.

Rolf

[TS]

Hello,

du bekommst als POST oder GET Parameter immer nur Strings. Das musst Du also nicht wirklich prüfen. Prüfen könntest Du, ob die Parameter gefüllt oder leer waren (mit empty($_POST['...']).

Nee, das ist zwar vermutlich hier trotzdem brauchbar, weil man keine Passworte akzeptieren will, die mur ein Digit haben, aber es ist formal falsch!

Mit empty() würden auch die Werte "0" und " " als leer identifiziert. Das will man sicherlich nicht.

Die Prüfung müsste z. B. auf

   if(isset($_POST['password1']) && (strlen($_POST['password1']) > 7))
   {
       ### ...
   }

lauten.

Reihenfolge:

1. Vorhandensein der POST-Parameter mit isset() prüfen
2. Übereinstimmung prüfen (===, identisch)
3. Mindestlänge prüfen
4. verlangten Zeichenvorrat prüfen
   4a. auf erlaubte Zeichen prüfen
   4b. auf geforderte Anzahl von Versalien, Gemeinen, Ziffern, Sonderzeichen prüfen
5. hashen (bitte nicht mehr mit md5(), sondern mit password_hash() )
6. eintragen
7. ggf. Bestätigungsmail (selbstverständlich ohne das Passwort!) senden

Und der ganze Vorgang muss über TLS laufen.

Glück Auf
Tom vom Berg

[vapita]

Hallo Tom,

vielen Dank für den Input. Ich hatte es dann wohl nicht ganz richtig:

$password = $userData['password'];
$passwordCheck = $userData['password_check'];
if($password == $passwordCheck){
    $passwordHashed = $this->passwordEncoder->hash($password);
    $newUser->setPassword($passwordHashed);
} else {
    $this->flash->add('Die Passwörter stimmen nicht überein!','danger');
    break;
}

Habe es nun nach deiner Empfehlung so geändert:

$password = $userData['password'];
$passwordCheck = $userData['password_check'];
if(!isset($password) or !isset($passwordCheck)){
    $this->flash->add('Es muss ein Passwort angegeben werden!','danger');
    break;
}
if($password === $passwordCheck){
    $passwordHashed = $this->passwordEncoder->hash($password);
    $newUser->setPassword($passwordHashed);
} else {
    $this->flash->add('Die Passwörter stimmen nicht überein!','danger');
    break;
}

Punkt 3 und 4 müsste ich noch erledigen.

Beste Grüße und vielen Dank

vapita

[TS]

Hello vapita,

vielen Dank für den Input. Ich hatte es dann wohl nicht ganz richtig:

$password = $userData['password'];
$passwordCheck = $userData['password_check'];

Warum arbeitest Du nicht direkt mit den $_POST-Parametern?
Das mehrmalige Umkopieren von Parametern ist kontraproduktiv.

   <p><input type="password" name="password[a]"></p>
   <p><input type="password" name="password[b]"></p>

   if(isset($_POST['password']['a'], $_POST['password']['b']))
   {
       # ...
   }
   else # Fehlermeldung

Glück Auf
Tom vom Berg

[vapita]

Warum arbeitest Du nicht direkt mit den $_POST-Parametern?
Das mehrmalige Umkopieren von Parametern ist kontraproduktiv.

   <p><input type="password" name="password[a]"></p>
   <p><input type="password" name="password[b]"></p>

Ich hole mir die Daten über eine Request-Klasse, die mir die Eingaben gleich filtert:

public function getQuery(string $FormFieldName)
{
    try {
        if ($FormFieldName !== NULL){
            $query = filter_input(INPUT_POST, $FormFieldName, FILTER_SANITIZE_STRIPPED);
            return $this->query = $query;
        } else {
            throw new Exception($query_exception);
        }
    } catch (Exception $query_exception){
        Logger::newMessage($query_exception);
        Logger::customErrorMsg($query_exception);
    }

}

Im Controller speichere ich dann die Daten in einer Variablen, bzw. im Array $userData, da ich dieses später noch in der View benötige, um die Benutzereingaben nach einer Fehlermeldung als Formwerte zu erhalten.

$userData['username'] = $this->request->getQuery('username');
$userData['email'] = $this->request->getQuery('email');
$userData['password'] = $this->request->getQuery('password');
$userData['firstname'] = $this->request->getQuery('firstname');
$userData['lastname'] = $this->request->getQuery('lastname');

Ich habe den Code nochmals angepasst und folgendes ist dabei herausgekommen:

if(isset($userData['password']['a'],$userData['password']['b'])) {
    if($userData['password']['a'] === $userData['password']['b']){
        $passwordHashed = $this->passwordEncoder->hash($userData['password']['a']);
        $newUser->setPassword($passwordHashed);
    } else {
        $this->flash->add('Die Passwörter stimmen nicht überein!','danger');
        break;
    }
} else {
    $this->flash->add('Es muss ein Passwort angegeben werden!','danger');
    break;
}

Beste Grüße

vapita

[Rolf B]

Hallo vapita,

einerseits kannst Du direkt mit $_GET oder $_POST arbeiten, wie Tom vorschlug, aber wenn es für Dich Gründe gibt, dieses $userData Array zu verwenden (z.B. weil es ein Parameter für deine Methode ist), dann solltest Du die isset-Abfrage an der Stelle machen, wo Du die $_POST-Daten nach $userData überträgst.

Denn andernfalls kommt die blöde Notice, dass Du auf einen undefinierten Key zugreifst, beim Übertragen nach $userData.

Aber da hab ich grad was gelernt: Ab PHP 7 gibt es den "null coalescing operator" ??. Der beinhaltet eine isset Abfrage!

$_POST['password'] ?? "" liefert den geposteten Wert des Passworts, und wenn die POST-Daten keinen Eintrag für password enthalten, sorgt ?? zum einen dafür, dass die Notice nicht kommt, und zum anderen setzt er "" als Ersatzwert.

Also, wenn Du PHP 7 aufwärts verwendest, kannst Du es so machen:

$userData['password'] = $_POST['password'] ?? "";
$userData['password_check'] = $_POST['password_check'] ?? "";

und hast für fehlende Passworte einen Leerstring in $userData stehen.

Beim Hashen würde ich dann als erstes die Gleichheit testen, und zwar ohne mir die Mühe zu machen, das check-Passwort in eine Variable zu laden. Danach muss die erforderliche Passwordkomplexität geprüft werden (lang genug, Zeichenmix), und DANN kann man hashen. Hier wäre meine Frage, was dein passwordEncoder Objekt tut. Ist das was selbstgemachtes? Oder nur eine Hülle um die eingebaute password_hash Funktion von PHP? Wenn es nicht password_hash ist: Wegschmeißen. In PHP selbstgebautes Hashing ist entweder zu trivial oder zu langsam, und die älteren Passwortfunktionen sind unsicher. Wenn Du crypt() verwendest, ok, das tut password_hash auch, aber letztere sorgt für starkes Salz und bietet mit password_verify und password_needs_rehash sinnvolle Tools dazu. Falls Du md5() verwendest: Nein. Zu unsicher.

$password = $userData['password'];
if($password !== $userData['passwordCheck']) {
    $this->flash->add('Die Passwörter stimmen nicht überein!','danger');
    break;
}
if (!$this->verifyPasswordComplexity($password)) {
    $this->flash->add('Das Password ist nicht stark genug!','danger');
    break;
}
$passwordHashed = password_hash($password);   // reicht!
$newUser->setPassword($passwordHashed);

(Wieso eigentlich break? Läuft das in einer Schleife?! Benutzt Du eine Schleifenkonstruktion, um einen GOTO zu simulieren?)

Rolf

[vapita]

Hallo Rolf,

Denn andernfalls kommt die blöde Notice, dass Du auf einen undefinierten Key zugreifst, beim Übertragen nach $userData.

Das ist ein guter Hinweis. Ist mir tatsächlich schon passiert. Dann werde ich das nochmals anpassen.

Hier wäre meine Frage, was dein passwordEncoder Objekt tut. Ist das was selbstgemachtes? Oder nur eine Hülle um die eingebaute password_hash Funktion von PHP?

Genau, es ist nur eine Hülle. Als Algo benutze ich PASSWORD_DEFAULT.

class Password
{

    /**
     * @param $plain_password
     * @return false|string|null
     */
    public function hash($plain_password)
    {
       return password_hash($plain_password, PASSWORD_DEFAULT);
    }

    /**
     * @param $plain_password
     * @param $correct_hash
     * @return bool
     */
    public function validate($plain_password, $correct_hash)
    {
       return password_verify($plain_password, $correct_hash);
    }

}

$password = $userData['password'];
if($password !== $userData['passwordCheck']) {
    $this->flash->add('Die Passwörter stimmen nicht überein!','danger');
    break;
}
if (!$this->verifyPasswordComplexity($password)) {
    $this->flash->add('Das Password ist nicht stark genug!','danger');
    break;
}
$passwordHashed = password_hash($password);   // reicht!
$newUser->setPassword($passwordHashed);

Danke, danke, so werde ich es umsetzen.

(Wieso eigentlich break? Läuft das in einer Schleife?! Benutzt Du eine Schleifenkonstruktion, um einen GOTO zu simulieren?)

Ja, ich wollte tiefe IF-Schleifen-Schachteln vermeiden und nutze dafür:

do{
    if($error){
        break;
    }
} while (false);

Beste Grüße

vapita

[Rolf B]

Hallo vapita,

IF-Schleifen

Brrrrrr

Wenn Methoden zu komplex werden, teilt man sie auf. Ab PHP 7 ist ein Funktions-/Methodenaufruf nicht mehr so teuer. Die Idee ist, dass eine Methode sich um genau eine Aufgabe kümmert. Wird sie länger als 50 Zeilen, läuft zumeist was falsch.

Wenn Du eine Methode hast, die drölf Prüfungen macht und nach jeder Prüfung mit BREAK aussteigen kann, dann solltest Du jede dieser Prüfungen in eine eigene, private Methode auslagern, und sie einfach nacheinander aufrufen.

public function do_a_lot() {
   $success = $this->do_thing_1()
           && $this->do_thing_2()
           ...
           && $this->do_thing_13()
   if (!$success) {
      // Log Error - falls noch nötig
   }  
}

Die Einzelmethoden müssen nur mit return false; aussteigen, um den Ablauf abzubrechen. PHP verwendet wie einige andere Sprachen auch die sogenannte Kurzschluss-Logik: Wenn bei A && B oder A || B nach der Auswertung von A schon feststeht, wie das Ergebnis ist, wird B nicht mehr angefasst. Bei && ist es klar, wenn A zu FALSE wird, und bei || ist es klar, wenn A zu TRUE wird.

In $this->doThis() && $this->doThat() wird doThat nur aufgerufen, wenn doThis TRUE zurückgibt (oder etwas, das der Type Juggler zu TRUE macht).

Es kommt auch vor, dass man in den einzelnen Schritten Teilergebnisse bestimmt, die in den folgenden Schritten gebraucht werden. Sie dann als Parameter zur übergeben, ist unhandlich. Sowas deutet aber darauf hin, dass diese Methode lieber ein eigenes Objekt sein möchte, ein Worker. Statt die Monstermethode aufzurufen, instanziierst Du den Worker, rufst seine offizielle Methode auf (z.B. "Run" oder "Validate" oder was grad passt) und der macht dann intern rum. Die privaten Methoden, die drinstecken, verwenden private Properties, um sich Daten zuzuspielen. Das ist absolut okay.

Der Vorteil ist auch, dass man einen solchen Worker wiederverwenden und isoliert testen kann.

Rolf

[TS]

Hello vapita,

Ja, ich wollte tiefe IF-Schleifen-Schachteln vermeiden und nutze dafür:

Ach, die berühmten IF-SCHLEIFEN ;-)

Oder gibt es sie doch?

do{
    if($error){
        break;
    }
} while (false);

Das Ganze kannst Du auch als leicht lesbare Funktion aufbauen.

function (insert_new_pass($pass1, $pass2))
{

   if ( !test1() ) return 'Fehler: Eine Passwortangabe fehlt';
   if ( !test2() ) return 'Fehler: Passworte stimmen nicht überein';  
   if ( !test3() ) return 'Fehler: Passwortlänge zu kurz';  
   if ( !test4() ) return 'Fehler: Unerlaubte Zeichen im Passwort';  
   if ( !test5() ) return 'Fehler: Es muss mindestens ein Groß, ein Klein, eine Ziffer, ein Sonderzeichen im Passwort enthalten sein'; 

   # ...

   if ( !db_insert_password() ) return 'Fehler: Eintragung in die DB nicht möglich';  

   return true;
}

Prüfen musst Du dann nach dem Aufruf der Funktion auf

if (($message = insert_new_pass()) === true ## weitermachen, sonst $message ausgeben.

Und anstelle der statischen Fehlermeldungen und TRUE könntest Du auch Fehlernummern und 0 zurückgeben. Dann kannst Du die Fehlermeldungen anhand der Nummer sprachabhängig ausgeben lassen.

Glück Auf
Tom vom Berg

[vapita]

Hallo Tom,

Ach, die berühmten IF-SCHLEIFEN ;-)

Oder gibt es sie doch?

Da hatte ich wohl einen Knoten im Gehirn. Es ist natürlich eine IF-Abfrage, keine -Schleife. Jedenfalls nicht in meinem Fall. Danke für den Hinweis auf die nette Lektüre. 😉

Das Ganze kannst Du auch als leicht lesbare Funktion aufbauen.

function (insert_new_pass($pass1, $pass2))
{

   if ( !test1() ) return 'Fehler: Eine Passwortangabe fehlt';
   if ( !test2() ) return 'Fehler: Passworte stimmen nicht überein';  
   if ( !test3() ) return 'Fehler: Passwortlänge zu kurz';  
   if ( !test4() ) return 'Fehler: Unerlaubte Zeichen im Passwort';  
   if ( !test5() ) return 'Fehler: Es muss mindestens ein Groß, ein Klein, eine Ziffer, ein Sonderzeichen im Passwort enthalten sein'; 

   # ...

   if ( !db_insert_password() ) return 'Fehler: Eintragung in die DB nicht möglich';  

   return true;
}

Prüfen musst Du dann nach dem Aufruf der Funktion auf

if (($message = insert_new_pass()) === true ## weitermachen, sonst $message ausgeben.

Und anstelle der statischen Fehlermeldungen und TRUE könntest Du auch Fehlernummern und 0 zurückgeben. Dann kannst Du die Fehlermeldungen anhand der Nummer sprachabhängig ausgeben lassen.

Ich habe die Anregungen umgesetzt und die Password-Klasse etwas umgearbeitet:

    /**
     * @param $plain_password
     * @return false|string|null
     */
    public function hash($plain_password)
    {
        $plain_password = (is_array($plain_password)) ? array_pop($plain_password) : $plain_password;
        return password_hash($plain_password, PASSWORD_DEFAULT);
    }

    /**
     * @param string $plain_password
     * @param string $correct_hash
     * @return bool
     */
    public function verify(string $plain_password, string $correct_hash)
    {
       return password_verify($plain_password, $correct_hash);
    }

    /**
     * @param $password
     * @return bool
     */
    private function isString($password){
        if(is_array($password)){
            foreach ($password as $item){
                $passwordIsString = (is_string($item)) ?? false;
            }
            return ($passwordIsString) ?? false;
        } else {
            return (is_string($password)) ?? false;
        }
    }

Ich prüfe, ob das Passwort (oder die Passwörter, wenn es ein Array ist) ein String ist.

    /**
     * @param array $password
     * @return bool
     */
    private function isEqual(array $password){
        return ($password[0] == $password[1]) ?? false;
    }

    /**
     * @param $password
     * @return bool
     */
    private function hasMinLength($password){
        $password = (is_array($password)) ? array_pop($password) : $password;
        return (strlen($password) > 7) ?? false;
    }

Ich prüfe einmal mit strlen() die Zeichenlänge, jedoch brauch ich das gar nicht mehr, da ich bereits mit isComplex() prüfe, ob das Passwort lang genug ist, oder?

    /**
     * @param $password
     * @return bool
     */
    private function isAllowed($password){
        // TODO: auf erlaubte Zeichen prüfen
        return true;
    }

Ich bin mir noch nicht sicher, wie ich prüfe, ob nicht erlaubte Zeichen enthalten sind, bzw. welche das eigentlich sein sollen. Ich würde es wahrscheinlich auf lateinische Buchstaben, arabische Ziffern und die Sonderzeichen @#-_$%^&+=§!? begrenzen.

    /**
     * @param $password
     * @return bool
     */
    private function isComplex($password){
        $password = (is_array($password)) ? array_pop($password) : $password;

        /** Das Passwort muss mindestens
         * - einen Kleinbuchstaben,
         * - einen Großbuchstaben,
         * - eine Ziffer und
         * - ein Sonderzeichen @#-_$%^&+=§!?
         * enthalten.
         * Das Passwort muss zwischen 8 und 20 Zeichen lang sein.
         */
        return (preg_match('/^(?=.*\d)(?=.*[@#\-_$%^&+=§!\?])(?=.*[a-z])(?=.*[A-Z])[0-9A-Za-z@#\-_$%^&+=§!\?]{8,20}$/',$password))? true : false;
    }

}

Hier prüfe ich ja eigentlich auch nochmals die Zeichenkettenlänge. Entferne ich dies hier wieder oder lasse ich einfach die hasMinLentgh() weg?

    /**
     * @param array $password
     * @return int
     */
    public function validate(array $password):int
    {
        if(!$this->isString($password))     return 1601;    // kein String
        if(!$this->isEqual($password))      return 1602;    // stimmt nicht überein
        if(!$this->hasMinLength($password)) return 1603;    // ist nicht lang genug
        if(!$this->isAllowed($password))    return 1604;    // verwendet nicht erlaubte Zeichen
        if(!$this->isComplex($password))    return 1605;    // ist nicht komplex genug
        return 0; // alles in Ordnung
    }

So ist es tatsächlich übersichtlicher. Da hab ich wieder was gelernt. Vielen Dank dafür.

Die Idee mit den Fehlercodes gefällt mir gut. In der View rufe ich die Fehlermeldung dann über ein Array ab.

Beste Grüße

vapita

[Rolf B]

Hallo vapita,

schön, dass Du deinen Code säubern und strukturieren möchtest.

Aber der Meckerfritze ist wieder da 😉

Ich habe dein Array/String Konzept noch nicht verstanden. Mal gehst Du die Array-Einträge durch, mal nimmst Du nur den ersten Eintrag. Ich hoffe, dein um ein Vierteljahr älteres Ich (dein schärfster Kritiker überhaupt) wird Dich dafür nicht hassen.

Ich glaube, du hast jetzt ganz allgemein versucht, Arrays zu behandeln. Hast Du Dir vorab Gedanken gemacht, ob ein Array als Input für diese Klasse überhaupt irgendwie sinnvoll ist und welchen sinnvollen Umgang es damit geben sollte? Arrays in $_POST kommen ja nur vor, wenn Du das im Form so vorsiehst, oder Dir irgendwer das bösartig hineinpostet. Heißt: Wenn Du vorhast, in deinem Login- oder Register-Form Passwort-Arrays einzubauen, dann überlege Dir, wie Du damit umgehen müsstest. Wenn Du das nicht vorhast, weise sie als manipulierte Eingabe zurück.

Methode hash:

• (is_array(...)) muss man nicht extra klammern.
• array_pop kann fehlschlagen, wenn das Array leer ist
• ist es tatsächlich sinnvoll, wenn hash nur den ersten Array-Eintrag hasht, wenn es ein Array bekommt? Könnte es nicht auch sinnvoll sein, jeden Eintrag einzeln zu hashen und ein Array aus Hashes zurückzugeben? Aber das hängt vom Grundkonzept für Arrays ab, siehe oben.

Methode verify:

• An sich okay, aber sie kann nicht mit Arrays umgehen. Konzept?

Methode isString:

• Ist falsch. Gib ihr ARRAY(1,2,"Hallo") als Eingabe und sie sagt TRUE. Gib ihr ARRAY(1,"Hallo",3) als Eingabe und sie sagt FALSE. Weil sie immer das Ergebnis für den letzten Eintrag zurückgibt.
• Ist ungeschickt. Es freut mich ja, dass Du Dich in den null coalesce Operator ?? verliebt hast, aber man braucht ihn nur dort, wo NULL auch wirklich vorkommen kann. Deine Array-Prüfung soll vermutlich eine "ist alles ein String" Prüfung durchführen. Dafür setzt man $passwordIsString vor der Schleife auf TRUE und in der Schleife schaut man, ob ein Eintrag ein Nicht-String ist. Wenn ja, setzt man $passwordIsString auf FALSE und bricht mit break aus der Schleife aus.
• Null Coalescing braucht man in dieser Methode nirgends. Auch im else-Zweig nicht. Die PHP Doku sagt, dass is_string immer einen booleschen Wert liefert, niemals NULL.

Methode isEqual:

• Der == oder === Operator liefern immer ein bool. ?? ist unnötig.
• Kleinigkeit: Hier wäre ein typsicherer Vergleich mit === besser. Es wird wohl auch mit == funktionieren, weil Du vorher auf Strings testest.

Methode hasMinLength:

• Array-Konzept...
• ?? unnötig, > liefert immer einen bool

Methode isComplex:

• Array-Konzept...
• Ein Konstrukt der Art (bedingung) ? true : false ist redundant, es sei denn, die Bedingung liefert auch mal was anderes als einen booleschen Wert. Das ist bei preg_match tatsächlich der Fall, es liefert 1 für "gefunden" oder 0 für "nicht gefunden", und FALSE für "da ging was schief". Schief gehen kann aber nur der Fall, dass deine Regex falsch ist. Ich finde, du solltest FALSE separat prüfen (mit ===, um es von 0 unterscheiden zu können) und dann einen Error loggen.

Sorry für die lange Liste...

Rolf

[Der Martin]

Hallo Rolf,

Ich hoffe, dein um ein Vierteljahr älteres Ich (dein schärfster Kritiker überhaupt)

das lese oder höre ich immer wieder - und kann es doch selbst nicht nachvollziehen.

Wenn ich heute meinen eigenen Code von vor einem halben Jahr, oder auch von vor zehn Jahren ansehe, dann fällt mir zwar manchmal auf: "Das würde ich heute anders lösen."
Aber es ist sehr, sehr selten so, dass ich nicht mehr verstehe, was ich damals getan habe. Je nach Komplexität des Codes brauche ich normalerweise zwischen einer halben Stunde und einem halben Tag, bis ich wieder komplett "im Fluss" bin.

Und ich behaupte: Dazu tragen maßgeblich eine übersichtliche Strukturierung, sinnvoll gewählte sprechende Bezeichner und informative Kommentare bei.

Deswegen kann ich nur immer wieder jedem empfehlen: Investiert ruhig eine Stunde mehr Zeit, um den Code sauber zu form(ul|at)ieren und zu dokumentieren, das zahlt sich später aus.

Live long and pros healthy,
 Martin

[vapita]

Hallo Rolf,

habe deinen Post erst eben gesehen. Ich danke dir für deine großartige Antwort. Ich brauche noch ein wenig Zeit, um es umzusetzen.

Bis später und beste Grüße

vapita

[vapita]

Hallo Rolf,

Aber der Meckerfritze ist wieder da 😉

So soll es aber auch sein. Schließlich wächst man nicht am Lob, sondern an der Kritik. Besonders, wenn sie so konstruktiv ist.

Arrays in $_POST kommen ja nur vor, wenn Du das im Form so vorsiehst, oder Dir irgendwer das bösartig hineinpostet. Heißt: Wenn Du vorhast, in deinem Login- oder Register-Form Passwort-Arrays einzubauen, dann überlege Dir, wie Du damit umgehen müsstest. Wenn Du das nicht vorhast, weise sie als manipulierte Eingabe zurück.

Grundsätzlich kommen vom Form nur Strings an, jedoch habe ich im Controller das Passwort und das Kontrollpasswort in ein Array gesetzt, da ich das so übersichtlicher fand.

Methode hash:

Die Methode nutze ich erst nach der Passwortvalidierung, da kommt nun gar nicht erst ein Array an:

public static function hash(string $plain_password): string
{
   return password_hash($plain_password, PASSWORD_DEFAULT);
}

und das User-Objekt macht dann vor dem Speichern in der Datenbank:

# Entity/User.php
...
return PasswordService::hash($this->password);
...

Methode verify:

Diese Methode kommt dann erst beim Login zum Einsatz:

public static function verify(string $plain_password, string $correct_hash): bool
    {
        return password_verify($plain_password, $correct_hash);
    }

Methode isString:

Kommt das Passwort als Array (Passwort und Kontrollpasswort), soll jedes auf String überprüft werden.

private static function isString($password): bool
    {
        if(is_array($password) && !empty($password)){
            $passwordIsString = true;
            foreach ($password as $item){
                $passwordIsString = is_string($item);
            }
            return $passwordIsString;
        } else {
            return is_string($password);
        }
    }

Methode isEqual:

private static function isEqual(array $password): bool
    {
        return ($password[0] === $password[1]);
    }

Methode hasMinLength:

Hier reicht es, meiner Meinung nach, nur das Passwort auf Länge zu prüfen, da das Kontrollpasswort sowieso identisch sein muss.

private static function hasMinLength($password): bool
    {
        $password = is_array($password) && !empty($password) ?   array_pop($password) : $password;
        return (strlen($password) > 7);
    }

Methode isComplex:

Hier soll auch nur das Passwort, nicht das Kontrollpasswort, sollte es sich um ein Passwort-Array handeln, überprüft werden. Bin mir noch nicht sicher, ob ich das so richtig gemacht habe:

private static function isComplex($password): bool
    {
        try{
            $password = (is_array($password) && !empty($password)) ? array_pop($password) : $password;
            $pregMatch = (preg_match('/^(?=.*\d)(?=.*[@#\-_$%^&+=§!\?])(?=.*[a-z])(?=.*[A-Z])[0-9A-Za-z@#\-_$%^&+=§!\?]{8,20}$/',$password));
            if (false === $pregMatch){
                throw new Exception();
            }
            return $pregMatch;
        } catch (Exception $exception){
            (new Logger)->newMessage($exception);
            (new Logger)->customErrorMsg($exception);
        }
    }

Sorry für die lange Liste...

ich kann mich nur für die Unterstzütung und Mühe bedanken.

Beste Grüße

vapita

[Rolf B]

Hallo vapita,

hash() - ok 😀
verify() - ok 😀

Den Grund, warum deine Methoden Arrays verarbeiten sollen, habe ich jetzt endlich verstanden 🤦‍♂️. Aber warum sie auch Strings können sollen, hm. Password::verify hat einen auf array getypten Parameter, da können die privaten Methoden nichts anderes bekommen. Du kannst in diesen privaten Helfern davon ausgehen, dass ein Array herein kommt und sie ebenfalls auf array typen.

isString() - vom Algorithmus her nach wie vor nicht ok. Wenn Du in der Schleife einfach nur an $passwordIsString zuweist, ist das Ergebnis immer das vom letzten Schleifendurchlauf. Es gibt zwei richtige Techniken: (1) verwende eine UND Verknüpfung, (2) weise nur zu, wenn die Prüfung FALSE ergibt.

Und der ?? Operator in der Schleife ist nach wie vor unnötig, is_string gibt niemals NULL zurück.

Also, entweder (vom Prinzip her) so:

$passwordIsString = true;
foreach ($array as $item) {
   if (!is_string($item))
      $passwordIsString = false;
}

oder so

$passwordIsString = true;
foreach ($array as $item) {
   $passwordIsString = $passwordIsString && is_string($item);
}

Man muss das im zweiten Beispiel leider so umständlich schreiben. PHP kennt zwar kombinierte Zuweisungen bei Arithmetik (+=, -= etc), aber kein &&=.

Wenn nach der Schleife - so wie bei Dir - nichts mehr passiert, kannst Du es auch vereinfachen, wobei die Päpste strukturierter Programmierung darüber die Stirn runzeln dürften. Ich nicht, ich mache sowas gern.

foreach ($array as $item) {
   if (!is_string($item)) return false;
}
return true;

Mit funktionalen Ansätzen wie array_reduce und Pfeilfunktionen (ab PHP 7.4) komme ich Dir jetzt besser nicht 🙊

isEqual() - nicht prinzipiell falsch, aber hier ist nochmal das Konzept gefragt. Wenn isString ein beliebig langes Array verarbeiten kann, sollte das isEqual dann nicht auch tun? Die Methoden einer Klasse sollten eine logische Konsistenz aufweisen. Wenn Du beliebig viele PW verarbeiten können willst:

function isEqual(array $password) {
   if (count($password) < 2) return true;   // 0 oder 1 PW übergeben
   $comp = $password[0];
   foreach ($password as $pw)
      if ($pw !== $comp) return false;
   return true;
}

Diese Methode ist zwar leicht ineffizient, weil sie den ersten Eintrag des Arrays mit sich selbst vergleicht, aber andernfalls könntest Du nicht foreach benutzen, es müsste eine for ($i=1; $i<count($password); $i++) Schleife sein. Das ist auch nicht besser.

isComplex - ich finde die Anforderungen zu strikt. Normal ist es, drei von vier dieser Kategorien zu verlangen. Alle vier ist sehr streng, und vor allem ist der Zwang zu @#-_$%^&+=§!? zu streng. Ich möchte das Passwort "Lämmlein*17" verwenden! Vermutlich kann man meine folgenden Überlegungen heiß diskutieren; es ist nur meine eigene Meinung aus 36 Berufsjahren, die ich als Opfer der IT-Security verbracht habe, nicht als Täter.

Auf Internationalisierungsprobleme will ich gar nicht erst eingehen (was macht ein Russe, der kyrillisch tippt? Was macht ein Chinese mit seinen Ideogrammen?). Um hier besser zu werden, müsste man mittels Unicode-Kategorie prüfen. \p{Lu} sind Großbuchstaben, \p{Ll} Kleinbuchstaben und \p{Nd} Ziffern. Und dann noch ein Zeichen, was in diesen Kategorien nicht ist. Dafür muss man, denke ich, vier mal matchen, und man darf nicht preg_match nehmen, sondern es muss mb_ereg sein (diese Funktion ist Unicode-fähig und kann mit UTF-8 codierten Strings umgehen). Nicht mb_ereg_match, die verankert die Prüfung am Beginn. Wir wollen aber nur die Existenz eines Zeichens testen.

function isComplex($password) {
  $password = (is_array($password)) ? array_pop($password) : $password;

  $spaceCount = mb_strlen($p) - mb_strlen(mb_ereg_replace("\p{Zs}", "", $p));
  $hasUpper   = mb_ereg("\p{Lu}", $p);
  $hasLower   = mb_ereg("\p{Ll}", $p);
  $hasOther   = mb_ereg("\p{Lo}", $p);
  $hasNumber  = mb_ereg("\p{Nd}", $p);
  $hasOther   = mb_ereg("(?!(\p{Nd}|\p{Ll}|\p{Lu}|\p{Zs})).", $p);

}

Und jetzt braucht man eine Entscheidungsregeln. Ich würde beispielsweise ein Passwort akzeptieren, dass nur (kleinbuchstaben oder großbuchstaben) und space enthält, aber länger als 25 Zeichen ist (siehe "correct horse battery staple"). Der Space-Anteil sollte aber unter - sagenwirmal - 15% liegen (das korrekte Pferd hat schon 10,7%!). Die $hasOther Kategorie trifft viele Fremdalphabete, z.B. finde ich bei Thai oder Arabisch nur \p{Lo } Zeichen. Diese 字亦属^[1].

Diese Regeln solltest Du jetzt für Dich formulieren, und dann der Reihe nach prüfen, ob sie erfüllt sind.

Oder Du bleibst bei deiner Regex, wenn Dir das jetzt alles zu mystisch wird.

Rolf

---

1. Das sollte "Buchstaben (gemeint sind die CJK-Ideogramme), auch, gehören dazu" bedeuten, wenn ich die Zeichenliste richtig deute 🤣. That's all greek chinese to me… ↩︎

[dedlfix]

Tach!

2. Übereinstimmung prüfen (===, identisch)

Warum typsicher? $_POST liefert Strings oder Arrays. Strings mit Arrays zu vergleichen liefert false. Gleiche Arrays liefern true, egal ob typsicher oder nicht. Der typsichere Vergleich ergibt keinen Unterschied zum nicht typsicheren und der unerlaubte Fall Array wird nicht bemerkt. Es ist hier also egal, ob typsicher verglichen wird oder nicht.

4a. auf erlaubte Zeichen prüfen

Welche Zeichen sollten verboten werden? Oder auch andersrum gefragt, welche sollen erlaubt sein? Warum soll das eingeschränkt sein?

dedlfix.

[TS]

Hello,

2. Übereinstimmung prüfen (===, identisch)

Warum typsicher? $_POST liefert Strings oder Arrays. Strings mit Arrays zu vergleichen liefert false. Gleiche Arrays liefern true, egal ob typsicher oder nicht. Der typsichere Vergleich ergibt keinen Unterschied zum nicht typsicheren und der unerlaubte Fall Array wird nicht bemerkt. Es ist hier also egal, ob typsicher verglichen wird oder nicht.

Ok, sehe ich ein.

Man müsste also die beiden $_POST-Parameter auf is_string() prüfen, damit nicht nachher der Hash von "array" gebildet wird.

4a. auf erlaubte Zeichen prüfen

Welche Zeichen sollten verboten werden? Oder auch andersrum gefragt, welche sollen erlaubt sein? Warum soll das eingeschränkt sein?

Das kann ich nicht beantworten. Hängt sicherlich von Umfeld und Reichweite der Anwendung ab.

Sinnvoll könnte die Einschränkung auf international zugängliche Zeichen sein, damit ich mich auch auf einem chinedsischen Computer an meinem Account anmelden kann ;-P

Glück Auf
Tom vom Berg

[localhorst]

Hallo TS,
hallo Alle,

Man müsste also die beiden $_POST-Parameter auf is_string() prüfen, damit nicht nachher der Hash von "array" gebildet wird.

Oh, wieder was gelernt. Das wäre ja wirklich nicht schön.

4a. auf erlaubte Zeichen prüfen

Welche Zeichen sollten verboten werden? Oder auch andersrum gefragt, welche sollen erlaubt sein? Warum soll das eingeschränkt sein?

Das kann ich nicht beantworten. Hängt sicherlich von Umfeld und Reichweite der Anwendung ab.

Sinnvoll könnte die Einschränkung auf international zugängliche Zeichen sein, damit ich mich auch auf einem chinedsischen Computer an meinem Account anmelden kann ;-P

Vielleicht willst Du das Passwort ja auf deinem alten Fernschreiber drucken?

LG + Gesundheit
Localhorst

[kai345]

Sinnvoll könnte die Einschränkung auf international zugängliche Zeichen sein, damit ich mich auch auf einem chinedsischen Computer an meinem Account anmelden kann ;-P

Vielleicht willst Du das Passwort ja auf deinem alten Fernschreiber drucken?

[TS]

Hello,

Sinnvoll könnte die Einschränkung auf international zugängliche Zeichen sein, damit ich mich auch auf einem chinedsischen Computer an meinem Account anmelden kann ;-P

Vielleicht willst Du das Passwort ja auf deinem alten Fernschreiber drucken?

Als Lochmuster hättest Du nun noch ein visuelles (also nicht codiertes) "selfHTML it toll" stanzen können ;-)

Glück Auf
Tom vom Berg

[kai345]

Als Lochmuster hättest Du nun noch ein visuelles (also nicht codiertes) "selfHTML it toll" stanzen können ;-)

[Robert B.]

Moin Kai,

Als Lochmuster hättest Du nun noch ein visuelles (also nicht codiertes) "selfHTML it toll" stanzen können ;-)

das wäre als Card ja mal richtig smart 😂

Viele Grüße
Robert

[Robert B.]

Moin Kai,

Vielleicht willst Du das Passwort ja auf deinem alten Fernschreiber drucken?

wenn man die Löcher in eine Karte stanzt, kann man es als „Token-Lochkarte“ ähnlich einer Smartcard verwenden.

Viele Grüße
Robert

[TS]

Hello,

Vielleicht willst Du das Passwort ja auf deinem alten Fernschreiber drucken?

wenn man die Löcher in eine Karte stanzt, kann man es als „Token-Lochkarte“ ähnlich einer Smartcard verwenden.

Wie jetzt? Darf man Smartcards lochen?

Das erinnert mich an die Bilder von CDs, die jemand zum Abheften gelocht hatte. Leider wird Google auch immer humorloser, und ich fand die Bilder nicht mehr.

Glück Auf
Tom vom Berg

[MudGuard]

Hi,

Das erinnert mich an die Bilder von CDs, die jemand zum Abheften gelocht hatte.

Da waren die 3,5er Disketten praktischer, die waren schon im richtigen Abstand vorgelocht …

cu,
Andreas a/k/a MudGuard

[Der Martin]

Hallo,

Das erinnert mich an die Bilder von CDs, die jemand zum Abheften gelocht hatte.

Da waren die 3,5er Disketten praktischer, die waren schon im richtigen Abstand vorgelocht …

stimmt, und die 5¼"-Disketten konnte man mit etwas Vorsicht auch so lochen, dass die eigentliche Disk in der Plastikhülle nicht beschädigt wurde.

Live long and pros healthy,
 Martin

[Robert B.]

Moin,

und wenn es fürs Lochen nicht ganz reicht, gibt es immer noch CDs, DVDs und Blueray-Discs.

Viele Grüße
Robert

[dedlfix]

Tach!

Man müsste also die beiden $_POST-Parameter auf is_string() prüfen, damit nicht nachher der Hash von "array" gebildet wird.

Da du ja die Komplexität prüfen möchtest, würde das eigentlich da schon durchfallen. Aber selbst wenn das Array es bis zum password_hash() schafft, würde die Unternehmung dort zu Ende sein, denn dann gibt es einen fatalen Fehler wegen falschen Typs.

dedlfix.

[localhorst]

Hallo dedlfix,
hallo Alle,

Man müsste also die beiden $_POST-Parameter auf is_string() prüfen, damit nicht nachher der Hash von "array" gebildet wird.

Da du ja die Komplexität prüfen möchtest,

Was bedeutet hier "Komplexität prüfen"?
Ich hab da zwar was gefunden, werde aber nicht richtig schlauer daraus.

LG + Gesundheit
Localhorst

[Rolf B]

Hallo TS,

damit nicht nachher der Hash von "array" gebildet wird.

Das ist tatsächlich eine üble Sache.

Im Normalfall wird das nicht vorkommen, solange das Form korrekt aufgebaut ist. Aber wehe, ein Script-Kiddie postet irgendwelchen vorsätzlichen Müll.

Ich habe das gerade mal ausprobiert. Laut Doku liefert password_hash ja FALSE, wenn ein Fehler auftritt. Wenn man ein Array als ersten Parameter hineingibt, wirft PHP eine Warnung, dass da ein Array statt eines Strings kam.

Und die Rückgabe ist: NULL. Nicht FALSE. Auch, wenn man Müll als Passwortalgorithmus übergibt, ist die Rückgabe NULL. Nicht false. Kommt mir wie ein böser Bug vor.

Man muss die Rückgabe von password_hash also auf NULL und FALSE prüfen, um festzustellen, dass da was schief ging.

Und die Prüfung der Eingabe auf is_array ist sicherlich auch sinnvoll. Da hilft aber tatsächlich filter_input - das liefert FALSE wenn im Parameter ein Array steht. Und NULL wenn der Parameter nicht existiert, ein paar ifs wird man also bauen müssen.

Rolf

[dedlfix]

Tach!

Ich habe das gerade mal ausprobiert. Laut Doku liefert password_hash ja FALSE, wenn ein Fehler auftritt.

Du testest mit PHP 7? Teste mal mit PHP 8, da gibt es nicht nur eine Warnung.

dedlfix.

[Rolf B]

Hallo dedlfix,

sehr schön, dass PHP so langsam typsicherer wird.

Rolf

[1unitedpower]

sehr schön, dass PHP so langsam typsicherer wird.

Wenn es etwas mehr sein darf, kann ich phan empfehlen. Das ist ein Typchecker und ein statisches Analysetool aus der Feder von PHP-Erfinder Rasmus Lerdorf. phan geht über das PHP-interne Typsystem hinaus und erlaubt bspw. auch Generics, aka. parametrischer Polymorphismus.

[localhorst]

Hallo dedlfix,
hallo Alle,

2. Übereinstimmung prüfen (===, identisch)

Warum typsicher? $_POST liefert Strings oder Arrays. Strings mit Arrays zu vergleichen liefert false. Gleiche Arrays liefern true, egal ob typsicher oder nicht. Der typsichere Vergleich ergibt keinen Unterschied zum nicht typsicheren und der unerlaubte Fall Array wird nicht bemerkt. Es ist hier also egal, ob typsicher verglichen wird oder nicht.

Was kommt raus, wenn man

var_dump("□□□□□□□□"=="00000000");
oder
var_dump("□□□□□□□□"==="00000000");
oder auch
linke und rechte Seite vertauscht.

ausführt? Die Kästchen sollen Leerzeichen sein.

Ich habe leider kein php zur Verfügung im Moment.

LG + Gesundheit
Localhorst

[Tabellenkalk]

Hallo,

Was kommt raus, wenn man

var_dump("□□□□□□□□"=="00000000");
oder
var_dump("□□□□□□□□"==="00000000");
oder auch
linke und rechte Seite vertauscht.

ausführt?

Merkwürdige Frage, auch wenn ich das jetzt neugierigerweise tatsächlich getestet habe. Was soll da anderes als False rauskommen?

Hast du Anlass zu erwarten, dass da True kommen könnte?

Gruß
Kalk

[Der Martin]

Mahlzeit,

var_dump("□□□□□□□□"=="00000000");
oder
var_dump("□□□□□□□□"==="00000000");
oder auch
linke und rechte Seite vertauscht.

Merkwürdige Frage, auch wenn ich das jetzt neugierigerweise tatsächlich getestet habe. Was soll da anderes als False rauskommen?

ich habe tatsächlich auch leichte Zweifel gehabt.

Hast du Anlass zu erwarten, dass da True kommen könnte?

Ja. PHP konvertiert manchmal unerwartet Strings in Numbers. Dabei können seltsame Ergebnisse entstehen - hier beispielsweise, weil sowohl ein String aus ein paar Blanks, als auch ein String mit ein paar Nullen auf den Zahlenwert 0 abgebildet wird. Ein true beim nicht typsicheren Vergleich hätte mich daher nicht gewundert.

Live long and pros healthy,
 Martin

[dedlfix]

Tach!

Ich habe leider kein php zur Verfügung im Moment.

Teste es doch mit der PHP Sandbox.

dedlfix.

[Rolf B]

Hallo localhorst,

hier ist eins, mit vielen Versionen zur Auswahl:

https://sandbox.onlinephpfunctions.com/

Edit: Verd*mmt, Dedlfix war im Fotofinish schneller.

Rolf

[TS]

Hello WAN-Horst, ;-P

2. Übereinstimmung prüfen (===, identisch)

Warum typsicher? $_POST liefert Strings oder Arrays. Strings mit Arrays zu vergleichen liefert false. Gleiche Arrays liefern true, egal ob typsicher oder nicht. Der typsichere Vergleich ergibt keinen Unterschied zum nicht typsicheren und der unerlaubte Fall Array wird nicht bemerkt. Es ist hier also egal, ob typsicher verglichen wird oder nicht.

Was kommt raus, wenn man

var_dump("□□□□□□□□"=="00000000");
oder
var_dump("□□□□□□□□"==="00000000");
oder auch
linke und rechte Seite vertauscht.

ausführt? Die Kästchen sollen Leerzeichen sein.

Ich habe leider kein php zur Verfügung im Moment.

Ich habe auch nur Version (7.4.5):

Interessant ist diese Variante:

   var_dump("00" == "0");
   var_dump("00" === "0");

ergibt

   bool(true)
   bool(false)

Der Identitätsoperator erscheint mir daher doch richtig zu sein!

Glück Auf
Tom vom Berg

[localhorst]

Hallo TS,
hallo Alle,

2. Übereinstimmung prüfen (===, identisch)

Warum typsicher? $_POST liefert Strings oder Arrays. Strings mit Arrays zu vergleichen liefert false. Gleiche Arrays liefern true, egal ob typsicher oder nicht. Der typsichere Vergleich ergibt keinen Unterschied zum nicht typsicheren und der unerlaubte Fall Array wird nicht bemerkt. Es ist hier also egal, ob typsicher verglichen wird oder nicht.

Was kommt raus, wenn man

var_dump("□□□□□□□□"=="00000000");
oder
var_dump("□□□□□□□□"==="00000000");
oder auch
linke und rechte Seite vertauscht.

ausführt? Die Kästchen sollen Leerzeichen sein.

Ich habe leider kein php zur Verfügung im Moment.

Ich habe auch nur Version (7.4.5):

Interessant ist diese Variante:

   var_dump("00" == "0");
   var_dump("00" === "0");

ergibt

   bool(true)
   bool(false)

Der Identitätsoperator erscheint mir daher doch richtig zu sein!

Na hoppla!
Danke für den Test.

Da schlägt dann wohl die Typumwandlung zu?
Ist ja ne echt böse Falle!

♡♡♡ ♡♡♡ ♡♡♡ ♡♡♡
Hinweis:
Meine Antworten beziehen sich (meistens) auf den gesamten bisherigen Thread. Der/die jeweilige direkte Vorposter/in sollte sich daher nicht alleine angesprochen fühlen.
♡♡♡ ♡♡♡ ♡♡♡ ♡♡♡

LG + Gesundheit
Localhorst

[TS]

Hello Horst,

Ich habe auch nur Version (7.4.5):

Interessant ist diese Variante:

   var_dump("00" == "0");
   var_dump("00" === "0");

ergibt

   bool(true)
   bool(false)

Der Identitätsoperator erscheint mir daher doch richtig zu sein!

Na hoppla!
Danke für den Test.

Da schlägt dann wohl die Typumwandlung zu?
Ist ja ne echt böse Falle!

Ja. Und genau diese Lücke hatte ich auch im Hinterhirn, bis @dedlfix mich dann in die Irre geführt hat ;-(

Ohne dein Nachquängeln hätte ich ihm blind vertraut. :-O

Glück Auf
Tom vom Berg

[Rolf B]

Hallo TS,

PHP nennt das Numeric Strings

Ein Punkt für Kollege Bastelstube. PHP ist grottig, es bräuchte wie JavaScript einen "Strict Mode", der all diese hysterisch gewachsenen Quirks abschaltet.

Deswegen gleich das Büßerhemd anzuziehen, sich den Knotenstrick ums Bein zu schnüren und Perl zu nehmen scheint mir aber doch übertrieben.

Rolf

[TS]

Hello,

PHP nennt das Numeric Strings

Ja danke.
Ich bin ja noch ganz neu hier (ca. seit 1999), da kann man sowas nicht wissen ;-P

Glück Auf
Tom vom Berg

[dedlfix]

Tach!

PHP nennt das Numeric Strings

"When a string needs to be evaluated as number (e.g. arithmetic operations, int type declaration, etc.) …"

Eigentlich werden hier zwei String verglichen und nicht String mit Zahl. Ich sehe da kein "needs to be evaluated as number". Das ist für mich eine unerwartete Typkonvertierung.

dedlfix.

[TS]

Hello @dedlfix,

PHP nennt das Numeric Strings

"When a string needs to be evaluated as number (e.g. arithmetic operations, int type declaration, etc.) …"

Eigentlich werden hier zwei String verglichen und nicht String mit Zahl. Ich sehe da kein "needs to be evaluated as number". Das ist für mich eine unerwartete Typkonvertierung.

Früher hieß das immer:
Kommt immer darauf an, was links steht.

Bei

var_dump(0 == '000');

hätte ich eine Typkonvertierung erwartet. Aber die Sache mit den Nullen-Strings ist schon lange bekannt und mMn ein schwerer logischer Bug.

Sollte man da mal eine Message verfassen, oder ist das in PHP V8 sowieso Schnee von gestern?

Glück Auf
Tom vom Berg

[dedlfix]

Tach!

Interessant ist diese Variante:

   var_dump("00" == "0");
   var_dump("00" === "0");

ergibt

   bool(true)
   bool(false)

Der Identitätsoperator erscheint mir daher doch richtig zu sein!

Gut, einen Punkt für den ===. Aber dann ist da immer noch Punkt 3 und 4b, an dem die Prüfung scheitert.

dedlfix.

[TS]

Hello @dedlfix,

Interessant ist diese Variante:

   var_dump("00" == "0");
   var_dump("00" === "0");

ergibt

   bool(true)
   bool(false)

Der Identitätsoperator erscheint mir daher doch richtig zu sein!

Gut, einen Punkt für den ===. Aber dann ist da immer noch Punkt 3 und 4b, an dem die Prüfung scheitert.

Es ist zudem ein Spezialfall, vermutlich an dieser Stelle der einzige. Aber ein einziger Spezialfall reicht i.d.R., um ein System kompromittieren zu können, egal ob willentlich oder unwissentlich.

Wenn man das geplante Passwort-Modul als Ganzes betrachtet, wird die Lücke an der einen Stelle durch nachfolgende Prüfungen geschlossen. Wenn man aber nun - warum auch immer - einzelne Funktionen für andere Zwecke benutzt, dann kann das schon mächtig in die Hose gehen.

Darum immer meine Forderung:

Jede Funktion muss genau das leisten, was man von ihr erwartet, nicht weniger und nicht mehr (also keine Seiteneffekte), egal, ob man sie einzeln oder im Verbund mit anderen verwendet.

Glück Auf
Tom vom Berg

[klawischnigg]

Hi there,

Reihenfolge:

1. Vorhandensein der POST-Parameter mit isset() prüfen
2. Übereinstimmung prüfen (===, identisch)
3. Mindestlänge prüfen
4. verlangten Zeichenvorrat prüfen
   4a. auf erlaubte Zeichen prüfen
   4b. auf geforderte Anzahl von Versalien, Gemeinen, Ziffern, Sonderzeichen prüfen
5. hashen (bitte nicht mehr mit md5(), sondern mit password_hash() )
6. eintragen
7. ggf. Bestätigungsmail (selbstverständlich ohne das Passwort!) senden

Eines hast Du noch vergessen:

Im RAM mit dem Mikroskop nachschauen, wie die beiden Passwörter auf Bit-Ebene ausschauen...

(scnr)

[localhorst]

Hallo klawischnigg,
hallo Alle,

Eines hast Du noch vergessen:

Im RAM mit dem Mikroskop nachschauen, wie die beiden Passwörter auf Bit-Ebene ausschauen...

Das hier ist für mich seit langem der lehrreichste Thread in Bezug auf Client-Server Webapplikationen. Ich finde es daher nicht angemessen, sachliche Postings und deren Urheber ins Lächerliche zu ziehen.

(scnr)

Das hilft dann auch nicht mehr.

Oder kannst Du definitiv auf ein Speicherleck bei PHP in diesem Zusammenhang hinweisen? Dann würde ich mein Genöle sofort zurücknehmen und Dich um Verzeihung bitten.

♡♡♡ ♡♡♡ ♡♡♡ ♡♡♡
Hinweis:
Meine Antworten beziehen sich (meistens) auf den gesamten bisherigen Thread. Der/die jeweilige direkte Vorposter/in sollte sich daher nicht alleine angesprochen fühlen.
♡♡♡ ♡♡♡ ♡♡♡ ♡♡♡

LG + Gesundheit
Localhorst

[klawischnigg]

Hi there,

Eines hast Du noch vergessen:

Im RAM mit dem Mikroskop nachschauen, wie die beiden Passwörter auf Bit-Ebene ausschauen...

Das hier ist für mich seit langem der lehrreichste Thread in Bezug auf Client-Server Webapplikationen. Ich finde es daher nicht angemessen, sachliche Postings und deren Urheber ins Lächerliche zu ziehen.

Ja, fein. Ich mach seit 20 Jahren Client-Server-Webapplikationen, und ich muß sagen, man kann halt alles übertreiben. Wenn ich jedes eingegeben Passwort auf seinen Zeichvorrat überprüft hätte, dann hätte ich dieses Posting, über das Du Dich so echauffierst, vermutlich nie geschrieben, denn dann wär' ich in der Zwischenzeit verhungert.

Und was das vorgebliche "ins Lächerliche ziehen" betrifft, das wird der liebe TS schon aushalten, zumal ich absolut nichts erkennen kann, womit ich den Urheber persönlich hätte beleidigen oder ins Lächerliche hätte ziehen wollen.

(scnr)

Das hilft dann auch nicht mehr.

Soll's ja auch nicht. Das war ja auch nicht als Abschwächung gedacht. In der Sache selbst bleib ich sicher dabei - man kann alles übertreiben. Ausser natürlich, man sieht die Entwicklung von Web-Applikationen als akademisches Angelegenheit, dann kann man natürlich Zeit in Probleme investieren, die es ohne die Applikation gar nie gegeben hätte.

Oder kannst Du definitiv auf ein Speicherleck bei PHP in diesem Zusammenhang hinweisen?

Sicher nicht, PHP ist für mich nur ein Werkzeug, das halt irgendwie funktionieren sollte, wie eine Schaufel. Wenn ich ein Loch grabe, dann analysier ich auch nicht den Bagger...

[vapita]

Hallo Rolf,

ich hatte im Auge, dass password_hash einen String erwartet, daher die Idee.

Stimmt, man kann sich einen hash sparen, wenn man vorher vergleicht.

Danke für die Info.

Beste Grüße

vapita

[TS]

Hello vapita,

ich hatte im Auge, dass password_hash einen String erwartet, daher die Idee.

Stimmt, man kann sich einen hash sparen, wenn man vorher vergleicht.

Man darf nur einmal hashen, wenn man password_hash() und später zum Vergleichen password_verify() benutzen will.

Die Funktion password_hash() produziert nämlich bei jedem Aufruf einen neuen Passwortschlüssel. Dieser enthält dann alle Daten (mit Ausnahme des Passwortes im Klartext), die password_verify() später zum Vergleichen benötigt.

Glück Auf
Tom vom Berg

[Robert B.]

Moin,

angenommen, ich habe soeben per POST zwei Passwörter erhalten, die ich nun vergleichen und dann speichern möchte. Welche Reihenfolge ist am sinnvollsten? Oder ist es eigentlich egal?

1. Ich prüfe, ob sie sich gleichen. Wenn ja -> hashen und speichern.
2. Ich hashe beide Passwörter und prüfe, ob sie sich gleichen. Wenn ja -> speichern.
3. Ich prüfe erstmal, ob es sich jeweils um einen String handelt. Wenn ja, vergleiche ich sie dann und hashe und speichere ggf.
4. Ich prüfe, ob es Strings sind. Wenn ja, hashe und vergleiche ich sie und speichere dann ggf.

Ich habe mal eine richtige Liste daraus gemacht.

Oder gibt es gar ein besseres Vorgehen?

Was ist denn deine Vermutung, welches Vorgehen am sinnvollsten ist?

Die Optionen 3 und 4 brauchst du eigentlich nicht gesondert zu berücksichtigen, denn standardmäßig sind alle per POST übertragenen Werte erst einmal Strings. Du solltest allerdings die Passwörter nicht nur Hashen, sondern auch Salzen.

Viele Grüße
Robert

[Rolf B]

Hallo Robert,

das macht password_hash schon automatisch mit einem Zufallssalz. Ein eigenes, festes Salz zu verwenden, ist kontraproduktiv.

Die Option, Salz mitzugeben, gibt es eh nur bei BCRYPT und ist seit PHP 7 missbilligt.

Rolf

[vapita]

Hallo Robert,

danke erstmal. Ja, Option 3 und 4 sind vom Tisch. Ich habe die erste Variante benutzt, hatte aber auch überlegt, ob auch die anderen sinnvoll oder sinnvoller wären. Inzwischen kam jedoch heraus, dass man den Benutzer ja noch mit Mindestzeichenkettenlängen und Zeichenkomplexität ärgern kann. Also ist die Variante 1+ wohl die bisher sinnvollste.

Zum einen möchte ich den Code möglichst übersichtlich gestalten, damit auch Außenstehende leicht nachvollziehen können, was ich da angestellt habe. Andererseits traue ich den Benutzereingaben nicht und möchte möglichst alle Fehlerquellen ausschließen.

Daher prüfe ich zum Beispiel auch zusätzlich, ob es sich überhaupt um einen POST-Request handelt und ob ein gültiger CSRF-Token vom Formular mitgegeben wurde, ehe die Formular-Verarbeitung startet.

Du solltest allerdings die Passwörter nicht nur Hashen, sondern auch Salzen.

Daran hab ich auch noch nicht gedacht. Guter Hinweis.

Beste Grüße

vapita

[Rolf B]

Hallo vapita,

Daran hab ich auch noch nicht gedacht.

Doch, hast Du. password_hash salzt automatisch, es sei denn, du streust das Salz von Hand. Was aber nicht mehr erwünscht ist.

Rolf

[vapita]

Hallo Rolf,

Doch, hast Du. password_hash salzt automatisch, es sei denn, du streust das Salz von Hand. Was aber nicht mehr erwünscht ist.

Ich sollte vielleicht doch einmal einen genaueren Blick auf die PHP-Docs werfen. Danke auch für diesen Hinweis.

Beste Grüße

vapita

[Robert B.]

Moin vapita,

Ich sollte vielleicht doch einmal einen genaueren Blick auf die PHP-Docs werfen.

Das sollte man auf jeden Fall immer tun.

Viele Grüße
Robert

[Robert B.]

Moin vapita,

danke erstmal. Ja, Option 3 und 4 sind vom Tisch. Ich habe die erste Variante benutzt, hatte aber auch überlegt, ob auch die anderen sinnvoll oder sinnvoller wären.

Option 2 hat den Nachteil, dass du unnötig Rechenzeit verschwendest. Und im Fall von password_hash wird es auch nicht zum Erfolg führen, da der Salt zweiter Aufrufe hintereinander unterschiedlich ist.

Inzwischen kam jedoch heraus, dass man den Benutzer ja noch mit Mindestzeichenkettenlängen und Zeichenkomplexität ärgern kann. Also ist die Variante 1+ wohl die bisher sinnvollste.

Passwörter sollten für den Nutzer kein Ärgernis, sondern gut zu merken sein. Ich empfehle da immer den xkcd-Comic zur Passwortkomplexität.

Zum einen möchte ich den Code möglichst übersichtlich gestalten, damit auch Außenstehende leicht nachvollziehen können, was ich da angestellt habe. Andererseits traue ich den Benutzereingaben nicht und möchte möglichst alle Fehlerquellen ausschließen.

Was sind denn „Fehlerquellen“ in Bezug auf Passwörter?

Viele Grüße
Robert

[Tabellenkalk]

Hallo,

Passwörter sollten für den Nutzer kein Ärgernis, sondern gut zu merken sein. Ich empfehle da immer den xkcd-Comic zur Passwortkomplexität.

Ich hab von der Theorie dahinter keine Ahnung, wo kommen die xx Bits of Entropy her? Wenn das mit den vier Worten wirklich empfehlenswert ist, dann fänd ich das echt gut!

Gruß
Kalk

[Rolf B]

Hallo Tabellenkalk,

letztlich geht es um die mögliche Anzahl von Passworten und damit Kombinatorik. Meine Eselsbrücke ist da immer "Symbolzahl" hoch "Stellen", d.h. wenn ich 7 Symbole habe und 4 Stellen, habe ich $$7^4=2401$$ mögliche Wörter, die ich damit bilden kann.

Die Bitzahl der Entropie ist dann einfach der Exponent der Zweierpotenz, die diese Anzahl darstellt. Wenn ich die Anzahl kenne, muss ich also den Zweierlogarithmus dieser Anzahl bilden. Wenn man dafür keinen Rechner hat, kann man den Logarithmus zur Basis 2 oder e nehmen und die Umrechnungsformel verwenden: $$\displaystyle \log_2 a = \frac{\log_{10}a}{\log_{10}2}= \frac{\ln a}{\ln2}$$).

Die Entropie einer Variation von Troubadour ist schwierig zu berechnen. Man muss von der Anzahl brauchbarer Wörter im Wörterbuch ausgehen - z.B. 100000. Die Betonung liegt auf „brauchbar“, also sind es vermutlich deutlich weniger. Der Zweierlogarithmus von 100000 ist 16,6. Das sind also 16,6 Bits Entropie. Hinzu kommen die Sonderzeichen und die Variationen von 1337-Speak, die man einbaut; die 28 Bits sind vermutlich ein "educated guess".

Bei der Vierwort-Konstruktion ist es einfacher. Wähle ich ein Wort aus einer festgelegten Liste von 2048 Wörtern, sind das 11 Bits Entropie. Weil 2^11=2048. Wähle ich vier Wörter aus dieser Liste und lasse dabei auch Wiederholungen zu (correct correct correct correct), sind es gemäß der Eselsbrücke $$\displaystyle (2¹¹)^4 = 2^{44}$$ Möglichkeiten. Das Schöne daran ist, dass die Wortliste nicht geheim zu sein braucht.

Lasse ich keine Wiederholungen zu, sinkt die Entropie nur minimal. Aus der Möglichkeitenzahl $$2^{44}$$ wird dann $$2048\cdot 2047\cdot 2046\cdot 2045$$, der Logarithmus dieses Produkts ist die Summe der Logarithmen der Faktoren, also $$43{,}996$$.

Das gilt natürlich nur solange, wie man einen guten Zufallszahlengenerator hat und die ausgewürfelten Wörter auch wirklich vorurteilsfrei nimmt. Verwendet man einen primitiven Kongruenzgenerator, oder würfelt man solange, bis man eine „schöne“ Kombi bekommt, sinkt die Entropie deutlich, weil das Symbole oder Symbolkombinationen aus dem Möglichkeitsraum streicht, bzw. die möglichen Symbolfolgen begrenzt.

Leider hat sich "correct horse battery staple" noch nicht bis zum BSI herumgesprochen. Die empfehlen nach wie vor den Tr0b4dur&9, und wie Randall anführt, führen Diskussionen über diese Idee mit denen, die das Thema nicht wirklich verstehen, schnell zu heißem Streit.

Ich weiß allerdings selbst nicht so genau, ob ich beim Anmelden ständig correct horse battery staple tippen will, oder doch lieber nur 10 wilde Zeichen. Denn ich muss mich auf der Arbeit regelmäßig an verschiedenen Systemen anmelden; wir haben verschiedene Netzwerkdomänen und auch unterschiedliche User-IDs (für normale und Admintätigkeiten).

Rolf

[Tabellenkalk]

Hallo,

vielen Dank für die Erläuterung!

Also ist der korrekte Pferdebatteriestapel zwar sicherer aber nicht praktikabel, denn einerseits hat man viel mehr zu tippen und andererseits enthält er kein Sonderzeichen o.ä. was von manchen Logins vorgeschrieben wird…

Gruß
Kalk

[Rolf B]

Hallo Tabellenkalk,

was von manchen Logins vorgeschrieben wird…

Was auch an der Empfehlung des BSI für sichere Passwörter liegt. Randall kämpft mit diesem Vorschlag gegen Windmühlen an.

Oh. Update. Sie haben das aktualisiert, es stehen jetzt auch "lange Passworte mit 2 Zeichenarten, z.B. mehrere Wörter" auf der Liste.

Rolf

[tk]

Hallo vapita,

angenommen, ich habe soeben per POST zwei Passwörter erhalten, die ich nun vergleichen und dann speichern möchte. Welche Reihenfolge ist am sinnvollsten? Oder ist es eigentlich egal?

Nein, ist es nicht: password_hash() spuckt auch für den gleichen String nicht den gleichen Hash aus, du musst die Passwörter also erst vergleichen und dann hashen.

Was du mit der Prüfung auf String genau meinst/bezweckst, weiß ich nicht - aber Daten die per POST (bzw. auch GET) reinkommen sind immer Strings, die Prüfung kannst du dir also sparen.

Oder gibt es gar ein besseres Vorgehen?

Vorgehen wofür? Vielleicht beschreibst du mal etwas genauer was du da wirklich vor hast bzw. was du da genau programmierst.

Gruß,
Tobias

[localhorst]

Hallo vapita,
hallo Alle,

hier ging es zunächst nur um die Reihenfolge der Arbeitsschritte. Inzwischen haben wir aber eine Menge weiterer Erkenntnisse gewonnen.

Ich würde nun gerne noch lernen, wie man die Teilaufgaben richtig nach dem MVC-Modell verteilt. Es erscheint mir nicht sinnvoll, z.B. den Datenbankeintrag auch direkt in dieser Funktion vornehmen zu lassen, oder die HTML-Antwort komplett zu generieren, usw.

Außerdem müsste man vor der Eintragung eines Passworthashes in die Tabelle wohl auch die Berechtigung dafür prüfen, also z.B. das alte Passwort abfragen und vergleichen, oder so. Und bei Neuanlage des Accounts würde man vielleicht noch einen Opt-In Roundturn benötigen, um Spammer zu verhindern.

Ich würde mich deshalb freuen, wenn die Diskussion hier noch nicht abreißen würde.

LG + Gesundheit
Localhorst

[Rolf B]

Hallo localhorst,

wir wissen nicht, ob Vapita MVC verwendet bzw. schonmal was davon gehört hat. Aber wenn wir ein MVC-Design voraussetzen, dann würde sich das so aufteilen:

• Übernehmen der Formdaten, ggf. auch grundsätzliche Validierung (Array/String, steht was drin) ist Teil der View-Schicht. Die View-Schicht besteht aus zwei Teilen: Zum einen der Webbrowser, und zum anderen der serverseitige Code zum Senden an den und Empfangen vom Browser. Da jeder Request jungfräulich am Server beginnt, ist es Sache des verwendeten Controllers, die Übernahme der Eingabedaten zu orchestrieren. Das ist meistens so gebaut, dass der Controller die Übertragung der Daten ins Modell direkt durchführt, aber eigentlich ist es Teil der View-Schicht. Wenn ich also einen LoginView auf dem Browser habe und der sich zurückmeldet, dann müsste der LoginController eigentlich User-Model und LoginView erzeugen, die beiden verkabeln und es dann dem View überlassen, $_GET und $_POST Daten auszuwerten (MVC Prinzip: Der View kennt das Model, aber das Model nicht den View. Der Controller kennt View und Model, aber beide kennen den Controller nicht).

• Speichern der Userdaten (Name, Passworthash, Rechte, bla bla) ist Teil des Model. Ihre Persistierung gehört in den DB-Layer, der von MVC nicht explizit erwähnt wird, aber Teil des Models ist. Der Modellschicht den passenden DB-Layer unterzuschieben sollte konfigurativ erfolgen, oder im Web zentral von der allgemeinen Requeststeuerung erledigt werden.

• Eine Passwortklasse ist eher auf der Controller-Ebene anzusiedeln, es ist eine Diensteklasse für die Aktionen Login, Register und ChangePassword des User-Controllers

Diskutieren kann man, ob eine Passwort-Klasse den Job haben sollte, einen User anzulegen (db_insert_password). Das gehört meiner Vorstellung nach anders sortiert, ich wollte nur die Diskussion nicht zu weit aufblähen und neue Themenfelder eröffnen. Ein User-Objekt hat einen Namen, einen Passworthash, Rechte, etc. Und ein Login-Controller verwendet die Passwort-Klasse, um Passwörter zu verschlüsseln und zu überprüfen. Den Hash bekommt er vom User geliefert. Beim Anlegen eines neuen Users oder beim Passwortwechsel kann der Hash natürlich auch geändert werden.

Rolf

[vapita]

Hallo an alle,

ja, ich nutze das MVC-Pattern. Ich habe für User-Angelegenheiten einen sogenannten UserController erstellt, der derzeit die Methoden:

• index
• login
• logout
• register

enthält. Weitere Methoden folgen noch. Etwa für das Aktualisieren der Benutzerdaten. Eben dann auch etwa das Erneuern des Passworts. Ich habe sämtliche Passwortvalidierungen in eine Passwort-Klasse gepackt und diese dann vom Controller aus aufgerufen. Das müllt mir mir dann aber immer noch den Controller zu sehr voll, weswegen ich zusätzlich noch eine User-Klasse erstellt habe, von der aus auch die Passwortvalidierungen durchgeführt werden.

Die User-Klasse und die Passwort-Klasse würde ich als Services unterhalb des Controllers ansiedeln. Der Controller selbst steuert dann, was die View ausgibt oder von dort kommt. (z.B. Formulardaten und Fehlermeldungen). Außerdem steuert er auch, was er vom Model haben will oder was das Model speichern soll.

Nun muss ich erstmal den UserController aufräumen. Gewisse Tests aus der Passwort-Klasse können ja auch für E-Mail-Adressen, Benutzernamen oder sonstiges verwendet werden. Da kommt dann der UserService ins Spiel, der allgemeine Abfragen erledigt.

Gern melde ich mich später mit dem Ergebnis zurück.

Beste Grüße

vapita

[vapita]

Hallo nochmal,

Ich habe jetzt den UserController etwas aufgeräumt. Ich bin mir nicht sicher, ob man es so machen sollte, zumindest gab es nach einem ersten Test keine unerwarteten Fehler. Auszugsweise dazu einmal die register-Methode:

# UserController.php

...

public function register (){
    if($this->getUser()){
        $this->redirect('302','base/index');
    }
    $userData = null;
    if($this->request->isPostRequest() && $this->request->isFormSubmitted()){
        $userData = [
            'username' => $this->request->getQuery('username'),
            'email' => $this->request->getQuery('email'),
            'password' => [
                $this->request->getQuery('password_a'),
                $this->request->getQuery('password_b'),
            ],
            'firstname' => $this->request->getQuery('firstname'),
            'lastname' => $this->request->getQuery('lastname'),
        ];
        $userRepository = $this->getRepository(User::class);
        if(is_object($user = $this->user->validate(new User(),$userRepository,$userData))) {
            $em = $this->getEntityManager();
            $em->persist($user);
            $this->flash->add(200);
            $this->redirect('302','user/login');
        }  else {
            $this->flash->add($user,'danger');
        }
    }
    $this->view->render('user/register.html.twig',[
        'flash' => $this->flash,
        'user' => $userData
    ]);
}

...

Sobald das Registrierungsformular abgesendet wurde, startet die Überprüfung, andernfalls wird einfach nur das Formular präsentiert. In der IF-Abfrage wird zuerst das Array $userData mit den POST-Daten befüllt. Die Request-Methode getQuery() macht folgendes im Hintergrund:

# Request.php

...

public function getQuery(string $FormFieldName)
{
    try {
        if ($FormFieldName !== NULL){
            $query = filter_input(INPUT_POST, $FormFieldName, FILTER_SANITIZE_STRIPPED);
            return $this->query = $query;
        } else {
            throw new Exception($query_exception);
        }
    } catch (Exception $query_exception){
        Logger::newMessage($query_exception);
        Logger::customErrorMsg($query_exception);
    }
}

...

Dann beginnt die eigentliche Prüfung in der User-Klasse von der aus auch das Passwort überprüft wird:

# User.php
class User
{

    /**
     * @var Password
     */
    public Password $password;

    /**
     * User constructor.
     */
    function __construct(){
        $this->password = new Password();
    }

    /**
     * @param $user         // User-Objekt, dass die Datenbanktabelle widerspiegelt
     * @param $repository   // Model-Repository, das die Methoden zum Abrufen enthält
     * @param array $data   // Formulardaten, die per POST übergeben worden sind
     * @return int|mixed    // Gibt entweder Fehlercode oder das User-Objekt zurück
     */
    public function validate($user, $repository, array $data)
    {
        if(0 != ($usernameLastError = $this->isString('username',$data,21031))) return $usernameLastError;
        if(0 != ($usernameLastError = $this->isUnique($repository,'username',$data,21011))) return $usernameLastError;
        if(0 != ($emailLastError = $this->isUnique($repository,'email',$data,21012))) return $emailLastError;
        if(0 != ($emailLastError = $this->isEmail('email',$data,21022))) return $emailLastError;
        if(0 != ($passwordLastError = $this->password->validate($data['password']))) return $passwordLastError;
        if(0 != ($firstnameLastError = $this->isString('firstname',$data,21034))) return $firstnameLastError;
        if(0 != ($lastnameLastError = $this->isString('lastname',$data,21035))) return $lastnameLastError;

        $user->setUsername($data['username']);
        $user->setEmail($data['email']);
        $user->setPassword($this->password->hash($data['password']));
        $user->setFirstname($data['firstname']);
        $user->setLastname($data['lastname']);
        $user->setIsActive(1);
        $user->setISBlocked(0);

        return $user;
    }

    /**
     * @param $repository
     * @param $needle
     * @param $array
     * @param int $errorCode
     * @return int|mixed
     */
    protected function isUnique($repository, $needle, $array, $errorCode = 2101){
        return ($repository->findOneBy([$needle => $array[$needle]])) ? $errorCode : 0;
    }

    /**
     * @param $needle
     * @param $array
     * @param int $errorCode
     * @return int|mixed
     */
    protected function isEmail($needle, $array, $errorCode = 2102){
        return (!filter_var($array[$needle], FILTER_VALIDATE_EMAIL)) ? $errorCode : 0;
    }

    /**
     * @param $needle
     * @param $array
     * @param int $errorCode
     * @return int|mixed
     */
    protected function isString($needle, $array, $errorCode = 2103){
        return (!is_string($array[$needle])) ? $errorCode : 0;
    }

}

Die Passwort-Klasse sieht nun so aus:

# Password.php
class Password
{

    /**
     * @param $plain_password
     * @return false|string|null
     */
    public function hash($plain_password)
    {
        $plain_password = (is_array($plain_password)) ? array_pop($plain_password) : $plain_password;
        return password_hash($plain_password, PASSWORD_DEFAULT);
    }

    /**
     * @param string $plain_password
     * @param string $correct_hash
     * @return bool
     */
    public function verify(string $plain_password, string $correct_hash)
    {
       return password_verify($plain_password, $correct_hash);
    }

    /**
     * @param array $password
     * @return int
     */
    public function validate(array $password):int
    {
        if(!$this->isString($password))     return 1601;    // kein String
        if(!$this->isEqual($password))      return 1602;    // stimmt nicht überein
        if(!$this->hasMinLength($password)) return 1603;    // ist nicht lang genug
        if(!$this->isAllowed($password))    return 1604;    // verwendet nicht erlaubte Zeichen
        if(!$this->isComplex($password))    return 1605;    // ist nicht komplex genug
        return 0; // alles in Ordnung
    }

    /**
     * @param $password
     * @return bool
     */
    private function isString($password){
        if(is_array($password)){
            foreach ($password as $item){
                $passwordIsString = (is_string($item)) ?? false;
            }
            return ($passwordIsString) ?? false;
        } else {
            return (is_string($password)) ?? false;
        }
    }

    /**
     * @param array $password
     * @return bool
     */
    private function isEqual(array $password){
        return ($password[0] == $password[1]) ?? false;
    }

    /**
     * @param $password
     * @return bool
     */
    private function hasMinLength($password){
        $password = (is_array($password)) ? array_pop($password) : $password;
        return (strlen($password) > 7) ?? false;
    }

    /**
     * @param $password
     * @return bool
     */
    private function isAllowed($password){
        // TODO: auf erlaubte Zeichen prüfen
        return true;
    }

    /**
     * @param $password
     * @return bool
     */
    private function isComplex($password){
        $password = (is_array($password)) ? array_pop($password) : $password;

        /** Das Passwort muss mindestens
         * - einen Kleinbuchstaben,
         * - einen Großbuchstaben,
         * - eine Ziffer und
         * - ein Sonderzeichen @#-_$%^&+=§!?
         * enthalten.
         * Das Passwort muss zwischen 8 und 20 Zeichen lang sein.
         */
        return (preg_match('/^(?=.*\d)(?=.*[@#\-_$%^&+=§!\?])(?=.*[a-z])(?=.*[A-Z])[0-9A-Za-z@#\-_$%^&+=§!\?]{8,20}$/',$password))? true : false;
    }

}

Hab ich da noch etwas gravierendes verkehrt gemacht oder könnte man es im groben und ganzen so stehen lassen?

Beste Grüße

vapita

[Rolf B]

Hallo vapita,

du baust da schon hübsch sortiert was zusammen. Was ich ändern würde:

getQuery

dazu hatte ich schon mal was geschrieben. Diese Methode ist so, wie sie ist, Quatsch. Wenn Du schon Exceptions wirfst, dann lass sie auch fliegen und fang sie nicht gleich wieder ein. Was Du mit $queryException machst, ist mir schleierhaft: mit define festgelegte Konstanten tragen kein $, eine globale Variable müsste mit global deklariert werden, und statische Konstanten in der Klasse sollte ein Request:: oder self:: Präfix tragen. Soweit ich das sehe, läufst Du im Errorhandling auf eine undefinierte Variable. Das gleiche gilt für die Fehlerwerte aus User::validate.

register

Was befindet sich, wenn Du $this->user->validate aufrufst, im user Property des UserControllers? Wird da von getUser was hineingelegt? Aber welcher - wenn Du registrierst, gibt es ja keinen angemeldeten User. Dies scheint mir falsch konstruiert. Entweder sollte validate eine statische Methode sein, die Du mit User::validate(...) aufrufst, oder Du änderst das Vorgehen so, dass Du einfach erstmal den User konstruierst und darauf dann validate als Instanzmethode rufst. Hier wäre übrigens eine Stelle, wo man durchaus try/catch verwenden kann.

try {
   ...
   $user = new User($userData);
   $user->validate($userRepository);
   $this->getEntityManager()->persist($user);
   $this->flash->add(200);
   $this->redirect('302', 'user/login');
}
catch (Exception $validationException) {
   this->flash->add($validationException->getMessage(), 'danger');
}

Über Entitymanager und Repository wollte ich gerade auch schon anfangen, aber aber klugerweise mal gegoogelt. Du verwendest Symfony und Doctrine - das ist also so vorgegeben für Dich. Jetzt verstehe ich auch, warum Du fleißig set-Methoden verwendest. Das muss so, sonst bekommt Doctrine nicht mit, dass sich Propertywerte ändern.

class Password

Das sieht nach einer statischen Klasse aus. Eigentlich brauchst Du davon keine Instanz. Mach die Methoden static und ruf dann bspw. Password::hash(...) auf.

Rolf

[vapita]

Hallo Rolf,

register

Was befindet sich, wenn Du $this->user->validate aufrufst, im user Property des UserControllers?

Das ist die User-Klasse mit den Validierungen.

Ich habe $this->user = new User(); im Abstrakten Controller verwendet von dem Jeder Controller erbt. Jedoch erscheint mir das inzwischen nicht mehr sinnvoll, da ich ja die User-Validierung gar nicht in jedem Controller benötige.

try {
   ...
   $user = new User($userData);
   $user->validate($userRepository);
   $this->getEntityManager()->persist($user);
   $this->flash->add(200);
   $this->redirect('302', 'user/login');
}
catch (Exception $validationException) {
   this->flash->add($validationException->getMessage(), 'danger');
}

Das ist ein guter Tipp. Danke dafür.

Über Entitymanager und Repository wollte ich gerade auch schon anfangen, aber aber klugerweise mal gegoogelt. Du verwendest Symfony und Doctrine - das ist also so vorgegeben für Dich. Jetzt verstehe ich auch, warum Du fleißig set-Methoden verwendest. Das muss so, sonst bekommt Doctrine nicht mit, dass sich Propertywerte ändern.

Das sieht tatsächlich so aus. Ist es aber gar nicht. Hinter der EntityManager Klasse befindet sich noch ein unaufgeräumtes Kinderzimmer. Ich habe mich allerdings von Doctrine inspirieren lassen. Allerdings nutze ich die Twig Engine für die View.

# EntityManager.php
use \ReflectionClass;
use \ReflectionProperty;
use \ReflectionException;
use \Exception;
use Btinet\Ringhorn\Logger;

class EntityManager
{
    protected $db;
    protected $entity;

    function __construct()
    {
       $this->db = new Database();
    }

    public function persist($entity, $id = false){
        self::generateReflectionClass($entity);
        $class_name = strtolower($this->entity->getShortName());
        foreach($this->entity->getProperties() as $property){
            foreach ($property as $key => $value){
                if($key == 'name'){
                    $rp = new ReflectionProperty($entity, $value);
                    if($rp->isInitialized($entity)){
                        $mvalue = ucfirst($value);
                        $method = "get$mvalue";

                        $data[$value] = $entity->$method();
                    }
                }
            }
        };
        if ($id){
            $row = $this->db->select("SELECT * FROM $class_name WHERE id = :id", ['id' => $id]);
            if ($row){
                return $this->db->update($class_name, $data, ['id' => $id]);
            }
        } else {
            return $this->db->insert($class_name, $data);
        }
    }

    public function remove($entity, $id)
    {
        self::generateReflectionClass($entity);
        $class_name = strtolower($this->entity->getShortName());
        if ($id) {
            $row = $this->db->select("SELECT * FROM $class_name WHERE id = :id", ['id' => $id]);
            if ($row) {
                return $this->db->delete($class_name, ['id' => $id]);
            }
        } else {
            return false;
        }
    }

    public function truncate($entity)
    {
        self::generateReflectionClass($entity);
        $class_name = strtolower($this->entity->getShortName());
        try {
            return $this->db->truncate($class_name);
        } catch (Exception $e){
            Logger::newMessage($e);
            Logger::customErrorMsg($e);
        }
    }

    protected function generateReflectionClass($entity){
        try {
            return $this->entity = new ReflectionClass($entity);
        } catch (ReflectionException $reflectionException){
            Logger::newMessage($reflectionException);
            Logger::customErrorMsg($reflectionException);
        }
    }

}

Der Manager setzt im Prinzip für jede initialisierte Property der Entity-Klassen die Werte für die SQL-Abfragen. Daher die ganzen set-Methoden.

Beste Grüße

vapita

[Raketenlagermeister]

Ich hab da was rumliegen, was das ein Formular für die Passworteingabe "baut" und die beiden Passwörter erst im Browser und dann auf dem Server checkt. Über die Qualität kann man sich streiten, es funktioniert aber:

Ansehen:

https://home.fastix.org/Tests/PWForm/ (Die Dateien mit der Endung '.txt' kann man sich anschauen.)

Testen:

https://home.fastix.org/Tests/PWForm/PWForm.php

Das Hashen käme, wenn der Passwort-Check erfolgreich ist.

[Peter Pan (no reg.)]

Hi,

https://home.fastix.org/Tests/PWForm/ (Die Dateien mit der Endung '.txt' kann man sich anschauen.)

Nur aus Neugier: warum nicht als *.phps?

[Raketenlagermeister]

Nur aus Neugier: warum nicht als *.phps?

weil ich

for s in *\.php; do ln "${s}" "${s}.txt"; done

getippt habe.

[Peter Pan (no reg.)]

Hi,

manchmal ist keine Antwort auch ok. Oder sogar besser.

[Raketenlagermeister]

Freilich hätte ich auch

for s in *\.php; do ln "${s}" "${s}s"; done

tippen können.

Aber ich bin mir nicht sicher was der Webserver (und dann der Browser von jeder{mann|frau|divers}, dann wömöglich das befasste OS ) damit das tun, was Du wohl im Auge hast.

[Rolf B]

Hallo Raketenlagermeister,

Aber ich bin mir nicht sicher was der Webserver (...) damit das tun, was Du wohl im Auge hast.

Das solltest Du aber sein, wenn es dein eigener Webserver ist. Denn damit der PHPS Abruf funktioniert, musst Du dort einen CGI/FastCGI Handler für PHPS Files eingerichtet haben, der PHP mit der -s Option ausführt.

Wobei es auch einen Trick mit einem Rewrite-Handler gibt, der auf ein Adapterscript umleitet, in dem highlight_file aufgerufen wird. Das spart das Kopieren von PHP nach PHPS, nur ist das auf einem öffentlich zugänglichen Server ein Einfallstor für Gift, Galle und Schabernack…

Aber das hab ich gerade mal zum Spaß bei mir lokal gemacht, dolle Sache 😀. Als alter Microsoftie natürlich auf dem IIS, aber das geht sicherlich auch im Indianerzelt.

Jetzt such ich nur noch einen in PHP geschriebenen Highlighter für HTML, CSS und JS - das sucht sich im Netz so schlecht. Die wollen mir alle in JS geschriebene Highlighter für PHP andrehen, ich mag aber keinen Node dahinterklemmen.

Rolf

[Auge]

Hallo

Jetzt such ich nur noch einen in PHP geschriebenen Highlighter für HTML, CSS und JS - das sucht sich im Netz so schlecht.

Früher™️ hätte ich wohl GeShi empfohlen, aber da dort seit über einem Jahr auch nichts mehr passiert ^[1], fällt mir das schwer.

Die wollen mir alle in JS geschriebene Highlighter für PHP andrehen, ich mag aber keinen Node dahinterklemmen.

highlight.js läuft im Browser und ist von seinem Lieferumfang her konfigurierbar.

Tschö, Auge

---

1. letzes Release: Oktober 2019, letzter Commit: Juni 2020 ↩︎

[Rolf B]

Hallo Auge,

jaaaa, okeeeee - ich wollt's halt am Server tun, um nicht auf JS im Browser angewiesen zu sein.

Aber ich schau es mir mal an.

Rolf

[vapita]

Hallo Auge,

jaaaa, okeeeee - ich wollt's halt am Server tun, um nicht auf JS im Browser angewiesen zu sein.

Aber ich schau es mir mal an.

Rolf

Hallo Rolf,

ich habe dies hier im Keller gefunden. Ein PHP Port von Highlight.js:

Highlight PHP

Beste Grüße

vapita

[Auge]

Hallo

jaaaa, okeeeee - ich wollt's halt am Server tun, um nicht auf JS im Browser angewiesen zu sein.

ich habe dies hier im Keller gefunden. Ein PHP Port von Highlight.js:

Highlight PHP

Schade, da ist seit Mai 2019 auch nichts mehr geschehen. 😕

edit: Hach Mist! Reingefallen!

Das verlinkte Repo ist ein Fork dieses Projekts und fand der letzte Commit im Januar dieses Jahres statt.

Tschö, Auge

[vapita]

Hallo Auge,

richtig. Dein Link stellt die bessere Wahl dar.

Beste Grüße

vapita

[Raktenlagermeister]

Jetzt such ich nur noch einen in PHP geschriebenen Highlighter für HTML, CSS und JS - das sucht sich im Netz so schlecht.

Früher™️ hätte ich wohl GeShi empfohlen, aber da dort seit über einem Jahr auch nichts mehr passiert [^1], fällt mir das schwer.

Jepp. Habe Geshi für mich selbst mehrfach umgeschrieben.

Erst, damit das Zeug CSS-Klassen statt Farbangaben ausspuckt, zuletzt auf das es mit PHP8 läufe.

Und ich verwende tunlichst einen eigenen Cache…

[Matthias Apsel]

Hallo Raktenlagermeister,

Erst, damit das Zeug CSS-Klassen statt Farbangaben ausspuckt,

HTML-Klassen 😜

Bis demnächst
Matthias

[kai345]

Jepp. Habe Geshi für mich selbst mehrfach umgeschrieben.

Erst, damit das Zeug CSS-Klassen statt Farbangaben ausspuckt, […]

Wäre mir zu viel Aufwand. Mir hat $geshi->enable_classes(); eigentlich immer gereicht, auch wenn die erzeugten Klassen-Namen nicht sehr schön sind.

[Henry]

Hallo Rolf,

Jetzt such ich nur noch einen in PHP geschriebenen Highlighter für HTML, CSS und JS - das sucht sich im Netz so schlecht.

Warum in PHP?

Die wollen mir alle in JS geschriebene Highlighter für PHP andrehen, ich mag aber keinen Node dahinterklemmen.

Kann ich verstehen, auch sonst so die üblichen Verdächtigen ,mit ihrem GitHub Gedönse usw.. Habe in den letzten Wochen aber ebenso verzweifelt gesucht und bin fündig gworden. Für mich der flexibelste und resourceschonende Highlighter, gleichzeitig auf Wunsch sogar Editor ist, Codemirror.

https://codemirror.net/

Gruss
Henry

[Raketenlagermeister]

WK1, Oberitalien.

[Raketenlagermeister]

Ich darf das. Ich bin der Urheber des Fotos.

[Matthias Apsel]

Hallo Raketenlagermeister,

WK1, Oberitalien.

Dobbiaco?

Bis demnächst
Matthias

[Raketenlagermeister]

Dobbiaco

Pflaumenschnapsberg

[Matthias Apsel]

Hallo Raketenlagermeister,

Dobbiaco

Pflaumenschnapsberg

Sacrario Militare di San Candido

Bis demnächst
Matthias

[Raketenlagermeister]

Dobbiaco

Pflaumenschnapsberg

Sacrario Militare di San Candido

Fast. Monte Grappa wäre superkorrekt gewesen.

[Raktenlagermeister]

Dobbiaco

Pflaumenschnapsberg

Sacrario Militare di San Candido

Fast. Monte Grappa wäre superkorrekt gewesen.

Ist halt nur kein Pflaumenschnaps, sondern ein Weinresteschnaps. Aber was geht mich Schnaps an?

[Matthias Apsel]

Hallo Raktenlagermeister,

ich meine, das wäre hier https://de.wikipedia.org/wiki/Innichen#/media/Datei:BeinhausInnichen.2008-06-28.png, aber am Monte Grappa war ich auch schon.

Bis demnächst
Matthias

[Raktenlagermeister]

Sieht nur ähnlich aus.

Die Anlage auf dem Monte Grappa ist tatsächlich sehr ähnlich, aber um Klassen größer…

Warum sich viele damit auskennen: Diese Kriegerfriedhöfe bieten den mit großen Fahrzeugen Reisenden von der EU und Deutschland finanzierte Toiletten, ruhige Parkplätze und (in Griechenland nicht zu empfehlen) Trinkasser...

[Matthias Apsel]

Hallo Raktenlagermeister,

Die Anlage auf dem Monte Grappa ist tatsächlich sehr ähnlich, aber um Klassen größer…

In der Tat. https://de.wikipedia.org/wiki/Monte_Grappa#/media/Datei:MONTE_GRAPPA_(6899070536).jpg Dort war ich, nach dem ich nun dieses Bild gesehen habe, tatsächlich noch nicht. Ich hatte einen großen Soldatenfriedhof mitten im Wald im Hinterkopf. Wo das aber nun genau war, kann ich grade nicht sagen.

Bis demnächst
Matthias

[Raketenlagermeister]

1. Frage:

Kann man irgendwie mit JS feststellen, ob der Benutzer z.B. dem Vorschlag des Mozillas für ein „sicheres Passwort“ gefolgt ist?

2. Die Demo hat ein paar Updates bekommen:

Zuerst einmal wird die „Kompliziertsanforderungen“ des Passwort clientseitig geprüft. Die „Kompliziertsanforderungen“ diskutiere ich nicht, weil diese Geschäftspolitik sind. Im Beispiel ist es: 8 Zeichen lang, Große (A-Z) und kleine (a-z) Buchstaben drin, außerdem Ziffern und „Sonderzeichen“ (Achtung: Umlaute gelten als solche)

Geprüft wird zunächst browserseitig mit den HTML5-Methoden (regulärer Ausdruck). Sind diese nicht verfügbar alternativ mit Javascript.

Auf jeden Fall wird nach der Prüfung der „Kompliziertsanforderungen" die Übereinstimmung mit Javascript geprüft und ggf. der Button, der den Request auslösen soll, bei durchweg positiven Tests aktiviert. (Bei deaktivierten Javascript bleibt der Button aktiv, ggf. greift dann noch HTML5 ein wenn die Anforderungen nicht erfüllt sind.)

Serverseitig wird nach Empfang nochmals geprüft ob die Passwörter kompliziert genug sind und ob diese übereinstimmen. Schließlich weiss niemand ob der Benutzer einen Browser benutzt oder etwas wie wget, curl und co.

Ergänzt habe ich eine Möglichkeit, sich die Inhalte der Passwortfelder für eine konfigurierbare Zeitspanne anzusehen.

Was nach wie vor nicht in dem Skript steht ist die nachfolgende Verwendung des positiv erkannten Passworts. Klar ist, dass hier die password_* Funktionen verwendet werden sollten. Allerdings sollte anno 2021 überlegt werden, ob man noch eine zweite Sicherheitsstufe einbaut (z.B. Versand eines geheimen, zufälligen Bestätigungs-Tokens an eine Telefonnummer (SMS), per Mail oder mit einem anderen Dienst) und Abfrage des Tokens über die Webseite. Das ist aber Geschäftspolitik und nicht Gegenstand meiner Betrachtungen.

Was nach wie vor gilt: Das ist nur Zeug, welches auf meiner „Festplatte“ herumlag. Es funktioniert und genügt als Demo oder Denkansatz - ist aber vom Verwender so umzubauen, dass es zu seinem Projekt passt (z.B. hinsichtlich der Lagerstätte der Voreinstellungen, weiterer Formular-Felder für Benutzername, Telefonnummer, Mailadresse und so weiter).

Im Übrigen verzichte ich auf Tests, ob denn irgendetwas anderes als Text übertragen wurde. Grund: „Wer sowas einträgt kann sich später auch gerne damit anmelden…“. Außerdem sind genug Funktionen (trim!) drin, die für eine „sonstwas zu String“-Konvertierung sorgen und entweder zu einem hartem Fehler oder zu einem leerem Ergebnis und damit zur Ungültigkeit und ergo zum Abbruch führen.

Warum trim()? Manche kopieren das Passwort aus einer Liste. Da könnten Spaces oder Zeilenumbrüche am Anfang oder Ende in die Eingabe geraten.

Die Demo:

https://home.fastix.org/Tests/PWForm/ (Die Dateien mit der Endung '.txt' kann man sich anschauen.)

Testen:

https://home.fastix.org/Tests/PWForm/PWForm.php

Das Hashen käme, wenn der Passwort-Check erfolgreich ist.