[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Fri May 11 12:51:40 UTC 2018


The branch master has been updated
       via  0d8da77908df1aa3186b00113aab1b338cba9b07 (commit)
       via  9e064bc1701599a15d0111a252b70fe45f2d2da8 (commit)
       via  48a03162db6d5c1b66fd18e2d92461716178d986 (commit)
      from  3cb7c5cfef25463bd197b0c12ca7966f525ebf73 (commit)


- Log -----------------------------------------------------------------
commit 0d8da77908df1aa3186b00113aab1b338cba9b07
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 10 12:33:51 2018 +0100

    Test an old style PSK callback with no cert will prefer SHA-256
    
    If using an old style PSK callback and no certificate is configured for
    the server, we should prefer ciphersuites based on SHA-256, because that
    is the default hash for those callbacks as specified in the TLSv1.3 spec.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6215)

commit 9e064bc1701599a15d0111a252b70fe45f2d2da8
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 10 12:01:06 2018 +0100

    Provide documentation for the -psk_session option
    
    The s_client/s_server docs were missing documentation for this option.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6215)

commit 48a03162db6d5c1b66fd18e2d92461716178d986
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 10 11:51:45 2018 +0100

    Prefer SHA-256 ciphersuites if using old style PSKs
    
    If we have no certificate and we are using "old style" PSKs then we will
    always default to using SHA-256 for that PSK. However we may have selected
    a ciphersuite that is not based on SHA-256. Therefore if we see that there
    are no certificates and we have been configured for "old style" PSKs then
    we should prefer SHA-256 based ciphersuites during the selection process.
    
    Fixes #6197
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6215)

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

Summary of changes:
 doc/man1/s_client.pod |   8 +++
 doc/man1/s_server.pod |   6 ++
 ssl/s3_lib.c          |  33 ++++++++++-
 test/sslapitest.c     | 156 ++++++++++++++++++++++++++++----------------------
 test/ssltestlib.c     |  15 +++--
 5 files changed, 142 insertions(+), 76 deletions(-)

diff --git a/doc/man1/s_client.pod b/doc/man1/s_client.pod
index 5d33e1c..19a8139 100644
--- a/doc/man1/s_client.pod
+++ b/doc/man1/s_client.pod
@@ -71,6 +71,9 @@ B<openssl> B<s_client>
 [B<-crlf>]
 [B<-ign_eof>]
 [B<-no_ign_eof>]
+[B<-psk_identity identity>]
+[B<-psk key>]
+[B<-psk_session file>]
 [B<-quiet>]
 [B<-ssl3>]
 [B<-tls1>]
@@ -409,6 +412,11 @@ given as a hexadecimal number without leading 0x, for example -psk
 1a2b3c4d.
 This option must be provided in order to use a PSK cipher.
 
+=item B<-psk_session file>
+
+Use the pem encoded SSL_SESSION data stored in B<file> as the basis of a PSK.
+Note that this will only work if TLSv1.3 is negotiated.
+
 =item B<-ssl3>, B<-tls1>, B<-tls1_1>, B<-tls1_2>, B<-tls1_3>, B<-no_ssl3>, B<-no_tls1>, B<-no_tls1_1>, B<-no_tls1_2>, B<-no_tls1_3>
 
 These options require or disable the use of the specified SSL or TLS protocols.
diff --git a/doc/man1/s_server.pod b/doc/man1/s_server.pod
index 184ddc9..e577af8 100644
--- a/doc/man1/s_server.pod
+++ b/doc/man1/s_server.pod
@@ -157,6 +157,7 @@ B<openssl> B<s_server>
 [B<-psk_identity val>]
 [B<-psk_hint val>]
 [B<-psk val>]
+[B<-psk_session file>]
 [B<-srpvfile infile>]
 [B<-srpuserseed val>]
 [B<-ssl3>]
@@ -597,6 +598,11 @@ given as a hexadecimal number without leading 0x, for example -psk
 1a2b3c4d.
 This option must be provided in order to use a PSK cipher.
 
