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

Password hashing and validation problems #188

Open
aAlexanderLeben opened this issue Jul 15, 2024 · 1 comment
Open

Password hashing and validation problems #188

aAlexanderLeben opened this issue Jul 15, 2024 · 1 comment

Comments

@aAlexanderLeben
Copy link

Describe the bug

There is no pre-validation of the main.extra.new_password from config_panel.toml which leads to errors at installation step.

Context

  • Hardware: Raspberry Pi at home
  • YunoHost version: 11.2.20.2 (stable)
  • I have access to my server: Through SSH | through the webadmin
  • Are you in a special context or did you perform some particular tweaking on your YunoHost instance?: no
    • If yes, please explain: n/a
  • Using, or trying to install package version/branch: 0.107.52~ynh1
  • If upgrading, current package version: n/a
  • Is DNS over HTTP or DNS over QUIC activated?: yes

Steps to reproduce

  1. Navigate to https://<your_yunohost>/yunohost/admin/#/apps/install/adguardhome in your browser
  2. Fill in the required fields
  3. In the password field, enter a password containing non-ASCII characters, e.g. VWÎQF¯ê6Ò´3ÜÂû°QOÔQà8±©¨6àaÕ¨Há§ïN¨ZµÆøYpsVçiUBò¼T¸H²NügÙÒ¸ûÃ6¤0
  4. Try to install the app
  5. Watch installation fail way into the installation

Expected behavior

A check of the password should be performed before attempting an installation.

Logs

https://paste.yunohost.org/raw/uxiduferob

@aAlexanderLeben
Copy link
Author

Problems occur in install#L102 when calling

password=$(python3 -c "import bcrypt; print(bcrypt.hashpw(b\"$password\", bcrypt.gensalt(rounds=10)).decode())")

Problem 1: Non-ASCII characters

Trying to construct a byte literal using b\"$password\" produces a syntax error, e.g.

>>> test_byte = b"ê"
  File "<stdin>", line 1
    test_byte = b"ê"
                ^^^^
SyntaxError: bytes can only contain ASCII literal characters

From https://docs.python.org/3/reference/lexical_analysis.html#string-and-bytes-literals

Bytes literals [...] may only contain ASCII characters;

Problem 2: Passwords longer than 72 characters are truncated

See https://github.com/pyca/bcrypt/tree/main?tab=readme-ov-file#maximum-password-length

>>> import bcrypt
>>> salt = bcrypt.gensalt(rounds=10)

>>> ok_password = b"2wvKFD7C0FbfATxP9aK5eWa2BbiwEuaFjRkM4tSo6c72atn20CJYV2NRBDrfVvg451ZHbaYK"
>>> problematic_password = ok_password + b"padding"

>>> bcrypt.hashpw(ok_password, salt) == bcrypt.hashpw(problematic_password, salt)
True

Possible Solution

Add validation to config_panel.toml, not very sure about the syntax, but you should get the idea. Maybe a lower limit should be introduced as well. And maybe there are more perks to it that I did not notice.

Eplaination of the regexp [ -~] can be found on https://catonmat.net/my-favorite-regex

[main.extra.new_password]
pattern.regexp = '[ -~]{,72}'
pattern.error.en = "Your password must only contain ASCII characters and cannot have more than 72 characters

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

1 participant