diff options
| author | Matt Caswell <matt@openssl.org> | 2017-05-08 16:05:16 +0100 |
|---|---|---|
| committer | Matt Caswell <matt@openssl.org> | 2017-05-09 17:23:58 +0100 |
| commit | 66d4bf6b20d8769a3c2bf1a0c4cb3155365601e7 (patch) | |
| tree | e939c6ab2ac257942633985b178a439665a09572 | |
| parent | c96ec6f8048c7c2210c5d45276000190051c229f (diff) | |
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)
| -rw-r--r-- | include/openssl/ssl.h | 1 | ||||
| -rw-r--r-- | ssl/ssl_err.c | 1 | ||||
| -rw-r--r-- | ssl/statem/statem_clnt.c | 18 |
3 files changed, 19 insertions, 1 deletions
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index e89e97f7f1..54028f66c9 100644 --- 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 18a38dfe47..06cd8521e5 100644 --- 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 a66dd40806..6bff9d47d3 100644 --- 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 |