- markus@cvs.openbsd.org 2012/12/11 22:31:18

     [PROTOCOL authfile.c cipher.c cipher.h kex.h mac.c myproposal.h]
     [packet.c ssh_config.5 sshd_config.5]
     add encrypt-then-mac (EtM) modes to openssh by defining new mac algorithms
     that change the packet format and compute the MAC over the encrypted
     message (including the packet size) instead of the plaintext data;
     these EtM modes are considered more secure and used by default.
     feedback and ok djm@

1 files changed, 76 insertions, 37 deletions

diff --git a/packet.c b/packet.c
index b75c081f0..be8907854 100644
--- a/packet.c
+++ b/packet.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: packet.c,v 1.177 2012/09/17 13:04:11 markus Exp $ */
+/* $OpenBSD: packet.c,v 1.178 2012/12/11 22:31:18 markus Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -275,7 +275,7 @@ packet_stop_discard(void)
 static void
 packet_start_discard(Enc *enc, Mac *mac, u_int packet_length, u_int discard)
 {
-        if (enc == NULL || !cipher_is_cbc(enc->cipher))
+        if (enc == NULL || !cipher_is_cbc(enc->cipher) || (mac && mac->etm))
                 packet_disconnect("Packet corrupt");
         if (packet_length != PACKET_MAX_SIZE && mac && mac->enabled)
                 active_state->packet_discard_mac = mac;
@@ -709,7 +709,7 @@ packet_send1(void)
             buffer_len(&active_state->outgoing_packet));
         cipher_crypt(&active_state->send_context, cp,
             buffer_ptr(&active_state->outgoing_packet),
-            buffer_len(&active_state->outgoing_packet));
+            buffer_len(&active_state->outgoing_packet), 0);
 
 #ifdef PACKET_DEBUG
         fprintf(stderr, "encrypted: ");
@@ -845,9 +845,8 @@ static void
 packet_send2_wrapped(void)
 {
         u_char type, *cp, *macbuf = NULL;
-        u_char padlen, pad;
-        u_int packet_length = 0;
-        u_int i, len;
+        u_char padlen, pad = 0;
+        u_int i, len, aadlen = 0;
         u_int32_t rnd = 0;
         Enc *enc   = NULL;
         Mac *mac   = NULL;
@@ -860,6 +859,7 @@ packet_send2_wrapped(void)
                 comp = &active_state->newkeys[MODE_OUT]->comp;
         }
         block_size = enc ? enc->block_size : 8;
+        aadlen = mac && mac->enabled && mac->etm ? 4 : 0;
 
         cp = buffer_ptr(&active_state->outgoing_packet);
         type = cp[5];
@@ -892,6 +892,7 @@ packet_send2_wrapped(void)
          * calc size of padding, alloc space, get random data,
          * minimum padding is 4 bytes
          */
+        len -= aadlen; /* packet length is not encrypted for EtM modes */
         padlen = block_size - (len % block_size);
         if (padlen < 4)
                 padlen += block_size;
@@ -919,29 +920,37 @@ packet_send2_wrapped(void)
                 /* clear padding */
                 memset(cp, 0, padlen);
         }
-        /* packet_length includes payload, padding and padding length field */
-        packet_length = buffer_len(&active_state->outgoing_packet) - 4;
+        /* sizeof (packet_len + pad_len + payload + padding) */
+        len = buffer_len(&active_state->outgoing_packet);
         cp = buffer_ptr(&active_state->outgoing_packet);
-        put_u32(cp, packet_length);
+        /* packet_length includes payload, padding and padding length field */
+        put_u32(cp, len - 4);
         cp[4] = padlen;
-        DBG(debug("send: len %d (includes padlen %d)", packet_length+4, padlen));
+        DBG(debug("send: len %d (includes padlen %d, aadlen %d)",
+            len, padlen, aadlen));
 
         /* compute MAC over seqnr and packet(length fields, payload, padding) */
-        if (mac && mac->enabled) {
+        if (mac && mac->enabled && !mac->etm) {
                 macbuf = mac_compute(mac, active_state->p_send.seqnr,
-                    buffer_ptr(&active_state->outgoing_packet),
-                    buffer_len(&active_state->outgoing_packet));
+                    buffer_ptr(&active_state->outgoing_packet), len);
                 DBG(debug("done calc MAC out #%d", active_state->p_send.seqnr));
         }
         /* encrypt packet and append to output buffer. */
-        cp = buffer_append_space(&active_state->output,
-            buffer_len(&active_state->outgoing_packet));
+        cp = buffer_append_space(&active_state->output, len);
         cipher_crypt(&active_state->send_context, cp,
             buffer_ptr(&active_state->outgoing_packet),
-            buffer_len(&active_state->outgoing_packet));
+            len - aadlen, aadlen);
         /* append unencrypted MAC */
-        if (mac && mac->enabled)
+        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);
+                        DBG(debug("done calc MAC(EtM) out #%d",
+                            active_state->p_send.seqnr));
+                }
                 buffer_append(&active_state->output, macbuf, mac->mac_len);
+        }
 #ifdef PACKET_DEBUG
         fprintf(stderr, "encrypted: ");
         buffer_dump(&active_state->output);
@@ -952,8 +961,8 @@ packet_send2_wrapped(void)
         if (++active_state->p_send.packets == 0)
                 if (!(datafellows & SSH_BUG_NOREKEY))
                         fatal("XXX too many packets with same key");
-        active_state->p_send.blocks += (packet_length + 4) / block_size;
-        active_state->p_send.bytes += packet_length + 4;
+        active_state->p_send.blocks += len / block_size;
+        active_state->p_send.bytes += len;
         buffer_clear(&active_state->outgoing_packet);
 
         if (type == SSH2_MSG_NEWKEYS)
