[PM-12555] Pkcs8 sshkey import & general ssh key import tests #11048

quexten · 2024-09-14T19:16:04Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-12555
https://bitwarden.atlassian.net/browse/PM-10408

📔 Objective

Since ssh_key does not have support for PKCS#8 keys, this PR adds PKCS#8 support, for: unencrypted RSA, encrypted RSA, unencrypted PKCS#8 ed25519. We do not yet have test key files for encrypted pkcs#8 ed25519 because openssh does not support exporting these as pkcs#8, but theoretically these are supported too. The test ed25519 pkcs#8 key is exported from 1Password, which does support pkcs#8 ed25519 (it is the default of the export format).

Additionally, this adds a number of tests for parsing, both regarding working keys, and incorrect keys (wrong password; unsupported key type).

📸 Screenshots

⏰ 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

codecov · 2024-09-14T19:23:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.80%. Comparing base (b4af394) to head (d6ab873).
Report is 1 commits behind head on auth/beeep/ssh-agent.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@                   Coverage Diff                    @@
##           auth/beeep/ssh-agent   #11048      +/-   ##
========================================================
- Coverage                 34.81%   34.80%   -0.01%     
========================================================
  Files                      2681     2681              
  Lines                     82135    82135              
  Branches                  15466    15466              
========================================================
- Hits                      28594    28588       -6     
- Misses                    52637    52643       +6     
  Partials                    904      904

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

github-actions · 2024-09-14T19:25:02Z

Checkmarx One – Scan Summary & Details – be67bbaf-31c5-46f3-b6ae-d0f0191d8a80

New Issues

Issue	Source File / Package	Checkmarx Insight
Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 583	Attack Vector
Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 60	Attack Vector
Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 56	Attack Vector
Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 50	Attack Vector

coroiu

Looks like magic to me 🌈 😄

