Fixed #26033 -- Added argon2 password hasher. #5876

bwesterb · 2015-12-26T12:22:43Z

argon2 is the winner of the Password Hashing Competition.

There are two python bindings available for argon2: argon2 and argon2-cffi. The first uses ctypes and the latter cffi. As cffi works for most environments, I choose that one.
argon2 has four parameters: time_cost, memory_cost, parallelism and type. I choose to copy the defaults of argon2-cffi. Section 9 of the argon2 specification discusses recommended parameters.

https://code.djangoproject.com/ticket/26033#ticket

timgraham · 2015-12-26T22:56:46Z

Thanks for the patch. Design decisions (such as the choice of what algorithms to support in core) are usually made on the django-developers mailing list. I would post this idea there to get feedback.

bwesterb · 2015-12-26T23:37:06Z

Ok thanks. Post.

hynek · 2016-01-02T17:32:32Z

Since Florian mentioned he’d like some feedback from me or @dstufft on the mailing list:

Sadly I can’t really comment on the Django portions but the usage of Argon2 libraries is rather straight forward as long as you stay away from the argon2.low_level.core() binding. The authors mention in the paper that no parameter value is actually insecure.

I have two minor general comments:

I wouldn’t test with a time cost of 8. :) If you want to check whether arguments get propagated, I would recommend to go down to 1 and not up lest you wanna slow down your test suite.
If you want to keep using argon2-cffi’s defaults, you can also just use the relevant constants.

bwesterb · 2016-01-03T13:50:42Z

I decreased the time cost for the test from 8 to 1 as suggested by @hynek.
I am ok with using argon2 constants directly. However, this would be different from the other PasswordHashers that hardcode their default. We can always change this after it has been merged.

This PR is ready to be merged.

apollo13 · 2016-01-03T20:29:36Z

Is there a specific reason to go with the low_level api?
Edit:// Nevermind, looks as if the highlevel API doesn't allow custom salts…

apollo13 · 2016-01-03T20:41:45Z

django/contrib/auth/hashers.py

Please use argon2.DEFAULT_HASH_LENGTH here,

Done. We do have to be careful: if argon2.DEFAULT_HASH_LENGTH is increased significantly in the future, encode can return a string longer than the maximum 128.

bwesterb · 2016-01-04T08:47:31Z

@apollo13 argon2-cffi's high-level API (argon2.PasswordHasher) does not allow one to specify a salt (on purpose). It generates a new random salt itself. As Django's API wants to generate a salt itself, we use the lower-level API.

timgraham · 2016-01-05T19:06:47Z

django/contrib/auth/hashers.py

Please use 4 space hanging indent:

argon2.low_level.hash_secret( force_bytes(password), ..., )

bwesterb · 2016-01-07T16:49:41Z

@timgraham I saw you set the "missing documentation" label on the issue tracker. Could you help me out and tell me which documentation I should write?

timgraham · 2016-01-07T16:52:40Z

docs/topics/auth/passwords.txt -- see the similar docs for bcrypt there. Also a mention in the 1.10 release notes. p.s. You can squash commits too.

timgraham · 2016-01-07T16:55:23Z

django/contrib/auth/hashers.py

As above, please use hanging indent.

bwesterb · 2016-01-07T21:58:56Z

@timgraham Thanks. I added the documentation and release notes. Please feel free to change the wording, tone and contents as you like.

Before you merge, I will squash the commits. (For now it is easier to keep track of what changed in the PR.)

By the way, UnsaltedSHA1PasswordHasher and UnsaltedMD5PasswordHasher were missing in the documentation. Was this on purpose. I've added them for the moment, though.

aaugustin · 2016-01-08T08:40:11Z

docs/releases/1.10.txt

"the other entries" isn't entirely accurate; very few websites well need to keep anything other than PBKDF2PasswordHasher, the default hasher since Django got its current password hashing mechanism.

I would say:

PASSWORD_HASHERS = [ 'django.contrib.auth.hashers.Argon2PasswordHasher', 'django.contrib.auth.hashers.PBKDF2PasswordHasher', ]

If your database contains password encoded with other hashers, you need to add them to the list, or else the corresponding users won't be able to log in. See the documentation of :setting:PASSWORD_HASHERS for details.

Some people might blindly copy and paste the displayed code to enable Argon2, without reading the remark after it. It does not hurt to list all hashers, so that seems to me to be the safe choice.

This is also how the documentation is written to enable BCrypt. See topics/auth/passwords.txt.

Off course, I could change it everywhere.

Keeping it consistent is a reasonable choice for now. Leave it as is.

I would link to "argon2_usage" instead of repeating the example usage parts of it in the release notes.

