[openssl-commits] [openssl] OpenSSL_1_1_0-stable update

Matt Caswell matt at openssl.org
Thu Dec 7 13:49:55 UTC 2017


The branch OpenSSL_1_1_0-stable has been updated
       via  4749aba5a24a646cc1e84b1e4d21e6f52399da33 (commit)
       via  b9ddcd7aa6ed7cb4d8b165895339ca66170f2da5 (commit)
       via  e502cc86df9dafded1694fceb3228ee34d11c11a (commit)
      from  6fca9feeffc18472c8153fd0bf8b0a95cc05c504 (commit)


- Log -----------------------------------------------------------------
commit 4749aba5a24a646cc1e84b1e4d21e6f52399da33
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Dec 6 13:54:37 2017 +0000

    Update CHANGES and NEWS for the new release
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit b9ddcd7aa6ed7cb4d8b165895339ca66170f2da5
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Nov 29 13:56:15 2017 +0000

    Add a test for CVE-2017-3737
    
    Test reading/writing to an SSL object after a fatal error has been
    detected. This CVE only affected 1.0.2, but we should add it to other
    branches for completeness.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

commit e502cc86df9dafded1694fceb3228ee34d11c11a
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Nov 24 11:35:50 2017 +0100

    bn/asm/rsaz-avx2.pl: fix digit correction bug in rsaz_1024_mul_avx2.
    
    Credit to OSS-Fuzz for finding this.
    
    CVE-2017-3738
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>

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

Summary of changes:
 CHANGES                                            |  21 +++-
 NEWS                                               |   2 +-
 crypto/bn/asm/rsaz-avx2.pl                         |  15 ++-
 test/build.info                                    |   7 +-
 test/fatalerrtest.c                                | 124 +++++++++++++++++++++
 .../{90-test_sslapi.t => 90-test_fatalerr.t}       |   6 +-
 6 files changed, 161 insertions(+), 14 deletions(-)
 create mode 100644 test/fatalerrtest.c
 copy test/recipes/{90-test_sslapi.t => 90-test_fatalerr.t} (77%)

diff --git a/CHANGES b/CHANGES
index a8cea3a..3d4e835 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,7 +9,26 @@
 
  Changes between 1.1.0g and 1.1.0h [xx XXX xxxx]
 
-  *)
+  *) rsaz_1024_mul_avx2 overflow bug on x86_64
+
+     There is an overflow bug in the AVX2 Montgomery multiplication procedure
+     used in exponentiation with 1024-bit moduli. No EC algorithms are affected.
+     Analysis suggests that attacks against RSA and DSA as a result of this
+     defect would be very difficult to perform and are not believed likely.
+     Attacks against DH1024 are considered just feasible, because most of the
+     work necessary to deduce information about a private key may be performed
+     offline. The amount of resources required for such an attack would be
+     significant. However, for an attack on TLS to be meaningful, the server
+     would have to share the DH1024 private key among multiple clients, which is
+     no longer an option since CVE-2016-0701.
+
+     This only affects processors that support the AVX2 but not ADX extensions
+     like Intel Haswell (4th generation).
+
+     This issue was reported to OpenSSL by David Benjamin (Google). The issue
+     was originally found via the OSS-Fuzz project.
+     (CVE-2017-3738)
+     [Andy Polyakov]
 
  Changes between 1.1.0f and 1.1.0g [2 Nov 2017]
 
diff --git a/NEWS b/NEWS
index 3a58d25..8b5b971 100644
--- a/NEWS
+++ b/NEWS
@@ -7,7 +7,7 @@
 
   Major changes between OpenSSL 1.1.0g and OpenSSL 1.1.0h [under development]
 
-      o
+      o rsaz_1024_mul_avx2 overflow bug on x86_64 (CVE-2017-3738)
 
   Major changes between OpenSSL 1.1.0f and OpenSSL 1.1.0g [2 Nov 2017]
 
diff --git a/crypto/bn/asm/rsaz-avx2.pl b/crypto/bn/asm/rsaz-avx2.pl
index 0c1b236..46d746b 100755
--- a/crypto/bn/asm/rsaz-avx2.pl
+++ b/crypto/bn/asm/rsaz-avx2.pl
@@ -246,7 +246,7 @@ $code.=<<___;
 	vmovdqu		32*8-128($ap), $ACC8
 
 	lea	192(%rsp), $tp0			# 64+128=192
