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

Remove arbitrary restrictions on passwords #1285

Closed
omentic opened this issue Jun 16, 2023 · 7 comments
Closed

Remove arbitrary restrictions on passwords #1285

omentic opened this issue Jun 16, 2023 · 7 comments

Comments

@omentic
Copy link

omentic commented Jun 16, 2023

Describe the bug

I went to register for a new account on a Matrix server, only to be greeted by the following screen:

2023-06-15-190237

This is generally considered bad practice and is not required by the Matrix spec. I suggest removing all restrictions and instead warning for bad practices, similar to what Element does. Though Element also uses a better heuristic: which boils down to length good, dictionary words bad

Reproduction

No response

Expected behavior

No response

Platform and versions

n/a

Additional context

No response

@kfiven
Copy link
Collaborator

kfiven commented Jun 16, 2023

https://spec.matrix.org/v1.7/client-server-api/#notes-on-password-management says:

Clients SHOULD enforce that the password provided is suitably complex. The password SHOULD include a lower-case letter, an upper-case letter, a number and a symbol and be at a minimum 8 characters in length. Servers MAY reject weak passwords with an error code M_WEAK_PASSWORD.

@omentic
Copy link
Author

omentic commented Jun 16, 2023

Oh, that's interesting. That is bad practice and should be removed from the spec, see matrix-org/matrix-spec-proposals#2000 (comment)

I wonder why Element doesn't follow that. Maybe someone changed it and forgot the spec mentioned it?

@kfiven
Copy link
Collaborator

kfiven commented Jun 18, 2023

I could find any discussion changing it. Don't know why element is doing that way but correct way to get this fixed everywhere would be by amending the spec.

@omentic
Copy link
Author

omentic commented Jun 20, 2023

After some discussion in the matrix-spec channel, the consensus is that it's not worth changing it in the spec due to matrix-org/matrix-spec-proposals#3861 coming rather soon. I do think that Cinny should absolutely remove the hard restrictions though - and only suggest creating a stronger password via https://github.com/dropbox/zxcvbn (what Element uses) or similar.

Notably the conclusion was that the spec is in fact bad here, for the reasons I linked to in my comment above! The basic idea is that if you are an attacker trying to bruteforce a password, the only things that are relevant are 1) length and 2) not being a "common" password, where common means made out of dictionary words or on a common-password list (because that cuts way down on the bruteforcing needed).

And from a usability side it's not great because I need to switch to Element just to make my account 🙁

@kfiven
Copy link
Collaborator

kfiven commented Jun 20, 2023

My password manager at default easily creates such password so I don't see much of usability issue here. Regarding the zxcvbn, we have a PR, but it has some issues.

And as already raised on that PR hard restrictions as compared to zxcvbn doesn't effect the loading time of app during login/sign up, so IMO it's a net positive trade-off.

@omentic
Copy link
Author

omentic commented Jun 20, 2023

Would you be fine removing the hard restrictions? My password manager creates passwords that do conflict by default (long, random ASCII streams).

@deltanedas
Copy link

having a limit makes no sense, so what if i want it to be 128 chars long instead of 127

@ajbura ajbura mentioned this issue Jan 21, 2024
9 tasks
@kfiven kfiven closed this as completed Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ URL navigation and New UI
Development

No branches or pull requests

4 participants