Idea #20241
closed
API server accepts modern SSH key types (elliptic curve/ecdsa/ed25519)
Added by Brett Smith almost 2 years ago.
Updated over 1 year ago.
Release relationship:
Auto
Description
The API server validates SSH public keys: see public_key_must_be_unique
in services/api/app/models/authorized_key.rb
.
This validation uses the sshkey gem which claims to only support RSA and DSA keys.
We would like to support all the same key types as OpenSSH: dsa, ecdsa, ecdsa-sk, ed25519, ed25519-sk, rsa
Figure out what our options are for validating other public key types, and implement one.
Related issues
1 (1 open — 0 closed)
- Blocks Feature #20224: Workbench 2 accepts modern SSH key types (elliptic curve/ecdsa/ed25519) added
I searched Rubygems for another gem that does the same job as sshkey and didn't find one (but admittedly it's hard to narrow down the search, it's possible I missed one). I'm gonna investigate if there are other reasonable ways to do this.
Based on this StackOverflow, one option seems to be to pipe the key to ssh-keygen -l -f -
. ssh-keygen -l
tries to generate a public key's fingerprint. If it can't parse a key in the input it exits nonzero, so you can just discard all the output and look at the exit code. I tested to confirm this actually works.
One upside of this approach is it means there'll be a better match between the key types Arvados accepts, and the key types the server is likely to actually understand. There can still be mismatch from OpenSSH server configuration but it's at least closer.
If we implement this by running ssh-keygen in a subprocess we should add a package dependency on openssh-client (e.g. installing into a docker container that doesn't have openssh-server).
- Story points set to 2.0
- Target version changed from Future to To be scheduled
- Target version changed from To be scheduled to Development 2023-04-26 sprint
- Assigned To set to Tom Clegg
- Status changed from New to In Progress
The go ssh library can validate all the keys currently accepted by openssh-client, without forking a subprocess.
(Subprocesses of Passenger workers in particular always seem to turn into problems eventually.)
So I'm going to see if this can be done entirely in controller.
20241-validate-ssh-keys @ ec17f6971109186961283443f2df6d5802bea401 -- developer-run-tests: #3610
Most of the example keys in tests were trivial to generate with ssh-keygen. I had to use an authenticator dongle to generate an ecdsa-sk key.
For completeness I would like to add an ed25519-sk key, but I haven't figured out how to generate one. Meanwhile, all the other key types just worked, so I expect ed25519-sk will also just work.
Tom Clegg wrote in #note-9:
20241-validate-ssh-keys @ ec17f6971109186961283443f2df6d5802bea401 -- developer-run-tests: #3610
This looks good to me. All the code's great. My only thought is it would be nice if at least one test key had a comment with whitespace. This is totally legit (ssh-keygen -C "test key"
) and something that some Mac clients seem to like to do. The current code seems to handle it fine, but a test would be good to make sure we don't accidentally break that in the future.
For completeness I would like to add an ed25519-sk key, but I haven't figured out how to generate one.
Hm, that's the type I have, it should conceptually be the same as ecdsa-sk. I could contribute a public key to the branch if you want?
Thanks.
- Status changed from In Progress to Resolved
Also available in: Atom
PDF