+=item B<-psk_session file>
+
+Use the pem encoded SSL_SESSION data stored in B<file> as the basis of a PSK.
+Note that this will only work if TLSv1.3 is negotiated.
+
 =item B<-listen>
 
 This option can only be used in conjunction with one of the DTLS options above.
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index f797497..c5f2235 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -4108,8 +4108,9 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
 {
     const SSL_CIPHER *c, *ret = NULL;
     STACK_OF(SSL_CIPHER) *prio, *allow;
-    int i, ii, ok;
+    int i, ii, ok, prefer_sha256 = 0;
     unsigned long alg_k = 0, alg_a = 0, mask_k = 0, mask_a = 0;
+    const EVP_MD *mdsha256 = EVP_sha256();
 #ifndef OPENSSL_NO_CHACHA
     STACK_OF(SSL_CIPHER) *prio_chacha = NULL;
 #endif
@@ -4190,7 +4191,24 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
         allow = srvr;
     }
 
-    if (!SSL_IS_TLS13(s)) {
+    if (SSL_IS_TLS13(s)) {
+        int j;
+
+        /*
+         * If we allow "old" style PSK callbacks, and we have no certificate (so
+         * we're not going to succeed without a PSK anyway), and we're in
+         * TLSv1.3 then the default hash for a PSK is SHA-256 (as per the
+         * TLSv1.3 spec). Therefore we should prioritise ciphersuites using
+         * that.
+         */
+        if (s->psk_server_callback != NULL) {
+            for (j = 0; j < SSL_PKEY_NUM && !ssl_has_cert(s, j); j++);
+            if (j == SSL_PKEY_NUM) {
+                /* There are no certificates */
+                prefer_sha256 = 1;
+            }
+        }
+    } else {
         tls1_set_cert_validity(s);
         ssl_set_masks(s);
     }
@@ -4262,6 +4280,17 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
                 continue;
             }
 #endif
+            if (prefer_sha256) {
+                const SSL_CIPHER *tmp = sk_SSL_CIPHER_value(allow, ii);
+
+                if (ssl_md(tmp->algorithm2) == mdsha256) {
+                    ret = tmp;
+                    break;
+                }
+                if (ret == NULL)
+                    ret = tmp;
+                continue;
+            }
             ret = sk_SSL_CIPHER_value(allow, ii);
             break;
         }
diff --git a/test/sslapitest.c b/test/sslapitest.c
index 0a3d515..bce15db 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -2781,6 +2781,13 @@ static int test_ciphersuite_change(void)
     return testresult;
 }
 
+/*
+ * Test TLSv1.3 PSKs
+ * Test 0 = Test new style callbacks
+ * Test 1 = Test both new and old style callbacks
+ * Test 2 = Test old style callbacks
+ * Test 3 = Test old style callbacks with no certificate
+ */
 static int test_tls13_psk(int idx)
 {
     SSL_CTX *sctx = NULL, *cctx = NULL;
@@ -2796,15 +2803,21 @@ static int test_tls13_psk(int idx)
 
     if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), TLS_client_method(),
                                        TLS1_VERSION, TLS_MAX_VERSION,
-                                       &sctx, &cctx, cert, privkey)))
+                                       &sctx, &cctx, idx == 3 ? NULL : cert,
+                                       idx == 3 ? NULL : privkey)))
         goto end;
 
