Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

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

Closed
skolarianer opened this issue Jul 4, 2018 · 14 comments
Closed
Assignees
Labels
Milestone

Comments

@skolarianer
Copy link

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:
contao/core-bundle#481
contao/core-bundle#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
Copy link
Member

leofeyer commented Jul 5, 2018

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
Copy link
Member

leofeyer commented Jul 5, 2018

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

@leofeyer
Copy link
Member

leofeyer commented Jul 9, 2018

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
Copy link
Member

Toflar commented Jul 9, 2018

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
Copy link
Member

ausi commented Jul 9, 2018

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
Copy link
Member

Toflar commented Jul 9, 2018

Yes 👍

@aschempp
Copy link
Member

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 symfony/symfony#27910

@aschempp
Copy link
Member

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
Copy link
Member

das Cookie und die DB enthalten denselben Hash

Bei mir nicht?

@aschempp
Copy link
Member

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
Copy link
Member

Nein, bei mir nicht.

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

@aschempp
Copy link
Member

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

@leofeyer
Copy link
Member

Ja, bei mir auch. 😄

@leofeyer
Copy link
Member

leofeyer commented Sep 7, 2018

@leofeyer leofeyer added this to the 3.5.36 milestone Sep 7, 2018
@leofeyer leofeyer added the defect label Sep 7, 2018
@leofeyer leofeyer self-assigned this Sep 7, 2018
@leofeyer leofeyer closed this as completed Sep 7, 2018
leofeyer added a commit that referenced this issue Sep 10, 2018
Description
-----------

This PR implements #8888.

Commits
-------

bcdc91a Hash the autologin token in the database (see #8888)
leofeyer added a commit to contao/contao that referenced this issue Sep 10, 2018
Description
-----------

This PR implements contao/core#8888 for Contao 4.4.

Commits
-------

c084683 Hash the autologin token in the database (see contao/core#8888)
45d4fe5 Use "kernel.secret" instead of "encryptionKey"
leofeyer added a commit to contao/core-bundle that referenced this issue Sep 10, 2018
Description
-----------

This PR implements contao/core#8888 for Contao 4.4.

Commits
-------

c0846830 Hash the autologin token in the database (see contao/core#8888)
45d4fe5f Use "kernel.secret" instead of "encryptionKey"
leofeyer added a commit to contao/contao that referenced this issue Sep 12, 2018
Description
-----------

This PR implements contao/core#8888 for Contao 4.6. Thanks to our database token provider, the implementation was not too hard. 😄

Commits
-------

e518eed Hash the autologin token in the database (see contao/core#8888)
435a310 Fix the tests
leofeyer added a commit to contao/core-bundle that referenced this issue Sep 12, 2018
Description
-----------

This PR implements contao/core#8888 for Contao 4.6. Thanks to our database token provider, the implementation was not too hard. 😄

Commits
-------

e518eedc Hash the autologin token in the database (see contao/core#8888)
435a3109 Fix the tests
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants