upstream: ssh-keygen -Y find-principals fixes based on feedback

from Markus:

use "principals" instead of principal, as allowed_signers lines may list
multiple.

When the signing key is a certificate, emit only principals that match
the certificate principal list.

NB. the command -Y name changes: "find-principal" => "find-principals"

ok markus@

OpenBSD-Commit-ID: ab575946ff9a55624cd4e811bfd338bf3b1d0faf

diff --git a/ssh-keygen.1 b/ssh-keygen.1
index 5d33902f7c1b6a552d0079d597c6a29cd7601f14..b4a873920394ffa15ba0b19db71588d14380015a 100644 (file)
--- a/ssh-keygen.1
+++ b/ssh-keygen.1
@@ -1,4 +1,4 @@
-.\"    $OpenBSD: ssh-keygen.1,v 1.195 2020/01/23 07:16:38 jmc Exp $
+.\"    $OpenBSD: ssh-keygen.1,v 1.196 2020/01/23 23:31:52 djm Exp $
 .\"
 .\" Author: Tatu Ylonen <ylo@cs.hut.fi>
 .\" Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -138,7 +138,7 @@
 .Fl f Ar krl_file
 .Ar
 .Nm ssh-keygen
-.Fl Y Cm find-principal
+.Fl Y Cm find-principals
 .Fl s Ar signature_file
 .Fl f Ar allowed_signers_file
 .Nm ssh-keygen
@@ -618,8 +618,8 @@ The maximum is 3.
 Specifies a path to a library that will be used when creating
 FIDO authenticator-hosted keys, overriding the default of using
 the internal USB HID support.
-.It Fl Y Cm find-principal
-Find the principal associated with the public key of a signature,
+.It Fl Y Cm find-principals
+Find the principal(s) associated with the public key of a signature,
 provided using the
 .Fl s
 flag in an authorized signers file provided using the
@@ -628,7 +628,8 @@ flag.
 The format of the allowed signers file is documented in the
 .Sx ALLOWED SIGNERS
 section below.
-If a matching principal is found, it is returned on standard output.
+If one or more matching principals are found, they are returned on
+standard output.
 .It Fl Y Cm check-novalidate
 Checks that a signature generated using
 .Nm

diff --git a/ssh-keygen.c b/ssh-keygen.c
index ce94a5ab03a9ad48b001374a8fd2d567be93df7c..363da70db7b25bf0fc51019545c4b1f74a54b4eb 100644 (file)
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.387 2020/01/23 07:54:04 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.388 2020/01/23 23:31:52 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -2758,11 +2758,11 @@ done:
 }
 
 static int
-sig_find_principal(const char *signature, const char *allowed_keys) {
+sig_find_principals(const char *signature, const char *allowed_keys) {
        int r, ret = -1, sigfd = -1;
        struct sshbuf *sigbuf = NULL, *abuf = NULL;
        struct sshkey *sign_key = NULL;
-       char *principal = NULL;
+       char *principals = NULL;
 
        if ((abuf = sshbuf_new()) == NULL)
                fatal("%s: sshbuf_new() failed", __func__);
@@ -2782,12 +2782,11 @@ sig_find_principal(const char *signature, const char *allowed_keys) {
        }
        if ((r = sshsig_get_pubkey(sigbuf, &sign_key)) != 0) {
                error("%s: sshsig_get_pubkey: %s",
-                     __func__, ssh_err(r));
+                   __func__, ssh_err(r));
                goto done;
        }
-
-       if ((r = sshsig_find_principal(allowed_keys, sign_key,
-                                     &principal)) != 0) {
+       if ((r = sshsig_find_principals(allowed_keys, sign_key,
+           &principals)) != 0) {
                error("%s: sshsig_get_principal: %s",
                      __func__, ssh_err(r));
                goto done;
@@ -2795,7 +2794,7 @@ sig_find_principal(const char *signature, const char *allowed_keys) {
        ret = 0;
 done:
        if (ret == 0 ) {
-               printf("Found matching principal: %s\n", principal);
+               printf("Found matching principal: %s\n", principals);
        } else {
                printf("Could not find matching principal.\n");
        }
@@ -2804,7 +2803,7 @@ done:
        sshbuf_free(sigbuf);
        sshbuf_free(abuf);
        sshkey_free(sign_key);
-       free(principal);
+       free(principals);
        return ret;
 }
 
@@ -3093,7 +3092,7 @@ usage(void)
            "       ssh-keygen -k -f krl_file [-u] [-s ca_public] [-z version_number]\n"
            "                  file ...\n"
            "       ssh-keygen -Q -f krl_file file ...\n"
-           "       ssh-keygen -Y find-principal -s signature_file -f allowed_signers_file\n"
+           "       ssh-keygen -Y find-principals -s signature_file -f allowed_signers_file\n"
            "       ssh-keygen -Y check-novalidate -n namespace -s signature_file\n"
            "       ssh-keygen -Y sign -f key_file -n namespace file ...\n"
            "       ssh-keygen -Y verify -f allowed_signers_file -I signer_identity\n"
@@ -3357,18 +3356,18 @@ main(int argc, char **argv)
        argc -= optind;
 
        if (sign_op != NULL) {
-               if (strncmp(sign_op, "find-principal", 14) == 0) {
+               if (strncmp(sign_op, "find-principals", 15) == 0) {
                        if (ca_key_path == NULL) {
-                               error("Too few arguments for find-principal:"
+                               error("Too few arguments for find-principals:"
                                      "missing signature file");
                                exit(1);
                        }
                        if (!have_identity) {
-                               error("Too few arguments for find-principal:"
+                               error("Too few arguments for find-principals:"
                                      "missing allowed keys file");
                                exit(1);
                        }
-                       return sig_find_principal(ca_key_path, identity_file);
+                       return sig_find_principals(ca_key_path, identity_file);
                }
                if (cert_principals == NULL || *cert_principals == '\0') {
                        error("Too few arguments for sign/verify: "

diff --git a/sshsig.c b/sshsig.c
index e9f4baa762decc12b071c1b0a8346236b8ea0b0b..e63a36e1ec43c83986c061db3955b893045f044c 100644 (file)
--- a/sshsig.c
+++ b/sshsig.c
@@ -868,13 +868,64 @@ sshsig_check_allowed_keys(const char *path, const struct sshkey *sign_key,
 }
 
 static int
-get_matching_principal_from_line(const char *path, u_long linenum, char *line,
+cert_filter_principals(const char *path, u_long linenum,
+    char **principalsp, const struct sshkey *cert)
+{
+       char *cp, *oprincipals, *principals;
+       const char *reason;
+       struct sshbuf *nprincipals;
+       int r = SSH_ERR_INTERNAL_ERROR, success = 0;
+
+       oprincipals = principals = *principalsp;
+       *principalsp = NULL;
+
+       if ((nprincipals = sshbuf_new()) == NULL)
+               return SSH_ERR_ALLOC_FAIL;
+
+       while ((cp = strsep(&principals, ",")) != NULL && *cp != '\0') {
+               if (strcspn(cp, "!?*") != strlen(cp)) {
+                       debug("%s:%lu: principal \"%s\" not authorized: "
+                           "contains wildcards", path, linenum, cp);
+                       continue;
+               }
+               /* Check against principals list in certificate */
+               if ((r = sshkey_cert_check_authority(cert, 0, 1,
+                   cp, &reason)) != 0) {
+                       debug("%s:%lu: principal \"%s\" not authorized: %s",
+                           path, linenum, cp, reason);
+                       continue;
+               }
+               if ((r = sshbuf_putf(nprincipals, "%s%s",
+                   sshbuf_len(nprincipals) != 0 ? "," : "", cp)) != 0) {
+                       error("%s: buffer error", __func__);
+                       goto out;
+               }
+       }
+       if (sshbuf_len(nprincipals) == 0) {
+               error("%s:%lu: no valid principals found", path, linenum);
+               r = SSH_ERR_KEY_CERT_INVALID;
+               goto out;
+       }
+       if ((principals = sshbuf_dup_string(nprincipals)) == NULL) {
+               error("%s: buffer error", __func__);
+               goto out;
+       }
+       /* success */
+       success = 1;
+       *principalsp = principals;
+ out:
+       sshbuf_free(nprincipals);
+       free(oprincipals);
+       return success ? 0 : r;
+}
+
+static int
+get_matching_principals_from_line(const char *path, u_long linenum, char *line,
     const struct sshkey *sign_key, char **principalsp)
 {
        struct sshkey *found_key = NULL;
        char *principals = NULL;
        int r, found = 0;
-       const char *reason = NULL;
        struct sshsigopt *sigopts = NULL;
 
        if (principalsp != NULL)
@@ -894,11 +945,12 @@ get_matching_principal_from_line(const char *path, u_long linenum, char *line,
                found = 1;
        } 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,
-                   principals, &reason)) != 0) {
-                       error("%s:%lu: certificate not authorized: %s",
-                           path, linenum, reason);
+               /* Remove principals listed in file but not allowed by cert */
+               if ((r = cert_filter_principals(path, linenum,
+                   &principals, sign_key)) != 0) {
+                       /* error already displayed */
+                       debug("%s:%lu: cert_filter_principals: %s",
+                           path, linenum, ssh_err(r));
                        goto done;
                }
                debug("%s:%lu: matched certificate CA key", path, linenum);
@@ -920,8 +972,8 @@ get_matching_principal_from_line(const char *path, u_long linenum, char *line,
 }
 
 int
-sshsig_find_principal(const char *path, const struct sshkey *sign_key,
-    char **principal)
+sshsig_find_principals(const char *path, const struct sshkey *sign_key,
+    char **principals)
 {
        FILE *f = NULL;
        char *line = NULL;
@@ -939,8 +991,8 @@ sshsig_find_principal(const char *path, const struct sshkey *sign_key,
 
        while (getline(&line, &linesize, f) != -1) {
                linenum++;
-               r = get_matching_principal_from_line(path, linenum, line,
-                   sign_key, principal);
+               r = get_matching_principals_from_line(path, linenum, line,
+                   sign_key, principals);
                free(line);
                line = NULL;
                if (r == SSH_ERR_KEY_NOT_FOUND)

diff --git a/sshsig.h b/sshsig.h
index 939e3dfe0bd1f936500ab2a730311933c458a453..63cc1ad1a2036b924aea66738b42256111d2c2c7 100644 (file)
--- a/sshsig.h
+++ b/sshsig.h
@@ -93,13 +93,12 @@ struct sshsigopt *sshsigopt_parse(const char *opts,
 void sshsigopt_free(struct sshsigopt *opts);
 
 /* Get public key from signature */
-int
-sshsig_get_pubkey(struct sshbuf *signature, struct sshkey **pubkey);
+int sshsig_get_pubkey(struct sshbuf *signature, struct sshkey **pubkey);
 
 /* Find principal in allowed_keys file, given a sshkey. Returns
  * 0 on success.
  */
-int sshsig_find_principal(const char *path, const struct sshkey *sign_key,
+int sshsig_find_principals(const char *path, const struct sshkey *sign_key,
     char **principal);
 
 #endif /* SSHSIG_H */