[openssl] OpenSSL_1_1_1-stable update

kaduk at mit.edu kaduk at mit.edu
Fri May 14 18:47:07 UTC 2021


The branch OpenSSL_1_1_1-stable has been updated
       via  b743b16113ca0e30c383191c804de37dbfc4f12e (commit)
       via  df1fd3c986f5a58b6dc87d2c4bb565a8f1e688fa (commit)
       via  5d88a9c62c81e38918becae96a842986e2e0940e (commit)
       via  1ab7b9991ba00a1423ec6c5898a70e11d1337cfb (commit)
      from  a812f8fc8f3c9ba30e5ecd2c168cca0613f15dcd (commit)


- Log -----------------------------------------------------------------
commit b743b16113ca0e30c383191c804de37dbfc4f12e
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Mon Mar 29 23:05:22 2021 -0700

    Update expected results for tls13kexmodes tests
    
    One of the scenarios constructed in these tests was erroneously
    producing successful handshakes until the previous commits, but should
    have been failing.  Update our expected behavior to match the
    specification requirements, and adjust the commentary slightly for
    a test case relevant for the other preceding commit.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    
    (cherry picked from commit 80c25611abd7067815943187f36f5e1879201678)
    
    (Merged from https://github.com/openssl/openssl/pull/15255)

commit df1fd3c986f5a58b6dc87d2c4bb565a8f1e688fa
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Mon Mar 29 21:27:49 2021 -0700

    Don't send key_share for PSK-only key exchange
    
    TLS 1.3 allows for the "psk_ke" and "psk_dhe_ke" key-exchange modes.
    Only the latter mode introduces a new ephemeral (Diffie-Hellman)
    key exchange, with the PSK being the only key material used in the
    former case.
    
    It's a compliance requirement of RFC 8446 that the server MUST NOT
    send a KeyShareEntry when using the "psk_ke" mode, but prior to
    this commit we would send a key-share based solely on whether the
    client sent one.  This bug goes unnoticed in our internal test suite
    since openssl communicating with openssl can never negotiate the
    PSK-only key-exchange mode.  However, we should still be compliant
    with the spec, so check whether the DHE mode was offered and don't
    send a key-share if it wasn't.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    
    (cherry picked from commit e776858bce32d473bd7a69c616ad7f6c2f979dfc)
    
    (Merged from https://github.com/openssl/openssl/pull/15255)

commit 5d88a9c62c81e38918becae96a842986e2e0940e
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Mon Mar 29 23:08:10 2021 -0700

    make update
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15255)

commit 1ab7b9991ba00a1423ec6c5898a70e11d1337cfb
Author: Benjamin Kaduk <bkaduk at akamai.com>
Date:   Mon Mar 29 23:03:49 2021 -0700

    Improve RFC 8446 PSK key exchange mode compliance
    
    It's a MUST-level requirement that if the client sends a pre_shared_key
    extension not accompanied by a psk_key_exchange_modes extension, the
    server must abort the handshake.  Prior to this commit the server
    would continue on.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    
    (cherry picked from commit efe0f315354b020213097885c79ce856a2f5ac68)
    
    (Merged from https://github.com/openssl/openssl/pull/15255)

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

Summary of changes:
 crypto/err/openssl.txt               |  2 ++
 include/openssl/sslerr.h             |  2 ++
 ssl/ssl_err.c                        |  3 +++
 ssl/statem/extensions.c              | 19 ++++++++++++++++++-
 ssl/statem/extensions_srvr.c         |  7 +++++++
 test/recipes/70-test_tls13kexmodes.t | 12 +++++-------
 6 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index e0e60ffa38..017a9a6652 100644
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -1160,6 +1160,7 @@ SSL_F_FINAL_EC_PT_FORMATS:485:final_ec_pt_formats
 SSL_F_FINAL_EMS:486:final_ems
 SSL_F_FINAL_KEY_SHARE:503:final_key_share
 SSL_F_FINAL_MAXFRAGMENTLEN:557:final_maxfragmentlen
+SSL_F_FINAL_PSK:639:final_psk
 SSL_F_FINAL_RENEGOTIATE:483:final_renegotiate
 SSL_F_FINAL_SERVER_NAME:558:final_server_name
 SSL_F_FINAL_SIG_ALGS:497:final_sig_algs
@@ -2741,6 +2742,7 @@ SSL_R_MISSING_DSA_SIGNING_CERT:165:missing dsa signing cert
 SSL_R_MISSING_ECDSA_SIGNING_CERT:381:missing ecdsa signing cert
 SSL_R_MISSING_FATAL:256:missing fatal
 SSL_R_MISSING_PARAMETERS:290:missing parameters
+SSL_R_MISSING_PSK_KEX_MODES_EXTENSION:310:missing psk kex modes extension
 SSL_R_MISSING_RSA_CERTIFICATE:168:missing rsa certificate
 SSL_R_MISSING_RSA_ENCRYPTING_CERT:169:missing rsa encrypting cert
 SSL_R_MISSING_RSA_SIGNING_CERT:170:missing rsa signing cert
diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
index 9060fd1b75..701d61c6e9 100644
--- a/include/openssl/sslerr.h
+++ b/include/openssl/sslerr.h
@@ -70,6 +70,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_FINAL_EMS                                  486
 # define SSL_F_FINAL_KEY_SHARE                            503
 # define SSL_F_FINAL_MAXFRAGMENTLEN                       557
+# define SSL_F_FINAL_PSK                                  639
 # define SSL_F_FINAL_RENEGOTIATE                          483
 # define SSL_F_FINAL_SERVER_NAME                          558
 # define SSL_F_FINAL_SIG_ALGS                             497
@@ -592,6 +593,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_R_MISSING_ECDSA_SIGNING_CERT                 381
 # define SSL_R_MISSING_FATAL                              256
 # define SSL_R_MISSING_PARAMETERS                         290
+# define SSL_R_MISSING_PSK_KEX_MODES_EXTENSION            310
 # define SSL_R_MISSING_RSA_CERTIFICATE                    168
 # define SSL_R_MISSING_RSA_ENCRYPTING_CERT                169
 # define SSL_R_MISSING_RSA_SIGNING_CERT                   170
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index d0c69821b5..324f2ccbb0 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -85,6 +85,7 @@ static const ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_PACK(ERR_LIB_SSL, SSL_F_FINAL_KEY_SHARE, 0), "final_key_share"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_FINAL_MAXFRAGMENTLEN, 0),
      "final_maxfragmentlen"},