-    /*
-     * We use a ciphersuite with SHA256 to ease testing old style PSK callbacks
-     * which will always default to SHA256
-     */
-    if (!TEST_true(SSL_CTX_set_ciphersuites(cctx, "TLS_AES_128_GCM_SHA256")))
-        goto end;
+    if (idx != 3) {
+        /*
+         * We use a ciphersuite with SHA256 to ease testing old style PSK
+         * callbacks which will always default to SHA256. This should not be
+         * necessary if we have no cert/priv key. In that case the server should
+         * prefer SHA256 automatically.
+         */
+        if (!TEST_true(SSL_CTX_set_ciphersuites(cctx,
+                                                "TLS_AES_128_GCM_SHA256")))
+            goto end;
+    }
 
     /*
      * Test 0: New style callbacks only
@@ -2816,7 +2829,7 @@ static int test_tls13_psk(int idx)
         SSL_CTX_set_psk_find_session_callback(sctx, find_session_cb);
     }
 #ifndef OPENSSL_NO_PSK
-    if (idx == 1 || idx == 2) {
+    if (idx >= 1) {
         SSL_CTX_set_psk_client_callback(cctx, psk_client_cb);
         SSL_CTX_set_psk_server_callback(sctx, psk_server_cb);
     }
@@ -2827,36 +2840,41 @@ static int test_tls13_psk(int idx)
     psk_client_cb_cnt = 0;
     psk_server_cb_cnt = 0;
 
-    /* Check we can create a connection if callback decides not to send a PSK */
-    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
-                                             NULL, NULL))
-            || !TEST_true(create_ssl_connection(serverssl, clientssl,
-                                                SSL_ERROR_NONE))
-            || !TEST_false(SSL_session_reused(clientssl))
-            || !TEST_false(SSL_session_reused(serverssl)))
-        goto end;
-
-    if (idx == 0 || idx == 1) {
-        if (!TEST_true(use_session_cb_cnt == 1)
-                || !TEST_true(find_session_cb_cnt == 0)
-                   /*
-                    * If no old style callback then below should be 0
-                    * otherwise 1
-                    */
-                || !TEST_true(psk_client_cb_cnt == idx)
-                || !TEST_true(psk_server_cb_cnt == 0))
-            goto end;
-    } else {
-        if (!TEST_true(use_session_cb_cnt == 0)
-                || !TEST_true(find_session_cb_cnt == 0)
-                || !TEST_true(psk_client_cb_cnt == 1)
-                || !TEST_true(psk_server_cb_cnt == 0))
+    if (idx != 3) {
+        /*
+         * Check we can create a connection if callback decides not to send a
+         * PSK
+         */
+        if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+                                                 NULL, NULL))
+                || !TEST_true(create_ssl_connection(serverssl, clientssl,
+                                                    SSL_ERROR_NONE))
+                || !TEST_false(SSL_session_reused(clientssl))
+                || !TEST_false(SSL_session_reused(serverssl)))
             goto end;
-    }
 
-    shutdown_ssl_connection(serverssl, clientssl);
-    serverssl = clientssl = NULL;
-    use_session_cb_cnt = psk_client_cb_cnt = 0;
+        if (idx == 0 || idx == 1) {
+            if (!TEST_true(use_session_cb_cnt == 1)
+                    || !TEST_true(find_session_cb_cnt == 0)
+                       /*
+                        * If no old style callback then below should be 0
+                        * otherwise 1
+                        */
+                    || !TEST_true(psk_client_cb_cnt == idx)
+                    || !TEST_true(psk_server_cb_cnt == 0))
+                goto end;
+        } else {
+            if (!TEST_true(use_session_cb_cnt == 0)
+                    || !TEST_true(find_session_cb_cnt == 0)
+                    || !TEST_true(psk_client_cb_cnt == 1)
+                    || !TEST_true(psk_server_cb_cnt == 0))
+                goto end;
+        }
+
+        shutdown_ssl_connection(serverssl, clientssl);
+        serverssl = clientssl = NULL;
+        use_session_cb_cnt = psk_client_cb_cnt = 0;
+    }
 
     if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
                                              NULL, NULL)))
@@ -2937,39 +2955,41 @@ static int test_tls13_psk(int idx)
     use_session_cb_cnt = find_session_cb_cnt = 0;
     psk_client_cb_cnt = psk_server_cb_cnt = 0;
 
