Verify that if we have an HRR then something will change

It is invalid if we receive an HRR but no change will result in
ClientHello2.

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

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index e89e97f7f1241937e0014f3bcdf4e5fdcb1339d4..54028f66c9d346643cfcd4691279b7433e4a93a5 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2680,6 +2680,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_R_NO_CERTIFICATES_RETURNED                   176
 # define SSL_R_NO_CERTIFICATE_ASSIGNED                    177
 # define SSL_R_NO_CERTIFICATE_SET                         179
+# define SSL_R_NO_CHANGE_FOLLOWING_HRR                    205
 # define SSL_R_NO_CIPHERS_AVAILABLE                       181
 # define SSL_R_NO_CIPHERS_SPECIFIED                       183
 # define SSL_R_NO_CIPHER_MATCH                            185

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 18a38dfe475bb502cbaee881d0badf386e3df278..06cd8521e54d988e29a27f721482f53e646ff035 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -646,6 +646,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = {
     {ERR_REASON(SSL_R_NO_CERTIFICATES_RETURNED), "no certificates returned"},
     {ERR_REASON(SSL_R_NO_CERTIFICATE_ASSIGNED), "no certificate assigned"},
     {ERR_REASON(SSL_R_NO_CERTIFICATE_SET), "no certificate set"},
+    {ERR_REASON(SSL_R_NO_CHANGE_FOLLOWING_HRR), "no change following hrr"},
     {ERR_REASON(SSL_R_NO_CIPHERS_AVAILABLE), "no ciphers available"},
     {ERR_REASON(SSL_R_NO_CIPHERS_SPECIFIED), "no ciphers specified"},
     {ERR_REASON(SSL_R_NO_CIPHER_MATCH), "no cipher match"},

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index a66dd4080647dce18c9135021fba128f37b322f9..6bff9d47d32d5ad50680c9d9011a7c16f9de539c 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1609,7 +1609,11 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
         goto f_err;
     }
 
-    if (!PACKET_as_length_prefixed_2(pkt, &extpkt)) {
+    if (!PACKET_as_length_prefixed_2(pkt, &extpkt)
+               /* Must have a non-empty extensions block */
+            || PACKET_remaining(&extpkt) == 0
+               /* Must be no trailing data after extensions */
+            || PACKET_remaining(pkt) != 0) {
         al = SSL_AD_DECODE_ERROR;
         SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_BAD_LENGTH);
         goto f_err;
@@ -1622,6 +1626,18 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt)
         goto f_err;
 
     OPENSSL_free(extensions);
+    extensions = NULL;
+
+    if (s->ext.tls13_cookie_len == 0 && s->s3->tmp.pkey != NULL) {
+        /*
+         * We didn't receive a cookie or a new key_share so the next
+         * ClientHello will not change
+         */
+        al = SSL_AD_ILLEGAL_PARAMETER;
+        SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST,
+               SSL_R_NO_CHANGE_FOLLOWING_HRR);
+        goto f_err;
+    }
 
     /*
      * Re-initialise the Transcript Hash. We're going to prepopulate it with