From 8692bcf07edfddf2b25b397d2c839c7365e0bc90 Mon Sep 17 00:00:00 2001 From: markus Date: Tue, 13 Jan 2015 19:31:40 +0000 Subject: [PATCH] adapt mac.c to ssherr.h return codes (de-fatal) and simplify dependencies ok djm@ --- usr.bin/ssh/kex.h | 19 ++------ usr.bin/ssh/mac.c | 94 +++++++++++++++++++------------------- usr.bin/ssh/mac.h | 30 ++++++++++-- usr.bin/ssh/monitor_wrap.c | 4 +- usr.bin/ssh/packet.c | 35 +++++++++----- 5 files changed, 100 insertions(+), 82 deletions(-) diff --git a/usr.bin/ssh/kex.h b/usr.bin/ssh/kex.h index 49cc244ec35..a7fb9123a67 100644 --- a/usr.bin/ssh/kex.h +++ b/usr.bin/ssh/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,9 +26,7 @@ #ifndef KEX_H #define KEX_H -#include -#include -#include +#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; diff --git a/usr.bin/ssh/mac.c b/usr.bin/ssh/mac.c index 0842f989424..7572f17112b 100644 --- a/usr.bin/ssh/mac.c +++ b/usr.bin/ssh/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. * @@ -25,23 +25,16 @@ #include -#include - #include -#include - -#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 #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; } diff --git a/usr.bin/ssh/mac.h b/usr.bin/ssh/mac.h index fbe18c463bb..e5f6b84d9ed 100644 --- a/usr.bin/ssh/mac.h +++ b/usr.bin/ssh/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 + +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/usr.bin/ssh/monitor_wrap.c b/usr.bin/ssh/monitor_wrap.c index 4dec4448c58..41de5aebd15 100644 --- a/usr.bin/ssh/monitor_wrap.c +++ b/usr.bin/ssh/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 * Copyright 2002 Markus Friedl @@ -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); diff --git a/usr.bin/ssh/packet.c b/usr.bin/ssh/packet.c index 21839ecbaa3..e6a3de2b01f 100644 --- a/usr.bin/ssh/packet.c +++ b/usr.bin/ssh/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 * Copyright (c) 1995 Tatu Ylonen , 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."); -- 2.20.1