Add support for client side parsing of the PSK extension

Requires a refactor of the ServerHello parsing, so that we parse first and
then subsequently process. This is because the resumption information is
held in the extensions block which is parsed last - but we need to know that
information earlier.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index e528689c70e168670a70c8c5971c5dfd99b8b6cd..52be4064fa2b99cc1e2aca33dce4c9103fced158 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2326,6 +2326,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_PARSE_CTOS_RENEGOTIATE                 464
 # define SSL_F_TLS_PARSE_CTOS_USE_SRTP                    465
 # define SSL_F_TLS_PARSE_STOC_KEY_SHARE                   445
 # define SSL_F_TLS_PARSE_CTOS_RENEGOTIATE                 464
 # define SSL_F_TLS_PARSE_CTOS_USE_SRTP                    465
 # define SSL_F_TLS_PARSE_STOC_KEY_SHARE                   445

+# define SSL_F_TLS_PARSE_STOC_PSK                         502

 # define SSL_F_TLS_PARSE_STOC_RENEGOTIATE                 448
 # define SSL_F_TLS_PARSE_STOC_USE_SRTP                    446
 # define SSL_F_TLS_POST_PROCESS_CLIENT_HELLO              378
 # define SSL_F_TLS_PARSE_STOC_RENEGOTIATE                 448
 # define SSL_F_TLS_PARSE_STOC_USE_SRTP                    446
 # define SSL_F_TLS_POST_PROCESS_CLIENT_HELLO              378

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 2186f799fc882c9c7d255688f85462e7af6b7f5d..9b02c58737782229f372ebe11685a37930cb5243 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -373,6 +373,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
      "tls_parse_ctos_renegotiate"},
     {ERR_FUNC(SSL_F_TLS_PARSE_CTOS_USE_SRTP), "tls_parse_ctos_use_srtp"},
     {ERR_FUNC(SSL_F_TLS_PARSE_STOC_KEY_SHARE), "tls_parse_stoc_key_share"},
      "tls_parse_ctos_renegotiate"},
     {ERR_FUNC(SSL_F_TLS_PARSE_CTOS_USE_SRTP), "tls_parse_ctos_use_srtp"},
     {ERR_FUNC(SSL_F_TLS_PARSE_STOC_KEY_SHARE), "tls_parse_stoc_key_share"},

+    {ERR_FUNC(SSL_F_TLS_PARSE_STOC_PSK), "tls_parse_stoc_psk"},

     {ERR_FUNC(SSL_F_TLS_PARSE_STOC_RENEGOTIATE),
      "tls_parse_stoc_renegotiate"},
     {ERR_FUNC(SSL_F_TLS_PARSE_STOC_USE_SRTP), "tls_parse_stoc_use_srtp"},
     {ERR_FUNC(SSL_F_TLS_PARSE_STOC_RENEGOTIATE),
      "tls_parse_stoc_renegotiate"},
     {ERR_FUNC(SSL_F_TLS_PARSE_STOC_USE_SRTP), "tls_parse_stoc_use_srtp"},


