[openssh-commits] [openssh] 01/02: upstream: ssh-keygen -Y find-principals was verifying key validity

git+noreply at mindrot.org git+noreply at mindrot.org
Fri Nov 19 08:13:06 AEDT 2021


This is an automated email from the git hooks/post-receive script.

djm pushed a commit to branch master
in repository openssh.

commit c74aa0eb73bd1edf79947d92d9c618fc3424c4a6
Author: djm at openbsd.org <djm at openbsd.org>
Date:   Thu Nov 18 03:50:41 2021 +0000

    upstream: ssh-keygen -Y find-principals was verifying key validity
    
    when using ca certs but not with simple key lifetimes within the allowed
    signers file.
    
    Since it returns the first keys principal it finds this could
    result in a principal with an expired key even though a valid
    one is just below.
    
    patch from Fabian Stelzer; feedback/ok djm markus
    
    OpenBSD-Commit-ID: b108ed0a76b813226baf683ab468dc1cc79e0905
---
 sshsig.c | 101 +++++++++++++++++++++++----------------------------------------
 1 file changed, 36 insertions(+), 65 deletions(-)

diff --git a/sshsig.c b/sshsig.c
index b05e7415..5039a0fb 100644
--- a/sshsig.c
+++ b/sshsig.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshsig.c,v 1.22 2021/11/05 03:10:58 djm Exp $ */
+/* $OpenBSD: sshsig.c,v 1.23 2021/11/18 03:50:41 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -869,17 +869,21 @@ cert_filter_principals(const char *path, u_long linenum,
 static int
 check_allowed_keys_line(const char *path, u_long linenum, char *line,
     const struct sshkey *sign_key, const char *principal,
-    const char *sig_namespace, uint64_t verify_time)
+    const char *sig_namespace, uint64_t verify_time, char **principalsp)
 {
 	struct sshkey *found_key = NULL;
+	char *principals = NULL;
 	int r, success = 0;
 	const char *reason = NULL;
 	struct sshsigopt *sigopts = NULL;
 	char tvalid[64], tverify[64];
 
+	if (principalsp != NULL)
+		*principalsp = NULL;
+
 	/* Parse the line */
 	if ((r = parse_principals_key_and_options(path, linenum, line,
-	    principal, NULL, &found_key, &sigopts)) != 0) {
+	    principal, &principals, &found_key, &sigopts)) != 0) {
 		/* error already logged */
 		goto done;
 	}
@@ -889,14 +893,28 @@ check_allowed_keys_line(const char *path, u_long linenum, char *line,
 		debug("%s:%lu: matched key", path, linenum);
 	} else if (sigopts->ca && sshkey_is_cert(sign_key) &&
 	    sshkey_equal_public(sign_key->cert->signature_key, found_key)) {
-		/* Match of certificate's CA key */
-		if ((r = sshkey_cert_check_authority(sign_key, 0, 1, 0,
-		    verify_time, principal, &reason)) != 0) {
-			error("%s:%lu: certificate not authorized: %s",
-			    path, linenum, reason);
-			goto done;
+		if (principal) {
+			/* Match certificate CA key with specified principal */
+			if ((r = sshkey_cert_check_authority(sign_key, 0, 1, 0,
+			    verify_time, principal, &reason)) != 0) {
+				error("%s:%lu: certificate not authorized: %s",
+				    path, linenum, reason);
+				goto done;
+			}
+			debug("%s:%lu: matched certificate CA key",
+			    path, linenum);
+		} else {
+			/* No principal specified - find all matching ones */
+			if ((r = cert_filter_principals(path, linenum,
+			    &principals, sign_key, verify_time)) != 0) {
+				/* error already displayed */
+				debug_r(r, "%s:%lu: cert_filter_principals",
+				    path, linenum);
+				goto done;
+			}
+			debug("%s:%lu: matched certificate CA key",
+			    path, linenum);
 		}
-		debug("%s:%lu: matched certificate CA key", path, linenum);
 	} else {
 		/* Didn't match key */
 		goto done;
@@ -933,6 +951,11 @@ check_allowed_keys_line(const char *path, u_long linenum, char *line,
 	success = 1;
 
  done:
+	if (success && principalsp != NULL) {
+		*principalsp = principals;
+		principals = NULL; /* transferred */
+	}
+	free(principals);
 	sshkey_free(found_key);
 	sshsigopt_free(sigopts);
 	return success ? 0 : SSH_ERR_KEY_NOT_FOUND;
@@ -960,7 +983,7 @@ sshsig_check_allowed_keys(const char *path, const struct sshkey *sign_key,
 	while (getline(&line, &linesize, f) != -1) {
 		linenum++;
 		r = check_allowed_keys_line(path, linenum, line, sign_key,
-		    principal, sig_namespace, verify_time);
+		    principal, sig_namespace, verify_time, NULL);
 		free(line);
 		line = NULL;
 		linesize = 0;
@@ -979,58 +1002,6 @@ sshsig_check_allowed_keys(const char *path, const struct sshkey *sign_key,
 	return r == 0 ? SSH_ERR_KEY_NOT_FOUND : r;
 }
 
-static int
-get_matching_principals_from_line(const char *path, u_long linenum, char *line,
-    const struct sshkey *sign_key, uint64_t verify_time, char **principalsp)
-{
-	struct sshkey *found_key = NULL;
-	char *principals = NULL;
-	int r, found = 0;
-	struct sshsigopt *sigopts = NULL;
-
-	if (principalsp != NULL)
-		*principalsp = NULL;
-
-	/* Parse the line */
-	if ((r = parse_principals_key_and_options(path, linenum, line,
-	    NULL, &principals, &found_key, &sigopts)) != 0) {
-		/* error already logged */
-		goto done;
-	}
-
-	if (!sigopts->ca && sshkey_equal(found_key, sign_key)) {
-		/* Exact match of key */
-		debug("%s:%lu: matched key", path, linenum);
-		/* success */
-		found = 1;
-	} else if (sigopts->ca && sshkey_is_cert(sign_key) &&
-	    sshkey_equal_public(sign_key->cert->signature_key, found_key)) {
-		/* Remove principals listed in file but not allowed by cert */
-		if ((r = cert_filter_principals(path, linenum,
-		    &principals, sign_key, verify_time)) != 0) {
-			/* error already displayed */
-			debug_r(r, "%s:%lu: cert_filter_principals",
-			    path, linenum);
-			goto done;
-		}
-		debug("%s:%lu: matched certificate CA key", path, linenum);
-		/* success */
-		found = 1;
-	} else {
-		/* Key didn't match */
-		goto done;
-	}
- done:
-	if (found && principalsp != NULL) {
-		*principalsp = principals;
-		principals = NULL; /* transferred */
-	}
-	free(principals);
-	sshkey_free(found_key);
-	sshsigopt_free(sigopts);
-	return found ? 0 : SSH_ERR_KEY_NOT_FOUND;
-}
-
 int
 sshsig_find_principals(const char *path, const struct sshkey *sign_key,
     uint64_t verify_time, char **principals)
@@ -1051,8 +1022,8 @@ sshsig_find_principals(const char *path, const struct sshkey *sign_key,
 
 	while (getline(&line, &linesize, f) != -1) {
 		linenum++;
-		r = get_matching_principals_from_line(path, linenum, line,
-		    sign_key, verify_time, principals);
+		r = check_allowed_keys_line(path, linenum, line,
+		    sign_key, NULL, NULL, verify_time, principals);
 		free(line);
 		line = NULL;
 		linesize = 0;

-- 
To stop receiving notification emails like this one, please contact
djm at mindrot.org.


More information about the openssh-commits mailing list