Some wishes regarding revoked keys

Damien Miller djm at mindrot.org
Fri Sep 7 13:57:26 AEST 2018



On Fri, 7 Sep 2018, Alexander E. Patrakov wrote:

> Hello.
> 
> I am trying to play through the following test scenario about 
> certificate revocation on Ubuntu 18.04, which has OpenSSH of this version:
> OpenSSH_7.6p1 Ubuntu-4, OpenSSL 1.0.2n  7 Dec 2017
> 
> 1. A CA key is created
> ssh-keygen -t ed25519 -f ca
> 
> 2. The CA public key is added to ~/.ssh/authorized_keys on some server:
> cert-authority ssh-ed25519 AAAA...e ca at yoga
> 
> 3. A user key is created on a different laptop:
> ssh-keygen -t ed25519 -f user
> 
> 4. The CA gets user.pub, signs the user key and applies restrictions, 
> then transfers user-cert.pub back to the user's laptop
> ssh-keygen -s ca -I user -V 20180906:20180908 -O clear -O permit-pty user
> (yes I know, at this point the CA has made a mistake of not specifying 
> the unique serial, but still there is a unique ID supplied with "-I 
> user", so nothing fatal)
> 
> 5. The CA destroys its copy of the user.pub and user-cert.pub files, 
> because a guide (e.g. https://ef.gy/hardening-ssh) says it is a good idea.

IMO #4 and #5 are bad CA practices and are the root of your problems.

That being said, I think it's a great idea for the sshd authentication
log messages to contain enough information to be able to revoke keys
under all circumstances.

The following patches implement this. They include the cert fingerprint
in the sshd "Accepted publickey ..." message and allow revocation by
the fingerprint hash. E.g.

Given the log message of:

Accepted publickey for djm from 127.0.0.1 port 54091 ssh2: RSA-CERT SHA256:hNggUg6sCnYZFlaauRzwfqVSRsep3OpSngU7xQEj0ac ID regress user key for djm (serial 19564) CA RSA SHA256:b6AasnX04A2zkjc6oWYEPhI01QsjK0h2YmjWdxCMe2U

You'll be able to prepare a KRL spec:

echo "SHA256:hNggUg6sCnYZFlaauRzwfqVSRsep3OpSngU7xQEj0ac" > /tmp/krlspec
ssh-keygen -kf /tmp/krl /tmp/krlspec

Unfortunately, since I had to add a new KRL section to list keys by
SHA256 hash, these KRLs will only be readable by OpenSSH with the patch
(hopefully it will ship in OpenSSH 7.9). A sshd that gets a KRL with
unknown entries will refuse *all* keys.

-d

diff --git a/auth.c b/auth.c
index a6b294c..78f145a 100644
--- a/auth.c
+++ b/auth.c
@@ -205,22 +205,26 @@ format_method_key(Authctxt *authctxt)
 {
 	const struct sshkey *key = authctxt->auth_method_key;
 	const char *methinfo = authctxt->auth_method_info;
-	char *fp, *ret = NULL;
+	char *fp, *cafp, *ret = NULL;
 
 	if (key == NULL)
 		return NULL;
 
 	if (sshkey_is_cert(key)) {
-		fp = sshkey_fingerprint(key->cert->signature_key,
+		fp = sshkey_fingerprint(key,
 		    options.fingerprint_hash, SSH_FP_DEFAULT);
-		xasprintf(&ret, "%s ID %s (serial %llu) CA %s %s%s%s",
-		    sshkey_type(key), key->cert->key_id,
+		cafp = sshkey_fingerprint(key->cert->signature_key,
+		    options.fingerprint_hash, SSH_FP_DEFAULT);
+		xasprintf(&ret, "%s %s ID %s (serial %llu) CA %s %s%s%s",
+		    sshkey_type(key), fp == NULL ? "(null)" : fp,
+		    key->cert->key_id,
 		    (unsigned long long)key->cert->serial,
 		    sshkey_type(key->cert->signature_key),
-		    fp == NULL ? "(null)" : fp,
+		    cafp == NULL ? "(null)" : cafp,
 		    methinfo == NULL ? "" : ", ",
 		    methinfo == NULL ? "" : methinfo);
 		free(fp);
+		free(cafp);
 	} else {
 		fp = sshkey_fingerprint(key, options.fingerprint_hash,
 		    SSH_FP_DEFAULT);
@@ -238,7 +242,7 @@ auth_log(Authctxt *authctxt, int authenticated, int partial,
     const char *method, const char *submethod)
 {
 	struct ssh *ssh = active_state; /* XXX */
-	void (*authlog) (const char *fmt,...) = verbose;
+	int level = SYSLOG_LEVEL_VERBOSE;
 	const char *authmsg;
 	char *extra = NULL;
 
@@ -250,7 +254,7 @@ auth_log(Authctxt *authctxt, int authenticated, int partial,
 	    !authctxt->valid ||
 	    authctxt->failures >= options.max_authtries / 2 ||
 	    strcmp(method, "password") == 0)
-		authlog = logit;
+		level = SYSLOG_LEVEL_INFO;
 
 	if (authctxt->postponed)
 		authmsg = "Postponed";
@@ -264,7 +268,7 @@ auth_log(Authctxt *authctxt, int authenticated, int partial,
 			extra = xstrdup(authctxt->auth_method_info);
 	}
 
-	authlog("%s %s%s%s for %s%.100s from %.200s port %d ssh2%s%s",
+	do_log2(level, "%s %s%s%s for %s%.100s from %.200s port %d ssh2%s%s",
 	    authmsg,
 	    method,
 	    submethod != NULL ? "/" : "", submethod == NULL ? "" : submethod,
diff --git a/PROTOCOL.krl b/PROTOCOL.krl
index f319bad..33bffdc 100644
--- a/PROTOCOL.krl
+++ b/PROTOCOL.krl
@@ -36,6 +36,7 @@ The available section types are:
 #define KRL_SECTION_EXPLICIT_KEY		2
 #define KRL_SECTION_FINGERPRINT_SHA1		3
 #define KRL_SECTION_SIGNATURE			4
+#define KRL_SECTION_FINGERPRINT_SHA256		5
 
 2. Certificate section
 
@@ -127,18 +128,19 @@ must be a raw key (i.e. not a certificate).
 
 This section may appear multiple times.
 
-4. SHA1 fingerprint sections
+4. SHA1/SHA256 fingerprint sections
 
-These sections, identified as KRL_SECTION_FINGERPRINT_SHA1, revoke
-plain keys (i.e. not certificates) by listing their SHA1 hashes:
+These sections, identified as KRL_SECTION_FINGERPRINT_SHA1 and
+KRL_SECTION_FINGERPRINT_SHA256, revoke plain keys (i.e. not
+certificates) by listing their hashes:
 
 	string	public_key_hash[0]
 	....
 
 This section must contain at least one "public_key_hash". The hash blob
-is obtained by taking the SHA1 hash of the public key blob. Hashes in
-this section must appear in numeric order, treating each hash as a big-
-endian integer.
+is obtained by taking the SHA1 or SHA256 hash of the public key blob.
+Hashes in this section must appear in numeric order, treating each hash
+as a big-endian integer.
 
 This section may appear multiple times.
 
diff --git a/krl.c b/krl.c
index 19c27e4..ecee295 100644
--- a/krl.c
+++ b/krl.c
@@ -94,6 +94,7 @@ struct ssh_krl {
 	char *comment;
 	struct revoked_blob_tree revoked_keys;
 	struct revoked_blob_tree revoked_sha1s;
+	struct revoked_blob_tree revoked_sha256s;
 	struct revoked_certs_list revoked_certs;
 };
 
@@ -134,6 +135,7 @@ ssh_krl_init(void)
 		return NULL;
 	RB_INIT(&krl->revoked_keys);
 	RB_INIT(&krl->revoked_sha1s);
+	RB_INIT(&krl->revoked_sha256s);
 	TAILQ_INIT(&krl->revoked_certs);
 	return krl;
 }
@@ -176,6 +178,11 @@ ssh_krl_free(struct ssh_krl *krl)
 		free(rb->blob);
 		free(rb);
 	}
+	RB_FOREACH_SAFE(rb, revoked_blob_tree, &krl->revoked_sha256s, trb) {
+		RB_REMOVE(revoked_blob_tree, &krl->revoked_sha256s, rb);
+		free(rb->blob);
+		free(rb);
+	}
 	TAILQ_FOREACH_SAFE(rc, &krl->revoked_certs, entry, trc) {
 		TAILQ_REMOVE(&krl->revoked_certs, rc, entry);
 		revoked_certs_free(rc);
@@ -406,25 +413,47 @@ ssh_krl_revoke_key_explicit(struct ssh_krl *krl, const struct sshkey *key)
 	return revoke_blob(&krl->revoked_keys, blob, len);
 }
 
-int
-ssh_krl_revoke_key_sha1(struct ssh_krl *krl, const struct sshkey *key)
+static int
+revoke_by_hash(struct revoked_blob_tree *target, const u_char *p, size_t len)
 {
 	u_char *blob;
-	size_t len;
 	int r;
 
-	debug3("%s: revoke type %s by sha1", __func__, sshkey_type(key));
-	if ((r = sshkey_fingerprint_raw(key, SSH_DIGEST_SHA1,
-	    &blob, &len)) != 0)
+	/* need to copy hash, as revoke_blob steals ownership */
+	if ((blob = malloc(len)) == NULL)
+		return SSH_ERR_SYSTEM_ERROR;
+	memcpy(blob, p, len);
+	if ((r = revoke_blob(target, blob, len)) != 0) {
+		free(blob);
 		return r;
-	return revoke_blob(&krl->revoked_sha1s, blob, len);
+	}
+	return 0;
+}
+
+int
+ssh_krl_revoke_key_sha1(struct ssh_krl *krl, const u_char *p, size_t len)
+{
+	debug3("%s: revoke by sha1", __func__);
+	if (len != 20)
+		return SSH_ERR_INVALID_FORMAT;
+	return revoke_by_hash(&krl->revoked_sha1s, p, len);
+}
+
+int
+ssh_krl_revoke_key_sha256(struct ssh_krl *krl, const u_char *p, size_t len)
+{
+	debug3("%s: revoke by sha1", __func__);
+	if (len != 32)
+		return SSH_ERR_INVALID_FORMAT;
+	return revoke_by_hash(&krl->revoked_sha256s, p, len);
 }
 
 int
 ssh_krl_revoke_key(struct ssh_krl *krl, const struct sshkey *key)
 {
+	/* XXX replace with SHA256? */
 	if (!sshkey_is_cert(key))
-		return ssh_krl_revoke_key_sha1(krl, key);
+		return ssh_krl_revoke_key_explicit(krl, key);
 
 	if (key->cert->serial == 0) {
 		return ssh_krl_revoke_cert_by_key_id(krl,
@@ -760,6 +789,18 @@ ssh_krl_to_blob(struct ssh_krl *krl, struct sshbuf *buf,
 		    (r = sshbuf_put_stringb(buf, sect)) != 0)
 			goto out;
 	}
+	sshbuf_reset(sect);
+	RB_FOREACH(rb, revoked_blob_tree, &krl->revoked_sha256s) {
+		KRL_DBG(("%s: hash len %zu ", __func__, rb->len));
+		if ((r = sshbuf_put_string(sect, rb->blob, rb->len)) != 0)
+			goto out;
+	}
+	if (sshbuf_len(sect) != 0) {
+		if ((r = sshbuf_put_u8(buf,
+		    KRL_SECTION_FINGERPRINT_SHA256)) != 0 ||
+		    (r = sshbuf_put_stringb(buf, sect)) != 0)
+			goto out;
+	}
 
 	for (i = 0; i < nsign_keys; i++) {
 		KRL_DBG(("%s: signature key %s", __func__,
@@ -912,6 +953,29 @@ parse_revoked_certs(struct sshbuf *buf, struct ssh_krl *krl)
 	return r;
 }
 
+static int
+blob_section(struct sshbuf *sect, struct revoked_blob_tree *target_tree,
+    size_t expected_len)
+{
+	u_char *rdata = NULL;
+	size_t rlen = 0;
+	int r;
+
+	while (sshbuf_len(sect) > 0) {
+		if ((r = sshbuf_get_string(sect, &rdata, &rlen)) != 0)
+			return r;
+		if (expected_len != 0 && rlen != expected_len) {
+			error("%s: bad length", __func__);
+			free(rdata);
+			return SSH_ERR_INVALID_FORMAT;
+		}
+		if ((r = revoke_blob(target_tree, rdata, rlen)) != 0) {
+			free(rdata);
+			return r;
+		}
+	}
+	return 0;
+}
 
 /* Attempt to parse a KRL, checking its signature (if any) with sign_ca_keys. */
 int
@@ -923,9 +987,9 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
 	char timestamp[64];
 	int r = SSH_ERR_INTERNAL_ERROR, sig_seen;
 	struct sshkey *key = NULL, **ca_used = NULL, **tmp_ca_used;
-	u_char type, *rdata = NULL;
+	u_char type;
 	const u_char *blob;
-	size_t i, j, sig_off, sects_off, rlen, blen, nca_used;
+	size_t i, j, sig_off, sects_off, blen, nca_used;
 	u_int format_version;
 
 	nca_used = 0;
@@ -1066,24 +1130,19 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
 				goto out;
 			break;
 		case KRL_SECTION_EXPLICIT_KEY:
+			if ((r = blob_section(sect,
+			    &krl->revoked_keys, 0)) != 0)
+				goto out;
+			break;
 		case KRL_SECTION_FINGERPRINT_SHA1:
-			while (sshbuf_len(sect) > 0) {
-				if ((r = sshbuf_get_string(sect,
-				    &rdata, &rlen)) != 0)
-					goto out;
-				if (type == KRL_SECTION_FINGERPRINT_SHA1 &&
-				    rlen != 20) {
-					error("%s: bad SHA1 length", __func__);
-					r = SSH_ERR_INVALID_FORMAT;
-					goto out;
-				}
-				if ((r = revoke_blob(
-				    type == KRL_SECTION_EXPLICIT_KEY ?
-				    &krl->revoked_keys : &krl->revoked_sha1s,
-				    rdata, rlen)) != 0)
-					goto out;
-				rdata = NULL; /* revoke_blob frees rdata */
-			}
+			if ((r = blob_section(sect,
+			    &krl->revoked_sha1s, 20)) != 0)
+				goto out;
+			break;
+		case KRL_SECTION_FINGERPRINT_SHA256:
+			if ((r = blob_section(sect,
+			    &krl->revoked_sha256s, 32)) != 0)
+				goto out;
 			break;
 		case KRL_SECTION_SIGNATURE:
 			/* Handled above, but still need to stay in synch */
@@ -1148,7 +1207,6 @@ ssh_krl_from_blob(struct sshbuf *buf, struct ssh_krl **krlp,
 	for (i = 0; i < nca_used; i++)
 		sshkey_free(ca_used[i]);
 	free(ca_used);
-	free(rdata);
 	sshkey_free(key);
 	sshbuf_free(copy);
 	sshbuf_free(sect);
@@ -1208,6 +1266,16 @@ is_key_revoked(struct ssh_krl *krl, const struct sshkey *key)
 		KRL_DBG(("%s: revoked by key SHA1", __func__));
 		return SSH_ERR_KEY_REVOKED;
 	}
+	memset(&rb, 0, sizeof(rb));
+	if ((r = sshkey_fingerprint_raw(key, SSH_DIGEST_SHA256,
+	    &rb.blob, &rb.len)) != 0)
+		return r;
+	erb = RB_FIND(revoked_blob_tree, &krl->revoked_sha256s, &rb);
+	free(rb.blob);
+	if (erb != NULL) {
+		KRL_DBG(("%s: revoked by key SHA256", __func__));
+		return SSH_ERR_KEY_REVOKED;
+	}
 
 	/* Next, explicit keys */
 	memset(&rb, 0, sizeof(rb));
diff --git a/krl.h b/krl.h
index 675496c..17cd70a 100644
--- a/krl.h
+++ b/krl.h
@@ -29,6 +29,7 @@
 #define KRL_SECTION_EXPLICIT_KEY	2
 #define KRL_SECTION_FINGERPRINT_SHA1	3
 #define KRL_SECTION_SIGNATURE		4
+#define KRL_SECTION_FINGERPRINT_SHA256	5
 
 /* KRL_SECTION_CERTIFICATES subsection types */
 #define KRL_SECTION_CERT_SERIAL_LIST	0x20
@@ -51,7 +52,8 @@ int ssh_krl_revoke_cert_by_serial_range(struct ssh_krl *krl,
 int ssh_krl_revoke_cert_by_key_id(struct ssh_krl *krl,
     const struct sshkey *ca_key, const char *key_id);
 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 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,
     const struct sshkey **sign_keys, u_int nsign_keys);
diff --git a/ssh-keygen.1 b/ssh-keygen.1
index dd6e7e5..5bb616f 100644
--- a/ssh-keygen.1
+++ b/ssh-keygen.1
@@ -814,7 +814,20 @@ option.
 Revokes the specified key.
 If a certificate is listed, then it is revoked as a plain public key.
 .It Cm sha1 : Ar public_key
-Revokes the specified key by its SHA1 hash.
+Revokes the specified key by including its SHA1 hash in the KRL.
+.It Cm sha256 : Ar public_key
+Revokes the specified key by including its SHA256 hash in the KRL.
+KRLs that revoke keys by SHA256 hash are not supported by OpenSSH versions
+prior to 7.9.
+.It Cm hash : Ar fingerprint
+Revokes a key using by fingerprint hash, as obtained from a
+.Xr sshd 8
+authentication log message or the
+.Nm
+.Fl l
+flag.
+Only SHA256 fingerprints are supported here and resultant KRLs are
+not supported by OpenSSH versions prior to 7.9.
 .El
 .Pp
 KRLs may be updated using the
diff --git a/ssh-keygen.c b/ssh-keygen.c
index 12a5add..21cc52f 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -2063,6 +2063,41 @@ load_krl(const char *path, struct ssh_krl **krlp)
 	sshbuf_free(krlbuf);
 }
 
+static void
+hash_to_blob(const char *cp, u_char **blobp, size_t *lenp,
+    const char *file, u_long lnum)
+{
+	char *tmp;
+	size_t tlen;
+	struct sshbuf *b;
+	int r;
+
+	if (strncmp(cp, "SHA256:", 7) != 0)
+		fatal("%s:%lu: unsupported hash algorithm", file, lnum);
+	cp += 7;
+
+	/*
+	 * OpenSSH base64 hashes omit trailing '='
+	 * characters; put them back for decode.
+	 */
+	tlen = strlen(cp);
+	tmp = xmalloc(tlen + 4 + 1);
+	strlcpy(tmp, cp, tlen + 1);
+	while ((tlen % 4) != 0) {
+		tmp[tlen++] = '=';
+		tmp[tlen] = '\0';
+	}
+	if ((b = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((r = sshbuf_b64tod(b, tmp)) != 0)
+		fatal("%s:%lu: decode hash failed: %s", file, lnum, ssh_err(r));
+	free(tmp);
+	*lenp = sshbuf_len(b);
+	*blobp = xmalloc(*lenp);
+	memcpy(*blobp, sshbuf_ptr(b), *lenp);
+	sshbuf_free(b);
+}
+
 static void
 update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
     const struct sshkey *ca, struct ssh_krl *krl)
@@ -2070,9 +2105,10 @@ update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
 	struct sshkey *key = NULL;
 	u_long lnum = 0;
 	char *path, *cp, *ep, *line = NULL;
-	size_t linesize = 0;
+	u_char *blob = NULL;
+	size_t blen = 0, linesize = 0;
 	unsigned long long serial, serial2;
-	int i, was_explicit_key, was_sha1, r;
+	int i, was_explicit_key, was_sha1, was_sha256, was_hash, r;
 	FILE *krl_spec;
 
 	path = tilde_expand_filename(file, pw->pw_uid);
@@ -2087,7 +2123,7 @@ update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
 		printf("Revoking from %s\n", path);
 	while (getline(&line, &linesize, krl_spec) != -1) {
 		lnum++;
-		was_explicit_key = was_sha1 = 0;
+		was_explicit_key = was_sha1 = was_sha256 = was_hash = 0;
 		cp = line + strspn(line, " \t");
 		/* Trim trailing space, comments and strip \n */
 		for (i = 0, r = -1; cp[i] != '\0'; i++) {
@@ -2152,6 +2188,11 @@ update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
 			cp = cp + strspn(cp, " \t");
 			if (ssh_krl_revoke_cert_by_key_id(krl, ca, cp) != 0)
 				fatal("%s: revoke key ID failed", __func__);
+		} else if (strncasecmp(cp, "hash:", 5) == 0) {
+			cp += 5;
+			cp = cp + strspn(cp, " \t");
+			hash_to_blob(cp, &blob, &blen, file, lnum);
+			r = ssh_krl_revoke_key_sha256(krl, blob, blen);
 		} else {
 			if (strncasecmp(cp, "key:", 4) == 0) {
 				cp += 4;
@@ -2161,7 +2202,10 @@ update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
 				cp += 5;
 				cp = cp + strspn(cp, " \t");
 				was_sha1 = 1;
-			} else {
+			} else if (strncasecmp(cp, "sha256:", 7) == 0) {
+				cp += 7;
+				cp = cp + strspn(cp, " \t");
+				was_sha256 = 1;
 				/*
 				 * Just try to process the line as a key.
 				 * Parsing will fail if it isn't.
@@ -2174,13 +2218,28 @@ update_krl_from_file(struct passwd *pw, const char *file, int wild_ca,
 				    path, lnum, ssh_err(r));
 			if (was_explicit_key)
 				r = ssh_krl_revoke_key_explicit(krl, key);
-			else if (was_sha1)
-				r = ssh_krl_revoke_key_sha1(krl, key);
-			else
+			else if (was_sha1) {
+				if (sshkey_fingerprint_raw(key,
+				    SSH_DIGEST_SHA1, &blob, &blen) != 0) {
+					fatal("%s:%lu: fingerprint failed",
+					    file, lnum);
+				}
+				r = ssh_krl_revoke_key_sha1(krl, blob, blen);
+			} else if (was_sha256) {
+				if (sshkey_fingerprint_raw(key,
+				    SSH_DIGEST_SHA256, &blob, &blen) != 0) {
+					fatal("%s:%lu: fingerprint failed",
+					    file, lnum);
+				}
+				r = ssh_krl_revoke_key_sha256(krl, blob, blen);
+			} else
 				r = ssh_krl_revoke_key(krl, key);
 			if (r != 0)
 				fatal("%s: revoke key failed: %s",
 				    __func__, ssh_err(r));
+			freezero(blob, blen);
+			blob = NULL;
+			blen = 0;
 			sshkey_free(key);
 		}
 	}


More information about the openssh-unix-dev mailing list