Skip to content

fix(auth): correctly size keys for encrypted token passwords - #63804

Open
joshtrichards wants to merge 8 commits into
masterfrom
jtr/fix-auth-newToken
Open

fix(auth): correctly size keys for encrypted token passwords#63804
joshtrichards wants to merge 8 commits into
masterfrom
jtr/fix-auth-newToken

Conversation

@joshtrichards

@joshtrichards joshtrichards commented Aug 30, 2026

Copy link
Copy Markdown
Member
  • Resolves: #

Summary

Correct the RSA key-size selection used to encrypt passwords associated with authentication tokens.

A 2048-bit RSA key using OAEP with SHA-1 can encrypt up to 214 bytes, but the existing code only selected a 4096-bit key for passwords longer than 250 bytes. Passwords between 215 and 250 bytes could therefore be assigned a key that was too small, causing password encryption to fail. Because that failure was not checked, the code subsequently attempted to Base64-encode an unset ciphertext value.

This change:

  • uses the 214-byte RSA-OAEP plaintext limit when selecting between 2048-bit and 4096-bit keys;
  • only generates a 4096-bit key when encrypted password storage (auth.storeCryptedPassword) is enabled;
  • validates the encrypted-password storage limit before generating a key;
  • clarifies that the storage limit is measured in bytes;
  • checks the result of openssl_public_encrypt() before encoding or storing its output.

Checking the encryption result also provides a controlled failure when an existing token with a 2048-bit key cannot accommodate a subsequently changed, longer password. The short-to-long password transition was already a problem, as reported in #45090, but its encryption failure was not handled explicitly. Regenerating existing token key pairs to accommodate a new, longer password (and defining the associated user experience) remain outside the scope of this PR.

Tests

Added unit coverage for:

  • rejecting passwords above the encrypted-password storage limit;
  • allowing long passwords when encrypted password storage is disabled;
  • using a 2048-bit key at the 214-byte OAEP boundary;
  • using a 4096-bit key above the OAEP boundary;
  • explicitly failing a short-to-long password transition when the existing token key is too small.

TODO

  • Backport

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Validate the encrypted password length before generating a key, use the RSA-OAEP plaintext limit when selecting the key size, and avoid generating
4096-bit keys when encrypted password storage is disabled.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Check the result of OpenSSL public-key encryption and fail explicitly instead of encoding and storing missing or invalid ciphertext. This also
provides a controlled failure when a token created with a 2048-bit key cannot accommodate a subsequently changed, longer password.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Replace and strengthen testGenerateTokenLongPassword():

- validate the new message
- confirm that no token is inserted
- rename test for clarity
- use constant rather than hard-coded arbitrary 500

Signed-off-by: Josh <josh.t.richards@gmail.com>
Cover both sides of 214-byte password boundary.

These tests specifically catch the old incorrect 250 threshold: the 215-byte case would fail under the old implementation.

Signed-off-by: Josh <josh.t.richards@gmail.com>
This verifies both that the maximum is not applied and that a 4096-bit key is not generated unnecessarily.

Signed-off-by: Josh <josh.t.richards@gmail.com>
Adds a short-to-long transition regression test for encryptPassword().

Verifies an existing 2048-bit existing token fails explicitly when password is storage is enabled and the password is changed to a >214-byte password.

Assisted-by: Copilot:gpt-5.6-sol

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards
joshtrichards force-pushed the jtr/fix-auth-newToken branch from 2823f1b to 9003ba1 Compare August 31, 2026 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants