[openssl-commits] [openssl] master update

Matt Caswell matt at openssl.org
Fri Mar 9 11:33:02 UTC 2018


The branch master has been updated
       via  532f95783e2bff4d7f4e8086297ed8e0b25561f7 (commit)
       via  4a192c77b7bf794a9283a8e7fb4f7bee0d7bff56 (commit)
       via  f3d40db1b947f546541b815a0363de3120c16506 (commit)
       via  e73c6eaeff82615d20845692c5c72ba9dfa895f5 (commit)
      from  a7fb4fa1708c65c0932133dca64a53d0237312e3 (commit)


- Log -----------------------------------------------------------------
commit 532f95783e2bff4d7f4e8086297ed8e0b25561f7
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Mar 8 13:45:22 2018 +0000

    Test the new PSK behaviour
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5554)

commit 4a192c77b7bf794a9283a8e7fb4f7bee0d7bff56
Author: Matt Caswell <matt at openssl.org>
Date:   Thu Mar 8 08:20:23 2018 +0000

    Update documentation for the new PSK behaviour
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5554)

commit f3d40db1b947f546541b815a0363de3120c16506
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Mar 6 16:41:51 2018 +0000

    Fallback on old style PSK callbacks if the new style ones aren't present
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5554)

commit e73c6eaeff82615d20845692c5c72ba9dfa895f5
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Mar 6 14:12:10 2018 +0000

    Tolerate TLSv1.3 PSKs that are a different size to the hash size
    
    We also default to SHA256 as per the spec if we do not have an explicit
    digest defined.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5554)

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

Summary of changes:
 apps/s_client.c                              |  16 +--
 apps/s_server.c                              |  10 +-
 doc/man3/SSL_CTX_set_psk_client_callback.pod |  66 +++++-----
 doc/man3/SSL_CTX_use_psk_identity_hint.pod   |  70 ++++++-----
 ssl/statem/extensions.c                      |  12 +-
 ssl/statem/extensions_clnt.c                 |  53 +++++++++
 ssl/statem/extensions_srvr.c                 |  56 ++++++++-
 test/sslapitest.c                            | 172 ++++++++++++++++++++++++---
 8 files changed, 351 insertions(+), 104 deletions(-)

diff --git a/apps/s_client.c b/apps/s_client.c
index a319d21..1ed853d 100644
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -197,19 +197,13 @@ static int psk_use_session_cb(SSL *s, const EVP_MD *md,
             return 0;
         }
 
-        if (key_len == EVP_MD_size(EVP_sha256()))
-            cipher = SSL_CIPHER_find(s, tls13_aes128gcmsha256_id);
-        else if (key_len == EVP_MD_size(EVP_sha384()))
-            cipher = SSL_CIPHER_find(s, tls13_aes256gcmsha384_id);
-
+        /* We default to SHA-256 */
+        cipher = SSL_CIPHER_find(s, tls13_aes128gcmsha256_id);
         if (cipher == NULL) {
-            /* Doesn't look like a suitable TLSv1.3 key. Ignore it */
-            OPENSSL_free(key);
-            *id = NULL;
-            *idlen = 0;
-            *sess = NULL;
-            return 1;
+            BIO_printf(bio_err, "Error finding suitable ciphersuite\n");
+            return 0;
         }
+
         usesess = SSL_SESSION_new();
         if (usesess == NULL
                 || !SSL_SESSION_set1_master_key(usesess, key, key_len)
diff --git a/apps/s_server.c b/apps/s_server.c
index ff9ee5a..bc1d1e5 100644
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -208,14 +208,10 @@ static int psk_find_session_cb(SSL *ssl, const unsigned char *identity,
         return 0;
     }
 
-    if (key_len == EVP_MD_size(EVP_sha256()))
-        cipher = SSL_CIPHER_find(ssl, tls13_aes128gcmsha256_id);
-    else if (key_len == EVP_MD_size(EVP_sha384()))
-        cipher = SSL_CIPHER_find(ssl, tls13_aes256gcmsha384_id);
-
+    /* We default to SHA256 */
+    cipher = SSL_CIPHER_find(ssl, tls13_aes128gcmsha256_id);
     if (cipher == NULL) {
-        /* Doesn't look like a suitable TLSv1.3 key. Ignore it */
-        OPENSSL_free(key);
+        BIO_printf(bio_err, "Error finding suitable ciphersuite\n");
         return 0;
     }
 
diff --git a/doc/man3/SSL_CTX_set_psk_client_callback.pod b/doc/man3/SSL_CTX_set_psk_client_callback.pod
index e771072..5350d79 100644
--- a/doc/man3/SSL_CTX_set_psk_client_callback.pod
+++ b/doc/man3/SSL_CTX_set_psk_client_callback.pod
@@ -14,48 +14,33 @@ SSL_set_psk_use_session_callback
 
  #include <openssl/ssl.h>
 
- typedef unsigned int (*SSL_psk_client_cb_func)(SSL *ssl,
-                                                const char *hint,
-                                                char *identity,
-                                                unsigned int max_identity_len,
-                                                unsigned char *psk,
-                                                unsigned int max_psk_len);
  typedef int (*SSL_psk_use_session_cb_func)(SSL *ssl, const EVP_MD *md,
                                             const unsigned char **id,
                                             size_t *idlen,
                                             SSL_SESSION **sess);
 
- void SSL_CTX_set_psk_client_callback(SSL_CTX *ctx, SSL_psk_client_cb_func cb);
- void SSL_set_psk_client_callback(SSL *ssl, SSL_psk_client_cb_func cb);
 
  void SSL_CTX_set_psk_use_session_callback(SSL_CTX *ctx,
                                            SSL_psk_use_session_cb_func cb);
  void SSL_set_psk_use_session_callback(SSL *s, SSL_psk_use_session_cb_func cb);
 
-=head1 DESCRIPTION
 
-TLSv1.3 Pre-Shared Keys (PSKs) and PSKs for TLSv1.2 and below are not
-compatible.
+ typedef unsigned int (*SSL_psk_client_cb_func)(SSL *ssl,
+                                                const char *hint,
+                                                char *identity,
+                                                unsigned int max_identity_len,
+                                                unsigned char *psk,
+                                                unsigned int max_psk_len);
 
-A client application wishing to use PSK ciphersuites for TLSv1.2 and below must
-provide a callback function. This function will be called when the client is
-sending the ClientKeyExchange message to the server.
+ void SSL_CTX_set_psk_client_callback(SSL_CTX *ctx, SSL_psk_client_cb_func cb);
+ void SSL_set_psk_client_callback(SSL *ssl, SSL_psk_client_cb_func cb);
 
-The purpose of the callback function is to select the PSK identity and
-the pre-shared key to use during the connection setup phase.
 
-The callback is set using functions SSL_CTX_set_psk_client_callback()
-or SSL_set_psk_client_callback(). The callback function is given the
-connection in parameter B<ssl>, a B<NULL>-terminated PSK identity hint
-sent by the server in parameter B<hint>, a buffer B<identity> of
-length B<max_identity_len> bytes where the resulting
-B<NULL>-terminated identity is to be stored, and a buffer B<psk> of
-length B<max_psk_len> bytes where the resulting pre-shared key is to
-be stored.
+=head1 DESCRIPTION
 
-A client application wishing to use TLSv1.3 PSKs must set a different callback
-using either SSL_CTX_set_psk_use_session_callback() or
-SSL_set_psk_use_session_callback() as appropriate.
+A client application wishing to use TLSv1.3 PSKs should use either
+SSL_CTX_set_psk_use_session_callback() or SSL_set_psk_use_session_callback() as
+appropriate. These functions cannot be used for TLSv1.2 and below PSKs.
 
 The callback function is given a pointer to the SSL connection in B<ssl>.
 
@@ -113,6 +98,33 @@ case no PSK will be sent to the server but the handshake will continue. To do
 this the callback should return successfully and ensure that B<*sess> is
 NULL. The contents of B<*id> and B<*idlen> will be ignored.
 
+A client application wishing to use PSK ciphersuites for TLSv1.2 and below must
+provide a different callback function. This function will be called when the
+client is sending the ClientKeyExchange message to the server.
+
+The purpose of the callback function is to select the PSK identity and
+the pre-shared key to use during the connection setup phase.
+
+The callback is set using functions SSL_CTX_set_psk_client_callback()
+or SSL_set_psk_client_callback(). The callback function is given the
+connection in parameter B<ssl>, a B<NULL>-terminated PSK identity hint
+sent by the server in parameter B<hint>, a buffer B<identity> of
+length B<max_identity_len> bytes where the resulting
+B<NUL>-terminated identity is to be stored, and a buffer B<psk> of
+length B<max_psk_len> bytes where the resulting pre-shared key is to
+be stored.
+
+The callback for use in TLSv1.2 will also work in TLSv1.3 although it is
+recommended to use SSL_CTX_set_psk_use_session_callback()
+or SSL_set_psk_use_session_callback() for this purpose instead. If TLSv1.3 has
+been negotiated then OpenSSL will first check to see if a callback has been set
+via SSL_CTX_set_psk_use_session_callback() or SSL_set_psk_use_session_callback()
+and it will use that in preference. If no such callback is present then it will
+check to see if a callback has been set via SSL_CTX_set_psk_client_callback() or
+SSL_set_psk_client_callback() and use that. In this case the B<hint> value will
+always be NULL and the handshake digest will default to SHA-256 for any returned
+PSK.
+
 =head1 NOTES
 
 Note that parameter B<hint> given to the callback may be B<NULL>.
diff --git a/doc/man3/SSL_CTX_use_psk_identity_hint.pod b/doc/man3/SSL_CTX_use_psk_identity_hint.pod
index d41c0cc..2f4773d 100644
--- a/doc/man3/SSL_CTX_use_psk_identity_hint.pod
+++ b/doc/man3/SSL_CTX_use_psk_identity_hint.pod
@@ -16,30 +16,44 @@ SSL_set_psk_find_session_callback
 
  #include <openssl/ssl.h>
 
- typedef unsigned int (*SSL_psk_server_cb_func)(SSL *ssl,
-                                                const char *identity,
-                                                unsigned char *psk,
-                                                unsigned int max_psk_len);
-
  typedef int (*SSL_psk_find_session_cb_func)(SSL *ssl,
                                              const unsigned char *identity,
                                              size_t identity_len,
                                              SSL_SESSION **sess);
 
+
+ void SSL_CTX_set_psk_find_session_callback(SSL_CTX *ctx,
+                                            SSL_psk_find_session_cb_func cb);
+ void SSL_set_psk_find_session_callback(SSL *s, SSL_psk_find_session_cb_func cb);
+
+ typedef unsigned int (*SSL_psk_server_cb_func)(SSL *ssl,
+                                                const char *identity,
+                                                unsigned char *psk,
+                                                unsigned int max_psk_len);
+
  int SSL_CTX_use_psk_identity_hint(SSL_CTX *ctx, const char *hint);
  int SSL_use_psk_identity_hint(SSL *ssl, const char *hint);
 
  void SSL_CTX_set_psk_server_callback(SSL_CTX *ctx, SSL_psk_server_cb_func cb);
  void SSL_set_psk_server_callback(SSL *ssl, SSL_psk_server_cb_func cb);
 
- void SSL_CTX_set_psk_find_session_callback(SSL_CTX *ctx,
-                                            SSL_psk_find_session_cb_func cb);
- void SSL_set_psk_find_session_callback(SSL *s, SSL_psk_find_session_cb_func cb);
-
 =head1 DESCRIPTION
 
-TLSv1.3 Pre-Shared Keys (PSKs) and PSKs for TLSv1.2 and below are not
-compatible.
+A client application wishing to use TLSv1.3 PSKs should set a callback
+using either SSL_CTX_set_psk_use_session_callback() or
+SSL_set_psk_use_session_callback() as appropriate.
+
+The callback function is given a pointer to the SSL connection in B<ssl> and
+an identity in B<identity> of length B<identity_len>. The callback function
+should identify an SSL_SESSION object that provides the PSK details and store it
+in B<*sess>. The SSL_SESSION object should, as a minimum, set the master key,
+the ciphersuite and the protocol version. See
+L<SSL_CTX_set_psk_use_session_callback(3)> for details.
+
+It is also possible for the callback to succeed but not supply a PSK. In this
+case no PSK will be used but the handshake will continue. To do this the
+callback should return successfully and ensure that B<*sess> is
+NULL.
 
 Identity hints are not relevant for TLSv1.3. A server application wishing to use
 PSK ciphersuites for TLSv1.2 and below may call SSL_CTX_use_psk_identity_hint()
@@ -51,31 +65,25 @@ B<NULL> the current hint from B<ctx> or B<ssl> is deleted.
 In the case where PSK identity hint is B<NULL>, the server does not send the
 ServerKeyExchange message to the client.
 
-A server application for TLSv1.2 and below must provide a callback function
-which is called when the server receives the ClientKeyExchange message from the
-client. The purpose of the callback function is to validate the
-received PSK identity and to fetch the pre-shared key used during the
-connection setup phase. The callback is set using the functions
+A server application wishing to use PSKs for TLSv1.2 and below must provide a
+callback function which is called when the server receives the
+ClientKeyExchange message from the client. The purpose of the callback function
+is to validate the received PSK identity and to fetch the pre-shared key used
+during the connection setup phase. The callback is set using the functions
 SSL_CTX_set_psk_server_callback() or SSL_set_psk_server_callback(). The callback
 function is given the connection in parameter B<ssl>, B<NUL>-terminated PSK
 identity sent by the client in parameter B<identity>, and a buffer B<psk> of
 length B<max_psk_len> bytes where the pre-shared key is to be stored.
 
-A client application wishing to use TLSv1.3 PSKs must set a different callback
-using either SSL_CTX_set_psk_use_session_callback() or
-SSL_set_psk_use_session_callback() as appropriate.
-
-The callback function is given a pointer to the SSL connection in B<ssl> and
-an identity in B<identity> of length B<identity_len>. The callback function
-should identify an SSL_SESSION object that provides the PSK details and store it
-in B<*sess>. The SSL_SESSION object should, as a minimum, set the master key,
-the ciphersuite and the protocol version. See
-L<SSL_CTX_set_psk_use_session_callback(3)> for details.
-
-It is also possible for the callback to succeed but not supply a PSK. In this
-case no PSK will be used but the handshake will continue. To do this the
-callback should return successfully and ensure that B<*sess> is
-NULL.
+The callback for use in TLSv1.2 will also work in TLSv1.3 although it is
+recommended to use SSL_CTX_set_psk_find_session_callback()
+or SSL_set_psk_find_session_callback() for this purpose instead. If TLSv1.3 has
+been negotiated then OpenSSL will first check to see if a callback has been set
+via SSL_CTX_set_psk_find_session_callback() or SSL_set_psk_find_session_callback()
+and it will use that in preference. If no such callback is present then it will
+check to see if a callback has been set via SSL_CTX_set_psk_server_callback() or
+SSL_set_psk_server_callback() and use that. In this case the handshake digest
+will default to SHA-256 for any returned PSK.
 
 =head1 NOTES
 
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 6e3f8d1..8a8e524 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1426,7 +1426,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
     const char external_label[] = "ext binder";
     const char nonce_label[] = "resumption";
     const char *label;
-    size_t bindersize, labelsize, hashsize = EVP_MD_size(md);
+    size_t bindersize, labelsize, psklen, hashsize = EVP_MD_size(md);
     int ret = -1;
     int usepskfored = 0;
 
@@ -1444,16 +1444,12 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
         labelsize = sizeof(resumption_label) - 1;
     }
 
-    if (sess->master_key_length != hashsize) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PSK_DO_BINDER,
-                 SSL_R_BAD_PSK);
-        goto err;
-    }
-
     if (external) {
         psk = sess->master_key;
+        psklen = sess->master_key_length;
     } else {
         psk = tmppsk;
+        psklen = hashsize;
         if (!tls13_hkdf_expand(s, md, sess->master_key,
                                (const unsigned char *)nonce_label,
                                sizeof(nonce_label) - 1, sess->ext.tick_nonce,
@@ -1475,7 +1471,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
         early_secret = (unsigned char *)s->early_secret;
     else
         early_secret = (unsigned char *)sess->early_secret;
-    if (!tls13_generate_secret(s, md, NULL, psk, hashsize, early_secret)) {
+    if (!tls13_generate_secret(s, md, NULL, psk, psklen, early_secret)) {
         /* SSLfatal() already called */
         goto err;
     }
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index fa6c65b..9bf2d1c 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -744,6 +744,7 @@ EXT_RETURN tls_construct_ctos_early_data(SSL *s, WPACKET *pkt,
                                          unsigned int context, X509 *x,
                                          size_t chainidx)
 {
+    char identity[PSK_MAX_IDENTITY_LEN + 1];
     const unsigned char *id = NULL;
     size_t idlen = 0;
     SSL_SESSION *psksess = NULL;
@@ -763,6 +764,58 @@ EXT_RETURN tls_construct_ctos_early_data(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
+    if (psksess == NULL && s->psk_client_callback != NULL) {
+        unsigned char psk[PSK_MAX_PSK_LEN];
+        size_t psklen = 0;
+
+        memset(identity, 0, sizeof(identity));
+        psklen = s->psk_client_callback(s, NULL, identity, sizeof(identity) - 1,
+                                        psk, sizeof(psk));
+
+        if (psklen > PSK_MAX_PSK_LEN) {
+            SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE,
+                     SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA, ERR_R_INTERNAL_ERROR);
+            return EXT_RETURN_FAIL;
+        } else if (psklen > 0) {
+            const unsigned char tls13_aes128gcmsha256_id[] = { 0x13, 0x01 };
+            const SSL_CIPHER *cipher;
+
+            idlen = strlen(identity);
+            if (idlen > PSK_MAX_IDENTITY_LEN) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA,
+                         ERR_R_INTERNAL_ERROR);
+                return EXT_RETURN_FAIL;
+            }
+            id = (unsigned char *)identity;
+
+            /*
+             * We found a PSK using an old style callback. We don't know
+             * the digest so we default to SHA256 as per the TLSv1.3 spec
+             */
+            cipher = SSL_CIPHER_find(s, tls13_aes128gcmsha256_id);
+            if (cipher == NULL) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA,
+                         ERR_R_INTERNAL_ERROR);
+                return EXT_RETURN_FAIL;
+            }
+
+            psksess = SSL_SESSION_new();
+            if (psksess == NULL
+                    || !SSL_SESSION_set1_master_key(psksess, psk, psklen)
+                    || !SSL_SESSION_set_cipher(psksess, cipher)
+                    || !SSL_SESSION_set_protocol_version(psksess, TLS1_3_VERSION)) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                         SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA,
+                         ERR_R_INTERNAL_ERROR);
+                OPENSSL_cleanse(psk, psklen);
+                return EXT_RETURN_FAIL;
+            }
+            OPENSSL_cleanse(psk, psklen);
+        }
+    }
+
     SSL_SESSION_free(s->psksession);
     s->psksession = psksess;
     if (psksess != NULL) {
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index d1be32c..bcabb85 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1028,6 +1028,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     for (id = 0; PACKET_remaining(&identities) != 0; id++) {
         PACKET identity;
         unsigned long ticket_agel;
+        size_t idlen;
 
         if (!PACKET_get_length_prefixed_2(&identities, &identity)
                 || !PACKET_get_net_4(&identities, &ticket_agel)) {
@@ -1036,15 +1037,66 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
             return 0;
         }
 
+        idlen = PACKET_remaining(&identity);
         if (s->psk_find_session_cb != NULL
-                && !s->psk_find_session_cb(s, PACKET_data(&identity),
-                                           PACKET_remaining(&identity),
+                && !s->psk_find_session_cb(s, PACKET_data(&identity), idlen,
                                            &sess)) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_PARSE_CTOS_PSK, SSL_R_BAD_EXTENSION);
             return 0;
         }
 
+        if(sess == NULL
+                && s->psk_server_callback != NULL
+                && idlen <= PSK_MAX_IDENTITY_LEN) {
+            char *pskid = NULL;
+            unsigned char pskdata[PSK_MAX_PSK_LEN];
+            unsigned int pskdatalen;
+
+            if (!PACKET_strndup(&identity, &pskid)) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_PSK,
+                         ERR_R_INTERNAL_ERROR);
+                return 0;
+            }
+            pskdatalen = s->psk_server_callback(s, pskid, pskdata,
+                                                sizeof(pskdata));
+            OPENSSL_free(pskid);
+            if (pskdatalen > PSK_MAX_PSK_LEN) {
+                SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_PSK,
+                         ERR_R_INTERNAL_ERROR);
+                return 0;
+            } else if (pskdatalen > 0) {
+                const SSL_CIPHER *cipher;
+                const unsigned char tls13_aes128gcmsha256_id[] = { 0x13, 0x01 };
+
+                /*
+                 * We found a PSK using an old style callback. We don't know
+                 * the digest so we default to SHA256 as per the TLSv1.3 spec
+                 */
+                cipher = SSL_CIPHER_find(s, tls13_aes128gcmsha256_id);
+                if (cipher == NULL) {
+                    OPENSSL_cleanse(pskdata, pskdatalen);
+                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_PSK,
+                             ERR_R_INTERNAL_ERROR);
+                    return 0;
+                }
+
+                sess = SSL_SESSION_new();
+                if (sess == NULL
+                        || !SSL_SESSION_set1_master_key(sess, pskdata,
+                                                        pskdatalen)
+                        || !SSL_SESSION_set_cipher(sess, cipher)
+                        || !SSL_SESSION_set_protocol_version(sess,
+                                                             TLS1_3_VERSION)) {
+                    OPENSSL_cleanse(pskdata, pskdatalen);
+                    SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PARSE_CTOS_PSK,
+                             ERR_R_INTERNAL_ERROR);
+                    goto err;
+                }
+                OPENSSL_cleanse(pskdata, pskdatalen);
+            }
+        }
+
         if (sess != NULL) {
             /* We found a PSK */
             SSL_SESSION *sesstmp = ssl_session_dup(sess, 0);
diff --git a/test/sslapitest.c b/test/sslapitest.c
index ce24fad..c183ca8 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -1415,6 +1415,8 @@ static const char *srvid;
 
 static int use_session_cb_cnt = 0;
 static int find_session_cb_cnt = 0;
+static int psk_client_cb_cnt = 0;
+static int psk_server_cb_cnt = 0;
 
 static int use_session_cb(SSL *ssl, const EVP_MD *md, const unsigned char **id,
                           size_t *idlen, SSL_SESSION **sess)
@@ -1447,6 +1449,34 @@ static int use_session_cb(SSL *ssl, const EVP_MD *md, const unsigned char **id,
     return 1;
 }
 
+static unsigned int psk_client_cb(SSL *ssl, const char *hint, char *id,
+                                  unsigned int max_id_len,
+                                  unsigned char *psk,
+                                  unsigned int max_psk_len)
+{
+    unsigned int psklen = 0;
+
+    psk_client_cb_cnt++;
+
+    if (strlen(pskid) + 1 > max_id_len)
+        return 0;
+
+    /* We should only ever be called a maximum of twice per connection */
+    if (psk_client_cb_cnt > 2)
+        return 0;
+
+    if (clientpsk == NULL)
+        return 0;
+
+    /* We'll reuse the PSK we set up for TLSv1.3 */
+    if (SSL_SESSION_get_master_key(clientpsk, NULL, 0) > max_psk_len)
+        return 0;
+    psklen = SSL_SESSION_get_master_key(clientpsk, psk, max_psk_len);
+    strncpy(id, pskid, max_id_len);
+
+    return psklen;
+}
+
 static int find_session_cb(SSL *ssl, const unsigned char *identity,
                            size_t identity_len, SSL_SESSION **sess)
 {
@@ -1473,6 +1503,33 @@ static int find_session_cb(SSL *ssl, const unsigned char *identity,
     return 1;
 }
 
+static unsigned int psk_server_cb(SSL *ssl, const char *identity,
+                                  unsigned char *psk, unsigned int max_psk_len)
+{
+    unsigned int psklen = 0;
+
+    psk_server_cb_cnt++;
+
+    /* We should only ever be called a maximum of twice per connection */
+    if (find_session_cb_cnt > 2)
+        return 0;
+
+    if (serverpsk == NULL)
+        return 0;
+
+    /* Identity should match that set by the client */
+    if (strcmp(srvid, identity) != 0) {
+        return 0;
+    }
+
+    /* We'll reuse the PSK we set up for TLSv1.3 */
+    if (SSL_SESSION_get_master_key(serverpsk, NULL, 0) > max_psk_len)
+        return 0;
+    psklen = SSL_SESSION_get_master_key(serverpsk, psk, max_psk_len);
+
+    return psklen;
+}
+
 #define MSG1    "Hello"
 #define MSG2    "World."
 #define MSG3    "This"
@@ -1482,6 +1539,7 @@ static int find_session_cb(SSL *ssl, const unsigned char *identity,
 #define MSG7    "message."
 
 #define TLS13_AES_256_GCM_SHA384_BYTES  ((const unsigned char *)"\x13\x02")
+#define TLS13_AES_128_GCM_SHA256_BYTES  ((const unsigned char *)"\x13\x01")
 
 /*
  * Helper method to setup objects for early data test. Caller frees objects on
@@ -2440,7 +2498,7 @@ static int test_ciphersuite_change(void)
     return testresult;
 }
 
-static int test_tls13_psk(void)
+static int test_tls13_psk(int idx)
 {
     SSL_CTX *sctx = NULL, *cctx = NULL;
     SSL *serverssl = NULL, *clientssl = NULL;
@@ -2458,11 +2516,31 @@ static int test_tls13_psk(void)
                                        &cctx, cert, privkey)))
         goto end;
 
-    SSL_CTX_set_psk_use_session_callback(cctx, use_session_cb);
-    SSL_CTX_set_psk_find_session_callback(sctx, find_session_cb);
+    /*
+     * 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_cipher_list(cctx, "TLS13-AES-128-GCM-SHA256")))
+        goto end;
+
+    /*
+     * Test 0: New style callbacks only
+     * Test 1: New and old style callbacks (only the new ones should be used)
+     * Test 2: Old style callbacks only
+     */
+    if (idx == 0 || idx == 1) {
+        SSL_CTX_set_psk_use_session_callback(cctx, use_session_cb);
+        SSL_CTX_set_psk_find_session_callback(sctx, find_session_cb);
+    }
+    if (idx == 1 || idx == 2) {
+        SSL_CTX_set_psk_client_callback(cctx, psk_client_cb);
+        SSL_CTX_set_psk_server_callback(sctx, psk_server_cb);
+    }
     srvid = pskid;
     use_session_cb_cnt = 0;
     find_session_cb_cnt = 0;
+    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,
@@ -2470,21 +2548,37 @@ static int test_tls13_psk(void)
             || !TEST_true(create_ssl_connection(serverssl, clientssl,
                                                 SSL_ERROR_NONE))
             || !TEST_false(SSL_session_reused(clientssl))
-            || !TEST_false(SSL_session_reused(serverssl))
-            || !TEST_true(use_session_cb_cnt == 1)
-            || !TEST_true(find_session_cb_cnt == 0))
+            || !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))
+            goto end;
+    }
+
     shutdown_ssl_connection(serverssl, clientssl);
     serverssl = clientssl = NULL;