@@ -454,6 +455,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = {
     {ERR_REASON(SSL_R_BAD_PACKET_LENGTH), "bad packet length"},
     {ERR_REASON(SSL_R_BAD_PROTOCOL_VERSION_NUMBER),
      "bad protocol version number"},
     {ERR_REASON(SSL_R_BAD_PACKET_LENGTH), "bad packet length"},
     {ERR_REASON(SSL_R_BAD_PROTOCOL_VERSION_NUMBER),
      "bad protocol version number"},

+    {ERR_REASON(SSL_R_BAD_PSK_IDENTITY), "bad psk identity"},

     {ERR_REASON(SSL_R_BAD_RECORD_TYPE), "bad record type"},
     {ERR_REASON(SSL_R_BAD_RSA_ENCRYPT), "bad rsa encrypt"},
     {ERR_REASON(SSL_R_BAD_SIGNATURE), "bad signature"},
     {ERR_REASON(SSL_R_BAD_RECORD_TYPE), "bad record type"},
     {ERR_REASON(SSL_R_BAD_RSA_ENCRYPT), "bad rsa encrypt"},
     {ERR_REASON(SSL_R_BAD_SIGNATURE), "bad signature"},

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 5837720eb718da8c5860bc47c78142e46e7a78b5..4c66b3362fcc49b8bda653377eb77204f12d29f8 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -277,7 +277,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         TLSEXT_TYPE_psk,
         EXT_CLIENT_HELLO | EXT_TLS1_3_SERVER_HELLO | EXT_TLS_IMPLEMENTATION_ONLY
         | EXT_TLS1_3_ONLY,
         TLSEXT_TYPE_psk,
         EXT_CLIENT_HELLO | EXT_TLS1_3_SERVER_HELLO | EXT_TLS_IMPLEMENTATION_ONLY
         | EXT_TLS1_3_ONLY,

-        NULL, NULL, NULL, NULL, tls_construct_ctos_psk, NULL
+        NULL, NULL, tls_parse_stoc_psk, NULL, tls_construct_ctos_psk, NULL

     }
 };
 
     }
 };
 

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 366462ee8570c64c8b3608637ac2b9857ac0e850..04dbea11fd65589a8d501abb76bed67b08fa0479 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -1279,3 +1279,26 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, X509 *x, size_t chainidx,
 
     return 1;
 }
 
     return 1;
 }

+
+int tls_parse_stoc_psk(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al)
+{
+#ifndef OPENSSL_NO_TLS1_3
+    unsigned int identity;
+
+    if (!PACKET_get_net_2(pkt, &identity) || PACKET_remaining(pkt) != 0) {
+        *al = SSL_AD_HANDSHAKE_FAILURE;
+        SSLerr(SSL_F_TLS_PARSE_STOC_PSK, SSL_R_LENGTH_MISMATCH);
+        return 0;
+    }
+
+    if (s->session->ext.tick_identity != (int)identity) {
+        *al = SSL_AD_HANDSHAKE_FAILURE;
+        SSLerr(SSL_F_TLS_PARSE_STOC_PSK, SSL_R_BAD_PSK_IDENTITY);
+        return 0;
+    }
+
+    s->hit = 1;
+#endif
+
+    return 1;
+}

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 3bcd5902c1b69b8a5d99e2698665a8ee4755638b..3695c7568c4cc99e3c140923aff361635c8bef38 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1130,6 +1130,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
         goto f_err;
     }
 

