adapt mac.c to ssherr.h return codes (de-fatal) and simplify dependencies

ok djm@

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 <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;
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 <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;
 }
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 <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/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 <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);
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 <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.");