-    use_session_cb_cnt = 0;
+    use_session_cb_cnt = psk_client_cb_cnt = 0;
 
     if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
                                              NULL, NULL)))
         goto end;
 
     /* Create the PSK */
-    cipher = SSL_CIPHER_find(clientssl, TLS13_AES_256_GCM_SHA384_BYTES);
+    cipher = SSL_CIPHER_find(clientssl, TLS13_AES_128_GCM_SHA256_BYTES);
     clientpsk = SSL_SESSION_new();
     if (!TEST_ptr(clientpsk)
             || !TEST_ptr(cipher)
@@ -2500,14 +2594,27 @@ static int test_tls13_psk(void)
     /* Check we can create a connection and the PSK is used */
     if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))
             || !TEST_true(SSL_session_reused(clientssl))
-            || !TEST_true(SSL_session_reused(serverssl))
-            || !TEST_true(use_session_cb_cnt == 1)
-            || !TEST_true(find_session_cb_cnt == 1))
+            || !TEST_true(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)
+                || !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 == 1))
+            goto end;
+    }
+
     shutdown_ssl_connection(serverssl, clientssl);
     serverssl = clientssl = NULL;
     use_session_cb_cnt = find_session_cb_cnt = 0;
+    psk_client_cb_cnt = psk_server_cb_cnt = 0;
 
     if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
                                              NULL, NULL)))
