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

Investigate text encoding for passwords #186

Open
dkess opened this issue Sep 21, 2019 · 4 comments
Open

Investigate text encoding for passwords #186

dkess opened this issue Sep 21, 2019 · 4 comments

Comments

@dkess
Copy link
Member

dkess commented Sep 21, 2019

Interesting rootspam from last night:

 An exception occured in ocfweb:

Traceback (most recent call last):
  File "/opt/ocfweb/venv/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/opt/ocfweb/ocfweb/auth.py", line 53, in wrapper
    return fn(request, *args, **kwargs)
  File "/opt/ocfweb/ocfweb/account/register.py", line 75, in request_account
    RSA.importKey(CREATE_PUBLIC_KEY),
  File "/opt/ocfweb/venv/lib/python3.7/site-packages/ocflib/account/creation.py", line 435, in encrypt_password
    return RSA_CIPHER.encrypt(password.encode('ascii'))
UnicodeEncodeError: 'ascii' codec can't encode characters in position 0-5: ordinal not in range(128)

Request:
  * Host: www.ocf.berkeley.edu
  * Path: /account/register/
  * Method: POST
  * Secure: True

I assume this is from someone trying to use non-ascii character in their password. I am not sure what the best practices for this are, but we should investigate this further and see if we can avoid using the ascii encoding.

@zpfeiffer
Copy link

I'm also not sure what the best practice here is but as far as preventing this error goes it seems to be a problem with the password validation. validate_password should reject non-ascii characters here, but validate_password isn't called until the NewAccountRequest is created, by which point the non-ascii characters have already been passed to encrypt_password.

Perhaps a short term fix could be to validate the password before creating the NewAccountRequest and a long term fix could be to change the encoding used for encrypted passwords to something more general like password.encode('base64','strict') (assuming that no other software requires ascii-only passwords).

@dkess
Copy link
Member Author

dkess commented Oct 4, 2019

You have to be careful with full Unicode passwords since you don't want to allow passwords that users won't be able to type. Unicode is full of surprises and edge cases. For now, let's stick with ASCII and if you want to go beyond that, do some research on the state-of-the-art and implement what's done elsewhere in a separate, future commit.

@bzh-bzh
Copy link
Contributor

bzh-bzh commented Oct 4, 2019

Tfw no emoji passwords 😭

@emmatyping
Copy link
Member

emmatyping commented Oct 4, 2019

TIL https://tools.ietf.org/html/rfc8265

E: also I have someone in mind to ask about sources of how to handle Unicode correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants