upstream: sshsig tweaks and improvements from and suggested by

Markus

ok markus/me

OpenBSD-Commit-ID: ea4f46ad5a16b27af96e08c4877423918c4253e9

diff --git a/PROTOCOL.sshsig b/PROTOCOL.sshsig
index 806c35da6620369a5b31a1a8f84a0c0b39cfadf7..720e1f18a25ef87ad06b71d2e52fb93d46ac8c55 100644 (file)
--- a/PROTOCOL.sshsig
+++ b/PROTOCOL.sshsig
@@ -8,9 +8,9 @@ At present, only detached and armored signatures are supported.
 The Armored SSH signatures consist of a header, a base64
 encoded blob, and a footer.
 
-The header is the string “-----BEGIN SSH SIGNATURE-----”
+The header is the string "-----BEGIN SSH SIGNATURE-----"
 followed by a newline. The footer is the string
-“-----END SSH SIGNATURE-----” immediately after a newline.
+"-----END SSH SIGNATURE-----" immediately after a newline.
 
 The header MUST be present at the start of every signature.
 Files containing the signature MUST start with the header.

diff --git a/ssh-keygen.1 b/ssh-keygen.1
index 93c76ef8a66be0663350ec553fe2155adda9bfac..cbaf29809bb0b3d882d06f408bbab9c14300a396 100644 (file)
--- a/ssh-keygen.1
+++ b/ssh-keygen.1
@@ -1,4 +1,4 @@
-.\"    $OpenBSD: ssh-keygen.1,v 1.163 2019/09/03 08:34:19 djm Exp $
+.\"    $OpenBSD: ssh-keygen.1,v 1.164 2019/09/03 08:35:27 djm Exp $
 .\"
 .\" Author: Tatu Ylonen <ylo@cs.hut.fi>
 .\" Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -986,7 +986,7 @@ Indicates that this key is accepted as a certificate authority (CA) and
 that certificates signed by this CA may be accepted for verification.
 .It Cm namespaces="namespace-list"
 Specifies a pattern-list of namespaces that are accepted for this key.
-If this option is present, the the signature namespace embedded in the
+If this option is present, the signature namespace embedded in the
 signature object and presented on the verification command-line must
 match the specified list before the key will be considered acceptable.
 .El