-    /*
-     * Check that if the server rejects the PSK we can still connect, but with
-     * a full handshake
-     */
-    srvid = "Dummy Identity";
-    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
-                                             NULL, NULL))
-            || !TEST_true(create_ssl_connection(serverssl, clientssl,
-                                                SSL_ERROR_NONE))
-            || !TEST_false(SSL_session_reused(clientssl))
-            || !TEST_false(SSL_session_reused(serverssl)))
-        goto end;
-
-    if (idx == 0 || idx == 1) {
-        if (!TEST_true(use_session_cb_cnt == 1)
-                || !TEST_true(find_session_cb_cnt == 1)
-                || !TEST_true(psk_client_cb_cnt == 0)
-                   /*
-                    * If no old style callback then below should be 0
-                    * otherwise 1
-                    */
-                || !TEST_true(psk_server_cb_cnt == idx))
-            goto end;
-    } else {
-        if (!TEST_true(use_session_cb_cnt == 0)
-                || !TEST_true(find_session_cb_cnt == 0)
-                || !TEST_true(psk_client_cb_cnt == 1)
-                || !TEST_true(psk_server_cb_cnt == 1))
+    if (idx != 3) {
+        /*
+         * Check that if the server rejects the PSK we can still connect, but with
+         * a full handshake
+         */
+        srvid = "Dummy Identity";
+        if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+                                                 NULL, NULL))
+                || !TEST_true(create_ssl_connection(serverssl, clientssl,
+                                                    SSL_ERROR_NONE))
+                || !TEST_false(SSL_session_reused(clientssl))
+                || !TEST_false(SSL_session_reused(serverssl)))
             goto end;
-    }
 
-    shutdown_ssl_connection(serverssl, clientssl);
-    serverssl = clientssl = NULL;
+        if (idx == 0 || idx == 1) {
+            if (!TEST_true(use_session_cb_cnt == 1)
+                    || !TEST_true(find_session_cb_cnt == 1)
+                    || !TEST_true(psk_client_cb_cnt == 0)
+                       /*
+                        * If no old style callback then below should be 0
+                        * otherwise 1
+                        */
+                    || !TEST_true(psk_server_cb_cnt == idx))
+                goto end;
+        } else {
+            if (!TEST_true(use_session_cb_cnt == 0)
+                    || !TEST_true(find_session_cb_cnt == 0)
+                    || !TEST_true(psk_client_cb_cnt == 1)
+                    || !TEST_true(psk_server_cb_cnt == 1))
+                goto end;
+        }
+
+        shutdown_ssl_connection(serverssl, clientssl);
+        serverssl = clientssl = NULL;
+    }
     testresult = 1;
 
  end:
@@ -4640,7 +4660,7 @@ int setup_tests(void)
 #ifdef OPENSSL_NO_PSK
     ADD_ALL_TESTS(test_tls13_psk, 1);
 #else
-    ADD_ALL_TESTS(test_tls13_psk, 3);
+    ADD_ALL_TESTS(test_tls13_psk, 4);
 #endif  /* OPENSSL_NO_PSK */
     ADD_ALL_TESTS(test_custom_exts, 5);
     ADD_TEST(test_stateless);
diff --git a/test/ssltestlib.c b/test/ssltestlib.c
index 041ae26..c768963 100644
--- a/test/ssltestlib.c
+++ b/test/ssltestlib.c
@@ -594,12 +594,15 @@ int create_ssl_ctx_pair(const SSL_METHOD *sm, const SSL_METHOD *cm,
                                                             max_proto_version)))))
         goto err;
 
-    if (!TEST_int_eq(SSL_CTX_use_certificate_file(serverctx, certfile,
-                                                  SSL_FILETYPE_PEM), 1)
-            || !TEST_int_eq(SSL_CTX_use_PrivateKey_file(serverctx, privkeyfile,
-                                                        SSL_FILETYPE_PEM), 1)
-            || !TEST_int_eq(SSL_CTX_check_private_key(serverctx), 1))
-        goto err;
+    if (certfile != NULL && privkeyfile != NULL) {
+        if (!TEST_int_eq(SSL_CTX_use_certificate_file(serverctx, certfile,
+                                                      SSL_FILETYPE_PEM), 1)
+                || !TEST_int_eq(SSL_CTX_use_PrivateKey_file(serverctx,
+                                                            privkeyfile,
+                                                            SSL_FILETYPE_PEM), 1)
+                || !TEST_int_eq(SSL_CTX_check_private_key(serverctx), 1))
+            goto err;
+    }
 
 #ifndef OPENSSL_NO_DH
     SSL_CTX_set_dh_auto(serverctx, 1);


More information about the openssl-commits mailing list