Skip to content

Support non-ASCII passwords in basic_auth - #1782

Closed
Bountybette wants to merge 1 commit into
python-websockets:mainfrom
Bountybette:basic-auth-non-ascii
Closed

Bountybette wants to merge 1 commit into
python-websockets:mainfrom
Bountybette:basic-auth-non-ascii

Conversation

@Bountybette

Copy link
Copy Markdown

Checklist

  • I personally discussed the issue with a maintainer before opening this pull request. I understood and followed their requirements.
  • I reviewed the design choices and implementation details of the changes. I can explain them without relying on an AI coding agent.
  • I wrote a short, specific pull request description that makes it easy for maintainers to understand the problem and the solution.
  • I am aware that maintainers have AI coding agents. Merely throwing an AI at an issue wastes their time. It comes across as a lack of respect.

Problem

With basic_auth(credentials=("bob", "pässword")), bob can never log in: the server answers HTTP 500 and logs "process_request failed" with TypeError: comparing strings with non-ASCII characters is not supported, because hmac.compare_digest only accepts ASCII str. The same 500 happens for any client that sends a non-ASCII password for a known username, and for credentials that aren't valid UTF-8, where parse_authorization_basic lets UnicodeDecodeError escape. RFC 7617 test vectors with 123£ already parse fine, so only the comparison breaks.

Solution

Compare UTF-8 encoded bytes in the hard coded check_credentials of the asyncio, sync and legacy implementations (trio reuses the asyncio one), and turn UnicodeDecodeError into InvalidHeaderValue so invalid credentials get a 401. Restricting passwords to ASCII was the alternative, but RFC 7617 specifies UTF-8 and build_authorization_basic already sends UTF-8.

hmac.compare_digest raises TypeError on str containing non-ASCII
characters, so a correct non-ASCII password was answered with HTTP 500
instead of authenticating. Compare UTF-8 bytes instead.

Also reject credentials that aren't valid UTF-8 with HTTP 401 instead
of crashing on UnicodeDecodeError.
@aaugustin

Copy link
Copy Markdown
Member

Checklist? Second time you ignore it.

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

Successfully merging this pull request may close these issues.

2 participants