5 files changed, 102 insertions, 82 deletions

diff --git a/kex.h b/kex.h
index 4c40ec851..dbcc0816f 100644
--- a/kex.h
+++ b/kex.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.h,v 1.64 2014/05/02 03:27:54 djm Exp $ */
+/* $OpenBSD: kex.h,v 1.65 2015/01/13 19:31:40 markus Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -26,10 +26,9 @@
 #ifndef KEX_H
 #define KEX_H
 
-#include <signal.h>
-#include <openssl/evp.h>
-#include <openssl/hmac.h>
-#ifdef OPENSSL_HAS_ECC
+#include "mac.h"
+
+#if defined(WITH_OPENSSL) && defined(OPENSSL_HAS_ECC)
 #include <openssl/ec.h>
 #endif
 
@@ -82,8 +81,8 @@ enum kex_exchange {
 #define KEX_INIT_SENT   0x0001
 
 typedef struct Kex Kex;
-typedef struct Mac Mac;
 typedef struct Comp Comp;
+typedef struct sshmac Mac;
 typedef struct Enc Enc;
 typedef struct Newkeys Newkeys;
 
@@ -97,17 +96,6 @@ struct Enc {
         u_char  *key;
         u_char  *iv;
 };
-struct Mac {
-        char    *name;
-        int     enabled;
-        u_int   mac_len;
-        u_char  *key;
-        u_int   key_len;
-        int     type;
-        int     etm;            /* Encrypt-then-MAC */
-        struct ssh_hmac_ctx     *hmac_ctx;
-        struct umac_ctx         *umac_ctx;
-};
 struct Comp {
         int     type;
         int     enabled;
diff --git a/mac.c b/mac.c
index 402dc984c..11c30a1c5 100644
--- a/mac.c
+++ b/mac.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: mac.c,v 1.30 2014/04/30 19:07:48 naddy Exp $ */
+/* $OpenBSD: mac.c,v 1.31 2015/01/13 19:31:40 markus Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  *
@@ -27,22 +27,16 @@
 
 #include <sys/types.h>
 
-#include <stdarg.h>
 #include <string.h>
-#include <signal.h>
-
-#include "xmalloc.h"
-#include "log.h"
-#include "cipher.h"
-#include "buffer.h"
-#include "key.h"
-#include "kex.h"
-#include "mac.h"
-#include "misc.h"
+#include <stdio.h>
 
 #include "digest.h"
 #include "hmac.h"
 #include "umac.h"
+#include "mac.h"
+#include "misc.h"
+#include "ssherr.h"
+#include "sshbuf.h"
 
 #include "openbsd-compat/openssl-compat.h"
 
@@ -95,7 +89,7 @@ static const struct macalg macs[] = {
 char *
 mac_alg_list(char sep)
 {
-        char *ret = NULL;
+        char *ret = NULL, *tmp;
         size_t nlen, rlen = 0;
         const struct macalg *m;
 
@@ -103,20 +97,24 @@ mac_alg_list(char sep)
                 if (ret != NULL)
                         ret[rlen++] = sep;
                 nlen = strlen(m->name);
-                ret = xrealloc(ret, 1, rlen + nlen + 2);
+                if ((tmp = realloc(ret, rlen + nlen + 2)) == NULL) {
+                        free(ret);
+                        return NULL;
+                }
+                ret = tmp;
                 memcpy(ret + rlen, m->name, nlen + 1);
                 rlen += nlen;
         }
         return ret;
 }
 
-static void
-mac_setup_by_alg(Mac *mac, const struct macalg *macalg)
+static int
+mac_setup_by_alg(struct sshmac *mac, const struct macalg *macalg)
 {
         mac->type = macalg->type;
         if (mac->type == SSH_DIGEST) {
                 if ((mac->hmac_ctx = ssh_hmac_start(macalg->alg)) == NULL)
-                        fatal("ssh_hmac_start(alg=%d) failed", macalg->alg);
+                        return SSH_ERR_ALLOC_FAIL;
                 mac->key_len = mac->mac_len = ssh_hmac_bytes(macalg->alg);
         } else {
                 mac->mac_len = macalg->len / 8;
@@ -126,61 +124,60 @@ mac_setup_by_alg(Mac *mac, const struct macalg *macalg)
         if (macalg->truncatebits != 0)
                 mac->mac_len = macalg->truncatebits / 8;
         mac->etm = macalg->etm;
+        return 0;
 }
 
 int
-mac_setup(Mac *mac, char *name)
+mac_setup(struct sshmac *mac, char *name)
 {
         const struct macalg *m;
 
         for (m = macs; m->name != NULL; m++) {
                 if (strcmp(name, m->name) != 0)
                         continue;
-                if (mac != NULL) {
-                        mac_setup_by_alg(mac, m);
-                        debug2("mac_setup: setup %s", name);
-                }
-                return (0);
+                if (mac != NULL)
+                        return mac_setup_by_alg(mac, m);
+                return 0;
         }
-        debug2("mac_setup: unknown %s", name);
-        return (-1);
+        return SSH_ERR_INVALID_ARGUMENT;
 }
 
 int
-mac_init(Mac *mac)
+mac_init(struct sshmac *mac)
 {
         if (mac->key == NULL)
-                fatal("%s: no key", __func__);
+                return SSH_ERR_INVALID_ARGUMENT;
         switch (mac->type) {
         case SSH_DIGEST:
                 if (mac->hmac_ctx == NULL ||
                     ssh_hmac_init(mac->hmac_ctx, mac->key, mac->key_len) < 0)
-                        return -1;
+                        return SSH_ERR_INVALID_ARGUMENT;
                 return 0;
         case SSH_UMAC:
-                mac->umac_ctx = umac_new(mac->key);
+                if ((mac->umac_ctx = umac_new(mac->key)) == NULL)
+                        return SSH_ERR_ALLOC_FAIL;
                 return 0;
         case SSH_UMAC128:
                 mac->umac_ctx = umac128_new(mac->key);
                 return 0;
         default:
-                return -1;
+                return SSH_ERR_INVALID_ARGUMENT;
         }
 }
 
-u_char *
-mac_compute(Mac *mac, u_int32_t seqno, u_char *data, int datalen)
+int
+mac_compute(struct sshmac *mac, u_int32_t seqno, const u_char *data, int datalen,
+    u_char *digest, size_t dlen)
 {
         static union {
-                u_char m[EVP_MAX_MD_SIZE];
+                u_char m[SSH_DIGEST_MAX_LENGTH];
                 u_int64_t for_align;
         } u;
         u_char b[4];
         u_char nonce[8];
 
         if (mac->mac_len > sizeof(u))
-                fatal("mac_compute: mac too long %u %zu",
-                    mac->mac_len, sizeof(u));
+                return SSH_ERR_INTERNAL_ERROR;
 
         switch (mac->type) {
         case SSH_DIGEST:
@@ -190,10 +187,10 @@ mac_compute(Mac *mac, u_int32_t seqno, u_char *data, int datalen)
                     ssh_hmac_update(mac->hmac_ctx, b, sizeof(b)) < 0 ||
                     ssh_hmac_update(mac->hmac_ctx, data, datalen) < 0 ||
                     ssh_hmac_final(mac->hmac_ctx, u.m, sizeof(u.m)) < 0)
-                        fatal("ssh_hmac failed");
+                        return SSH_ERR_LIBCRYPTO_ERROR;
                 break;
         case SSH_UMAC:
-                put_u64(nonce, seqno);
+                POKE_U64(nonce, seqno);
                 umac_update(mac->umac_ctx, data, datalen);
                 umac_final(mac->umac_ctx, u.m, nonce);
                 break;
@@ -203,13 +200,18 @@ mac_compute(Mac *mac, u_int32_t seqno, u_char *data, int datalen)
                 umac128_final(mac->umac_ctx, u.m, nonce);
                 break;
         default:
-                fatal("mac_compute: unknown MAC type");
+                return SSH_ERR_INVALID_ARGUMENT;
         }
-        return (u.m);
+        if (digest != NULL) {
+                if (dlen > mac->mac_len)
+                        dlen = mac->mac_len;
+                memcpy(digest, u.m, dlen);
+        }
+        return 0;
 }
 
 void
-mac_clear(Mac *mac)
+mac_clear(struct sshmac *mac)
 {
         if (mac->type == SSH_UMAC) {
                 if (mac->umac_ctx != NULL)
@@ -231,17 +233,16 @@ mac_valid(const char *names)
         char *maclist, *cp, *p;
 
         if (names == NULL || strcmp(names, "") == 0)
-                return (0);
-        maclist = cp = xstrdup(names);
+                return 0;
+        if ((maclist = cp = strdup(names)) == NULL)
+                return 0;
         for ((p = strsep(&cp, MAC_SEP)); p && *p != '\0';
             (p = strsep(&cp, MAC_SEP))) {
                 if (mac_setup(NULL, p) < 0) {
-                        debug("bad mac %s [%s]", p, names);
                         free(maclist);
-                        return (0);
+                        return 0;
                 }
         }
-        debug3("macs ok: [%s]", names);
         free(maclist);
-        return (1);
+        return 1;
 }
diff --git a/mac.h b/mac.h
index fbe18c463..e5f6b84d9 100644
--- a/mac.h
+++ b/mac.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: mac.h,v 1.8 2013/11/07 11:58:27 dtucker Exp $ */
+/* $OpenBSD: mac.h,v 1.9 2015/01/13 19:31:40 markus Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  *
@@ -23,9 +23,29 @@
  * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef SSHMAC_H
+#define SSHMAC_H
+
+#include <sys/types.h>
+
+struct sshmac {
+        char    *name;
+        int     enabled;
+        u_int   mac_len;
+        u_char  *key;
+        u_int   key_len;
+        int     type;
+        int     etm;            /* Encrypt-then-MAC */
+        struct ssh_hmac_ctx     *hmac_ctx;
+        struct umac_ctx         *umac_ctx;
+};
+
 int      mac_valid(const char *);
 char    *mac_alg_list(char);
