- djm@cvs.openbsd.org 2014/06/24 01:13:21

     [Makefile.in auth-bsdauth.c auth-chall.c auth-options.c auth-rsa.c
     [auth2-none.c auth2-pubkey.c authfile.c authfile.h cipher-3des1.c
     [cipher-chachapoly.c cipher-chachapoly.h cipher.c cipher.h
     [digest-libc.c digest-openssl.c digest.h dns.c entropy.c hmac.h
     [hostfile.c key.c key.h krl.c monitor.c packet.c rsa.c rsa.h
     [ssh-add.c ssh-agent.c ssh-dss.c ssh-ecdsa.c ssh-ed25519.c
     [ssh-keygen.c ssh-pkcs11-client.c ssh-pkcs11-helper.c ssh-pkcs11.c
     [ssh-rsa.c sshbuf-misc.c sshbuf.h sshconnect.c sshconnect1.c
     [sshconnect2.c sshd.c sshkey.c sshkey.h
     [openbsd-compat/openssl-compat.c openbsd-compat/openssl-compat.h]
     New key API: refactor key-related functions to be more library-like,
     existing API is offered as a set of wrappers.

     with and ok markus@

     Thanks also to Ben Hawkes, David Tomaschik, Ivan Fratric, Matthew
     Dempsky and Ron Bowes for a detailed review a few months ago.

     NB. This commit also removes portable OpenSSH support for OpenSSL
     <0.9.8e.

1 files changed, 133 insertions, 104 deletions

diff --git a/ssh-dss.c b/ssh-dss.c
index 6b4abcb7d..02d1ec2d6 100644
--- a/ssh-dss.c
+++ b/ssh-dss.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-dss.c,v 1.31 2014/02/02 03:44:31 djm Exp $ */
+/* $OpenBSD: ssh-dss.c,v 1.32 2014/06/24 01:13:21 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -33,157 +33,186 @@
 #include <stdarg.h>
 #include <string.h>
 
-#include "xmalloc.h"
-#include "buffer.h"
+#include "sshbuf.h"
 #include "compat.h"
-#include "log.h"
-#include "key.h"
+#include "ssherr.h"
 #include "digest.h"
+#define SSHKEY_INTERNAL
+#include "sshkey.h"
 
 #define INTBLOB_LEN     20
 #define SIGBLOB_LEN     (2*INTBLOB_LEN)
 
 int
-ssh_dss_sign(const Key *key, u_char **sigp, u_int *lenp,
-    const u_char *data, u_int datalen)
+ssh_dss_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
+    const u_char *data, size_t datalen, u_int compat)
 {
-        DSA_SIG *sig;
+        DSA_SIG *sig = NULL;
         u_char digest[SSH_DIGEST_MAX_LENGTH], sigblob[SIGBLOB_LEN];
-        u_int rlen, slen, len, dlen = ssh_digest_bytes(SSH_DIGEST_SHA1);
-        Buffer b;
-
-        if (key == NULL || key_type_plain(key->type) != KEY_DSA ||
-            key->dsa == NULL) {
-                error("%s: no DSA key", __func__);
-                return -1;
-        }
-
-        if (ssh_digest_memory(SSH_DIGEST_SHA1, data, datalen,
-            digest, sizeof(digest)) != 0) {
-                error("%s: ssh_digest_memory failed", __func__);
-                return -1;
-        }
-
-        sig = DSA_do_sign(digest, dlen, key->dsa);
-        explicit_bzero(digest, sizeof(digest));
-
-        if (sig == NULL) {
-                error("ssh_dss_sign: sign failed");
-                return -1;
+        size_t rlen, slen, len, dlen = ssh_digest_bytes(SSH_DIGEST_SHA1);
+        struct sshbuf *b = NULL;
+        int ret = SSH_ERR_INVALID_ARGUMENT;
+
+        if (lenp != NULL)
+                *lenp = 0;
+        if (sigp != NULL)
+                *sigp = NULL;
+
+        if (key == NULL || key->dsa == NULL ||
+            sshkey_type_plain(key->type) != KEY_DSA)
+                return SSH_ERR_INVALID_ARGUMENT;
+        if (dlen == 0)
+                return SSH_ERR_INTERNAL_ERROR;
+
+        if ((ret = ssh_digest_memory(SSH_DIGEST_SHA1, data, datalen,
+            digest, sizeof(digest))) != 0)
+                goto out;
+
+        if ((sig = DSA_do_sign(digest, dlen, key->dsa)) == NULL) {
+                ret = SSH_ERR_LIBCRYPTO_ERROR;
+                goto out;
         }
 
         rlen = BN_num_bytes(sig->r);
         slen = BN_num_bytes(sig->s);
         if (rlen > INTBLOB_LEN || slen > INTBLOB_LEN) {
-                error("bad sig size %u %u", rlen, slen);
-                DSA_SIG_free(sig);
-                return -1;
+                ret = SSH_ERR_INTERNAL_ERROR;
+                goto out;
         }
         explicit_bzero(sigblob, SIGBLOB_LEN);
-        BN_bn2bin(sig->r, sigblob+ SIGBLOB_LEN - INTBLOB_LEN - rlen);
-        BN_bn2bin(sig->s, sigblob+ SIGBLOB_LEN - slen);
-        DSA_SIG_free(sig);
+        BN_bn2bin(sig->r, sigblob + SIGBLOB_LEN - INTBLOB_LEN - rlen);
+        BN_bn2bin(sig->s, sigblob + SIGBLOB_LEN - slen);
 
-        if (datafellows & SSH_BUG_SIGBLOB) {
-                if (lenp != NULL)
-                        *lenp = SIGBLOB_LEN;
+        if (compat & SSH_BUG_SIGBLOB) {
                 if (sigp != NULL) {
-                        *sigp = xmalloc(SIGBLOB_LEN);
+                        if ((*sigp = malloc(SIGBLOB_LEN)) == NULL) {
+                                ret = SSH_ERR_ALLOC_FAIL;
+                                goto out;
+                        }
                         memcpy(*sigp, sigblob, SIGBLOB_LEN);
                 }
+                if (lenp != NULL)
+                        *lenp = SIGBLOB_LEN;
+                ret = 0;
         } else {
                 /* ietf-drafts */
-                buffer_init(&b);
-                buffer_put_cstring(&b, "ssh-dss");
-                buffer_put_string(&b, sigblob, SIGBLOB_LEN);
-                len = buffer_len(&b);
-                if (lenp != NULL)
-                        *lenp = len;
+                if ((b = sshbuf_new()) == NULL) {
+                        ret = SSH_ERR_ALLOC_FAIL;
+                        goto out;
+                }
+                if ((ret = sshbuf_put_cstring(b, "ssh-dss")) != 0 ||
+                    (ret = sshbuf_put_string(b, sigblob, SIGBLOB_LEN)) != 0)
+                        goto out;
+                len = sshbuf_len(b);
                 if (sigp != NULL) {
-                        *sigp = xmalloc(len);
-                        memcpy(*sigp, buffer_ptr(&b), len);
+                        if ((*sigp = malloc(len)) == NULL) {
+                                ret = SSH_ERR_ALLOC_FAIL;
+                                goto out;
+                        }
+                        memcpy(*sigp, sshbuf_ptr(b), len);
                 }
-                buffer_free(&b);
+                if (lenp != NULL)
+                        *lenp = len;
+                ret = 0;
         }
-        return 0;
+ out:
+        explicit_bzero(digest, sizeof(digest));
+        if (sig != NULL)
+                DSA_SIG_free(sig);
+        if (b != NULL)
+                sshbuf_free(b);
+        return ret;
 }
+
 int
-ssh_dss_verify(const Key *key, const u_char *signature, u_int signaturelen,
-    const u_char *data, u_int datalen)
+ssh_dss_verify(const struct sshkey *key,
+    const u_char *signature, size_t signaturelen,
+    const u_char *data, size_t datalen, u_int compat)
 {
-        DSA_SIG *sig;
-        u_char digest[SSH_DIGEST_MAX_LENGTH], *sigblob;
-        u_int len, dlen = ssh_digest_bytes(SSH_DIGEST_SHA1);
-        int rlen, ret;
-        Buffer b;
-
-        if (key == NULL || key_type_plain(key->type) != KEY_DSA ||
-            key->dsa == NULL) {
-                error("%s: no DSA key", __func__);
-                return -1;
-        }
+        DSA_SIG *sig = NULL;
+        u_char digest[SSH_DIGEST_MAX_LENGTH], *sigblob = NULL;
+        size_t len, dlen = ssh_digest_bytes(SSH_DIGEST_SHA1);
+        int ret = SSH_ERR_INTERNAL_ERROR;
+        struct sshbuf *b = NULL;
+        char *ktype = NULL;
+
+        if (key == NULL || key->dsa == NULL ||
+            sshkey_type_plain(key->type) != KEY_DSA)
+                return SSH_ERR_INVALID_ARGUMENT;
+        if (dlen == 0)
+                return SSH_ERR_INTERNAL_ERROR;
 
         /* fetch signature */
-        if (datafellows & SSH_BUG_SIGBLOB) {
-                sigblob = xmalloc(signaturelen);
+        if (compat & SSH_BUG_SIGBLOB) {
+                if ((sigblob = malloc(signaturelen)) == NULL)
+                        return SSH_ERR_ALLOC_FAIL;
                 memcpy(sigblob, signature, signaturelen);
                 len = signaturelen;
         } else {
                 /* ietf-drafts */
-                char *ktype;
-                buffer_init(&b);
-                buffer_append(&b, signature, signaturelen);
-                ktype = buffer_get_cstring(&b, NULL);
+                if ((b = sshbuf_from(signature, signaturelen)) == NULL)
+                        return SSH_ERR_ALLOC_FAIL;
+                if (sshbuf_get_cstring(b, &ktype, NULL) != 0 ||
+                    sshbuf_get_string(b, &sigblob, &len) != 0) {
+                        ret = SSH_ERR_INVALID_FORMAT;
+                        goto out;
+                }
                 if (strcmp("ssh-dss", ktype) != 0) {
-                        error("%s: cannot handle type %s", __func__, ktype);
-                        buffer_free(&b);
-                        free(ktype);
-                        return -1;
+                        ret = SSH_ERR_KEY_TYPE_MISMATCH;
+                        goto out;
                 }
-                free(ktype);
-                sigblob = buffer_get_string(&b, &len);
-                rlen = buffer_len(&b);
-                buffer_free(&b);
-                if (rlen != 0) {
-                        error("%s: remaining bytes in signature %d",
-                            __func__, rlen);
-                        free(sigblob);
-                        return -1;
+                if (sshbuf_len(b) != 0) {
+                        ret = SSH_ERR_UNEXPECTED_TRAILING_DATA;
+                        goto out;
                 }
         }
 
         if (len != SIGBLOB_LEN) {
-                fatal("bad sigbloblen %u != SIGBLOB_LEN", len);
+                ret = SSH_ERR_INVALID_FORMAT;
+                goto out;
         }
 
         /* parse signature */
-        if ((sig = DSA_SIG_new()) == NULL)
-                fatal("%s: DSA_SIG_new failed", __func__);
-        if ((sig->r = BN_new()) == NULL)
-                fatal("%s: BN_new failed", __func__);
-        if ((sig->s = BN_new()) == NULL)
-                fatal("ssh_dss_verify: BN_new failed");
+        if ((sig = DSA_SIG_new()) == NULL ||
+            (sig->r = BN_new()) == NULL ||
+            (sig->s = BN_new()) == NULL) {
+                ret = SSH_ERR_ALLOC_FAIL;
+                goto out;
+        }
         if ((BN_bin2bn(sigblob, INTBLOB_LEN, sig->r) == NULL) ||
-            (BN_bin2bn(sigblob+ INTBLOB_LEN, INTBLOB_LEN, sig->s) == NULL))
-                fatal("%s: BN_bin2bn failed", __func__);
-
-        /* clean up */
-        explicit_bzero(sigblob, len);
-        free(sigblob);
+            (BN_bin2bn(sigblob+ INTBLOB_LEN, INTBLOB_LEN, sig->s) == NULL)) {
+                ret = SSH_ERR_LIBCRYPTO_ERROR;
+                goto out;
+        }
 
         /* sha1 the data */
-        if (ssh_digest_memory(SSH_DIGEST_SHA1, data, datalen,
-            digest, sizeof(digest)) != 0) {
-                error("%s: digest_memory failed", __func__);
-                return -1;
+        if ((ret = ssh_digest_memory(SSH_DIGEST_SHA1, data, datalen,
+            digest, sizeof(digest))) != 0)
+                goto out;
+
+        switch (DSA_do_verify(digest, dlen, sig, key->dsa)) {
+        case 1:
+                ret = 0;
+                break;
+        case 0:
+                ret = SSH_ERR_SIGNATURE_INVALID;
+                goto out;
+        default:
+                ret = SSH_ERR_LIBCRYPTO_ERROR;
+                goto out;
         }
 
-        ret = DSA_do_verify(digest, dlen, sig, key->dsa);
+ out:
         explicit_bzero(digest, sizeof(digest));
-
-        DSA_SIG_free(sig);
-
-        debug("%s: signature %s", __func__,
-            ret == 1 ? "correct" : ret == 0 ? "incorrect" : "error");
+        if (sig != NULL)
+                DSA_SIG_free(sig);
+        if (b != NULL)
+                sshbuf_free(b);
+        if (ktype != NULL)
+                free(ktype);
+        if (sigblob != NULL) {
+                explicit_bzero(sigblob, len);
+                free(sigblob);
+        }
         return ret;
 }