Skip to content
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

How can we make argon2 safe? #14702

Open
zeripath opened this issue Feb 16, 2021 · 9 comments
Open

How can we make argon2 safe? #14702

zeripath opened this issue Feb 16, 2021 · 9 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@zeripath
Copy link
Contributor

zeripath commented Feb 16, 2021

Argon2 is clearly causing multiple problems due to its memory requirements, especially on lower spec'd systems.

I cannot recommend argon2 in good conscience at present as its memory use can be unbounded especially under LFS.

Ref #14674

@zeripath
Copy link
Contributor Author

Ok there are multiple options here, any and potentially all of which should be implemented:

  • Act like GitHub and ban basic auth with passwords - require tokens instead.
  • Expose and make configurable the parameters used for hashing.
  • Add a max concurrent lock around password hashing.

I understand a user on discord is looking in to the second option right now - but I think we seriously need to consider both of the other options in addition.

@zeripath zeripath added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Feb 16, 2021
@lunny
Copy link
Member

lunny commented Feb 17, 2021

Argon2 memory usage could be configured by Gitea so that users could chose suitable variable according his system.

@zeripath
Copy link
Contributor Author

That is point 2: Expose and make configurable the parameters used for hashing.

@vladionescu
Copy link

Moving my comment over from #14294 (comment) (sorry! found that via the PR linked in latest release notes)

Mostly interested in what the symptoms are here that we're trying to solve by ditching Argon2.

What issues have we seen from this memory usage? I personally haven't encountered slowdowns, but my instance only has ~20 users so I am certainly not running a very busy Gitea.

Like a good KDF, Argon2 is designed to not be performant. If it was performant, it would be easy for attackers to brute force plaintext passwords should they ever get their hands on the hashes.

That said, it's also not meant to be a drag on the whole system. It has configurable parameters that influence memory usage, among other things. Have we considered tweaking those knobs or exposing them to admins, so they can be turned down for more resource constrained deployments?

@jolheiser
Copy link
Member

It's pertinent to point out that you can still use argon2, it just isn't the default anymore.

@zeripath
Copy link
Contributor Author

zeripath commented Mar 4, 2021

OK I absolutely do not want this issue to turn into other issues that go round in circles without solutions.

So further comments that do not address the memory issues with argon2 and the solutions proposed above or propose other solutions WILL be deleted.

Mostly interested in what the symptoms are here that we're trying to solve by ditching Argon2.

It is not prudent to explain further than I have stated above: its memory use can be unbounded especially under LFS.

@vladionescu If you want further explanation come to discord and chat to me directly.

@zeripath

This comment has been minimized.

@zeripath
Copy link
Contributor Author

zeripath commented Mar 8, 2021

Fundamentally the issue is that hashing passwords is supposed to be expensive and has to be expensive in order to provide protection against cracking. Changing algorithm and adjusting its parameters etc. is not going to form a complete solution to this problem - it may well form part of the solution in order to tune parameters for running systems and allow more concurrent password checks - but it is not the solution.

If we allow unlimited password validation we are going to run out resources simply because password validation is expensive.

Our only options are to limit the amount of concurrent password validation actions on the server and to move users of the API and HTTP from using passwords to using tokens which are far less expensive to validate.

@uwejan
Copy link

uwejan commented Jun 20, 2023

Hey guys, I am oticing an increase of memory consumtion of 4 MB on a single user request to verify a password?
That is too much is not it. is thre a way around this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

5 participants