support encrypt-then-mac (etm) MACs (#987)

Support for calculating MAC (message authentication code) on encrypted
data instead of plain text data.

This adds support for the following MACs:
- `hmac-sha1-etm@openssh.com`
- `hmac-sha2-256-etm@openssh.com`
- `hmac-sha2-512-etm@openssh.com`

Integration-patches-by: Viktor Szakats

* rebase on master
* fix checksec warnings
* fix compiler warning
* fix indent/whitespace/eol
* rebase/manual merge onto AES-GCM patch #797
* more manual merge of `libssh2_transport_send()` based
  on dfandrich/shellfish

Fixes #582
Closes #655
Closes #987

src/libssh2_priv.h

@@ -541,7 +541,8 @@ struct transportpacket
                                packet_length + padding_length + 4 +
                                mac_length. */
     unsigned char *payload; /* this is a pointer to a LIBSSH2_ALLOC()
-                               area to which we write decrypted data */
+                               area to which we write incoming packet data
+                               which is not yet decrypted in etm mode. */
     unsigned char *wptr;    /* write pointer into the payload to where we
                                are currently writing decrypted data */
 

src/mac.c

@@ -71,7 +71,8 @@ static LIBSSH2_MAC_METHOD mac_method_none = {
     0,
     NULL,
     mac_none_MAC,
-    NULL
+    NULL,
+    0
 };
 #endif /* defined(LIBSSH2DEBUG) && defined(LIBSSH2_MAC_NONE_INSECURE) */
 
@@ -138,8 +139,6 @@ mac_method_hmac_sha2_512_hash(LIBSSH2_SESSION * session,
     return 0;
 }
 
-
-
 static const LIBSSH2_MAC_METHOD mac_method_hmac_sha2_512 = {
     "hmac-sha2-512",
     64,
@@ -147,7 +146,19 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_sha2_512 = {
     mac_method_common_init,
     mac_method_hmac_sha2_512_hash,
     mac_method_common_dtor,
+    0
 };
+
+static const LIBSSH2_MAC_METHOD mac_method_hmac_sha2_512_etm = {
+    "hmac-sha2-512-etm@openssh.com",
+    64,
+    64,
+    mac_method_common_init,
+    mac_method_hmac_sha2_512_hash,
+    mac_method_common_dtor,
+    1
+};
+
 #endif
 
 
@@ -192,7 +203,19 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_sha2_256 = {
     mac_method_common_init,
     mac_method_hmac_sha2_256_hash,
     mac_method_common_dtor,
+    0
 };
+
+static const LIBSSH2_MAC_METHOD mac_method_hmac_sha2_256_etm = {
+    "hmac-sha2-256-etm@openssh.com",
+    32,
+    32,
+    mac_method_common_init,
+    mac_method_hmac_sha2_256_hash,
+    mac_method_common_dtor,
+    1
+};
+
 #endif
 
 
@@ -237,6 +260,17 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_sha1 = {
     mac_method_common_init,
     mac_method_hmac_sha1_hash,
     mac_method_common_dtor,
+    0
+};
+
+static const LIBSSH2_MAC_METHOD mac_method_hmac_sha1_etm = {
+    "hmac-sha1-etm@openssh.com",
+    20,
+    20,
+    mac_method_common_init,
+    mac_method_hmac_sha1_hash,
+    mac_method_common_dtor,
+    1
 };
 
 /* mac_method_hmac_sha1_96_hash
@@ -268,6 +302,7 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_sha1_96 = {
     mac_method_common_init,
     mac_method_hmac_sha1_96_hash,
     mac_method_common_dtor,
+    0
 };
 
 #if LIBSSH2_MD5
@@ -310,6 +345,7 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_md5 = {
     mac_method_common_init,
     mac_method_hmac_md5_hash,
     mac_method_common_dtor,
+    0
 };
 
 /* mac_method_hmac_md5_96_hash
@@ -339,6 +375,7 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_md5_96 = {
     mac_method_common_init,
     mac_method_hmac_md5_96_hash,
     mac_method_common_dtor,
+    0
 };
 #endif /* LIBSSH2_MD5 */
 
@@ -383,6 +420,7 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_ripemd160 = {
     mac_method_common_init,
     mac_method_hmac_ripemd160_hash,
     mac_method_common_dtor,
+    0
 };
 
 static const LIBSSH2_MAC_METHOD mac_method_hmac_ripemd160_openssh_com = {
@@ -392,17 +430,21 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_ripemd160_openssh_com = {
     mac_method_common_init,
     mac_method_hmac_ripemd160_hash,
     mac_method_common_dtor,
+    0
 };
 #endif /* LIBSSH2_HMAC_RIPEMD */
 
 static const LIBSSH2_MAC_METHOD *mac_methods[] = {
 #if LIBSSH2_HMAC_SHA256
     &mac_method_hmac_sha2_256,
+    &mac_method_hmac_sha2_256_etm,
 #endif
 #if LIBSSH2_HMAC_SHA512
     &mac_method_hmac_sha2_512,
+    &mac_method_hmac_sha2_512_etm,
 #endif
     &mac_method_hmac_sha1,
+    &mac_method_hmac_sha1_etm,
     &mac_method_hmac_sha1_96,
 #if LIBSSH2_MD5
     &mac_method_hmac_md5,
@@ -435,6 +477,7 @@ static const LIBSSH2_MAC_METHOD mac_method_hmac_aesgcm = {
     NULL,
     NULL,
     NULL,
+    0
 };
 #endif /* LIBSSH2_AES_GCM */
 

src/mac.h

@@ -57,6 +57,8 @@ struct _LIBSSH2_MAC_METHOD
                  size_t packet_len, const unsigned char *addtl,
                  size_t addtl_len, void **abstract);
     int (*dtor) (LIBSSH2_SESSION * session, void **abstract);
+
+    int etm; /* encrypt-then-mac */
 };
 
 typedef struct _LIBSSH2_MAC_METHOD LIBSSH2_MAC_METHOD;

src/transport.c

@@ -197,21 +197,81 @@ fullpacket(LIBSSH2_SESSION * session, int encrypted /* 1 or 0 */ )
         if(encrypted && !CRYPT_FLAG_L(session, INTEGRATED_MAC)) {
 
             /* Calculate MAC hash */
-            session->remote.mac->hash(session, macbuf,  /* store hash here */
+            int etm = session->remote.mac->etm;
-                                      session->remote.seqno,
+            size_t mac_len = session->remote.mac->mac_len;
-                                      p->init, 5,
+            if(etm) {
-                                      p->payload,
+                /* store hash here */
-                                      session->fullpacket_payload_len,
+                session->remote.mac->hash(session, macbuf,
-                                      &session->remote.mac_abstract);
+                                          session->remote.seqno,
+                                          p->payload, p->total_num - mac_len,
+                                          NULL, 0,
+                                          &session->remote.mac_abstract);
+            }
+            else {
+                /* store hash here */
+                session->remote.mac->hash(session, macbuf,
+                                          session->remote.seqno,
+                                          p->init, 5,
+                                          p->payload,
+                                          session->fullpacket_payload_len,
+                                          &session->remote.mac_abstract);
+            }
 
             /* Compare the calculated hash with the MAC we just read from
              * the network. The read one is at the very end of the payload
              * buffer. Note that 'payload_len' here is the packet_length
              * field which includes the padding but not the MAC.
              */
-            if(memcmp(macbuf, p->payload + session->fullpacket_payload_len,
+            if(memcmp(macbuf, p->payload + p->total_num - mac_len, mac_len)) {
-                       session->remote.mac->mac_len)) {
+                _libssh2_debug((session, LIBSSH2_TRACE_SOCKET,
+                               "Failed MAC check"));
                 session->fullpacket_macstate = LIBSSH2_MAC_INVALID;
+
+            }
+            else if(etm) {
+                /* MAC was ok and we start by decrypting the first block that
+                   contains padding length since this allows us to decrypt
+                   all other blocks to the right location in memory
+                   avoiding moving a larger block of memory one byte. */
+                unsigned char first_block[MAX_BLOCKSIZE];
+                ssize_t decrypt_size;
+                unsigned char *decrypt_buffer;
+                int blocksize = session->remote.crypt->blocksize;
+
+                rc = decrypt(session, p->payload + 4,
+                             first_block, blocksize, FIRST_BLOCK);
+                if(rc) {
+                    return rc;
+                }
+
+                /* we need buffer for decrypt */
+                decrypt_size = p->total_num - mac_len - 4;
+                decrypt_buffer = LIBSSH2_ALLOC(session, decrypt_size);
+                if(!decrypt_buffer) {
+                    return LIBSSH2_ERROR_ALLOC;
+                }
+
+                /* grab padding length and copy anything else
+                   into target buffer */
+                p->padding_length = first_block[0];
+                if(blocksize > 1) {
+                    memcpy(decrypt_buffer, first_block + 1, blocksize - 1);
+                }
+
+                /* decrypt all other blocks packet */
+                if(blocksize < decrypt_size) {
+                    rc = decrypt(session, p->payload + blocksize + 4,
+                                 decrypt_buffer + blocksize - 1,
+                                 decrypt_size - blocksize, LAST_BLOCK);
+                    if(rc) {
+                        LIBSSH2_FREE(session, decrypt_buffer);
+                        return rc;
+                    }
+                }
+
+                /* replace encrypted payload with plain text payload */
+                LIBSSH2_FREE(session, p->payload);
+                p->payload = decrypt_buffer;
             }
         }
 
@@ -348,6 +408,7 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
     }
 
     do {
+        int etm;
         if(session->socket_state == LIBSSH2_SOCKET_DISCONNECTED) {
             return LIBSSH2_ERROR_SOCKET_DISCONNECT;
         }
@@ -361,6 +422,8 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
                                    make the checks below work fine still */
         }
 
+        etm = encrypted && session->local.mac ? session->local.mac->etm : 0;
+
         /* read/use a whole big chunk into a temporary area stored in
            the LIBSSH2_SESSION struct. We will decrypt data from that
            buffer into the packet buffer so this temp one doesn't have
@@ -429,45 +492,55 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
                                  (5 bytes) packet length and padding length
                                  fields */
 
+            /* packet length is not encrypted in encode-then-mac mode
+               and we donøt need to decrypt first block */
+            ssize_t required_size = etm ? 4 : blocksize;
+
             /* No payload package area allocated yet. To know the
-               size of this payload, we need to decrypt the first
+               size of this payload, we need enough to decrypt the first
                blocksize data. */
 
-            if(numbytes < blocksize) {
+            if(numbytes < required_size) {
                 /* we can't act on anything less than blocksize, but this
                    check is only done for the initial block since once we have
                    got the start of a block we can in fact deal with fractions
-                 */
+                */
                 session->socket_block_directions |=
                     LIBSSH2_SESSION_BLOCK_INBOUND;
                 return LIBSSH2_ERROR_EAGAIN;
             }
 
-            if(encrypted) {
+            if(etm) {
-                /* first decrypted block */
+                p->packet_length = _libssh2_ntohu32(&p->buf[p->readidx]);
-                rc = decrypt(session, &p->buf[p->readidx],
-                             block, blocksize, FIRST_BLOCK);
-                if(rc != LIBSSH2_ERROR_NONE) {
-                    return rc;
-                }
-                /* Save the first 5 bytes of the decrypted package, to be
-                   used in the hash calculation later down.
-                   This is ignored in the INTEGRATED_MAC case. */
-                memcpy(p->init, block, 5);
             }
             else {
-                /* the data is plain, just copy it verbatim to
+                if(encrypted) {
-                   the working block buffer */
+                    /* first decrypted block */
-                memcpy(block, &p->buf[p->readidx], blocksize);
+                    rc = decrypt(session, &p->buf[p->readidx],
+                                 block, blocksize, FIRST_BLOCK);
+                    if(rc != LIBSSH2_ERROR_NONE) {
+                        return rc;
+                    }
+                    /* Save the first 5 bytes of the decrypted package, to be
+                       used in the hash calculation later down.
+                       This is ignored in the INTEGRATED_MAC case. */
+                    memcpy(p->init, block, 5);
+                }
+                else {
+                    /* the data is plain, just copy it verbatim to
+                       the working block buffer */
+                    memcpy(block, &p->buf[p->readidx], blocksize);
+                }
+
+                /* advance the read pointer */
+                p->readidx += blocksize;
+
+                /* we now have the initial blocksize bytes decrypted,
+                 * and we can extract packet and padding length from it
+                 */
+                p->packet_length = _libssh2_ntohu32(block);
             }
 
-            /* advance the read pointer */
-            p->readidx += blocksize;
-
-            /* we now have the initial blocksize bytes decrypted,
-             * and we can extract packet and padding length from it
-             */
-            p->packet_length = _libssh2_ntohu32(block);
             if(p->packet_length < 1) {
                 return LIBSSH2_ERROR_DECRYPT;
             }
@@ -475,19 +548,27 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
                 return LIBSSH2_ERROR_OUT_OF_BOUNDARY;
             }
 
-            /* padding_length has not been authenticated yet, but it won't
+            if(etm) {
-               actually be used (except for the sanity check immediately
+                /* we collect entire undecrypted packet including the
-               following) until after the entire packet is authenticated,
+                   packet length field that we run MAC over */
-               so this is safe. */
+                total_num = 4 + p->packet_length +
-            p->padding_length = block[4];
+                            session->remote.mac->mac_len;
-            if(p->padding_length > p->packet_length - 1) {
-                return LIBSSH2_ERROR_DECRYPT;
             }
+            else {
+                /* padding_length has not been authenticated yet, but it won't
+                   actually be used (except for the sanity check immediately
+                   following) until after the entire packet is authenticated,
+                   so this is safe. */
+                p->padding_length = block[4];
+                if(p->padding_length > p->packet_length - 1) {
+                    return LIBSSH2_ERROR_DECRYPT;
+                }
 
-            /* total_num is the number of bytes following the initial
+                /* total_num is the number of bytes following the initial
-               (5 bytes) packet length and padding length fields */
+                   (5 bytes) packet length and padding length fields */
-            total_num = p->packet_length - 1 +
+                total_num = p->packet_length - 1 +
-                        (encrypted ? session->remote.mac->mac_len : 0);
+                            (encrypted ? session->remote.mac->mac_len : 0);
+            }
 
             /* RFC4253 section 6.1 Maximum Packet Length says:
              *
@@ -511,13 +592,17 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
             /* init write pointer to start of payload buffer */
             p->wptr = p->payload;
 
-            if(blocksize > 5) {
+            if(!etm && blocksize > 5) {
                 /* copy the data from index 5 to the end of
                    the blocksize from the temporary buffer to
                    the start of the decrypted buffer */
                 if(blocksize - 5 <= (int) total_num) {
                     memcpy(p->wptr, &block[5], blocksize - 5);
                     p->wptr += blocksize - 5;       /* advance write pointer */
+                    if(etm) {
+                        /* advance past unencrypted packet length */
+                        p->wptr += 4;
+                    }
                 }
                 else {
                     if(p->payload)
@@ -531,7 +616,8 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
             p->data_num = p->wptr - p->payload;
 
             /* we already dealt with a blocksize worth of data */
-            numbytes -= blocksize;
+            if(!etm)
+                numbytes -= blocksize;
         }
 
         /* how much there is left to add to the current payload
@@ -544,7 +630,7 @@ int _libssh2_transport_read(LIBSSH2_SESSION * session)
             numbytes = remainpack;
         }
 
-        if(encrypted) {
+        if(encrypted && !etm) {
             /* At the end of the incoming stream, there is a MAC,
                and we don't want to decrypt that since we need it
                "raw". We MUST however decrypt the padding data
@@ -772,11 +858,12 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
     struct transportpacket *p = &session->packet;
     int encrypted;
     int compressed;
+    int etm;
     ssize_t ret;
     int rc;
     const unsigned char *orgdata = data;
     size_t orgdata_len = data_len;
-    size_t crypt_offset;
+    size_t crypt_offset, etm_crypt_offset;
 
     /*
      * If the last read operation was interrupted in the middle of a key
@@ -814,6 +901,8 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
 
     encrypted = (session->state & LIBSSH2_STATE_NEWKEYS) ? 1 : 0;
 
+    etm = encrypted && session->local.mac ? session->local.mac->etm : 0;
+
     compressed = session->local.comp &&
                  session->local.comp->compress &&
                  ((session->state & LIBSSH2_STATE_AUTHENTICATED) ||
@@ -875,8 +964,11 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
 
     packet_length = data_len + 1 + 4;   /* 1 is for padding_length field
                                            4 for the packet_length field */
-    /* subtract 4 bytes of the packet_length field when padding AES-GCM */
+    /* subtract 4 bytes of the packet_length field when padding AES-GCM
-    crypt_offset = (encrypted && CRYPT_FLAG_R(session, PKTLEN_AAD)) ? 4 : 0;
+       or with ETM */
+    crypt_offset = (etm || (encrypted && CRYPT_FLAG_R(session, PKTLEN_AAD)))
+                   ? 4 : 0;
+    etm_crypt_offset = etm ? 4 : 0;
 
     /* at this point we have it all except the padding */
 
@@ -928,7 +1020,7 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
            fields except the MAC field itself. This is skipped in the
            INTEGRATED_MAC case, where the crypto algorithm also does its
            own hash. */
-        if(!CRYPT_FLAG_R(session, INTEGRATED_MAC)) {
+        if(!etm && !CRYPT_FLAG_R(session, INTEGRATED_MAC)) {
             session->local.mac->hash(session, p->outbuf + packet_length,
                                      session->local.seqno, p->outbuf,
                                      packet_length, NULL, 0,
@@ -940,23 +1032,23 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
         /* Some crypto back-ends could handle a single crypt() call for
            encryption, but (presumably) others cannot, so break it up
            into blocksize-sized chunks to satisfy them all. */
-        for(i = 0; i < packet_length;
+        for(i = etm_crypt_offset; i < packet_length;
             i += session->local.crypt->blocksize) {
             unsigned char *ptr = &p->outbuf[i];
             size_t bsize = LIBSSH2_MIN(session->local.crypt->blocksize,
                                        (int)(packet_length-i));
-            /* The INTEGRATED_MAC case always has an extra call below,
+            /* The INTEGRATED_MAC case always has an extra call below, so it
-               so it will never be LAST_BLOCK up here. */
+               will never be LAST_BLOCK up here. */
             int firstlast = i == 0 ? FIRST_BLOCK :
                 (!CRYPT_FLAG_L(session, INTEGRATED_MAC)
                  && (i == packet_length - session->local.crypt->blocksize)
                    ? LAST_BLOCK: MIDDLE_BLOCK);
-            /* In the AAD case, the last block would be only 4 bytes
+            /* In the AAD case, the last block would be only 4 bytes because
-               because everything is offset by 4 since the initial
+               everything is offset by 4 since the initial packet_length isn't
-               packet_length isn't encrypted. In this case, combine that
+               encrypted. In this case, combine that last short packet with the
-               last short packet with the previous one since AES-GCM
+               previous one since AES-GCM crypt() assumes that the entire MAC
-               crypt() assumes that the entire MAC is available in that
+               is available in that packet so it can set that to the
-               packet so it can set that to the authentication tag. */
+               authentication tag. */
             if(!CRYPT_FLAG_L(session, INTEGRATED_MAC))
                 if(i > packet_length - 2*bsize) {
                     /* increase the final block size */
@@ -964,25 +1056,38 @@ int _libssh2_transport_send(LIBSSH2_SESSION *session,
                     /* advance the loop counter by the extra amount */
                     i += bsize - session->local.crypt->blocksize;
                 }
+            _libssh2_debug((session, LIBSSH2_TRACE_SOCKET,
+                           "crypting bytes %d-%d", i,
+                           i + session->local.crypt->blocksize - 1));
             if(session->local.crypt->crypt(session, ptr,
                                            bsize,
                                            &session->local.crypt_abstract,
                                            firstlast))
                 return LIBSSH2_ERROR_ENCRYPT;     /* encryption failure */
         }
-        /* Call crypt() one last time so it can be filled in with
+        /* Call crypt() one last time so it can be filled in with the MAC */
-           the MAC */
         if(CRYPT_FLAG_L(session, INTEGRATED_MAC)) {
             int authlen = session->local.mac->mac_len;
             assert((size_t)total_length <=
                    packet_length + session->local.crypt->blocksize);
-            if(session->local.crypt->crypt(session,
+            if(session->local.crypt->crypt(session, &p->outbuf[packet_length],
-                                           &p->outbuf[packet_length],
                                            authlen,
                                            &session->local.crypt_abstract,
                                            LAST_BLOCK))
                 return LIBSSH2_ERROR_ENCRYPT;     /* encryption failure */
         }
+
+        if(etm) {
+            /* Calculate MAC hash. Put the output at index packet_length,
+               since that size includes the whole packet. The MAC is
+               calculated on the entire packet (length plain the rest
+               encrypted), including all fields except the MAC field
+               itself. */
+            session->local.mac->hash(session, p->outbuf + packet_length,
+                                     session->local.seqno, p->outbuf,
+                                     packet_length, NULL, 0,
+                                     &session->local.mac_abstract);
+        }
     }
 
     session->local.seqno++;

tests/CMakeLists.txt

@@ -117,6 +117,9 @@ foreach(test
   hmac-sha1-96
   hmac-sha2-256
   hmac-sha2-512
+  hmac-sha1-etm@openssh.com
+  hmac-sha2-256-etm@openssh.com
+  hmac-sha2-512-etm@openssh.com
 )
   add_test(NAME test_${test} COMMAND "$<TARGET_FILE:test_read>")
   set_tests_properties(test_${test} PROPERTIES ENVIRONMENT "FIXTURE_TEST_MAC=${test}")

tests/Makefile.am

@@ -115,4 +115,7 @@ EXTRA_DIST = \
   test_read_hmac-sha1-96 \
   test_read_hmac-sha2-256 \
   test_read_hmac-sha2-512 \
-  test_read_rijndael-cbc
+  test_read_rijndael-cbc \
+  test_read_hmac-sha1-etm@openssh.com \
+  test_read_hmac-sha2-256-etm@openssh.com \
+  test_read_hmac-sha2-512-etm@openssh.com

tests/test_auth_keyboard_info_request.c

@@ -310,7 +310,7 @@ int main(void)
 
     for(i = 0; i < FAILED_MALLOC_TEST_CASES_LEN; i++) {
         int tc =  i + TEST_CASES_LEN + 1;
-        int malloc_call_num = 5 + i;
+        int malloc_call_num = 3 + i;
         test_case(tc,
                   failed_malloc_test_cases[i].data,
                   failed_malloc_test_cases[i].data_len,