adapt mac.c to ssherr.h return codes (de-fatal) and simplify dependencies
authormarkus <markus@openbsd.org>
Tue, 13 Jan 2015 19:31:40 +0000 (19:31 +0000)
committermarkus <markus@openbsd.org>
Tue, 13 Jan 2015 19:31:40 +0000 (19:31 +0000)
ok djm@

usr.bin/ssh/kex.h
usr.bin/ssh/mac.c
usr.bin/ssh/mac.h
usr.bin/ssh/monitor_wrap.c
usr.bin/ssh/packet.c

index 49cc244..a7fb912 100644 (file)
@@ -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,9 +26,7 @@
 #ifndef KEX_H
 #define KEX_H
 
-#include <openssl/evp.h>
-#include <openssl/hmac.h>
-#include <openssl/ec.h>
+#include "mac.h"
 
 #define KEX_COOKIE_LEN 16
 
@@ -79,8 +77,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;
 
@@ -94,17 +92,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;
index 0842f98..7572f17 100644 (file)
@@ -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.
  *
 
 #include <sys/types.h>
 
-#include <openssl/hmac.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"
 
 #define SSH_DIGEST     1       /* SSH_DIGEST_XXX */
 #define SSH_UMAC       2       /* UMAC (not integrated with OpenSSL) */
@@ -88,7 +81,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;
 
@@ -96,20 +89,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;
@@ -119,61 +116,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:
@@ -183,10 +179,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;
@@ -196,13 +192,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)
@@ -224,17 +225,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;
 }
index fbe18c4..e5f6b84 100644 (file)
@@ -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.
  *
  * 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 */
index 4dec444..41de5ae 100644 (file)
@@ -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>
@@ -492,7 +492,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);
index 21839ec..e6a3de2 100644 (file)
@@ -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
@@ -67,6 +67,7 @@
 #include "cipher.h"
 #include "key.h"
 #include "kex.h"
+#include "digest.h"
 #include "mac.h"
 #include "log.h"
 #include "canohost.h"
@@ -270,7 +271,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);
@@ -851,7 +852,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;
@@ -859,6 +860,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;
@@ -941,8 +943,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. */
@@ -955,8 +959,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));
                }
@@ -1259,8 +1265,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;
@@ -1360,8 +1367,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,
@@ -1374,9 +1383,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.");