+    /* We do this immediately so we know what format the ServerHello is in */

     protverr = ssl_choose_client_version(s, sversion);
     if (protverr != 0) {
         al = SSL_AD_PROTOCOL_VERSION;
     protverr = ssl_choose_client_version(s, sversion);
     if (protverr != 0) {
         al = SSL_AD_PROTOCOL_VERSION;


@@ -1145,8 +1146,6 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
         goto f_err;
     }
 

-    s->hit = 0;
-

     /* Get the session-id. */
     if (!SSL_IS_TLS13(s)) {
         if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {
     /* Get the session-id. */
     if (!SSL_IS_TLS13(s)) {
         if (!PACKET_get_length_prefixed_1(pkt, &session_id)) {


@@ -1173,63 +1172,103 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
         goto f_err;
     }
 

-    /*
-     * Check if we can resume the session based on external pre-shared secret.
-     * EAP-FAST (RFC 4851) supports two types of session resumption.
-     * Resumption based on server-side state works with session IDs.
-     * Resumption based on pre-shared Protected Access Credentials (PACs)
-     * works by overriding the SessionTicket extension at the application
-     * layer, and does not send a session ID. (We do not know whether EAP-FAST
-     * servers would honour the session ID.) Therefore, the session ID alone
-     * is not a reliable indicator of session resumption, so we first check if
-     * we can resume, and later peek at the next handshake message to see if the
-     * server wants to resume.
-     */
-    if (s->version >= TLS1_VERSION && !SSL_IS_TLS13(s)
-            && s->ext.session_secret_cb != NULL && s->session->ext.tick) {
-        const SSL_CIPHER *pref_cipher = NULL;
+    if (!SSL_IS_TLS13(s)) {
+        if (!PACKET_get_1(pkt, &compression)) {
+            SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH);
+            al = SSL_AD_DECODE_ERROR;
+            goto f_err;
+        }
+    } else {
+        compression = 0;
+    }
+
+    /* TLS extensions */
+    if (PACKET_remaining(pkt) == 0) {
+        PACKET_null_init(&extpkt);
+    } else if (!PACKET_as_length_prefixed_2(pkt, &extpkt)) {
+        al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_BAD_LENGTH);
+        goto f_err;
+    }
+
+    context = SSL_IS_TLS13(s) ? EXT_TLS1_3_SERVER_HELLO
+                              : EXT_TLS1_2_SERVER_HELLO;
+    if (!tls_collect_extensions(s, &extpkt, context, &extensions, &al))
+        goto f_err;
+
+    s->hit = 0;
+
+    if (SSL_IS_TLS13(s)) {
+        /* This will set s->hit if we are resuming */
+        if (!tls_parse_extension(s, TLSEXT_IDX_psk,
+                                 EXT_TLS1_3_SERVER_HELLO,
+                                 extensions, NULL, 0, &al))
+            goto f_err;
+    } else {

         /*
         /*

-         * s->session->master_key_length is a size_t, but this is an int for
-         * backwards compat reasons
+         * Check if we can resume the session based on external pre-shared
+         * secret. EAP-FAST (RFC 4851) supports two types of session resumption.
+         * Resumption based on server-side state works with session IDs.
+         * Resumption based on pre-shared Protected Access Credentials (PACs)
+         * works by overriding the SessionTicket extension at the application
+         * layer, and does not send a session ID. (We do not know whether
+         * EAP-FAST servers would honour the session ID.) Therefore, the session
+         * ID alone is not a reliable indicator of session resumption, so we
+         * first check if we can resume, and later peek at the next handshake
+         * message to see if the server wants to resume.

          */
          */

-        int master_key_length;
-        master_key_length = sizeof(s->session->master_key);
-        if (s->ext.session_secret_cb(s, s->session->master_key,
-                                     &master_key_length,
-                                     NULL, &pref_cipher,
-                                     s->ext.session_secret_cb_arg)
-                 && master_key_length > 0) {
-            s->session->master_key_length = master_key_length;
-            s->session->cipher = pref_cipher ?
-                pref_cipher : ssl_get_cipher_by_char(s, cipherchars);
-        } else {
-            SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
-            al = SSL_AD_INTERNAL_ERROR;
-            goto f_err;
+        if (s->version >= TLS1_VERSION
+                && s->ext.session_secret_cb != NULL && s->session->ext.tick) {
+            const SSL_CIPHER *pref_cipher = NULL;
+            /*
+             * s->session->master_key_length is a size_t, but this is an int for
+             * backwards compat reasons
+             */
+            int master_key_length;
+            master_key_length = sizeof(s->session->master_key);
+            if (s->ext.session_secret_cb(s, s->session->master_key,
+                                         &master_key_length,
+                                         NULL, &pref_cipher,
+                                         s->ext.session_secret_cb_arg)
+                     && master_key_length > 0) {
+                s->session->master_key_length = master_key_length;
+                s->session->cipher = pref_cipher ?
+                    pref_cipher : ssl_get_cipher_by_char(s, cipherchars);
+            } else {
+                SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, ERR_R_INTERNAL_ERROR);
+                al = SSL_AD_INTERNAL_ERROR;
+                goto f_err;
+            }

         }
         }

+
+        if (session_id_len != 0
+                && session_id_len == s->session->session_id_length
+                && memcmp(PACKET_data(&session_id), s->session->session_id,
+                          session_id_len) == 0)
+            s->hit = 1;

     }
 
     }
 

-    if (session_id_len != 0 && session_id_len == s->session->session_id_length
-        && memcmp(PACKET_data(&session_id), s->session->session_id,
-                  session_id_len) == 0) {
+    if (s->hit) {

         if (s->sid_ctx_length != s->session->sid_ctx_length
         if (s->sid_ctx_length != s->session->sid_ctx_length

-            || memcmp(s->session->sid_ctx, s->sid_ctx, s->sid_ctx_length)) {
+                || memcmp(s->session->sid_ctx, s->sid_ctx, s->sid_ctx_length)) {

             /* actually a client application bug */
             al = SSL_AD_ILLEGAL_PARAMETER;
             SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO,
                    SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT);
             goto f_err;
         }
             /* actually a client application bug */
             al = SSL_AD_ILLEGAL_PARAMETER;
             SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO,
                    SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT);
             goto f_err;
         }

-        s->hit = 1;

     } else {
         /*
          * If we were trying for session-id reuse but the server
     } else {
         /*
          * If we were trying for session-id reuse but the server

-         * didn't echo the ID, make a new SSL_SESSION.
+         * didn't resume, make a new SSL_SESSION.

          * In the case of EAP-FAST and PAC, we do not send a session ID,
          * so the PAC-based session secret is always preserved. It'll be
          * overwritten if the server refuses resumption.
          */
          * In the case of EAP-FAST and PAC, we do not send a session ID,
          * so the PAC-based session secret is always preserved. It'll be
          * overwritten if the server refuses resumption.
          */

-        if (s->session->session_id_length > 0) {
+        if (s->session->session_id_length > 0
+                || (SSL_IS_TLS13(s)
+                    && s->session->ext.tick_identity
+                       != TLSEXT_PSK_BAD_IDENTITY)) {

             s->ctx->stats.sess_miss++;
             if (!ssl_get_new_session(s, 0)) {
                 goto f_err;
             s->ctx->stats.sess_miss++;
             if (!ssl_get_new_session(s, 0)) {
                 goto f_err;


@@ -1299,17 +1338,6 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
         goto f_err;
     }
     s->s3->tmp.new_cipher = c;
         goto f_err;
     }
     s->s3->tmp.new_cipher = c;

-    /* lets get the compression algorithm */
-    /* COMPRESSION */
-    if (!SSL_IS_TLS13(s)) {
-        if (!PACKET_get_1(pkt, &compression)) {
-            SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH);
-            al = SSL_AD_DECODE_ERROR;
-            goto f_err;
-        }
-    } else {
-        compression = 0;
-    }

 
 #ifdef OPENSSL_NO_COMP
     if (compression != 0) {
 
 #ifdef OPENSSL_NO_COMP
     if (compression != 0) {


@@ -1353,19 +1381,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
     }
 #endif
 
     }
 #endif
 

-    /* TLS extensions */
-    if (PACKET_remaining(pkt) == 0) {
-        PACKET_null_init(&extpkt);
-    } else if (!PACKET_as_length_prefixed_2(pkt, &extpkt)) {
-        al = SSL_AD_DECODE_ERROR;
-        SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_BAD_LENGTH);
-        goto f_err;
-    }
-
-    context = SSL_IS_TLS13(s) ? EXT_TLS1_3_SERVER_HELLO
-                              : EXT_TLS1_2_SERVER_HELLO;
-    if (!tls_collect_extensions(s, &extpkt, context, &extensions, &al)
-            || !tls_parse_all_extensions(s, context, extensions, NULL, 0, &al))
+    if (!tls_parse_all_extensions(s, context, extensions, NULL, 0, &al))

         goto f_err;
 
 #ifndef OPENSSL_NO_SCTP
         goto f_err;
 
 #ifndef OPENSSL_NO_SCTP

diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 67858057f707df5dbb8d8e1cf8e6ac068a56c32e..99f67e51ad5edc44f8d19e95421cd75652a3fba4 100644 (file)
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -321,3 +321,4 @@ int tls_parse_stoc_etm(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al);
 int tls_parse_stoc_ems(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al);
 int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, X509 *x, size_t chainidx,
                              int *al);
 int tls_parse_stoc_ems(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al);
 int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, X509 *x, size_t chainidx,
                              int *al);

+int tls_parse_stoc_psk(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al);