+    {ERR_PACK(ERR_LIB_SSL, SSL_F_FINAL_PSK, 0), "final_psk"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_FINAL_RENEGOTIATE, 0), "final_renegotiate"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_FINAL_SERVER_NAME, 0), "final_server_name"},
     {ERR_PACK(ERR_LIB_SSL, SSL_F_FINAL_SIG_ALGS, 0), "final_sig_algs"},
@@ -948,6 +949,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
     "missing ecdsa signing cert"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_FATAL), "missing fatal"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_PARAMETERS), "missing parameters"},
+    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_PSK_KEX_MODES_EXTENSION),
+    "missing psk kex modes extension"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_RSA_CERTIFICATE),
     "missing rsa certificate"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_RSA_ENCRYPTING_CERT),
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index e1a3b1db67..07803537ba 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -56,6 +56,7 @@ static int final_sig_algs(SSL *s, unsigned int context, int sent);
 static int final_early_data(SSL *s, unsigned int context, int sent);
 static int final_maxfragmentlen(SSL *s, unsigned int context, int sent);
 static int init_post_handshake_auth(SSL *s, unsigned int context);
+static int final_psk(SSL *s, unsigned int context, int sent);
 
 /* Structure to define a built-in extension */
 typedef struct extensions_definition_st {
@@ -389,7 +390,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_3_SERVER_HELLO
         | SSL_EXT_TLS_IMPLEMENTATION_ONLY | SSL_EXT_TLS1_3_ONLY,
         NULL, tls_parse_ctos_psk, tls_parse_stoc_psk, tls_construct_stoc_psk,
-        tls_construct_ctos_psk, NULL
+        tls_construct_ctos_psk, final_psk
     }
 };
 