I would link to "argon2_usage" instead of repeating the example usage parts of it in the release notes.

Done.

timgraham · 2016-02-03T19:14:26Z

docs/topics/auth/passwords.txt

I'm going to suggest some edits to move argon2 discussion to a separate section with a few todos that I hope can complete: https://dpaste.de/aaF5 You can apply the patch with patch -p1 -i raw.diff. Thanks, this is coming along!

Done. I added some text on choosing parameters.

timgraham · 2016-02-05T14:54:20Z

docs/topics/auth/passwords.txt

Should we say what the defaults are? Should we consider increasing the defaults for each major release of Django like we do for PBKDF2? https://docs.djangoproject.com/en/dev/internals/howto-release-django/#new-stable-branch-tasks
Please use 1 space between sentences.

It is a good idea to review the defaults for each major release. The analogue of the current rule would be to increase memory_cost with approximately 20% for each major release. Maybe it would be a better idea to use actual hardware as a reference: adjust the parameters such that the hash takes a certain amount of milliseconds on the cheapest Amazon EC2 instance.

Maybe coordinate changing the defaults with @hynek ?

The defaults aren’t too likely to change very frequently since even the lowest settings are “safe enough”. Choosing the target verification time is highly individual so I guess the best approach for Django is to decide on a time and tune all checkers to meet it so people don't have to think about it? My own target was roughly 0.5ms–1ms “on my notebook”.

timgraham · 2016-03-01T19:20:17Z

Could you add a harden_runtime() method based on the patch in today's security release? 67b46ba

bwesterb · 2016-03-02T13:53:55Z

The problem. The runtime of PBKDF2 is practically linear in the number of iterations. For argon2, it is more complex. If I am not mistaken, it's runtime is proportional to

time_cost * memory_cost / max(available_threads, parallelism) + memory_cost * constant1 + constant2

The first term is for the main iteration of argon2, which goes over all blocks of memory time_cost times. The second is for the compression of all memory into one final block (B_final in the spec.) at the end whose hash will be the result of argon2. The third is for the computation of the initial hash of the parameters (H_0 in the spec.)

Suppose we double the time_cost. Then argon2 will run just shy of twice as fast: the second and the third term are the difference. Here is some simple code that verifies this: https://gist.github.com/bwesterb/fe0e8be93c978aaa911d The difference is even bigger than I expected: 10%.

What to do?
I see a few possibilities

Use a no-op harden_runtime and live with the issue.
Let Django compute the expected runtime of the default password hash beforehand and let the harden_runtime sleep (or spinwait) the difference.
Instead of changing the parameters of argon2, simply iterate argon2 itself. (In other words: use PBKDF2(argon2))

The last would be the best mitigation of the issue at hand, but would over time give us a hash which is a lot weaker than it could have been. Option 2 would also work if a hash is replaced by a different, but slower hash (instead of just when the parameters are changed).

@timgraham

apollo13 · 2016-03-02T16:02:13Z

