diff --git a/PROTOCOL.krl b/PROTOCOL.krl index f4213156e..1b59c76be 100644 --- a/PROTOCOL.krl +++ b/PROTOCOL.krl @@ -193,6 +193,10 @@ The "extension_contents" contains the body of the extension. 6. KRL signature sections +Note: KRL signatures are not supported by OpenSSH. OpenSSH >= 9.4 will +refuse to load KRLs that contain signatures. We recommend the use +of SSHSIG (`ssh-keygen -Y sign ...`) style signatures for KRLs instead. + The KRL_SECTION_SIGNATURE section serves a different purpose to the preceding ones: to provide cryptographic authentication of a KRL that is retrieved over a channel that does not provide integrity protection. @@ -215,4 +219,4 @@ Implementations that retrieve KRLs over untrusted channels must verify signatures. Signature sections are optional for KRLs distributed by trusted means. -$OpenBSD: PROTOCOL.krl,v 1.6 2023/07/17 03:57:21 djm Exp $ +$OpenBSD: PROTOCOL.krl,v 1.7 2023/07/17 04:01:10 djm Exp $ diff --git a/krl.c b/krl.c index f04ea27d7..c53fdd6ed 100644 --- a/krl.c +++ b/krl.c @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* $OpenBSD: krl.c,v 1.56 2023/07/17 03:57:21 djm Exp $ */ +/* $OpenBSD: krl.c,v 1.57 2023/07/17 04:01:10 djm Exp $ */ #include "includes.h" @@ -729,15 +729,13 @@ revoked_certs_generate(struct revoked_certs *rc, struct sshbuf *buf) } int -ssh_krl_to_blob(struct ssh_krl *krl, struct sshbuf *buf, - struct sshkey **sign_keys, u_int nsign_keys) +ssh_krl_to_blob(struct ssh_krl *krl, struct sshbuf *buf) { int r = SSH_ERR_INTERNAL_ERROR; struct revoked_certs *rc; struct revoked_blob *rb; struct sshbuf *sect; u_char *sblob = NULL; - size_t slen, i; if (krl->generated_date == 0) krl->generated_date = time(NULL); @@ -801,22 +799,7 @@ ssh_krl_to_blob(struct ssh_krl *krl, struct sshbuf *buf, (r = sshbuf_put_stringb(buf, sect)) != 0) goto out; } - - for (i = 0; i < nsign_keys; i++) { - KRL_DBG(("sig key %s", sshkey_ssh_name(sign_keys[i]))); - if ((r = sshbuf_put_u8(buf, KRL_SECTION_SIGNATURE)) != 0 || - (r = sshkey_puts(sign_keys[i], buf)) != 0) - goto out; - /* XXX support sk-* keys */ - if ((r = sshkey_sign(sign_keys[i], &sblob, &slen, - sshbuf_ptr(buf), sshbuf_len(buf), NULL, NULL, - NULL, 0)) != 0) - goto out; - KRL_DBG(("signature sig len %zu", slen)); - if ((r = sshbuf_put_string(buf, sblob, slen)) != 0) - goto out; - } - + /* success */ r = 0; out: free(sblob); @@ -1057,45 +1040,39 @@ extension_section(struct sshbuf *sect, struct ssh_krl *krl) return r; } -/* Attempt to parse a KRL, checking its signature (if any) with sign_ca_keys. */ +/* Attempt to parse a KRL */ int -ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp, - const struct sshkey **sign_ca_keys, size_t nsign_ca_keys) +ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp) { struct sshbuf *copy = NULL, *sect = NULL; struct ssh_krl *krl = NULL; char timestamp[64]; - int r = SSH_ERR_INTERNAL_ERROR, sig_seen; - struct sshkey *key = NULL, **ca_used = NULL, **tmp_ca_used; + int r = SSH_ERR_INTERNAL_ERROR; u_char type; - const u_char *blob; - size_t i, j, sig_off, sects_off, blen, nca_used; u_int format_version; - nca_used = 0; *krlp = NULL; - if (sshbuf_len(buf) < sizeof(KRL_MAGIC) - 1 || - memcmp(sshbuf_ptr(buf), KRL_MAGIC, sizeof(KRL_MAGIC) - 1) != 0) { - debug3_f("not a KRL"); - return SSH_ERR_KRL_BAD_MAGIC; - } - /* Take a copy of the KRL buffer so we can verify its signature later */ - if ((copy = sshbuf_fromb(buf)) == NULL) { - r = SSH_ERR_ALLOC_FAIL; - goto out; + /* KRL must begin with magic string */ + if ((r = sshbuf_cmp(buf, 0, KRL_MAGIC, sizeof(KRL_MAGIC) - 1)) != 0) { + debug2_f("bad KRL magic header"); + return r; } - if ((r = sshbuf_consume(copy, sizeof(KRL_MAGIC) - 1)) != 0) - goto out; if ((krl = ssh_krl_init()) == NULL) { error_f("alloc failed"); goto out; } - - if ((r = sshbuf_get_u32(copy, &format_version)) != 0) + /* Don't modify buffer */ + if ((copy = sshbuf_fromb(buf)) == NULL) { + r = SSH_ERR_ALLOC_FAIL; + goto out; + } + if ((r = sshbuf_consume(copy, sizeof(KRL_MAGIC) - 1)) != 0 || + (r = sshbuf_get_u32(copy, &format_version)) != 0) goto out; if (format_version != KRL_FORMAT_VERSION) { + error_f("unsupported KRL format version %u", format_version); r = SSH_ERR_INVALID_FORMAT; goto out; } @@ -1103,106 +1080,23 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp, (r = sshbuf_get_u64(copy, &krl->generated_date)) != 0 || (r = sshbuf_get_u64(copy, &krl->flags)) != 0 || (r = sshbuf_skip_string(copy)) != 0 || - (r = sshbuf_get_cstring(copy, &krl->comment, NULL)) != 0) + (r = sshbuf_get_cstring(copy, &krl->comment, NULL)) != 0) { + error_fr(r, "parse KRL header"); goto out; - + } format_timestamp(krl->generated_date, timestamp, sizeof(timestamp)); debug("KRL version %llu generated at %s%s%s", (long long unsigned)krl->krl_version, timestamp, *krl->comment ? ": " : "", krl->comment); - /* - * 1st pass: verify signatures, if any. This is done to avoid - * detailed parsing of data whose provenance is unverified. - */ - sig_seen = 0; - if (sshbuf_len(buf) < sshbuf_len(copy)) { - /* Shouldn't happen */ - r = SSH_ERR_INTERNAL_ERROR; - goto out; - } - sects_off = sshbuf_len(buf) - sshbuf_len(copy); - while (sshbuf_len(copy) > 0) { - if ((r = sshbuf_get_u8(copy, &type)) != 0 || - (r = sshbuf_get_string_direct(copy, &blob, &blen)) != 0) - goto out; - KRL_DBG(("first pass, section 0x%02x", type)); - if (type != KRL_SECTION_SIGNATURE) { - if (sig_seen) { - error("KRL contains non-signature section " - "after signature"); - r = SSH_ERR_INVALID_FORMAT; - goto out; - } - /* Not interested for now. */ - continue; - } - sig_seen = 1; - /* First string component is the signing key */ - if ((r = sshkey_from_blob(blob, blen, &key)) != 0) { - r = SSH_ERR_INVALID_FORMAT; - goto out; - } - if (sshbuf_len(buf) < sshbuf_len(copy)) { - /* Shouldn't happen */ - r = SSH_ERR_INTERNAL_ERROR; - goto out; - } - sig_off = sshbuf_len(buf) - sshbuf_len(copy); - /* Second string component is the signature itself */ - if ((r = sshbuf_get_string_direct(copy, &blob, &blen)) != 0) { - r = SSH_ERR_INVALID_FORMAT; - goto out; - } - /* Check signature over entire KRL up to this point */ - if ((r = sshkey_verify(key, blob, blen, - sshbuf_ptr(buf), sig_off, NULL, 0, NULL)) != 0) - goto out; - /* Check if this key has already signed this KRL */ - for (i = 0; i < nca_used; i++) { - if (sshkey_equal(ca_used[i], key)) { - error("KRL signed more than once with " - "the same key"); - r = SSH_ERR_INVALID_FORMAT; - goto out; - } - } - /* Record keys used to sign the KRL */ - tmp_ca_used = recallocarray(ca_used, nca_used, nca_used + 1, - sizeof(*ca_used)); - if (tmp_ca_used == NULL) { - r = SSH_ERR_ALLOC_FAIL; - goto out; - } - ca_used = tmp_ca_used; - ca_used[nca_used++] = key; - key = NULL; - } - - if (sshbuf_len(copy) != 0) { - /* Shouldn't happen */ - r = SSH_ERR_INTERNAL_ERROR; - goto out; - } - - /* - * 2nd pass: parse and load the KRL, skipping the header to the point - * where the section start. - */ - sshbuf_free(copy); - if ((copy = sshbuf_fromb(buf)) == NULL) { - r = SSH_ERR_ALLOC_FAIL; - goto out; - } - if ((r = sshbuf_consume(copy, sects_off)) != 0) - goto out; + /* Parse and load the KRL sections. */ while (sshbuf_len(copy) > 0) { sshbuf_free(sect); sect = NULL; if ((r = sshbuf_get_u8(copy, &type)) != 0 || (r = sshbuf_froms(copy, §)) != 0) goto out; - KRL_DBG(("second pass, section 0x%02x", type)); + KRL_DBG(("section 0x%02x", type)); switch (type) { case KRL_SECTION_CERTIFICATES: @@ -1247,51 +1141,12 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp, } } - /* Check that the key(s) used to sign the KRL weren't revoked */ - sig_seen = 0; - for (i = 0; i < nca_used; i++) { - if (ssh_krl_check_key(krl, ca_used[i]) == 0) - sig_seen = 1; - else { - sshkey_free(ca_used[i]); - ca_used[i] = NULL; - } - } - if (nca_used && !sig_seen) { - error("All keys used to sign KRL were revoked"); - r = SSH_ERR_KEY_REVOKED; - goto out; - } - - /* If we have CA keys, then verify that one was used to sign the KRL */ - if (sig_seen && nsign_ca_keys != 0) { - sig_seen = 0; - for (i = 0; !sig_seen && i < nsign_ca_keys; i++) { - for (j = 0; j < nca_used; j++) { - if (ca_used[j] == NULL) - continue; - if (sshkey_equal(ca_used[j], sign_ca_keys[i])) { - sig_seen = 1; - break; - } - } - } - if (!sig_seen) { - r = SSH_ERR_SIGNATURE_INVALID; - error("KRL not signed with any trusted key"); - goto out; - } - } - + /* Success */ *krlp = krl; r = 0; out: if (r != 0) ssh_krl_free(krl); - for (i = 0; i < nca_used; i++) - sshkey_free(ca_used[i]); - free(ca_used); - sshkey_free(key); sshbuf_free(copy); sshbuf_free(sect); return r; @@ -1425,7 +1280,7 @@ ssh_krl_file_contains_key(const char *path, const struct sshkey *key) oerrno = errno; goto out; } - if ((r = ssh_krl_from_blob(krlbuf, &krl, NULL, 0)) != 0) + if ((r = ssh_krl_from_blob(krlbuf, &krl)) != 0) goto out; debug2_f("checking KRL %s", path); r = ssh_krl_check_key(krl, key); diff --git a/krl.h b/krl.h index d0f469870..eb244767b 100644 --- a/krl.h +++ b/krl.h @@ -14,7 +14,7 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* $OpenBSD: krl.h,v 1.9 2023/07/17 03:57:21 djm Exp $ */ +/* $OpenBSD: krl.h,v 1.10 2023/07/17 04:01:10 djm Exp $ */ #ifndef _KRL_H #define _KRL_H @@ -57,10 +57,8 @@ int ssh_krl_revoke_key_explicit(struct ssh_krl *krl, const struct sshkey *key); int ssh_krl_revoke_key_sha1(struct ssh_krl *krl, const u_char *p, size_t len); int ssh_krl_revoke_key_sha256(struct ssh_krl *krl, const u_char *p, size_t len); int ssh_krl_revoke_key(struct ssh_krl *krl, const struct sshkey *key); -int ssh_krl_to_blob(struct ssh_krl *krl, struct sshbuf *buf, - struct sshkey **sign_keys, u_int nsign_keys); -int ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp, - const struct sshkey **sign_ca_keys, size_t nsign_ca_keys); +int ssh_krl_to_blob(struct ssh_krl *krl, struct sshbuf *buf); +int ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp); int ssh_krl_check_key(struct ssh_krl *krl, const struct sshkey *key); int ssh_krl_file_contains_key(const char *path, const struct sshkey *key); int krl_dump(struct ssh_krl *krl, FILE *f); diff --git a/ssh-keygen.c b/ssh-keygen.c index 93c3ff70e..9ccea624c 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-keygen.c,v 1.469 2023/07/14 05:31:44 djm Exp $ */ +/* $OpenBSD: ssh-keygen.c,v 1.470 2023/07/17 04:01:10 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1994 Tatu Ylonen , Espoo, Finland @@ -2223,7 +2223,7 @@ load_krl(const char *path, struct ssh_krl **krlp) if ((r = sshbuf_load_file(path, &krlbuf)) != 0) fatal_r(r, "Unable to load KRL %s", path); /* XXX check sigs */ - if ((r = ssh_krl_from_blob(krlbuf, krlp, NULL, 0)) != 0 || + if ((r = ssh_krl_from_blob(krlbuf, krlp)) != 0 || *krlp == NULL) fatal_r(r, "Invalid KRL file %s", path); sshbuf_free(krlbuf); @@ -2466,7 +2466,7 @@ do_gen_krl(struct passwd *pw, int updating, const char *ca_key_path, if ((kbuf = sshbuf_new()) == NULL) fatal("sshbuf_new failed"); - if (ssh_krl_to_blob(krl, kbuf, NULL, 0) != 0) + if (ssh_krl_to_blob(krl, kbuf) != 0) fatal("Couldn't generate KRL"); if ((r = sshbuf_write_file(identity_file, kbuf)) != 0) fatal("write %s: %s", identity_file, strerror(errno));