diff --git a/ssh-keygen.c b/ssh-keygen.c
index 76bc41b2f3089b8eba48eb5985406bcf4b8a03d0..527cfcf6322be23d5000b3fa8f882f260de36847 100644 (file)
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.344 2019/09/03 08:34:19 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.345 2019/09/03 08:35:27 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -2659,10 +2659,10 @@ verify(const char *signature, const char *sig_namespace, const char *principal,
        fp = NULL;
 
        if (revoked_keys != NULL) {
-           if ((r = sshkey_check_revoked(sign_key, revoked_keys)) != 0) {
-                   debug3("sshkey_check_revoked failed: %s", ssh_err(r));
-                   goto done;
-           }
+               if ((r = sshkey_check_revoked(sign_key, revoked_keys)) != 0) {
+                       debug3("sshkey_check_revoked failed: %s", ssh_err(r));
+                       goto done;
+               }
        }
 
        if ((r = sshsig_check_allowed_keys(allowed_keys, sign_key,

diff --git a/sshsig.c b/sshsig.c
index 0a1e14627db2dd23bcf00240794e87d6d4f024eb..c1f2d803ffdcb748256e9684953113cb86100538 100644 (file)
--- a/sshsig.c
+++ b/sshsig.c
@@ -230,7 +230,7 @@ sshsig_parse_preamble(struct sshbuf *buf)
                return r;
        }
 
-       if (sversion < SIG_VERSION) {
+       if (sversion > SIG_VERSION) {
                error("Signature version %lu is larger than supported "
                    "version %u", (unsigned long)sversion, SIG_VERSION);
                return SSH_ERR_INVALID_FORMAT;
@@ -241,7 +241,8 @@ sshsig_parse_preamble(struct sshbuf *buf)
 static int
 sshsig_check_hashalg(const char *hashalg)
 {
-       if (match_pattern_list(hashalg, HASHALG_ALLOWED, 0) == 1)
+       if (hashalg == NULL ||
+           match_pattern_list(hashalg, HASHALG_ALLOWED, 0) == 1)
                return 0;
        error("%s: unsupported hash algorithm \"%.100s\"", __func__, hashalg);
        return SSH_ERR_SIGN_ALG_UNSUPPORTED;
@@ -268,8 +269,6 @@ sshsig_peek_hashalg(struct sshbuf *signature, char **hashalgp)
                error("Couldn't parse signature blob: %s", ssh_err(r));
                goto done;
        }
-       if ((r = sshsig_check_hashalg(hashalg)) != 0)
-               goto done;
 
        /* success */
        r = 0;
@@ -293,6 +292,7 @@ sshsig_wrap_verify(struct sshbuf *signature, const char *hashalg,
        char *got_namespace = NULL, *sigtype = NULL, *sig_hashalg = NULL;
        size_t siglen;
 
+       debug("%s: verify message length %zu", __func__, sshbuf_len(h_message));
        if (sign_keyp != NULL)
                *sign_keyp = NULL;
 
@@ -301,9 +301,6 @@ sshsig_wrap_verify(struct sshbuf *signature, const char *hashalg,
                r = SSH_ERR_ALLOC_FAIL;
                goto done;
        }
-       if ((r = sshsig_check_hashalg(hashalg)) != 0)
-               goto done;
-
        if ((r = sshbuf_put(toverify, MAGIC_PREAMBLE,
            MAGIC_PREAMBLE_LEN)) != 0 ||
            (r = sshbuf_put_cstring(toverify, expect_namespace)) != 0 ||
@@ -382,36 +379,65 @@ done:
        return r;
 }
 
-int
-sshsig_sign_message(struct sshkey *key, const char *hashalg,
-    const struct sshbuf *message, const char *sig_namespace,
-    struct sshbuf **out, sshsig_signer *signer, void *signer_ctx)
+static int
+hash_buffer(const struct sshbuf *m, const char *hashalg, struct sshbuf **bp)
 {
-       u_char hash[SSH_DIGEST_MAX_LENGTH];
-       struct sshbuf *b = NULL;
+       char *hex, hash[SSH_DIGEST_MAX_LENGTH];
        int alg, r = SSH_ERR_INTERNAL_ERROR;
+       struct sshbuf *b = NULL;
 
-       if (out != NULL)
-               *out = NULL;
-       if (hashalg == NULL)
-           hashalg = HASHALG_DEFAULT;
+       *bp = NULL;
+       memset(hash, 0, sizeof(hash));
 
        if ((r = sshsig_check_hashalg(hashalg)) != 0)
                return r;
        if ((alg = ssh_digest_alg_by_name(hashalg)) == -1) {
                error("%s: can't look up hash algorithm %s",
-                   __func__, HASHALG_DEFAULT);
+                   __func__, hashalg);
                return SSH_ERR_INTERNAL_ERROR;
        }
-       if ((r = ssh_digest_buffer(alg, message, hash, sizeof(hash))) != 0) {
+       if ((r = ssh_digest_buffer(alg, m, hash, sizeof(hash))) != 0) {
                error("%s: ssh_digest_buffer failed: %s", __func__, ssh_err(r));
                return r;
        }
-       if ((b = sshbuf_from(hash, ssh_digest_bytes(alg))) == NULL) {
-               error("%s: sshbuf_from failed", __func__);
+       if ((hex = tohex(hash, ssh_digest_bytes(alg))) != NULL) {
+               debug3("%s: final hash: %s", __func__, hex);
+               freezero(hex, strlen(hex));
+       }
+       if ((b = sshbuf_new()) == NULL) {
                r = SSH_ERR_ALLOC_FAIL;
                goto out;
        }
+       if ((r = sshbuf_put(b, hash, ssh_digest_bytes(alg))) != 0) {
+               error("%s: sshbuf_put: %s", __func__, ssh_err(r));
+               goto out;
+       }
+       *bp = b;
+       b = NULL; /* transferred */
+       /* success */
+       r = 0;
+ out:
+       sshbuf_free(b);
+       explicit_bzero(hash, sizeof(hash));
+       return 0;
+}
+
+int
+sshsig_signb(struct sshkey *key, const char *hashalg,
+    const struct sshbuf *message, const char *sig_namespace,
+    struct sshbuf **out, sshsig_signer *signer, void *signer_ctx)
+{
+       struct sshbuf *b = NULL;
+       int r = SSH_ERR_INTERNAL_ERROR;
+
+       if (hashalg == NULL)
+               hashalg = HASHALG_DEFAULT;
+       if (out != NULL)
+               *out = NULL;
+       if ((r = hash_buffer(message, hashalg, &b)) != 0) {
+               error("%s: hash_buffer failed: %s", __func__, ssh_err(r));
+               goto out;
+       }
        if ((r = sshsig_wrap_sign(key, hashalg, b, sig_namespace, out,
            signer, signer_ctx)) != 0)
                goto out;
@@ -419,17 +445,15 @@ sshsig_sign_message(struct sshkey *key, const char *hashalg,
        r = 0;
  out:
        sshbuf_free(b);
-       explicit_bzero(hash, sizeof(hash));
        return r;
 }
 
 int
-sshsig_verify_message(struct sshbuf *signature, const struct sshbuf *message,
+sshsig_verifyb(struct sshbuf *signature, const struct sshbuf *message,
     const char *expect_namespace, struct sshkey **sign_keyp)
 {
-       u_char hash[SSH_DIGEST_MAX_LENGTH];
        struct sshbuf *b = NULL;
-       int alg, r = SSH_ERR_INTERNAL_ERROR;
+       int r = SSH_ERR_INTERNAL_ERROR;
        char *hashalg = NULL;
 
        if (sign_keyp != NULL)
@@ -437,18 +461,9 @@ sshsig_verify_message(struct sshbuf *signature, const struct sshbuf *message,
 
        if ((r = sshsig_peek_hashalg(signature, &hashalg)) != 0)
                return r;
-       if ((alg = ssh_digest_alg_by_name(hashalg)) == -1) {
-               error("%s: can't look up hash algorithm %s",
-                   __func__, HASHALG_DEFAULT);
-               return SSH_ERR_INTERNAL_ERROR;
-       }
-       if ((r = ssh_digest_buffer(alg, message, hash, sizeof(hash))) != 0) {
-               error("%s: ssh_digest_buffer failed: %s", __func__, ssh_err(r));
-               goto out;
-       }
-       if ((b = sshbuf_from(hash, ssh_digest_bytes(alg))) == NULL) {
-               error("%s: sshbuf_from failed", __func__);
-               r = SSH_ERR_ALLOC_FAIL;
+       debug("%s: signature made with hash \"%s\"", __func__, hashalg);
+       if ((r = hash_buffer(message, hashalg, &b)) != 0) {
+               error("%s: hash_buffer failed: %s", __func__, ssh_err(r));
                goto out;
        }
        if ((r = sshsig_wrap_verify(signature, hashalg, b, expect_namespace,
@@ -459,20 +474,29 @@ sshsig_verify_message(struct sshbuf *signature, const struct sshbuf *message,
  out:
        sshbuf_free(b);
        free(hashalg);
-       explicit_bzero(hash, sizeof(hash));
        return r;
 }
 
 static int
-hash_file(int fd, int hashalg, u_char *hash, size_t hashlen)
+hash_file(int fd, const char *hashalg, struct sshbuf **bp)
 {
-       char *hex, rbuf[8192];
+       char *hex, rbuf[8192], hash[SSH_DIGEST_MAX_LENGTH];
        ssize_t n, total = 0;
        struct ssh_digest_ctx *ctx;
-       int r, oerrno;
+       int alg, oerrno, r = SSH_ERR_INTERNAL_ERROR;
+       struct sshbuf *b = NULL;
+
+       *bp = NULL;
+       memset(hash, 0, sizeof(hash));
 
-       memset(hash, 0, hashlen);
-       if ((ctx = ssh_digest_start(hashalg)) == NULL) {
+       if ((r = sshsig_check_hashalg(hashalg)) != 0)
+               return r;
+       if ((alg = ssh_digest_alg_by_name(hashalg)) == -1) {
+               error("%s: can't look up hash algorithm %s",
+                   __func__, hashalg);
+               return SSH_ERR_INTERNAL_ERROR;
+       }
+       if ((ctx = ssh_digest_start(alg)) == NULL) {
                error("%s: ssh_digest_start failed", __func__);
                return SSH_ERR_INTERNAL_ERROR;
        }
@@ -484,7 +508,8 @@ hash_file(int fd, int hashalg, u_char *hash, size_t hashlen)
                        error("%s: read: %s", __func__, strerror(errno));
                        ssh_digest_free(ctx);
                        errno = oerrno;
-                       return SSH_ERR_SYSTEM_ERROR;
+                       r = SSH_ERR_SYSTEM_ERROR;
+                       goto out;
                } else if (n == 0) {
                        debug2("%s: hashed %zu bytes", __func__, total);
                        break; /* EOF */
@@ -493,20 +518,33 @@ hash_file(int fd, int hashalg, u_char *hash, size_t hashlen)
                if ((r = ssh_digest_update(ctx, rbuf, (size_t)n)) != 0) {
                        error("%s: ssh_digest_update: %s",
                            __func__, ssh_err(r));
-                       ssh_digest_free(ctx);
-                       return r;
+                       goto out;
                }
        }
-       if ((r = ssh_digest_final(ctx, hash, hashlen)) != 0) {
+       if ((r = ssh_digest_final(ctx, hash, sizeof(hash))) != 0) {
                error("%s: ssh_digest_final: %s", __func__, ssh_err(r));
-               ssh_digest_free(ctx);
+               goto out;
        }
-       if ((hex = tohex(hash, hashlen)) != NULL) {
+       if ((hex = tohex(hash, ssh_digest_bytes(alg))) != NULL) {
                debug3("%s: final hash: %s", __func__, hex);
                freezero(hex, strlen(hex));
        }
+       if ((b = sshbuf_new()) == NULL) {
+               r = SSH_ERR_ALLOC_FAIL;
+               goto out;
+       }
+       if ((r = sshbuf_put(b, hash, ssh_digest_bytes(alg))) != 0) {
+               error("%s: sshbuf_put: %s", __func__, ssh_err(r));
+               goto out;
+       }
+       *bp = b;
+       b = NULL; /* transferred */
        /* success */
+       r = 0;
+ out:
+       sshbuf_free(b);
        ssh_digest_free(ctx);
+       explicit_bzero(hash, sizeof(hash));
        return 0;
 }
 
@@ -515,31 +553,17 @@ sshsig_sign_fd(struct sshkey *key, const char *hashalg,
     int fd, const char *sig_namespace, struct sshbuf **out,
     sshsig_signer *signer, void *signer_ctx)
 {
-       u_char hash[SSH_DIGEST_MAX_LENGTH];
        struct sshbuf *b = NULL;
-       int alg, r = SSH_ERR_INTERNAL_ERROR;
+       int r = SSH_ERR_INTERNAL_ERROR;
 
+       if (hashalg == NULL)
+               hashalg = HASHALG_DEFAULT;
        if (out != NULL)
                *out = NULL;
-       if (hashalg == NULL)
-           hashalg = HASHALG_DEFAULT;
-
-       if ((r = sshsig_check_hashalg(hashalg)) != 0)
-               return r;
-       if ((alg = ssh_digest_alg_by_name(hashalg)) == -1) {
-               error("%s: can't look up hash algorithm %s",
-                   __func__, HASHALG_DEFAULT);
-               return SSH_ERR_INTERNAL_ERROR;
-       }
-       if ((r = hash_file(fd, alg, hash, sizeof(hash))) != 0) {
+       if ((r = hash_file(fd, hashalg, &b)) != 0) {
                error("%s: hash_file failed: %s", __func__, ssh_err(r));
                return r;
        }
-       if ((b = sshbuf_from(hash, ssh_digest_bytes(alg))) == NULL) {
-               error("%s: sshbuf_from failed", __func__);
-               r = SSH_ERR_ALLOC_FAIL;
-               goto out;
-       }
        if ((r = sshsig_wrap_sign(key, hashalg, b, sig_namespace, out,
            signer, signer_ctx)) != 0)
                goto out;
@@ -547,7 +571,6 @@ sshsig_sign_fd(struct sshkey *key, const char *hashalg,
        r = 0;
  out:
        sshbuf_free(b);
-       explicit_bzero(hash, sizeof(hash));
        return r;
 }
 
@@ -555,9 +578,8 @@ int
 sshsig_verify_fd(struct sshbuf *signature, int fd,
     const char *expect_namespace, struct sshkey **sign_keyp)
 {
-       u_char hash[SSH_DIGEST_MAX_LENGTH];
        struct sshbuf *b = NULL;
-       int alg, r = SSH_ERR_INTERNAL_ERROR;
+       int r = SSH_ERR_INTERNAL_ERROR;
        char *hashalg = NULL;
 
        if (sign_keyp != NULL)
@@ -565,18 +587,9 @@ sshsig_verify_fd(struct sshbuf *signature, int fd,
 
        if ((r = sshsig_peek_hashalg(signature, &hashalg)) != 0)
                return r;
-       if ((alg = ssh_digest_alg_by_name(hashalg)) == -1) {
-               error("%s: can't look up hash algorithm %s",
-                   __func__, HASHALG_DEFAULT);
-               return SSH_ERR_INTERNAL_ERROR;
-       }
-       if ((r = hash_file(fd, alg, hash, sizeof(hash))) != 0) {
+       debug("%s: signature made with hash \"%s\"", __func__, hashalg);
+       if ((r = hash_file(fd, hashalg, &b)) != 0) {
                error("%s: hash_file failed: %s", __func__, ssh_err(r));
-               return r;
-       }
-       if ((b = sshbuf_from(hash, ssh_digest_bytes(alg))) == NULL) {
-               error("%s: sshbuf_from failed", __func__);
-               r = SSH_ERR_ALLOC_FAIL;
                goto out;
        }
        if ((r = sshsig_wrap_verify(signature, hashalg, b, expect_namespace,
@@ -587,7 +600,6 @@ sshsig_verify_fd(struct sshbuf *signature, int fd,
  out:
        sshbuf_free(b);
        free(hashalg);
-       explicit_bzero(hash, sizeof(hash));
        return r;
 }
 
@@ -769,14 +781,14 @@ sshsig_check_allowed_keys(const char *path, const struct sshkey *sign_key,
                linenum++;
                r = check_allowed_keys_line(path, linenum, line, sign_key,
                    principal, sig_namespace);
+               free(line);
+               line = NULL;
                if (r == SSH_ERR_KEY_NOT_FOUND)
                        continue;
                else if (r == 0) {
                        /* success */
                        fclose(f);
-                       free(line);
                        return 0;
-                       /* XXX continue and check revocation? */
                } else
                        break;
        }

diff --git a/sshsig.h b/sshsig.h
index 92c675e3a05d9b8e606873c202d34d2c372ecfe6..fc1d607b3a8b4b2d8f00ae1079dfc03ca4a9e4ec 100644 (file)
--- a/sshsig.h
+++ b/sshsig.h
@@ -23,15 +23,28 @@ struct sshkey;
 typedef int sshsig_signer(struct sshkey *, u_char **, size_t *,
     const u_char *, size_t, const char *, u_int, void *);
 
+/* Buffer-oriented API */
+
 /*
- * Creates a detached SSH signature for a given message.
+ * Creates a detached SSH signature for a given buffer.
  * Returns 0 on success or a negative SSH_ERR_* error code on failure.
  * out is populated with the detached signature, or NULL on failure.
  */
-int sshsig_sign_message(struct sshkey *key, const char *hashalg,
+int sshsig_signb(struct sshkey *key, const char *hashalg,
     const struct sshbuf *message, const char *sig_namespace,
     struct sshbuf **out, sshsig_signer *signer, void *signer_ctx);
 
+/*
+ * Verifies that a detached signature is valid and optionally returns key
+ * used to sign via argument.
+ * Returns 0 on success or a negative SSH_ERR_* error code on failure.
+ */
+int sshsig_verifyb(struct sshbuf *signature,
+    const struct sshbuf *message, const char *sig_namespace,
+    struct sshkey **sign_keyp);
+
+/* File/FD-oriented API */
+
 /*
  * Creates a detached SSH signature for a given file.
  * Returns 0 on success or a negative SSH_ERR_* error code on failure.
@@ -41,15 +54,6 @@ int sshsig_sign_fd(struct sshkey *key, const char *hashalg,
     int fd, const char *sig_namespace, struct sshbuf **out,
     sshsig_signer *signer, void *signer_ctx);
 
-/*
- * Verifies that a detached signature is valid and optionally returns key
- * used to sign via argument.
- * Returns 0 on success or a negative SSH_ERR_* error code on failure.
- */
-int sshsig_verify_message(struct sshbuf *signature,
-    const struct sshbuf *message, const char *sig_namespace,
-    struct sshkey **sign_keyp);
-
 /*
  * Verifies that a detached signature over a file is valid and optionally
  * returns key used to sign via argument.
@@ -58,6 +62,8 @@ int sshsig_verify_message(struct sshbuf *signature,
 int sshsig_verify_fd(struct sshbuf *signature, int fd,
     const char *sig_namespace, struct sshkey **sign_keyp);
 
+/* Utility functions */
+
 /*
  * Return a base64 encoded "ASCII armoured" version of a raw signature.
  */