@@ -1718,3 +1719,19 @@ static int init_post_handshake_auth(SSL *s, unsigned int context)
 
     return 1;
 }
+
+/*
+ * If clients offer "pre_shared_key" without a "psk_key_exchange_modes"
+ * extension, servers MUST abort the handshake.
+ */
+static int final_psk(SSL *s, unsigned int context, int sent)
+{
+    if (s->server && sent && s->clienthello != NULL
+            && !s->clienthello->pre_proc_exts[TLSEXT_IDX_psk_kex_modes].present) {
+        SSLfatal(s, TLS13_AD_MISSING_EXTENSION, SSL_F_FINAL_PSK,
+                 SSL_R_MISSING_PSK_KEX_MODES_EXTENSION);
+        return 0;
+    }
+
+    return 1;
+}
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 3c7395c0eb..90e8bce19b 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1714,6 +1714,13 @@ EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt,
         }
         return EXT_RETURN_NOT_SENT;
     }
+    if (s->hit && (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) == 0) {
+        /*
+         * PSK ('hit') and explicitly not doing DHE (if the client sent the
+         * DHE option we always take it); don't send key share.
+         */
+        return EXT_RETURN_NOT_SENT;
+    }
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share)
             || !WPACKET_start_sub_packet_u16(pkt)
diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t
index 98989b4703..e8ab25f190 100644
--- a/test/recipes/70-test_tls13kexmodes.t
+++ b/test/recipes/70-test_tls13kexmodes.t
@@ -195,17 +195,14 @@ $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 11;
 ok(TLSProxy::Message->success(), "Initial connection");
 
-#Test 2: Attempt a resume with no kex modes extension. Should not resume
+#Test 2: Attempt a resume with no kex modes extension. Should fail (server
+#        MUST abort handshake with pre_shared key and no psk_kex_modes)
 $proxy->clear();
 $proxy->clientflags("-sess_in ".$session);
 my $testtype = DELETE_EXTENSION;
 $proxy->filter(\&modify_kex_modes_filter);
 $proxy->start();
-checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
-               checkhandshake::DEFAULT_EXTENSIONS
-               | checkhandshake::KEY_SHARE_SRV_EXTENSION
-               | checkhandshake::PSK_CLI_EXTENSION,
-               "Resume with no kex modes");
+ok(TLSProxy::Message->fail(), "Resume with no kex modes");
 
 #Test 3: Attempt a resume with empty kex modes extension. Should fail (empty
 #        extension is invalid)
@@ -243,6 +240,7 @@ checkhandshake($proxy, checkhandshake::RESUME_HANDSHAKE,
                "Resume with non-dhe kex mode");
 
 #Test 6: Attempt a resume with only unrecognised kex modes. Should not resume
+#        but rather fall back to full handshake
 $proxy->clear();
 $proxy->clientflags("-sess_in ".$session);
 $testtype = UNKNOWN_KEX_MODES;
@@ -252,7 +250,7 @@ checkhandshake($proxy, checkhandshake::DEFAULT_HANDSHAKE,
                | checkhandshake::PSK_KEX_MODES_EXTENSION
                | checkhandshake::KEY_SHARE_SRV_EXTENSION
                | checkhandshake::PSK_CLI_EXTENSION,
-               "Resume with empty kex modes");
+               "Resume with unrecognized kex mode");
 
 #Test 7: Attempt a resume with both non-dhe and dhe kex mode. Should resume with
 #        a key_share


More information about the openssl-commits mailing list