Support SSH keys on desktop 2024.12

[dani-garcia]

Added support for the new SSH item key type in desktop 2024.12+

To test it, you need a desktop build from bitwarden/clients:main, and to enable the feature flags on server.

[BlackDex]

I have not tested it. But looks good. Just one remark.

[larena1]

Nice, so Vaultwarden is only incompatible right now with recent web versions but all other clients fully work?

[BlackDex]

Nice, so Vaultwarden is only incompatible right now with recent web versions but all other clients fully work?

Once merged and released yes.

[quexten]

Note: this might cause problems for key rotation. If the web client does not support parsing and serializing key ciphers, then key rotation might either fail or even end up with undecryptable items (if skipped), so rotation should be blocked.

I'm not sure about what vaultwarden does for validation here, but upstream has rotation validators, that block rotation entirely for cases like this: https://github.com/bitwarden/server/blob/dfbc400520f4c474b28e1279a86b2fae1da052a9/src/Api/Vault/Validators/CipherRotationValidator.cs#L10

[BlackDex]

Note: this might cause problems for key rotation. If the web client does not support parsing and serializing key ciphers, then key rotation might either fail or even end up with undecryptable items (if skipped), so rotation should be blocked.

I'm not sure about what vaultwarden does for validation here, but upstream has rotation validators, that block rotation entirely for cases like this: https://github.com/bitwarden/server/blob/dfbc400520f4c474b28e1279a86b2fae1da052a9/src/Api/Vault/Validators/CipherRotationValidator.cs#L10

It probably will and i think it will cause issues.
We currently have some checks, but not the same specific check Bitwarden uses. Which is not that hard to add and would make it more safer.

[BlackDex]

We could check if there are any SSH type ciphers, if that is the case, cancel/abort the rekey process directly.
But the cipher count would the probably help there too, since if we do not return the SSH ciphers, it will be aborted too.
But a better descriptive message in those cases would be better.

[BlackDex]

As mentioned in the comments already. Before we add this feature, we should add the same kind of checks for the key-rotation to prevent missing keys in the rotation, and make them unavailable anymore, and thus make all SSH Key's unusable.

It might be nice to add a more descriptive warning in case someone is using a newer client then 2024.6.2 to run the key-rotation if there are SSH Ciphers. But a more general error would also be a good start for verifying all ciphers are inculded.

And i think we should do the same for all other rotated items like emergency request, folders etc...

[dani-garcia]

Nice, good call about the key rotation validation. I've improved the validation to match what Bitwarden is doing. We now check that we have all the ciphers, folders, sends, etc before doing the rotation