Matthew Garrett
2024-Oct-14 15:29 UTC
[RFC] Preferentially TOFU certificate authorities rather than host keys
There's currently no way to express trust for an SSH certificate CA other than by manually adding it to known_hosts. This patch modifies the automatic key write-out behaviour on user verification to associate the hostname with the CA rather than the host key, allowing environments making use of certificates to update (potentially compromised) host keys without needing to modify client configuration or force users to update their known_hosts. --- Posting at this point primarily for discussion rather than submission - if this is something that seems desirable then we probably also want the ability for servers to revoke old certificates. I have some patches for that, but don't want to spend too much time cleaning them up unless this seems like something that stands some chance of being accepted. This was inspired by https://github.blog/news-insights/company-news/we-updated-our-rsa-ssh-host-key/ - github accidentally leaked their RSA private key into a public repo and immediately rolled over to a new one. This created significant disruption to client systems which fired noisy warnings about host keys having changed, and organisations were forced to reassure their users that in this specific case they should go ahead and delete the old fingerprint but should still in general not do that. Everyone had a bad day. If Github had been using certificates, and if we had a way to engender trust in certificate CAs, they could have rolled to a new key and certificate signed with the same (hopefully offline) CA key with zero impact on users. Ideally they'd also be able to push a signed revocation statement that would invalidate the old certificate. hostfile.c | 9 +++++++-- sshconnect.c | 30 +++++++++++++++++++++++------- sshkey.c | 6 ++++++ sshkey.h | 1 + 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/hostfile.c b/hostfile.c index c5669c703..462ed8357 100644 --- a/hostfile.c +++ b/hostfile.c @@ -437,12 +437,15 @@ static int write_host_entry(FILE *f, const char *host, const char *ip, const struct sshkey *key, int store_hash) { - int r, success = 0; + int r, success = 0, cert = sshkey_is_cert(key); char *hashed_host = NULL, *lhost; lhost = xstrdup(host); lowercase(lhost); + if (cert) + fprintf(f, "%s ", CA_MARKER); + if (store_hash) { if ((hashed_host = host_hash(lhost, NULL, 0)) == NULL) { error_f("host_hash failed"); @@ -457,7 +460,9 @@ write_host_entry(FILE *f, const char *host, const char *ip, } free(hashed_host); free(lhost); - if ((r = sshkey_write(key, f)) == 0) + if ((cert && (r = sshca_write(key, f)) == 0)) + success = 1; + else if ((r = sshkey_write(key, f) == 0)) success = 1; else error_fr(r, "sshkey_write"); diff --git a/sshconnect.c b/sshconnect.c index 7cf6b6386..72bdc7d1f 100644 --- a/sshconnect.c +++ b/sshconnect.c @@ -964,7 +964,7 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo, HostStatus host_status = -1, ip_status = -1; struct sshkey *raw_key = NULL; char *ip = NULL, *host = NULL; - char hostline[1000], *hostp, *fp, *ra; + char hostline[1000], *hostp, *fp, *cafp, *ra; char msg[1024]; const char *type, *fail_reason = NULL; const struct hostkey_entry *host_found = NULL, *ip_found = NULL; @@ -973,6 +973,7 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo, int r, want_cert = sshkey_is_cert(host_key), host_ip_differ = 0; int hostkey_trusted = 0; /* Known or explicitly accepted by user */ struct hostkeys *host_hostkeys, *ip_hostkeys; + struct sshkey *cert = NULL; u_int i; /* @@ -1189,13 +1190,20 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo, "type are already known for this host."); } else xextendf(&msg1, "", "."); - fp = sshkey_fingerprint(host_key, options.fingerprint_hash, SSH_FP_DEFAULT); ra = sshkey_fingerprint(host_key, options.fingerprint_hash, SSH_FP_RANDOMART); if (fp == NULL || ra == NULL) fatal_f("sshkey_fingerprint failed"); + if (cert) { + cafp = sshkey_fingerprint(cert->cert->signature_key, + options.fingerprint_hash, SSH_FP_DEFAULT); + if (cafp == NULL) + fatal_f("sshkey_fingerprint failed"); + xextendf(&msg1, "\n", "%s CA certificate fingerprint is %s.", + type, cafp); + } xextendf(&msg1, "\n", "%s key fingerprint is %s.", type, fp); if (options.visual_host_key) @@ -1229,19 +1237,26 @@ check_host_key(char *hostname, const struct ssh_conn_info *cinfo, * If in "new" or "off" strict mode, add the key automatically * to the local known_hosts file. */ + if (cert) + host_key = cert; if (options.check_host_ip && ip_status == HOST_NEW) { snprintf(hostline, sizeof(hostline), "%s,%s", host, ip); hostp = hostline; if (options.hash_known_hosts) { /* Add hash of host and IP separately */ r = add_host_to_hostfile(user_hostfiles[0], - host, host_key, options.hash_known_hosts) && - add_host_to_hostfile(user_hostfiles[0], ip, - host_key, options.hash_known_hosts); + host, host_key, options.hash_known_hosts); + /* Don't add an IP entry if we're writing out a cert */ + if (!r && !cert) { + r = add_host_to_hostfile(user_hostfiles[0], ip, + host_key, options.hash_known_hosts); + } } else { - /* Add unhashed "host,ip" */ + if (cert) + /* Certificates are host-specific */ + hostp = host; r = add_host_to_hostfile(user_hostfiles[0], - hostline, host_key, + hostp, host_key, options.hash_known_hosts); } } else { @@ -1453,6 +1468,7 @@ fail: fatal_fr(r, "decode key"); if ((r = sshkey_drop_cert(raw_key)) != 0) fatal_r(r, "Couldn't drop certificate"); + cert = host_key; host_key = raw_key; goto retry; } diff --git a/sshkey.c b/sshkey.c index 73fb89ac2..903611937 100644 --- a/sshkey.c +++ b/sshkey.c @@ -1427,6 +1427,12 @@ sshkey_format_text(const struct sshkey *key, struct sshbuf *b) return r; } +int +sshca_write(const struct sshkey *key, FILE *f) +{ + return sshkey_write(key->cert->signature_key, f); +} + int sshkey_write(const struct sshkey *key, FILE *f) { diff --git a/sshkey.h b/sshkey.h index d0cdea0ce..71a111b8b 100644 --- a/sshkey.h +++ b/sshkey.h @@ -212,6 +212,7 @@ int sshkey_fingerprint_raw(const struct sshkey *k, const char *sshkey_type(const struct sshkey *); const char *sshkey_cert_type(const struct sshkey *); int sshkey_format_text(const struct sshkey *, struct sshbuf *); +int sshca_write(const struct sshkey *, FILE *); int sshkey_write(const struct sshkey *, FILE *); int sshkey_read(struct sshkey *, char **); u_int sshkey_size(const struct sshkey *); -- 2.46.2
Matthew Garrett
2024-Oct-14 22:12 UTC
[RFC] Preferentially TOFU certificate authorities rather than host keys
On Mon, Oct 14, 2024 at 04:29:56PM +0100, Matthew Garrett wrote:> There's currently no way to express trust for an SSH certificate CA other > than by manually adding it to known_hosts. This patch modifies the automatic > key write-out behaviour on user verification to associate the hostname with > the CA rather than the host key, allowing environments making use of > certificates to update (potentially compromised) host keys without needing > to modify client configuration or force users to update their known_hosts.Oh, and a couple of use-cases I forgot to mention - transparent association of a CA key with a hostname also allows for either transient hosts behind the same hostname without needing to retain private key material, and also makes it possible to have multiple hosts behind the same hostname without having to share key material. This seems especially useful for allowing hardware-backed key material to be used in more complex scenarios than are currently feasible.
Damien Miller
2024-Oct-16 02:58 UTC
[RFC] Preferentially TOFU certificate authorities rather than host keys
On Mon, 14 Oct 2024, Matthew Garrett wrote:> There's currently no way to express trust for an SSH certificate CA other > than by manually adding it to known_hosts. This patch modifies the automatic > key write-out behaviour on user verification to associate the hostname with > the CA rather than the host key, allowing environments making use of > certificates to update (potentially compromised) host keys without needing > to modify client configuration or force users to update their known_hosts.Thanks - this is an intriging idea. I'll need to consider it a bit; TOFU of CA trust anchors is not a common pattern and both the UI and the general ramifications need to be thought through.> Posting at this point primarily for discussion rather than submission - > if this is something that seems desirable then we probably also want the > ability for servers to revoke old certificates. I have some patches for > that, but don't want to spend too much time cleaning them up unless this > seems like something that stands some chance of being accepted. > > This was inspired by > https://github.blog/news-insights/company-news/we-updated-our-rsa-ssh-host-key/ > - github accidentally leaked their RSA private key into a public repo > and immediately rolled over to a new one. This created significant > disruption to client systems which fired noisy warnings about host keys > having changed, and organisations were forced to reassure their users > that in this specific case they should go ahead and delete the old > fingerprint but should still in general not do that. Everyone had a bad > day. > > If Github had been using certificates, and if we had a way to engender > trust in certificate CAs, they could have rolled to a new key and > certificate signed with the same (hopefully offline) CA key with zero > impact on users. Ideally they'd also be able to push a signed revocation > statement that would invalidate the old certificate.TOFU of CA trust anchors could help here, but we already have a graceful key rotation mechanism that is at least partially enabled by default. It isn't as effective as I'd like to be because of a number of corner cases that have yet to be filed smooth. See client_input_hostkeys() in clientloop.c for the whole mess. In the case of the Github incident, one thing that drastically reduced its effectiveness was the old default of disabling the rotation when multiple hostnames were found for the same key. This interacted badly with the old default of CheckHostIP, which caused both the name and address of hosts to be recorded in known_hosts. The CheckHostIP default has since been changed, but its droppings are still in lots of known_hosts files... -d