-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Argon2id Support for Basic Auth #7186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Example of caddy hash-password --plaintext "hiccup" --algorithm "argon2id"
$argon2id$v=19$m=47104,t=1,p=1$zJPvVe48N64JUa9MFlVhiw$b5Tznu0PxnA4TciY6qYe2BFPxncF1ePQaeNukHhH1cU Example of basic_auth /secret/* argon2id {
# Username "Bob", password "hiccup"
Bob $argon2id$v=19$m=47104,t=1,p=1$zJPvVe48N64JUa9MFlVhiw$b5Tznu0PxnA4TciY6qYe2BFPxncF1ePQaeNukHhH1cU
} In this Caddyfile we authenticate with |
See this tweet thread (that I happened to be involved in lmao) from people involved in the password hashing competition which chose argon many years ago: https://x.com/TerahashCorp/status/1155129705034653698 in the end argon is not as good as bcrypt for hashing under 1s which is absolutely the target for web scenarios (usually you want 0.3s or less so it doesn't feel bad for users waiting, and to not have too much load on servers with lots of concurrent users). We can certainly add argon support (news to me that go stdlib now has it, thought it was still would require an extra dependency), but I don't think we should change the default, bcrypt is still the best option. |
Totally fair point, Argon2id shows its real strength when you can allocate more time and memory, which isn’t always practical in web contexts. I ran some benchmarks that show the login with the default settings takes 0.026 seconds on my system. (first login, without cache). What do you think about ? |
We can allocate more memory but probably not much more time. Is there any way to tune it so that it takes at most 1/2 second to converge? |
Yes, there's these 4 new parameters:
Default values(match the recommended OWASP) are:
I didn't experience any slowness with this. My login was instant. |
Just to confirm, was this a very first login after starting the server? Caddy caches the results for future requests. Good chance this will get merged, but maybe after the next release (within a day or two I hope, if my computer is stable). |
Yes, this is for the very first login without any cache. Edit: Additional benchmarks (measured using
|
Makes sense to me. I'd been thinking we should lower the default bcrypt to 10 instead of 14. Maybe we do that now too. 14 is kinda ridiculously high lol, 12 or 13 is usually good for most apps with a traditional DB, but for basicauth the needs are not so equivalent and we had to do caching which is kinda cheating, to make it not hurt a lot. |
Sounds good to me. If there’s agreement, I’d be happy to include it in my PR. |
If the default bcrypt cost is changed, users of Caddy should be made aware of this change in the next release notes. Let's assume you have existing bcrypt hashes in you Caddy configuration, all of them with costs of 14. Now the default costs is changed to 12 and I guess the "FakeHash" is changed to 12 as well. This would allow a user enumaration because "fakehash" used for non existing users will be faster than a wrong password for an existing user with bcrypt cost of 14. |
Good point, to deal with that, we should probably ship fake hashes at each of the common cost factors and use the highest cost configured as the one to test against for unknown users. |
Yes, if we change the default bcrypt cost, then the fakehash must be updated in sync, otherwise there’s a measurable difference in response times that could lead to user enumeration. I think this is a separate concern from Argon2id support. To keep the PR clean and focused, it would make sense to handle the bcrypt cost change (and fakehash adjustment) in a dedicated PR. How that sound for you @francislavoie ? |
Yeah that's probably fine. Was just thinking it would make sense to sync the rough cost (time taken) from these argon settings to bcrypt's, but it's not an absolute blocker |
Absolutely, that makes a lot of sense. If I have some bandwidth this week, I can open a separate PR to handle that change. In the meantime, let me know if there’s anything else needed to get this branch ready for merge. |
Hello @mholt @francislavoie |
@GreyXor Sorry for the delay; can you rebase to resolve the conflicts? I am not super in favor of changing the default bcrypt cost (even in a separate PR) -- it adds a lot of complexity for very little / no gain, since the cache prevents slowdowns after the first login. |
Hello 👋,
According to the OWASP Password Storage Cheat Sheet, the recommended modern algorithm for password hashing is Argon2id, with bcrypt only being considered a legacy option.
Currently, Caddy’s
basicauth
directive only supports bcrypt. While bcrypt is still reasonably safe with sufficient work factor, Argon2id provides stronger protection against modern hardware attacks (GPU/ASIC/FPGA), thanks to its memory-hard design.Motivation
Proposal
This PR adds Argon2id support as a handler alongside bcrypt, with recommended defaults:
Implementation notes
golang.org/x/crypto/argon2
.$argon2id$v=19$m=…
).Backward compatibility
basicauth
configs continue to work unchanged.bcrypt
remains the default algorithm.Related doc/website PR : caddyserver/website#486