Skip to content

[PM-24127] Implement password-protected key envelope #335

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

Merged
merged 158 commits into from
Aug 19, 2025

Conversation

quexten
Copy link
Contributor

@quexten quexten commented Jun 27, 2025

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-24127

📔 Objective

The current masterkey logic is complex to understand for using teams (auth), and also prone to error. When any setting changes / gets out of sync, such as the email, or kdf, then decryption fails. The masterkey is further too widely scoped, used both in an authentication protocol, and in unlock decryption.

This PR introduces a PasswordProtectedKeyEnvelope. The goal is to protect a symmetric key with a password securely. Internally, this uses a KDF, and the KDF settings (argon2 parameters, and random salt) are stored on the serialized object.

That means that the only thing needed to unlock this structure is the correct password, everything else is stored on the object, making this process much less error prone. At the same time the interface is easier to use.

An example is provided to show usage.

A follow-up PR will add an unlock method / enrollment for PIN based on this new cryptographic API.

Note: Only argon2 is supported here. The PasswordProtectedKeyEnvelope's settings are completely decoupled from the account settings, and we don't need to provide backwards compatibility to non-recommended legacy cryptographic algorithms (pbkdf2).

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
    team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
    issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

…b.com:bitwarden/sdk-internal into km/beeep/safe-password-protected-key-envelope
@quexten quexten dismissed stale reviews from Thomas-Avery and justindbaur via e65a3d4 August 15, 2025 10:34
@quexten
Copy link
Contributor Author

quexten commented Aug 15, 2025

Noting here: I lifted in a fair bit of non-pin-specific changes from the follow-up PR to this PR since the last set of reviews.

Copy link
Member

@Hinton Hinton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My primary concerns are resolved. I'd still like to see some elegant way to force upgrades as adding it later will be significantly more complicated. And a note to the root readme to look in safe before implementing anything.

But I'll leave it up to the team to decide if it happens now or when the need arises.

Copy link
Contributor

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@quexten quexten merged commit aaae971 into main Aug 19, 2025
50 checks passed
@quexten quexten deleted the km/beeep/safe-password-protected-key-envelope branch August 19, 2025 13:49
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.

4 participants