-int      mac_setup(Mac *, char *);
-int      mac_init(Mac *);
-u_char  *mac_compute(Mac *, u_int32_t, u_char *, int);
-void     mac_clear(Mac *);
+int      mac_setup(struct sshmac *, char *);
+int      mac_init(struct sshmac *);
+int      mac_compute(struct sshmac *, u_int32_t, const u_char *, int,
+    u_char *, size_t);
+void     mac_clear(struct sshmac *);
+
+#endif /* SSHMAC_H */
diff --git a/monitor_wrap.c b/monitor_wrap.c
index 45dc16951..f4e11c966 100644
--- a/monitor_wrap.c
+++ b/monitor_wrap.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor_wrap.c,v 1.80 2014/04/29 18:01:49 markus Exp $ */
+/* $OpenBSD: monitor_wrap.c,v 1.81 2015/01/13 19:31:40 markus Exp $ */
 /*
  * Copyright 2002 Niels Provos <provos@citi.umich.edu>
  * Copyright 2002 Markus Friedl <markus@openbsd.org>
@@ -506,7 +506,7 @@ mm_newkeys_from_blob(u_char *blob, int blen)
         /* Mac structure */
         if (cipher_authlen(enc->cipher) == 0) {
                 mac->name = buffer_get_string(&b, NULL);
-                if (mac->name == NULL || mac_setup(mac, mac->name) == -1)
+                if (mac->name == NULL || mac_setup(mac, mac->name) != 0)
                         fatal("%s: can not setup mac %s", __func__, mac->name);
                 mac->enabled = buffer_get_int(&b);
                 mac->key = buffer_get_string(&b, &len);
diff --git a/packet.c b/packet.c
index 4674a203f..6b326f367 100644
--- a/packet.c
+++ b/packet.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.c,v 1.199 2014/10/24 02:01:20 lteo Exp $ */
+/* $OpenBSD: packet.c,v 1.200 2015/01/13 19:31:40 markus Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -72,6 +72,7 @@
 #include "cipher.h"
 #include "key.h"
 #include "kex.h"
+#include "digest.h"
 #include "mac.h"
 #include "log.h"
 #include "canohost.h"
@@ -275,7 +276,7 @@ packet_stop_discard(void)
                 (void) mac_compute(active_state->packet_discard_mac,
                     active_state->p_read.seqnr,
                     buffer_ptr(&active_state->incoming_packet),
-                    PACKET_MAX_SIZE);
+                    PACKET_MAX_SIZE, NULL, 0);
         }
         logit("Finished discarding for %.200s", get_remote_ipaddr());
         cleanup_exit(255);
@@ -863,7 +864,7 @@ packet_enable_delayed_compress(void)
 static void
 packet_send2_wrapped(void)
 {
-        u_char type, *cp, *macbuf = NULL;
+        u_char type, *cp, macbuf[SSH_DIGEST_MAX_LENGTH];
         u_char padlen, pad = 0;
         u_int i, len, authlen = 0, aadlen = 0;
         u_int32_t rnd = 0;
@@ -871,6 +872,7 @@ packet_send2_wrapped(void)
         Mac *mac   = NULL;
         Comp *comp = NULL;
         int block_size;
+        int r;
 
         if (active_state->newkeys[MODE_OUT] != NULL) {
                 enc  = &active_state->newkeys[MODE_OUT]->enc;
@@ -953,8 +955,10 @@ packet_send2_wrapped(void)
 
         /* compute MAC over seqnr and packet(length fields, payload, padding) */
         if (mac && mac->enabled && !mac->etm) {
-                macbuf = mac_compute(mac, active_state->p_send.seqnr,
-                    buffer_ptr(&active_state->outgoing_packet), len);
+                if ((r = mac_compute(mac, active_state->p_send.seqnr,
+                    buffer_ptr(&active_state->outgoing_packet), len,
+                    macbuf, sizeof(macbuf))) != 0)
+                        fatal("%s: mac_compute: %s", __func__, ssh_err(r));
                 DBG(debug("done calc MAC out #%d", active_state->p_send.seqnr));
         }
         /* encrypt packet and append to output buffer. */
@@ -967,8 +971,10 @@ packet_send2_wrapped(void)
         if (mac && mac->enabled) {
                 if (mac->etm) {
                         /* EtM: compute mac over aadlen + cipher text */
-                        macbuf = mac_compute(mac,
-                            active_state->p_send.seqnr, cp, len);
+                        if ((r = mac_compute(mac,
+                            active_state->p_send.seqnr, cp, len,
+                            macbuf, sizeof(macbuf))) != 0)
+                                fatal("%s: mac_compute: %s", __func__, ssh_err(r));
                         DBG(debug("done calc MAC(EtM) out #%d",
                             active_state->p_send.seqnr));
                 }
@@ -1272,8 +1278,9 @@ static int
 packet_read_poll2(u_int32_t *seqnr_p)
 {
         u_int padlen, need;
-        u_char *macbuf = NULL, *cp, type;
+        u_char type, *cp, macbuf[SSH_DIGEST_MAX_LENGTH];
         u_int maclen, authlen = 0, aadlen = 0, block_size;
+        int r;
         Enc *enc   = NULL;
         Mac *mac   = NULL;
         Comp *comp = NULL;
@@ -1373,8 +1380,10 @@ packet_read_poll2(u_int32_t *seqnr_p)
 #endif
         /* EtM: compute mac over encrypted input */
         if (mac && mac->enabled && mac->etm)
-                macbuf = mac_compute(mac, active_state->p_read.seqnr,
-                    buffer_ptr(&active_state->input), aadlen + need);
+                if ((r = mac_compute(mac, active_state->p_read.seqnr,
+                    buffer_ptr(&active_state->input), aadlen + need,
+                    macbuf, sizeof(macbuf))) != 0)
+                        fatal("%s: mac_compute: %s", __func__, ssh_err(r));
         cp = buffer_append_space(&active_state->incoming_packet, aadlen + need);
         if (cipher_crypt(&active_state->receive_context,
             active_state->p_read.seqnr, cp,
@@ -1387,9 +1396,11 @@ packet_read_poll2(u_int32_t *seqnr_p)
          */
         if (mac && mac->enabled) {
                 if (!mac->etm)
-                        macbuf = mac_compute(mac, active_state->p_read.seqnr,
+                        if ((r = mac_compute(mac, active_state->p_read.seqnr,
                             buffer_ptr(&active_state->incoming_packet),
-                            buffer_len(&active_state->incoming_packet));
+                            buffer_len(&active_state->incoming_packet),
+                            macbuf, sizeof(macbuf))) != 0)
+                                fatal("%s: mac_compute: %s", __func__, ssh_err(r));
                 if (timingsafe_bcmp(macbuf, buffer_ptr(&active_state->input),
                     mac->mac_len) != 0) {
                         logit("Corrupted MAC on input.");