contao / core

Contao 3 → see contao/contao for Contao 4
GNU Lesser General Public License v3.0
491 stars 213 forks source link

Das autologin-Token sollte gehasht in tl_member gespeichert werden #8888

Closed skolarianer closed 6 years ago

skolarianer commented 6 years ago

Das autologin-Token wird derzeit (Contao 3.5.35) im Klartext in tl_member gespeichert. Da mit diesem Token der Zugriff auf Nutzerkonten möglich ist, ist es IMHO als passwort-ähnlich einzustufen und sollte auch entsprechend behandelt werden.

Folgende PRs habe ich zu dem Thema gefunden: https://github.com/contao/core-bundle/pull/481 https://github.com/contao/core-bundle/pull/685

Als Zwischenlösung sollte doch auch Contao 3 durch das Hashen des autologin-Tokens besser abgesichert werden können.

Gemäß den folgenden Links kann hierzu unter bestimmten Voraussetzungen auch ein schwacher Hash-Algorithmus ausreichend sein. Ich bin mir allerdings nicht sicher, ob man dieser Empfehlung folgen sollte.

Mehr Infos zum Hashen von Tokens: https://stackoverflow.com/a/477578/7999799 (PART II) https://security.stackexchange.com/a/63438

leofeyer commented 6 years ago

Wie im Mumble am 5. Juli besprochen, wird @aschempp ein Ticket bei Symfony erstellen und die Problematik schildern, um herauszufinden, ob Symfony dieses Problem ebenfalls hat.

leofeyer commented 6 years ago

Das Problem würde dann auch alle anderen Tokens betreffen (activation, subscription etc.).

leofeyer commented 6 years ago

Symfony scheint das RememberMe-Token bereits zu hashen, somit müssen wir uns ab Contao 4.5 nur noch um unsere eigenen Token kümmern.

@contao/developers Was hatten wir nochmal besprochen bezüglich Hashing mit dem Secret?

Toflar commented 6 years ago

Ich würde das Secret zusätzlich zum Hash hinzufügen. Einfach weil das nochmal einen Sicherheitsfaktor mehr gibt. Also wenn man die DB und die Hashes hat, könnte man theoretisch ja (z.B. durch zugegeben ziemlich aufwändigem Rainbow-Table-Gedöns) das gültige Token finden. Mit dem Secret hätte man eine 2. Unbekannte und es wäre daher eigentlich unmöglich.

Also so:

$tokenHash = hash('sha256', $token . $secret);
ausi commented 6 years ago

Anstatt das Secret anzuhängen sollten wir wahrscheinlich besser eine hash Funktion verwenden die dafür vorbereitet ist wie hash_hmac().

Z. B. so wie wir es im Captcha-Cookie gemacht haben: https://github.com/contao/core-bundle/blob/d43ecb77af384633334a6c296d3f3db3e502df18/src/Resources/contao/forms/FormCaptcha.php#L173

Toflar commented 6 years ago

Yes 👍

aschempp commented 6 years ago

Wie im Mumble am 5. Juli besprochen, wird @aschempp ein Ticket bei Symfony erstellen und die Problematik schildern, um herauszufinden, ob Symfony dieses Problem ebenfalls hat.

See https://github.com/symfony/symfony/issues/27910

aschempp commented 6 years ago

Symfony scheint das RememberMe-Token bereits zu hashen, somit müssen wir uns ab Contao 4.5 nur noch um unsere eigenen Token kümmern.

Diese Aussage ist übrigens nicht korrekt, das Token wird aus einem Hash generiert, aber das Cookie und die DB enthalten denselben Hash (bzw. random bytes).

leofeyer commented 6 years ago

das Cookie und die DB enthalten denselben Hash

Bei mir nicht?

aschempp commented 6 years ago

Das Cookie enthält eine Kombination von Series und Token (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/RememberMe/PersistentTokenBasedRememberMeServices.php#L115), aber der Token-Wert ist derselbe wie in der DB.

leofeyer commented 6 years ago

Nein, bei mir nicht.

Cookie-Wert: dStXenBqKzdMOFZXbk5BWUxLSkJyMDFUZjNNb1BiVkRIUnJ2T2k… DB-Series: u+Wzpj+7L8VWnNAYLKJBr01Tf3MoPbVDHRrvOi4c… DB-Value: qzI+Ia9lMYBtt08gkFmI778OYaVyHjcaUBkjnHL

aschempp commented 6 years ago

Hast du meinen Kommentar bzw. den Code gelesen? Cookie-Wert = base64 implode von series + token

leofeyer commented 6 years ago

Ja, bei mir auch. 😄

leofeyer commented 6 years ago