-	vpbroadcastq	.Land_mask(%rip), $AND_MASK
+	vmovdqu	.Land_mask(%rip), $AND_MASK
 	jmp	.LOOP_GRANDE_SQR_1024
 
 .align	32
@@ -1077,10 +1077,10 @@ $code.=<<___;
 	vpmuludq	32*6-128($np),$Yi,$TEMP1
 	vpaddq		$TEMP1,$ACC6,$ACC6
 	vpmuludq	32*7-128($np),$Yi,$TEMP2
-	 vpblendd	\$3, $ZERO, $ACC9, $ACC9	# correct $ACC3
+	 vpblendd	\$3, $ZERO, $ACC9, $TEMP1	# correct $ACC3
 	vpaddq		$TEMP2,$ACC7,$ACC7
 	vpmuludq	32*8-128($np),$Yi,$TEMP0
-	 vpaddq		$ACC9, $ACC3, $ACC3		# correct $ACC3
+	 vpaddq		$TEMP1, $ACC3, $ACC3		# correct $ACC3
 	vpaddq		$TEMP0,$ACC8,$ACC8
 
 	mov	%rbx, %rax
@@ -1093,7 +1093,9 @@ $code.=<<___;
 	 vmovdqu	-8+32*2-128($ap),$TEMP2
 
 	mov	$r1, %rax
+	 vpblendd	\$0xfc, $ZERO, $ACC9, $ACC9	# correct $ACC3
 	imull	$n0, %eax
+	 vpaddq		$ACC9,$ACC4,$ACC4		# correct $ACC3
 	and	\$0x1fffffff, %eax
 
 	 imulq	16-128($ap),%rbx
@@ -1329,15 +1331,12 @@ ___
 #	But as we underutilize resources, it's possible to correct in
 #	each iteration with marginal performance loss. But then, as
 #	we do it in each iteration, we can correct less digits, and
-#	avoid performance penalties completely. Also note that we
-#	correct only three digits out of four. This works because
-#	most significant digit is subjected to less additions.
+#	avoid performance penalties completely.
 
 $TEMP0 = $ACC9;
 $TEMP3 = $Bi;
 $TEMP4 = $Yi;
 $code.=<<___;
-	vpermq		\$0, $AND_MASK, $AND_MASK
 	vpaddq		(%rsp), $TEMP1, $ACC0
 
 	vpsrlq		\$29, $ACC0, $TEMP1
@@ -1770,7 +1769,7 @@ $code.=<<___;
 
 .align	64
 .Land_mask:
-	.quad	0x1fffffff,0x1fffffff,0x1fffffff,-1
+	.quad	0x1fffffff,0x1fffffff,0x1fffffff,0x1fffffff
 .Lscatter_permd:
 	.long	0,2,4,6,7,7,7,7
 .Lgather_permd:
diff --git a/test/build.info b/test/build.info
index 0b52994..199fe13 100644
--- a/test/build.info
+++ b/test/build.info
@@ -16,7 +16,8 @@ IF[{- !$disabled{tests} -}]
           packettest asynctest secmemtest srptest memleaktest \
           dtlsv1listentest ct_test threadstest afalgtest d2i_test \
           ssl_test_ctx_test ssl_test x509aux cipherlist_test asynciotest \
-          bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test
+          bioprinttest sslapitest dtlstest sslcorrupttest bio_enc_test \
+          fatalerrtest
 
   SOURCE[aborttest]=aborttest.c
   INCLUDE[aborttest]=../include
@@ -146,6 +147,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[rsa_test]=.. ../include
   DEPEND[rsa_test]=../libcrypto
 
+  SOURCE[fatalerrtest]=fatalerrtest.c ssltestlib.c testutil.c
+  INCLUDE[fatalerrtest]=../include ..
+  DEPEND[fatalerrtest]=../libcrypto ../libssl
+
   SOURCE[evp_test]=evp_test.c
   INCLUDE[evp_test]=../include
   DEPEND[evp_test]=../libcrypto
