Bug 2021342 - Keep the enterprise-managed SDR unlocked and fail closed on unlock failure#1162
Open
beurdouche wants to merge 1 commit into
Open
Conversation
simon-friedberger
approved these changes
Jul 17, 2026
simon-friedberger
left a comment
simon-friedberger
left a comment
Member
There was a problem hiding this comment.
lgtm as long as the tests all pass, I suspect they don't so those would need fixing
lissyx
reviewed
Jul 17, 2026
lissyx
reviewed
Jul 17, 2026
Member
Author
|
Looking into this now. |
…d on unlock failure
The enterprise storage-encryption feature sets the NSS internal token's primary
password to the console-supplied primarySecret and unlocks it once at startup.
A primary-password prompt could still reach the user for a secret they cannot
supply (about:logins, password import, Sync on startup). Fixes:
- EnterpriseStorageEncryption.load set the password on a fresh token via
changePassword("", primarySecret) but never logged the token in (PK11_ChangePW
does not authenticate). Make the login explicit, verify pk11token.isLoggedIn,
and fail the launch with a dedicated exit code
(FeltEncryptionExitCode_SdrTokenUnlockFailed) if any step fails instead of
continuing half-unlocked.
- Handle that exit code in FeltProcessParent.handleRestartAfterAbnormalExit:
surface a dedicated "cannot unlock your profile" error
(felt-error-sdr-token-unlock-failed) instead of counting it as a crash and
looping through restarts.
- The ClearData CLEAR_AUTH_TOKENS path (SecretDecoderRing::LogoutAndTeardown)
ran PK11_LogoutAll, re-locking the internal token mid-session with no
re-login. Skip that logout while the primary password is enterprise-managed;
the non-token teardown (TLS cache, connection pruning) still runs. Logout()
is left unguarded as it has no production caller.
- Add an enterprise-gated regression test asserting the token stays logged in
through logoutAndTeardown.
beurdouche
force-pushed
the
bug-2021342-sdr-primary-password
branch
from
July 21, 2026 11:50
4cfc3d9 to
e76cd66
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The enterprise storage-encryption feature sets the NSS internal token's primary password to the console-supplied primarySecret and unlocks it once at startup. Two gaps let a primary-password prompt reach the user for a secret they cannot supply (about:logins, password import, Sync on startup):
EnterpriseStorageEncryption.load set the password on a fresh token via changePassword("", primarySecret) but never logged the token in (PK11_ChangePW does not authenticate), and its only error handling was console.error. Make the login explicit and symmetric across the fresh and returning branches, verify pk11token.isLoggedIn, and fail the launch with a dedicated exit code (FeltEncryptionExitCode_UnlockFailed) if any step fails instead of continuing half-unlocked.
Any CLEAR_AUTH_TOKENS clear (Clear Recent History, sanitize, site-data) routed through SecretDecoderRing::Logout / LogoutAndTeardown ran PK11_LogoutAll, re-locking the internal token mid-session with no re-login. Skip the token logout while the primary password is enterprise-managed; the rest of the teardown (TLS cache, connection pruning) still runs.