Add rsa extract public key

[quexten]

🎟️ Tracking

📔 Objective

Webcrypto provides non-useful errors when private keys are corrupt (invalid content on undecryptable wrapping decryption). This PR introduces a temporary function to immediately replace the derive_public_key function in clients so that the error logs become useful.

🚨 Breaking Changes

⏰ 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

[github-actions]

Checkmarx One – Scan Summary & Details – 668f630f-a48a-4ff4-b462-93ba88fced6d

Great job! No new security vulnerabilities introduced in this pull request

[github-actions]

🔍 SDK Breaking Change Detection Results

SDK Version: km/rsa-extract-public-key (a3f3b8c)
Completed: 2025-11-25 11:12:02 UTC
Total Time: 215s

Client	Status	Details
typescript	❌ Breaking changes detected	TypeScript compilation failed with new SDK version - View Details

---

Breaking change detection completed. View SDK workflow

[codecov]

Codecov Report

❌ Patch coverage is 0% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.46%. Comparing base (f75a58c) to head (a3f3b8c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/bitwarden-wasm-internal/src/pure_crypto.rs	0.00%	13 Missing ⚠️
...rates/bitwarden-crypto/src/enc_string/symmetric.rs	0.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #572      +/-   ##
==========================================
- Coverage   79.51%   79.46%   -0.05%     
==========================================
  Files         302      302              
  Lines       32311    32331      +20     
==========================================
  Hits        25691    25691              
- Misses       6620     6640      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude finished @quexten's task —— View job

---

PR Review Complete

Tasks

• Evaluate PR title and description quality
• Read changed files and understand implementation
• Review against repository guidelines (CLAUDE.md)
• Check for security concerns
• Analyze test coverage requirements
• Post comprehensive review findings

---

Summary

This PR adds a new rsa_extract_public_key function to extract public keys from encrypted RSA private keys, and makes EncString directly usable from WASM. The changes improve error reporting for corrupted private keys in webcrypto operations. While the implementation is functionally sound, there are concerns about business logic in WASM, missing tests, and lack of required justification for changes to bitwarden-crypto.

Findings

Finding 1: PR description lacks comprehensive objective and test plan details

💭 The PR objective mentions this is a "temporary function" but doesn't explain:

• Why is this temporary? What's the long-term solution?
• What specific error improvements will users see?
• When will this be replaced?

The test plan section (Reminders before review) shows "Written new unit and / or integration tests where applicable" as checked, but codecov reports 0% patch coverage with 20 lines missing coverage.

Finding 2: Business logic added to WASM without justification

❌ Per CLAUDE.md: "Business logic in WASM: bitwarden-wasm-internal contains only thin bindings. Move business logic to feature crates."

The new rsa_extract_public_key function in pure_crypto.rs (lines 324-336) contains substantial business logic:

• Decrypting private keys
• Converting key formats
• Extracting public keys

This should be moved to bitwarden-crypto as a reusable function, with pure_crypto.rs providing only a thin WASM binding.

Finding 3: Changes to bitwarden-crypto/ without required justification

❌ Per CLAUDE.md: "Changes to bitwarden-crypto/ core functionality: Generally speaking, this crate should not be modified. Changes need a comment explaining why."

The FromWasmAbi implementation for EncString (lines 88-96 in symmetric.rs) modifies core crypto functionality without explaining:

• Why is this change necessary?
• What are the implications for security?
• Why use unwrap_throw() instead of proper error handling?

Finding 4: Missing test coverage for new functionality

❌ The new rsa_extract_public_key function has zero test coverage. Required tests:

• Valid encrypted private key → correct public key extraction
• Invalid/corrupted encrypted data → proper error handling
• Wrong wrapping key → proper error handling
• Non-RSA key data → proper error handling

Finding 5: Unsafe error handling in FromWasmAbi implementation

⚠️ The FromWasmAbi implementation uses unwrap_throw() (symmetric.rs:95):

let s = unsafe { String::from_abi(abi) };
Self::from_str(&s).unwrap_throw()

While unwrap_throw() is appropriate for WASM contexts, this lacks documentation explaining:

• What happens when from_str fails
• Why this is safe for all possible input strings
• What error message the JS caller will receive

Finding 6: Function naming doesn't follow crate conventions

🎨 The function name rsa_extract_public_key uses rsa_ prefix, but other functions in the same file use descriptive names without algorithm prefixes:

• verifying_key_for_signing_key
• wrap_decapsulation_key
• unwrap_encapsulation_key

Consider renaming to extract_public_key_from_encrypted_private_key or similar for consistency.

Finding 7: Missing documentation about temporary nature

💭 The PR description mentions this is "temporary" to be replaced in clients, but there's no TODO comment or deprecation notice in the code. Consider adding:

/// TEMPORARY: This function provides better error messages for corrupt private keys.
/// Will be replaced by derive_public_key improvements in clients.
/// Tracking: [link to issue/discussion]

Action Items

1. ❌ REQUIRED: Add comprehensive unit tests for rsa_extract_public_key covering success and failure cases
2. ❌ REQUIRED: Add justification comment in symmetric.rs explaining why bitwarden-crypto changes are necessary
3. ❌ REQUIRED: Move business logic from rsa_extract_public_key to bitwarden-crypto crate, keep only thin WASM binding
4. ⚠️ RECOMMENDED: Document error behavior in FromWasmAbi implementation
5. 🎨 SUGGESTED: Consider renaming function for consistency with crate conventions
6. 🎨 SUGGESTED: Add TODO/tracking comment about temporary nature of the function

Good Practices Observed

• Proper use of Result<Vec<u8>, CryptoError> for error handling
• Correct key conversion through proper types (BitwardenLegacyKeyBytes, Pkcs8PrivateKeyBytes, etc.)
• Appropriate use of #[cfg(feature = "wasm")] for platform-specific code

---