* Add ssh agent, generator & import * Move ssh agent code to bitwarden-russh crate * Remove generator component * Cleanup * Cleanup * Remove left over sshGenerator reference * Cleanup * Add documentation to sshkeyimportstatus * Fix outdated variable name * Update apps/desktop/src/platform/preload.ts Co-authored-by: Andreas Coroiu <[email protected]> * Rename renderersshagent * Rename MainSshAgentService * Improve clarity of 'id' variables being used * Improve clarity of 'id' variables being used * Update apps/desktop/src/vault/app/vault/add-edit.component.html Co-authored-by: Andreas Coroiu <[email protected]> * Fix outdated cipher/messageid names * Rename SSH to Ssh * Make agent syncing more reactive * Move constants to top of class * Make sshkey cipher filtering clearer * Add stricter equality check on ssh key unlock * Fix build and messages * Fix incorrect featureflag name * Replace anonymous async function with switchmap pipe * Fix build * Update apps/desktop/desktop_native/napi/src/lib.rs Co-authored-by: Andreas Coroiu <[email protected]> * Revert incorrectly renamed 'Ssh' usages to SSH * Run cargo fmt * Clean up ssh agent sock path logic * Cleanup and split to platform specific files * Small cleanup * Pull out generator and importer into core * Rename renderersshagentservice to sshagentservice * Rename cipheruuid to cipher_id * Drop ssh dependencies from napi crate * Clean up windows build * Small cleanup * Small cleanup * Cleanup * Add rxjs pipeline for agent services * [PM-12555] Pkcs8 sshkey import & general ssh key import tests (#11048) * Add pkcs8 import and tests * Add key type unsupported error * Remove unsupported formats * Remove code for unsupported formats * Fix encrypted pkcs8 import * Add ed25519 pkcs8 unencrypted test file * SSH agent rxjs tweaks (#11148) * feat: rewrite sshagent.signrequest as purely observable * feat: fail the request when unlock times out * chore: clean up, add some clarifying comments * chore: remove unused dependency * fix: result `undefined` crashing in NAPI -> Rust * Allow concurrent SSH requests in rust * Remove unwraps * Cleanup and add init service init call * Fix windows * Fix timeout behavior on locked vault --------- Co-authored-by: Andreas Coroiu <[email protected]>

* [PM-10395] Add new item type ssh key (#10360) * Implement ssh-key cipher type * Fix linting * Fix edit and view components for ssh-keys on desktop * Fix tests * Remove ssh key type references * Remove add ssh key option * Fix typo * Add tests * [PM-10399] Add ssh key import export for bitwarden json (#10529) * Add ssh key import export for bitwarden json * Remove key type from ssh key export * [PM-10406] Add privatekey publickey and fingerprint to both add-edit and view co… (#11046) * Add privatekey publickey and fingerprint to both add-edit and view components * Remove wrong a11y title * Fix testid * [PM-10098] SSH Agent & SSH Key creation for Bitwarden Desktop (#10293) * Add ssh agent, generator & import * Move ssh agent code to bitwarden-russh crate * Remove generator component * Cleanup * Cleanup * Remove left over sshGenerator reference * Cleanup * Add documentation to sshkeyimportstatus * Fix outdated variable name * Update apps/desktop/src/platform/preload.ts Co-authored-by: Andreas Coroiu <[email protected]> * Rename renderersshagent * Rename MainSshAgentService * Improve clarity of 'id' variables being used * Improve clarity of 'id' variables being used * Update apps/desktop/src/vault/app/vault/add-edit.component.html Co-authored-by: Andreas Coroiu <[email protected]> * Fix outdated cipher/messageid names * Rename SSH to Ssh * Make agent syncing more reactive * Move constants to top of class * Make sshkey cipher filtering clearer * Add stricter equality check on ssh key unlock * Fix build and messages * Fix incorrect featureflag name * Replace anonymous async function with switchmap pipe * Fix build * Update apps/desktop/desktop_native/napi/src/lib.rs Co-authored-by: Andreas Coroiu <[email protected]> * Revert incorrectly renamed 'Ssh' usages to SSH * Run cargo fmt * Clean up ssh agent sock path logic * Cleanup and split to platform specific files * Small cleanup * Pull out generator and importer into core * Rename renderersshagentservice to sshagentservice * Rename cipheruuid to cipher_id * Drop ssh dependencies from napi crate * Clean up windows build * Small cleanup * Small cleanup * Cleanup * Add rxjs pipeline for agent services * [PM-12555] Pkcs8 sshkey import & general ssh key import tests (#11048) * Add pkcs8 import and tests * Add key type unsupported error * Remove unsupported formats * Remove code for unsupported formats * Fix encrypted pkcs8 import * Add ed25519 pkcs8 unencrypted test file * SSH agent rxjs tweaks (#11148) * feat: rewrite sshagent.signrequest as purely observable * feat: fail the request when unlock times out * chore: clean up, add some clarifying comments * chore: remove unused dependency * fix: result `undefined` crashing in NAPI -> Rust * Allow concurrent SSH requests in rust * Remove unwraps * Cleanup and add init service init call * Fix windows * Fix timeout behavior on locked vault --------- Co-authored-by: Andreas Coroiu <[email protected]> * Fix libc dependency being duplicated * fix SSH casing (#11840) * Move ssh agent behind feature flag (#11841) * Move ssh agent behind feature flag * Add separate flag for ssh agent * [PM-14215] fix unsupported key type error message (#11788) * Fix error message for import of unsupported ssh keys * Use triple equals in add-edit component for ssh keys --------- Co-authored-by: Andreas Coroiu <[email protected]> Co-authored-by: aj-bw <[email protected]>

Add pkcs8 import and tests

800b781

quexten mentioned this pull request Sep 14, 2024

[PM-10938] Import ssh-keys from 1password 1pux #10787

Draft

quexten added 4 commits September 14, 2024 21:47

Add key type unsupported error

2190736

Remove unsupported formats

6c06ebb

Remove code for unsupported formats

6ea5977

Fix encrypted pkcs8 import

b182bc7

quexten changed the title ~~[PM-10408] Pkcs8 sshkey import~~ [PM-10408] Pkcs8 sshkey import & general ssh key import tests Sep 23, 2024

quexten marked this pull request as ready for review September 23, 2024 16:01

quexten requested review from a team as code owners September 23, 2024 16:01

quexten requested a review from coroiu September 23, 2024 16:01

Add ed25519 pkcs8 unencrypted test file

d6ab873

quexten changed the title ~~[PM-10408] Pkcs8 sshkey import & general ssh key import tests~~ [PM-12555] Pkcs8 sshkey import & general ssh key import tests Sep 23, 2024

coroiu approved these changes Sep 24, 2024

View reviewed changes

quexten merged commit cb19189 into auth/beeep/ssh-agent Sep 24, 2024
37 of 38 checks passed

quexten deleted the km/pm-10408/pkcs8-sshkey-import branch September 24, 2024 10:41

quexten mentioned this pull request Sep 26, 2024

[PM-11924] Add ssh-key item type bitwarden/sdk-sm#1037

Merged

quexten mentioned this pull request Oct 1, 2024

PM-10393 SSH keys #10825

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PM-12555] Pkcs8 sshkey import & general ssh key import tests #11048

[PM-12555] Pkcs8 sshkey import & general ssh key import tests #11048

quexten commented Sep 14, 2024 •

edited

Loading

codecov bot commented Sep 14, 2024 •

edited

Loading

github-actions bot commented Sep 14, 2024 •

edited

Loading

coroiu left a comment

[PM-12555] Pkcs8 sshkey import & general ssh key import tests #11048

[PM-12555] Pkcs8 sshkey import & general ssh key import tests #11048

Conversation

quexten commented Sep 14, 2024 • edited Loading

🎟️ Tracking

📔 Objective

📸 Screenshots

⏰ Reminders before review

🦮 Reviewer guidelines

codecov bot commented Sep 14, 2024 • edited Loading

Codecov Report

github-actions bot commented Sep 14, 2024 • edited Loading

New Issues

coroiu left a comment

Choose a reason for hiding this comment

quexten commented Sep 14, 2024 •

edited

Loading

codecov bot commented Sep 14, 2024 •

edited

Loading

github-actions bot commented Sep 14, 2024 •

edited

Loading