@@ -1190,7 +1199,7 @@ packet_read_poll1(void)
         buffer_clear(&active_state->incoming_packet);
         cp = buffer_append_space(&active_state->incoming_packet, padded_len);
         cipher_crypt(&active_state->receive_context, cp,
-            buffer_ptr(&active_state->input), padded_len);
+            buffer_ptr(&active_state->input), padded_len, 0);
 
         buffer_consume(&active_state->input, padded_len);
 
@@ -1238,8 +1247,8 @@ static int
 packet_read_poll2(u_int32_t *seqnr_p)
 {
         u_int padlen, need;
-        u_char *macbuf, *cp, type;
-        u_int maclen, block_size;
+        u_char *macbuf = NULL, *cp, type;
+        u_int maclen, aadlen = 0, block_size;
         Enc *enc   = NULL;
         Mac *mac   = NULL;
         Comp *comp = NULL;
@@ -1254,8 +1263,22 @@ packet_read_poll2(u_int32_t *seqnr_p)
         }
         maclen = mac && mac->enabled ? mac->mac_len : 0;
         block_size = enc ? enc->block_size : 8;
+        aadlen = mac && mac->enabled && mac->etm ? 4 : 0;
 
-        if (active_state->packlen == 0) {
+        if (aadlen && active_state->packlen == 0) {
+                if (buffer_len(&active_state->input) < 4)
+                        return SSH_MSG_NONE;
+                cp = buffer_ptr(&active_state->input);
+                active_state->packlen = get_u32(cp);
+                if (active_state->packlen < 1 + 4 ||
+                    active_state->packlen > PACKET_MAX_SIZE) {
+#ifdef PACKET_DEBUG
+                        buffer_dump(&active_state->input);
+#endif
+                        logit("Bad packet length %u.", active_state->packlen);
+                        packet_disconnect("Packet corrupt");
+                }
+        } else if (active_state->packlen == 0) {
                 /*
                  * check if input size is less than the cipher block size,
                  * decrypt first block and extract length of incoming packet
@@ -1266,7 +1289,7 @@ packet_read_poll2(u_int32_t *seqnr_p)
                 cp = buffer_append_space(&active_state->incoming_packet,
                     block_size);
                 cipher_crypt(&active_state->receive_context, cp,
-                    buffer_ptr(&active_state->input), block_size);
+                    buffer_ptr(&active_state->input), block_size, 0);
                 cp = buffer_ptr(&active_state->incoming_packet);
                 active_state->packlen = get_u32(cp);
                 if (active_state->packlen < 1 + 4 ||
@@ -1279,13 +1302,21 @@ packet_read_poll2(u_int32_t *seqnr_p)
                             PACKET_MAX_SIZE);
                         return SSH_MSG_NONE;
                 }
-                DBG(debug("input: packet len %u", active_state->packlen+4));
                 buffer_consume(&active_state->input, block_size);
         }
-        /* we have a partial packet of block_size bytes */
-        need = 4 + active_state->packlen - block_size;
-        DBG(debug("partial packet %d, need %d, maclen %d", block_size,
-            need, maclen));
+        DBG(debug("input: packet len %u", active_state->packlen+4));
+        if (aadlen) {
+                /* only the payload is encrypted */
+                need = active_state->packlen;
+        } else {
+                /*
+                 * the payload size and the payload are encrypted, but we
+                 * have a partial packet of block_size bytes
+                 */
+                need = 4 + active_state->packlen - block_size;
+        }
+        DBG(debug("partial packet: block %d, need %d, maclen %d, aadlen %d",
+            block_size, need, maclen, aadlen));
         if (need % block_size != 0) {
                 logit("padding error: need %d block %d mod %d",
                     need, block_size, need % block_size);
@@ -1295,26 +1326,34 @@ packet_read_poll2(u_int32_t *seqnr_p)
         }
         /*
          * check if the entire packet has been received and
-         * decrypt into incoming_packet
+         * decrypt into incoming_packet:
+         * 'aadlen' bytes are unencrypted, but authenticated.
+         * 'need' bytes are encrypted, followed by
+         * 'maclen' bytes of message authentication code.
          */
-        if (buffer_len(&active_state->input) < need + maclen)
+        if (buffer_len(&active_state->input) < aadlen + need + maclen)
                 return SSH_MSG_NONE;
 #ifdef PACKET_DEBUG
         fprintf(stderr, "read_poll enc/full: ");
         buffer_dump(&active_state->input);
 #endif
-        cp = buffer_append_space(&active_state->incoming_packet, need);
+        /* 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);
+        cp = buffer_append_space(&active_state->incoming_packet, aadlen + need);
         cipher_crypt(&active_state->receive_context, cp,
-            buffer_ptr(&active_state->input), need);
-        buffer_consume(&active_state->input, need);
+            buffer_ptr(&active_state->input), need, aadlen);
+        buffer_consume(&active_state->input, aadlen + need);
         /*
          * compute MAC over seqnr and packet,
          * increment sequence number for incoming packet
          */
         if (mac && mac->enabled) {
-                macbuf = mac_compute(mac, active_state->p_read.seqnr,
-                    buffer_ptr(&active_state->incoming_packet),
-                    buffer_len(&active_state->incoming_packet));
+                if (!mac->etm)
+                        macbuf = mac_compute(mac, active_state->p_read.seqnr,
+                            buffer_ptr(&active_state->incoming_packet),
+                            buffer_len(&active_state->incoming_packet));
                 if (timingsafe_bcmp(macbuf, buffer_ptr(&active_state->input),
                     mac->mac_len) != 0) {
                         logit("Corrupted MAC on input.");