[PM-10098] SSH Agent & SSH Key Generator/Import for Bitwarden Desktop

[quexten]

🎟️ Tracking

https://github.com/bitwarden/server/pull/4575 https://github.com/bitwarden/clients/pull/10360 https://github.com/bitwarden/clients/pull/10293 https://bitwarden.atlassian.net/browse/PM-10098

📔 Objective

Implements the ssh agent feature and key generation on Bitwarden desktop. Private-keys are imported in OPENSSH format. There is no PKCS#8 support for now. Key creation in general is feature-flagged.

@bitwarden/team-platform-dev most of the code additions are currently assigned to you (since you own desktop native), but we will probably change the codeownership after initial review; that is still open. (I still very much look forward to the comments/improvements you have for the initial additions).

📸 Screenshots

https://github.com/user-attachments/assets/86ef6460-beda-48d5-9206-8c3b8e39e60f

⏰ 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[bot]]

Checkmarx One – Scan Summary & Details – 5d3bb449-8865-40eb-bf9f-1784744e1717

Fixed Issues

Severity	Issue	Source File / Package
	Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 633
	Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 60
	Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 56
	Client_Privacy_Violation	/apps/desktop/src/vault/app/vault/view.component.html: 50

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 8.78661% with 218 lines in your changes missing coverage. Please review.

Please upload report for BASE (feature/ssh-keys@65385f4). Learn more about missing BASE report.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...desktop/src/platform/services/ssh-agent.service.ts	0.00%	67 Missing :warning:
...esktop/src/platform/main/main-ssh-agent.service.ts	0.00%	47 Missing :warning:
.../desktop/src/vault/app/vault/add-edit.component.ts	0.00%	26 Missing :warning:
...top/src/platform/components/approve-ssh-request.ts	0.00%	15 Missing :warning:
apps/desktop/src/platform/preload.ts	0.00%	13 Missing :warning:
apps/desktop/src/main/window.main.ts	0.00%	7 Missing :warning:
...angular/src/vault/components/add-edit.component.ts	0.00%	6 Missing :warning:
libs/common/src/models/export/ssh-key.export.ts	0.00%	6 Missing :warning:
.../src/platform/services/desktop-settings.service.ts	0.00%	5 Missing :warning:
libs/common/src/models/export/cipher.export.ts	0.00%	4 Missing :warning:
... and 14 more

Additional details and impacted files

@@                 Coverage Diff                 @@
##             feature/ssh-keys   #10293   +/-   ##
===================================================
  Coverage                    ?   32.84%           
===================================================
  Files                       ?     2681           
  Lines                       ?    82146           
  Branches                    ?    15467           
===================================================
  Hits                        ?    26979           
  Misses                      ?    53078           
  Partials                    ?     2089           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Hinton]

I haven't gotten around to reviewing this @quexten. @dani-garcia is re-writing the IPC in https://github.com/bitwarden/clients/pull/9894 which is getting closer to being merged and is also being used in the passkey PoC. As I understand it we will be using IPC logic here as part of NamedPipeServerStream. Should we be streamlining this to use the same IPC logic for ssh agent?

[quexten]

I haven't gotten around to reviewing this @quexten. @dani-garcia is re-writing the IPC in #9894 which is getting closer to being merged and is also being used in the passkey PoC. As I understand it we will be using IPC logic here as part of NamedPipeServerStream. Should we be streamlining this to use the same IPC logic for ssh agent?

I asked @dani-garcia the same question, however I don't think it's possible since we are not doing IPC with another bitwarden process, but instead providing the SSH_AUTH_SOCK / on windows the named pipe that external programs (ssh/git/rsync/etc.) consume? (@dani-garcia feel free to correct me here if I'm missing something)

[dani-garcia]

I haven't gotten around to reviewing this @quexten. @dani-garcia is re-writing the IPC in #9894 which is getting closer to being merged and is also being used in the passkey PoC. As I understand it we will be using IPC logic here as part of NamedPipeServerStream. Should we be streamlining this to use the same IPC logic for ssh agent?

I asked @dani-garcia the same question, however I don't think it's possible since we are not doing IPC with another bitwarden process, but instead providing the SSH_AUTH_SOCK / on windows the named pipe that external programs (ssh/git/rsync/etc.) consume? (@dani-garcia feel free to correct me here if I'm missing something)

Yeah, with the SSH agent we're fairly constrained on what we can do by the protocol, so I don't know if there's much logic to reuse there with what we're doing for IPC between Bitwarden owned processes. I think the next step for us in this field could involve improving on the Rust IPC module and moving the message authentication and encryption into it so the code can be reused between passkeys and browser integration, and that won't be something useful for this use case, either.

We're currently using the interprocess crate to abstract over the UNIX socket / Windows named pipe separation though, so we might also be able to use that here, but that's honestly only a very small part of the code.

[quexten]

Just a few more comments

@quexten a thought I had was that we will probably want to document this in a similar way to https://contributing.bitwarden.com/architecture/deep-dives/passkeys/implementations/provider/browser-extension

Contributing docs draft (still needs work): https://github.com/bitwarden/contributing-docs/pull/426

[quexten]

@quexten have you run the rust files through a formatter? When I modify the files on my device I get a whole lot of reformatting. It's odd that the CI doesn't react to it, but could you please run prettier on everything?

@coroiu Might be the editor I use (Zed) messing with things. I'll take a look.

[daltonpearson]

Any chance that we'll get the option to disable the authorize dialog? I understand that it's a security measure but considering you already have to be logged in I think the auth/deny popup adds a bit too much friction for the hassle.