Project

General

Profile

Actions

Idea #20241

closed

API server accepts modern SSH key types (elliptic curve/ecdsa/ed25519)

Added by Brett Smith about 1 year ago. Updated 7 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Story points:
2.0
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.


Subtasks 1 (0 open1 closed)

Task #20324: Review 20241-validate-ssh-keysResolvedTom Clegg04/20/2023Actions

Related issues

Blocks Arvados - Feature #20224: Workbench 2 accepts modern SSH key types (elliptic curve/ecdsa/ed25519)NewActions
Actions #1

Updated by Brett Smith about 1 year ago

  • Blocks Feature #20224: Workbench 2 accepts modern SSH key types (elliptic curve/ecdsa/ed25519) added
Actions #2

Updated by Brett Smith about 1 year ago

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.

Actions #3

Updated by Brett Smith about 1 year ago

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.

Actions #4

Updated by Peter Amstutz 12 months ago

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).

Actions #5

Updated by Peter Amstutz 12 months ago

  • Story points set to 2.0
  • Target version changed from Future to To be scheduled
Actions #6

Updated by Peter Amstutz 12 months ago

  • Target version changed from To be scheduled to Development 2023-04-26 sprint
Actions #7

Updated by Tom Clegg 12 months ago

  • Assigned To set to Tom Clegg
Actions #8

Updated by Tom Clegg 12 months ago

  • 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.

Actions #9

Updated by Tom Clegg 12 months ago

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.

Actions #10

Updated by Brett Smith 11 months ago

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.

Actions #11

Updated by Tom Clegg 11 months ago

  • Status changed from In Progress to Resolved
Actions #12

Updated by Peter Amstutz 7 months ago

  • Release set to 66
Actions

Also available in: Atom PDF