diff --git a/test/fatalerrtest.c b/test/fatalerrtest.c
new file mode 100644
index 0000000..4a58839
--- /dev/null
+++ b/test/fatalerrtest.c
@@ -0,0 +1,124 @@
+/*
+ * Copyright 2017 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL license (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/err.h>
+#include "ssltestlib.h"
+#include "testutil.h"
+#include <string.h>
+
+static char *cert = NULL;
+static char *privkey = NULL;
+
+static int test_fatalerr(void)
+{
+    SSL_CTX *sctx = NULL, *cctx = NULL;
+    SSL *sssl = NULL, *cssl = NULL;
+    const char *msg = "Dummy";
+    BIO *wbio = NULL;
+    int ret = 0, len;
+    char buf[80];
+    unsigned char dummyrec[] = {
+        0x17, 0x03, 0x03, 0x00, 0x05, 'D', 'u', 'm', 'm', 'y'
+    };
+
+    if (!create_ssl_ctx_pair(SSLv23_method(), SSLv23_method(), &sctx, &cctx,
+                             cert, privkey)) {
+        printf("Failed to create SSL_CTX pair\n");
+        goto err;
+    }
+
+    /*
+     * Deliberately set the cipher lists for client and server to be different
+     * to force a handshake failure.
+     */
+    if (!SSL_CTX_set_cipher_list(sctx, "AES128-SHA")
+            || !SSL_CTX_set_cipher_list(cctx, "AES256-SHA")) {
+        printf("Failed to set cipher lists\n");
+        goto err;
+    }
+
+    if (!create_ssl_objects(sctx, cctx, &sssl, &cssl, NULL, NULL)) {
+        printf("Failed to create SSL objectx\n");
+        goto err;
+    }
+
+    wbio = SSL_get_wbio(cssl);
+    if (wbio == NULL) {
+        printf("Unexpected NULL bio received\n");
+        goto err;
+    }
+
+    if (create_ssl_connection(sssl, cssl)) {
+        printf("Unexpected success creating a connection\n");
+        goto err;
+    }
+
+    ERR_clear_error();
+
+    /* Inject a plaintext record from client to server */
+    if (BIO_write(wbio, dummyrec, sizeof(dummyrec)) <= 0) {
+        printf("Unexpected failure injecting dummy record\n");
+        goto err;
+    }
+
+    /* SSL_read()/SSL_write should fail because of a previous fatal error */
+    if ((len = SSL_read(sssl, buf, sizeof(buf - 1))) > 0) {
+        buf[len] = '\0';
+        printf("Unexpected success reading data: %s\n", buf);
+        goto err;
+    }
+    if (SSL_write(sssl, msg, strlen(msg)) > 0) {
+        printf("Unexpected success writing data\n");
+        goto err;
+    }
+
+    ret = 1;
+ err:
+    SSL_free(sssl);
+    SSL_free(cssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+
+    return ret;
+}
+
+int main(int argc, char *argv[])
+{
+    BIO *err = NULL;
+    int testresult = 1;
+
+    if (argc != 3) {
+        printf("Invalid argument count\n");
+        return 1;
+    }
+
+    cert = argv[1];
+    privkey = argv[2];
+
+    err = BIO_new_fp(stderr, BIO_NOCLOSE | BIO_FP_TEXT);
+
+    CRYPTO_set_mem_debug(1);
+    CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON);
+
+    ADD_TEST(test_fatalerr);
+
+    testresult = run_tests(argv[0]);
+
+#ifndef OPENSSL_NO_CRYPTO_MDEBUG
+    if (CRYPTO_mem_leaks(err) <= 0)
+        testresult = 1;
+#endif
+    BIO_free(err);
+
+    if (!testresult)
+        printf("PASS\n");
+
+    return testresult;
+}
diff --git a/test/recipes/90-test_sslapi.t b/test/recipes/90-test_fatalerr.t
similarity index 77%
copy from test/recipes/90-test_sslapi.t
copy to test/recipes/90-test_fatalerr.t
index efaae3b..361bc1f 100644
--- a/test/recipes/90-test_sslapi.t
+++ b/test/recipes/90-test_fatalerr.t
@@ -10,12 +10,12 @@
 use OpenSSL::Test::Utils;
 use OpenSSL::Test qw/:DEFAULT srctop_file/;
 
-setup("test_sslapi");
+setup("test_fatalerr");
 
 plan skip_all => "No TLS/SSL protocols are supported by this OpenSSL build"
     if alldisabled(grep { $_ ne "ssl3" } available_protocols("tls"));
 
 plan tests => 1;
 
-ok(run(test(["sslapitest", srctop_file("apps", "server.pem"),
-             srctop_file("apps", "server.pem")])), "running sslapitest");
+ok(run(test(["fatalerrtest", srctop_file("apps", "server.pem"),
+             srctop_file("apps", "server.pem")])), "running fatalerrtest");


More information about the openssl-commits mailing list