- 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, 127 deletions

diff --git a/ssh-rsa.c b/ssh-rsa.c
index c6f25b3ee..fec1953b4 100644
--- a/ssh-rsa.c
+++ b/ssh-rsa.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-rsa.c,v 1.51 2014/02/02 03:44:31 djm Exp $ */
+/* $OpenBSD: ssh-rsa.c,v 1.52 2014/06/24 01:13:21 djm Exp $ */
 /*
  * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org>
  *
@@ -25,163 +25,167 @@
 #include <stdarg.h>
 #include <string.h>
 
-#include "xmalloc.h"
-#include "log.h"
-#include "buffer.h"
-#include "key.h"
+#include "sshbuf.h"
 #include "compat.h"
-#include "misc.h"
-#include "ssh.h"
+#include "ssherr.h"
+#define SSHKEY_INTERNAL
+#include "sshkey.h"
 #include "digest.h"
 
-static int openssh_RSA_verify(int, u_char *, u_int, u_char *, u_int, RSA *);
+static int openssh_RSA_verify(int, u_char *, size_t, u_char *, size_t, RSA *);
 
 /* RSASSA-PKCS1-v1_5 (PKCS #1 v2.0 signature) with SHA1 */
 int
-ssh_rsa_sign(const Key *key, u_char **sigp, u_int *lenp,
-    const u_char *data, u_int datalen)
+ssh_rsa_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
+    const u_char *data, size_t datalen, u_int compat)
 {
         int hash_alg;
-        u_char digest[SSH_DIGEST_MAX_LENGTH], *sig;
-        u_int slen, dlen, len;
-        int ok, nid;
-        Buffer b;
+        u_char digest[SSH_DIGEST_MAX_LENGTH], *sig = NULL;
+        size_t slen;
+        u_int dlen, len;
+        int nid, ret = SSH_ERR_INTERNAL_ERROR;
+        struct sshbuf *b = NULL;
 
-        if (key == NULL || key_type_plain(key->type) != KEY_RSA ||
-            key->rsa == NULL) {
-                error("%s: no RSA key", __func__);
-                return -1;
-        }
+        if (lenp != NULL)
+                *lenp = 0;
+        if (sigp != NULL)
+                *sigp = NULL;
+
+        if (key == NULL || key->rsa == NULL ||
+            sshkey_type_plain(key->type) != KEY_RSA)
+                return SSH_ERR_INVALID_ARGUMENT;
+        slen = RSA_size(key->rsa);
+        if (slen <= 0 || slen > SSHBUF_MAX_BIGNUM)
+                return SSH_ERR_INVALID_ARGUMENT;
 
         /* hash the data */
         hash_alg = SSH_DIGEST_SHA1;
         nid = NID_sha1;
-        if ((dlen = ssh_digest_bytes(hash_alg)) == 0) {
-                error("%s: bad hash algorithm %d", __func__, hash_alg);
-                return -1;
-        }
-        if (ssh_digest_memory(hash_alg, data, datalen,
-            digest, sizeof(digest)) != 0) {
-                error("%s: ssh_digest_memory failed", __func__);
-                return -1;
-        }
-
-        slen = RSA_size(key->rsa);
-        sig = xmalloc(slen);
-
-        ok = RSA_sign(nid, digest, dlen, sig, &len, key->rsa);
-        explicit_bzero(digest, sizeof(digest));
+        if ((dlen = ssh_digest_bytes(hash_alg)) == 0)
+                return SSH_ERR_INTERNAL_ERROR;
+        if ((ret = ssh_digest_memory(hash_alg, data, datalen,
+            digest, sizeof(digest))) != 0)
+                goto out;
 
-        if (ok != 1) {
-                int ecode = ERR_get_error();
+        if ((sig = malloc(slen)) == NULL) {
+                ret = SSH_ERR_ALLOC_FAIL;
+                goto out;
+        }
 
-                error("%s: RSA_sign failed: %s", __func__,
-                    ERR_error_string(ecode, NULL));
-                free(sig);
-                return -1;
+        if (RSA_sign(nid, digest, dlen, sig, &len, key->rsa) != 1) {
+                ret = SSH_ERR_LIBCRYPTO_ERROR;
+                goto out;
         }
         if (len < slen) {
-                u_int diff = slen - len;
-                debug("slen %u > len %u", slen, len);
+                size_t diff = slen - len;
                 memmove(sig + diff, sig, len);
                 explicit_bzero(sig, diff);
         } else if (len > slen) {
-                error("%s: slen %u slen2 %u", __func__, slen, len);
-                free(sig);
-                return -1;
+                ret = SSH_ERR_INTERNAL_ERROR;
+                goto out;
         }
         /* encode signature */
-        buffer_init(&b);
-        buffer_put_cstring(&b, "ssh-rsa");
-        buffer_put_string(&b, sig, slen);
-        len = buffer_len(&b);
+        if ((b = sshbuf_new()) == NULL) {
+                ret = SSH_ERR_ALLOC_FAIL;
+                goto out;
+        }
+        if ((ret = sshbuf_put_cstring(b, "ssh-rsa")) != 0 ||
+            (ret = sshbuf_put_string(b, sig, slen)) != 0)
+                goto out;
+        len = sshbuf_len(b);
+        if (sigp != NULL) {
+                if ((*sigp = malloc(len)) == NULL) {
+                        ret = SSH_ERR_ALLOC_FAIL;
+                        goto out;
+                }
+                memcpy(*sigp, sshbuf_ptr(b), len);
+        }
         if (lenp != NULL)
                 *lenp = len;
-        if (sigp != NULL) {
-                *sigp = xmalloc(len);
-                memcpy(*sigp, buffer_ptr(&b), len);
+        ret = 0;
+ out:
+        explicit_bzero(digest, sizeof(digest));
+        if (sig != NULL) {
+                explicit_bzero(sig, slen);
+                free(sig);
         }
-        buffer_free(&b);
-        explicit_bzero(sig, slen);
-        free(sig);
-
+        if (b != NULL)
+                sshbuf_free(b);
         return 0;
 }
 
 int
-ssh_rsa_verify(const Key *key, const u_char *signature, u_int signaturelen,
-    const u_char *data, u_int datalen)
+ssh_rsa_verify(const struct sshkey *key,
+    const u_char *signature, size_t signaturelen,
+    const u_char *data, size_t datalen, u_int compat)
 {
-        Buffer b;
-        int hash_alg;
-        char *ktype;
-        u_char digest[SSH_DIGEST_MAX_LENGTH], *sigblob;
-        u_int len, dlen, modlen;
-        int rlen, ret;
+        char *ktype = NULL;
+        int hash_alg, ret = SSH_ERR_INTERNAL_ERROR;
+        size_t len, diff, modlen, dlen;
+        struct sshbuf *b = NULL;
+        u_char digest[SSH_DIGEST_MAX_LENGTH], *osigblob, *sigblob = NULL;
 
-        if (key == NULL || key_type_plain(key->type) != KEY_RSA ||
-            key->rsa == NULL) {
-                error("%s: no RSA key", __func__);
-                return -1;
-        }
+        if (key == NULL || key->rsa == NULL ||
+            sshkey_type_plain(key->type) != KEY_RSA ||
+            BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE)
+                return SSH_ERR_INVALID_ARGUMENT;
 
-        if (BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE) {
-                error("%s: RSA modulus too small: %d < minimum %d bits",
-                    __func__, BN_num_bits(key->rsa->n),
-                    SSH_RSA_MINIMUM_MODULUS_SIZE);
-                return -1;
+        if ((b = sshbuf_from(signature, signaturelen)) == NULL)
+                return SSH_ERR_ALLOC_FAIL;
+        if (sshbuf_get_cstring(b, &ktype, NULL) != 0) {
+                ret = SSH_ERR_INVALID_FORMAT;
+                goto out;
         }
-        buffer_init(&b);
-        buffer_append(&b, signature, signaturelen);
-        ktype = buffer_get_cstring(&b, NULL);
         if (strcmp("ssh-rsa", 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_get_string(b, &sigblob, &len) != 0) {
+                ret = SSH_ERR_INVALID_FORMAT;
+                goto out;
+        }
+        if (sshbuf_len(b) != 0) {
+                ret = SSH_ERR_UNEXPECTED_TRAILING_DATA;
+                goto out;
         }
         /* RSA_verify expects a signature of RSA_size */
         modlen = RSA_size(key->rsa);
         if (len > modlen) {
-                error("%s: len %u > modlen %u", __func__, len, modlen);
-                free(sigblob);
-                return -1;
+                ret = SSH_ERR_KEY_BITS_MISMATCH;
+                goto out;
         } else if (len < modlen) {
-                u_int diff = modlen - len;
-                debug("%s: add padding: modlen %u > len %u", __func__,
-                    modlen, len);
-                sigblob = xrealloc(sigblob, 1, modlen);
+                diff = modlen - len;
+                osigblob = sigblob;
+                if ((sigblob = realloc(sigblob, modlen)) == NULL) {
+                        sigblob = osigblob; /* put it back for clear/free */
+                        ret = SSH_ERR_ALLOC_FAIL;
+                        goto out;
+                }
                 memmove(sigblob + diff, sigblob, len);
                 explicit_bzero(sigblob, diff);
                 len = modlen;
         }
-        /* hash the data */
         hash_alg = SSH_DIGEST_SHA1;
         if ((dlen = ssh_digest_bytes(hash_alg)) == 0) {
-                error("%s: bad hash algorithm %d", __func__, hash_alg);
-                return -1;
-        }
-        if (ssh_digest_memory(hash_alg, data, datalen,
-            digest, sizeof(digest)) != 0) {
-                error("%s: ssh_digest_memory failed", __func__);
-                return -1;
+                ret = SSH_ERR_INTERNAL_ERROR;
+                goto out;
         }
+        if ((ret = ssh_digest_memory(hash_alg, data, datalen,
+            digest, sizeof(digest))) != 0)
+                goto out;
 
         ret = openssh_RSA_verify(hash_alg, digest, dlen, sigblob, len,
             key->rsa);
+ out:
+        if (sigblob != NULL) {
+                explicit_bzero(sigblob, len);
+                free(sigblob);
+        }
+        if (ktype != NULL)
+                free(ktype);
+        if (b != NULL)
+                sshbuf_free(b);
         explicit_bzero(digest, sizeof(digest));
-        explicit_bzero(sigblob, len);
-        free(sigblob);
-        debug("%s: signature %scorrect", __func__, (ret == 0) ? "in" : "");
         return ret;
 }
 
@@ -204,15 +208,15 @@ static const u_char id_sha1[] = {
 };
 
 static int
-openssh_RSA_verify(int hash_alg, u_char *hash, u_int hashlen,
-    u_char *sigbuf, u_int siglen, RSA *rsa)
+openssh_RSA_verify(int hash_alg, u_char *hash, size_t hashlen,
+    u_char *sigbuf, size_t siglen, RSA *rsa)
 {
-        u_int ret, rsasize, oidlen = 0, hlen = 0;
+        size_t ret, rsasize = 0, oidlen = 0, hlen = 0;
         int len, oidmatch, hashmatch;
         const u_char *oid = NULL;
         u_char *decrypted = NULL;
 
-        ret = 0;
+        ret = SSH_ERR_INTERNAL_ERROR;
         switch (hash_alg) {
         case SSH_DIGEST_SHA1:
                 oid = id_sha1;
@@ -223,37 +227,39 @@ openssh_RSA_verify(int hash_alg, u_char *hash, u_int hashlen,
                 goto done;
         }
         if (hashlen != hlen) {
-                error("bad hashlen");
+                ret = SSH_ERR_INVALID_ARGUMENT;
                 goto done;
         }
         rsasize = RSA_size(rsa);
-        if (siglen == 0 || siglen > rsasize) {
-                error("bad siglen");
+        if (rsasize <= 0 || rsasize > SSHBUF_MAX_BIGNUM ||
+            siglen == 0 || siglen > rsasize) {
+                ret = SSH_ERR_INVALID_ARGUMENT;
+                goto done;
+        }
+        if ((decrypted = malloc(rsasize)) == NULL) {
+                ret = SSH_ERR_ALLOC_FAIL;
                 goto done;
         }
-        decrypted = xmalloc(rsasize);
         if ((len = RSA_public_decrypt(siglen, sigbuf, decrypted, rsa,
             RSA_PKCS1_PADDING)) < 0) {
-                error("RSA_public_decrypt failed: %s",
-                    ERR_error_string(ERR_get_error(), NULL));
+                ret = SSH_ERR_LIBCRYPTO_ERROR;
                 goto done;
         }
-        if (len < 0 || (u_int)len != hlen + oidlen) {
-                error("bad decrypted len: %d != %d + %d", len, hlen, oidlen);
+        if (len < 0 || (size_t)len != hlen + oidlen) {
+                ret = SSH_ERR_INVALID_FORMAT;
                 goto done;
         }
         oidmatch = timingsafe_bcmp(decrypted, oid, oidlen) == 0;
         hashmatch = timingsafe_bcmp(decrypted + oidlen, hash, hlen) == 0;
-        if (!oidmatch) {
-                error("oid mismatch");
-                goto done;
-        }
-        if (!hashmatch) {
-                error("hash mismatch");
+        if (!oidmatch || !hashmatch) {
+                ret = SSH_ERR_SIGNATURE_INVALID;
                 goto done;
         }
-        ret = 1;
+        ret = 0;
 done:
-        free(decrypted);
+        if (decrypted) {
+                explicit_bzero(decrypted, rsasize);
+                free(decrypted);
+        }
         return ret;
 }