@@ -2523,14 +2630,27 @@ static int test_tls13_psk(void)
      */
     if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE))
             || !TEST_true(SSL_session_reused(clientssl))
-            || !TEST_true(SSL_session_reused(serverssl))
-            || !TEST_true(use_session_cb_cnt == 2)
-            || !TEST_true(find_session_cb_cnt == 2))
+            || !TEST_true(SSL_session_reused(serverssl)))
         goto end;
 
+    if (idx == 0 || idx == 1) {
+        if (!TEST_true(use_session_cb_cnt == 2)
+                || !TEST_true(find_session_cb_cnt == 2)
+                || !TEST_true(psk_client_cb_cnt == 0)
+                || !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 == 2)
+                || !TEST_true(psk_server_cb_cnt == 2))
+            goto end;
+    }
+
     shutdown_ssl_connection(serverssl, clientssl);
     serverssl = clientssl = NULL;
     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
@@ -2542,11 +2662,27 @@ static int test_tls13_psk(void)
             || !TEST_true(create_ssl_connection(serverssl, clientssl,
                                                 SSL_ERROR_NONE))
             || !TEST_false(SSL_session_reused(clientssl))
-            || !TEST_false(SSL_session_reused(serverssl))
-            || !TEST_true(use_session_cb_cnt == 1)
-            || !TEST_true(find_session_cb_cnt == 1))
+            || !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))
+            goto end;
+    }
+
     shutdown_ssl_connection(serverssl, clientssl);
     serverssl = clientssl = NULL;
     testresult = 1;
@@ -3506,7 +3642,7 @@ int setup_tests(void)
 #endif
 #ifndef OPENSSL_NO_TLS1_3
     ADD_TEST(test_ciphersuite_change);
-    ADD_TEST(test_tls13_psk);
+    ADD_ALL_TESTS(test_tls13_psk, 3);
     ADD_ALL_TESTS(test_custom_exts, 5);
     ADD_TEST(test_stateless);
     ADD_TEST(test_pha_key_update);


More information about the openssl-commits mailing list