[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Mon Oct 17 22:25:44 UTC 2016


The branch master has been updated
       via  cde6145ba19a2fce039cf054a89e49f67c623c59 (commit)
       via  e23d5071ec4c7aa6bb2b0f2c3e0fc2182ed7e63f (commit)
      from  b2e54eb834e2d5a79d03f12a818d68f82c0e3d13 (commit)


- Log -----------------------------------------------------------------
commit cde6145ba19a2fce039cf054a89e49f67c623c59
Author: David Woodhouse <David.Woodhouse at intel.com>
Date:   Fri Oct 14 00:26:38 2016 +0100

    Add SSL_OP_NO_ENCRYPT_THEN_MAC
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>

commit e23d5071ec4c7aa6bb2b0f2c3e0fc2182ed7e63f
Author: David Woodhouse <David.Woodhouse at intel.com>
Date:   Wed Oct 12 23:12:04 2016 +0100

    Fix encrypt-then-mac implementation for DTLS
    
    OpenSSL 1.1.0 will negotiate EtM on DTLS but will then not actually *do* it.
    
    If we use DTLSv1.2 that will hopefully be harmless since we'll tend to use
    an AEAD ciphersuite anyway. But if we're using DTLSv1, then we certainly
    will end up using CBC, so EtM is relevant — and we fail to interoperate with
    anything that implements EtM correctly.
    
    Fixing it in HEAD and 1.1.0c will mean that 1.1.0[ab] are incompatible with
    1.1.0c+... for the limited case of non-AEAD ciphers, where they're *already*
    incompatible with other implementations due to this bug anyway. That seems
    reasonable enough, so let's do it. The only alternative is just to turn it
    off for ever... which *still* leaves 1.0.0[ab] failing to communicate with
    non-OpenSSL implementations anyway.
    
    Tested against itself as well as against GnuTLS both with and without EtM.
    
    Reviewed-by: Tim Hudson <tjh at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>

-----------------------------------------------------------------------

Summary of changes:
 doc/ssl/SSL_CTX_set_options.pod |  8 ++++++++
 include/openssl/ssl.h           |  2 ++
 ssl/record/rec_layer_d1.c       | 10 +++++++++-
 ssl/record/ssl3_record.c        | 22 +++++++++++++++++++++-
 ssl/t1_lib.c                    | 14 +++++++++-----
 5 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/doc/ssl/SSL_CTX_set_options.pod b/doc/ssl/SSL_CTX_set_options.pod
index 635b470..63609f3 100644
--- a/doc/ssl/SSL_CTX_set_options.pod
+++ b/doc/ssl/SSL_CTX_set_options.pod
@@ -189,6 +189,14 @@ Allow legacy insecure renegotiation between OpenSSL and unpatched servers
 B<only>: this option is currently set by default. See the
 B<SECURE RENEGOTIATION> section for more details.
 
+=item SSL_OP_NO_ENCRYPT_THEN_MAC
+
+Normally clients and servers will transparently attempt to negotiate the
+RFC7366 Encrypt-then-MAC option on TLS and DTLS connection.
+
+If this option is set, Encrypt-then-MAC is disabled. Clients will not
+propose, and servers will not accept the extension.
+
 =back
 
 =head1 SECURE RENEGOTIATION
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index e0d82f2..7e626e0 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -318,6 +318,8 @@ typedef int (*custom_ext_parse_cb) (SSL *s, unsigned int ext_type,
 # define SSL_OP_NO_COMPRESSION                           0x00020000U
 /* Permit unsafe legacy renegotiation */
 # define SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION        0x00040000U
+/* Disable encrypt-then-mac */
+# define SSL_OP_NO_ENCRYPT_THEN_MAC                      0x00080000U
 /* Does nothing: retained for compatibility */
 # define SSL_OP_SINGLE_ECDH_USE                          0x0
 /* Does nothing: retained for compatibility */
diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index 1d16319..c9fd066 100644
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -1094,7 +1094,7 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
      * wb->buf
      */
 
-    if (mac_size != 0) {
+    if (!SSL_USE_ETM(s) && mac_size != 0) {
         if (s->method->ssl3_enc->mac(s, &wr,
                                      &(p[SSL3_RECORD_get_length(&wr) + eivlen]),
                                      1) < 0)
@@ -1112,6 +1112,14 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
     if (s->method->ssl3_enc->enc(s, &wr, 1, 1) < 1)
         goto err;
 
+    if (SSL_USE_ETM(s) && mac_size != 0) {
+        if (s->method->ssl3_enc->mac(s, &wr,
+                                     &(p[SSL3_RECORD_get_length(&wr)]),
+                                     1) < 0)
+            goto err;
+        SSL3_RECORD_add_length(&wr, mac_size);
+    }
+
     /* record length after mac and block padding */
     /*
      * if (type == SSL3_RT_APPLICATION_DATA || (type == SSL3_RT_ALERT && !
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index 32a97af..3236166 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -1314,6 +1314,26 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
     rr->data = rr->input;
     rr->orig_len = rr->length;
 
+    if (SSL_USE_ETM(s) && s->read_hash) {
+        unsigned char *mac;
+        mac_size = EVP_MD_CTX_size(s->read_hash);
+        OPENSSL_assert(mac_size <= EVP_MAX_MD_SIZE);
+        if (rr->orig_len < mac_size) {
+            al = SSL_AD_DECODE_ERROR;
+            SSLerr(SSL_F_DTLS1_PROCESS_RECORD, SSL_R_LENGTH_TOO_SHORT);
+            goto f_err;
+        }
+        rr->length -= mac_size;
+        mac = rr->data + rr->length;
+        i = s->method->ssl3_enc->mac(s, rr, md, 0 /* not send */ );
+        if (i < 0 || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0) {
+            al = SSL_AD_BAD_RECORD_MAC;
+            SSLerr(SSL_F_DTLS1_PROCESS_RECORD,
+                   SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
+            goto f_err;
+        }
+    }
+
     enc_err = s->method->ssl3_enc->enc(s, rr, 1, 0);
     /*-
      * enc_err is:
@@ -1338,7 +1358,7 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
 #endif
 
     /* r->length is now the compressed data plus mac */
-    if ((sess != NULL) &&
+    if ((sess != NULL) && !SSL_USE_ETM(s) &&
         (s->enc_read_ctx != NULL) && (EVP_MD_CTX_md(s->read_hash) != NULL)) {
         /* s->read_hash != NULL => mac_size != -1 */
         unsigned char *mac = NULL;
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 71c480f..87ebbf3 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1335,10 +1335,12 @@ int ssl_add_clienthello_tlsext(SSL *s, WPACKET *pkt, int *al)
         return 0;
     }
 
-    if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_encrypt_then_mac)
+    if (!(s->options & SSL_OP_NO_ENCRYPT_THEN_MAC)) {
+        if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_encrypt_then_mac)
             || !WPACKET_put_bytes_u16(pkt, 0)) {
-        SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
-        return 0;
+            SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR);
+            return 0;
+        }
     }
 
 #ifndef OPENSSL_NO_CT
@@ -2128,7 +2130,8 @@ static int ssl_scan_clienthello_tlsext(SSL *s, PACKET *pkt, int *al)
                 return 0;
         }
 #endif
-        else if (type == TLSEXT_TYPE_encrypt_then_mac)
+        else if (type == TLSEXT_TYPE_encrypt_then_mac &&
+                 !(s->options & SSL_OP_NO_ENCRYPT_THEN_MAC))
             s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC;
         /*
          * Note: extended master secret extension handled in
@@ -2448,7 +2451,8 @@ static int ssl_scan_serverhello_tlsext(SSL *s, PACKET *pkt, int *al)
 #endif
         else if (type == TLSEXT_TYPE_encrypt_then_mac) {
             /* Ignore if inappropriate ciphersuite */
-            if (s->s3->tmp.new_cipher->algorithm_mac != SSL_AEAD
+            if (!(s->options & SSL_OP_NO_ENCRYPT_THEN_MAC) &&
+                s->s3->tmp.new_cipher->algorithm_mac != SSL_AEAD
                 && s->s3->tmp.new_cipher->algorithm_enc != SSL_RC4)
                 s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC;
         } else if (type == TLSEXT_TYPE_extended_master_secret) {


More information about the openssl-commits mailing list