I am leaning towards option one. The security fix is mainly for pbkdf2
where we constantly increase iterations. I do not think that algos like
argon 2 need such an aggressive approach (I have to read up on argon
though, but for bcrypt adding 1 to the work factor means doubling the
workload -- not something you'd do lightly)

On Wed, Mar 2, 2016, 14:54 Bas Westerbaan notifications@github.com wrote:

The problem. The runtime of PBKDF2 is practically linear in the number
of iterations. For argon2, it is more complex. If I am not mistaken, it's
runtime is proportional to

time_cost * memory_cost / max(available_threads, parallelism) + memory_cost * constant1 + constant2

The first term is for the main iteration of argon2, which goes over all
blocks of memory time_cost times. The second is for the compression of
all memory into one final block (B_final in the spec.) at the end whose
hash will be the result of argon2. The third is for the computation of the
initial hash of the parameters (H_0 in the spec.)

Suppose we double the time_cost. Then argon2 will run just shy of twice
as fast: the second and the third term are the difference. Here is some
simple code that verifies this:
https://gist.github.com/bwesterb/fe0e8be93c978aaa911d The difference is
even bigger than I expected: 10%.

What to do?
I see a few possibilities

Use a no-op harden_runtime and live with the issue.

Let Django compute the expected runtime of the default password
hash beforehand and let the harden_runtime sleep (or spinwait) the
difference.

Instead of changing the parameters of argon2, simply iterate argon2
itself. (In other words: use PBKDF2(argon2))

The last would be the best mitigation of the issue at hand, but would over
time give us a hash which is a lot weaker than it could have been. Option 2
would also work if a hash is replaced by a different, but slower hash
(instead of just when the parameters are changed).

—
Reply to this email directly or view it on GitHub
#5876 (comment).

Argon2 is the winner of the Password Hashing Competition. https://password-hashing.net There are two python bindings available for Argon2: argon2 and argon2-cffi. The first uses ctypes and the latter cffi. As cffi works for most environments, I choose that one. Argon2 has four parameters: time_cost, memory_cost, parallelism and type. I choose to copy the defaults of argon2-cffi. Section 9 of the argon2 specification discusses recommended parameters. https://password-hashing.net/argon2-specs.pdf

bwesterb · 2016-03-05T11:07:16Z

@timgraham I added a no-op harden_runtime as suggested by @apollo13

timgraham · 2016-03-08T16:22:42Z

merged in b4250ea, thanks!

timgraham · 2016-04-19T15:44:38Z

We have a test failure with the release of argon2_cffi 16.1 which changes the hash and adds an extra dollar sign which causes problems with Django trying to split it. How do you think we should proceed? I guess updating Django and requiring the latest version of argon2_cffi might be okay. I just hope this won't be an ongoing issue where argon2_cffi makes changes that aren't compatible?

hynek · 2016-04-19T15:52:37Z

Argon2 has a new version 1.3 that introduced a version tag.

My suggestions:

fix the hash in your tests.
you can check what version the hash is and use that information for opportunistic re-hashing

JFTR Hashes are backward compatible. You can verif old hashes with the new release.

bwesterb · 2016-04-19T15:53:15Z

I will write a patch for this this week.

hynek · 2016-04-19T15:54:10Z

(Oh and I didn't change anything; it's the standard/official bindings that evolved.)

bwesterb · 2016-04-22T11:40:49Z

I created a PR: #6489

apollo13 reviewed Jan 3, 2016
View reviewed changes

timgraham changed the title ~~Add argon2 password hasher~~ Fixed #26033 -- Added argon2 password hasher. Jan 4, 2016

timgraham reviewed Jan 5, 2016
View reviewed changes

dougthor42 mentioned this pull request Jan 7, 2016

Plans to add Argon2 as a password hashing backend? pyca/cryptography#2643

Closed

timgraham reviewed Jan 7, 2016
View reviewed changes

bwesterb force-pushed the argon2 branch from 8d0157f to 008e0d9 Compare January 7, 2016 22:17

aaugustin reviewed Jan 8, 2016
View reviewed changes

bwesterb force-pushed the argon2 branch from 008e0d9 to 89d4bfc Compare February 3, 2016 11:49

timgraham reviewed Feb 3, 2016
View reviewed changes

bwesterb force-pushed the argon2 branch 2 times, most recently from 2d50fa6 to 57cf060 Compare February 4, 2016 19:27

bwesterb force-pushed the argon2 branch 2 times, most recently from 93c8ed5 to abc48a3 Compare February 4, 2016 19:36

timgraham reviewed Feb 5, 2016
View reviewed changes

bwesterb force-pushed the argon2 branch from abc48a3 to 7fb6b36 Compare February 6, 2016 11:30

bwesterb force-pushed the argon2 branch from 7fb6b36 to e43177c Compare February 28, 2016 13:15

bwesterb force-pushed the argon2 branch from e43177c to febdcf9 Compare March 5, 2016 11:06

timgraham closed this Mar 8, 2016

Uh oh!

Fixed #26033 -- Added argon2 password hasher. #5876

Fixed #26033 -- Added argon2 password hasher. #5876

Uh oh!

Conversation

bwesterb commented Dec 26, 2015

Uh oh!

timgraham commented Dec 26, 2015

Uh oh!

bwesterb commented Dec 26, 2015

Uh oh!

hynek commented Jan 2, 2016

Uh oh!

bwesterb commented Jan 3, 2016

Uh oh!

apollo13 commented Jan 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwesterb commented Jan 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwesterb commented Jan 7, 2016

Uh oh!

timgraham commented Jan 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwesterb commented Jan 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bwesterb Feb 3, 2016 via email

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timgraham commented Mar 1, 2016

Uh oh!

bwesterb commented Mar 2, 2016

Uh oh!

apollo13 commented Mar 2, 2016

Uh oh!

bwesterb commented Mar 5, 2016

Uh oh!

timgraham commented Mar 8, 2016

Uh oh!

timgraham commented Apr 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hynek commented Apr 19, 2016

Uh oh!

bwesterb commented Apr 19, 2016

Uh oh!

hynek commented Apr 19, 2016

Uh oh!

bwesterb commented Apr 22, 2016

timgraham commented Apr 19, 2016 •

edited

Loading