Basic input validation on SSH keys

[citrus-it]

This could be something that should be addressed in the API, but there appears to be no validation on what one can type into the SSH key box. As a minimum we should probably check that there are two or three words - <type> <value> [comment] so we don't end up with:

> select * from ssh_key order by time_created desc limit 1;
                   id                  | name | description |         time_created          |         time_modified         | time_deleted |             silo_user_id             | public_key
---------------------------------------+------+-------------+-------------------------------+-------------------------------+--------------+--------------------------------------+-------------
  ff3aea52-16d5-4387-ba6f-ed66f854708c | test |             | 2023-05-22 10:51:01.991649+00 | 2023-05-22 10:51:01.991649+00 | NULL         | 36a246df-a238-49de-9b61-c68f1f783c86 | waffle
(1 row)

cloud-init in guests will just reject this - it actually checks against a specific list of key types but we may not want to duplicate and maintain that list, and it will ultimately depend on what the guest is running for cloud-init (which might not even be cloud-init, just something that uses the same data source).

[david-crespo]

Broadly our approach is that the API should be ultimately responsible for avoiding broken input, but we try to do as much of the same validation client-side as we can, both for responsiveness and because we can usually provide better error messages than the API does. When the API includes a regex pattern in the OpenAPI spec, we get it for free on our end — we generate runtime parsers from those. In general we try to avoid doing our own validation that goes beyond what the API allows, so I would prefer to add it API side first and then use their regex.

https://github.com/oxidecomputer/console/blob/d67d2c7b5f31708208937ed6202e9cc6a39e19e6/libs/api/__generated__/validate.ts#L704-L716

As you suggest, right now there is no API-side validation on the public key:

https://github.com/oxidecomputer/omicron/blob/e83f7b69fb1685d36d8db01dede75d9c4f1cc58d/nexus/types/src/external_api/params.rs#L1301-L1309

[david-crespo]

Created https://github.com/oxidecomputer/omicron/issues/3180

[benjaminleonard]

Trying to think about the broadest possible validation that is still useful.

Ignoring the first part – I had initially thought we could check for ssh-(...) but based on that ssh_util.py file it looks like we can't rely on that.

What if we take that string, split the string by spaces and run atob() on the second part. If it doesn't throw we know it's base64 decodable and likely formatted like a public key? That might catch most of the obvious mistakes a user might make.

Thoughts @iliana

I also think that the validation probably shouldn't prevent submission. We could add a message that appears whenever the field is blurred, but that wouldn't catch a user entering it into the key field and submitting directly. It could run onBlur and whenever something is pasted into an input. Or we block submission initially if it doesn't seem valid, and the user can just submit again to ignore it.

[iliana]

Soft validation seems useful, that sounds like a good idea.

https://www.ibm.com/docs/en/zos/2.5.0?topic=daemon-format-authorized-keys-file is a pretty good reference of what is expected by ~/.ssh/authorized_keys and also cloud-init (albeit not a canonical reference), but the tl;dr is that an SSH key is made up of up to four words:

1. an optional "options" field
2. the key type (e.g. ssh-ed25519)
3. the base64-encoded key data
4. an optional comment field

So running atob() on the second part is not necessarily correct either; if someone with an SSH CA comes along and tries to paste in their CA pubkey with the cert-authority option, the second-field might be something like ssh-rsa. But those people probably know they're operating outside the ordinary and if soft validation doesn't stop them, it'll be fine.

What might be most careful is something like:

• Split the input into tokens and check there are 2-4 of them. If not, soft error
• If there are 2 tokens, atob() the second one; if that fails, soft error
• If there are 3 tokens, atob() the second one; if that fails, atob() the third one; if that fails, soft error
• If there are 4 tokens, atob() the third one, etc etc etc