Enforce supported_versions in the second ServerHello.
We forgot to do this in our original implementation on general ecosystem
grounds. It's also mandated starting draft-26.
Just to avoid unnecessary turbulence, since draft-23 is doomed to die
anyway, condition this on our draft-28 implementation. (We don't support
24 through 27.)
We'd actually checked this already on the Go side, but the spec wants a
different alert.
Change-Id: I0014cda03d7129df0b48de077e45f8ae9fd16976
Reviewed-on: https://boringssl-review.googlesource.com/28124
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 579e6a6..49f528b 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -290,6 +290,18 @@
return ssl_hs_error;
}
+ if (ssl_is_draft28(ssl->version)) {
+ // Recheck supported_versions, in case this is the second ServerHello.
+ uint16_t version;
+ if (!have_supported_versions ||
+ !CBS_get_u16(&supported_versions, &version) ||
+ version != ssl->version) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_SECOND_SERVERHELLO_VERSION_MISMATCH);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ return ssl_hs_error;
+ }
+ }
+
alert = SSL_AD_DECODE_ERROR;
if (have_pre_shared_key) {
if (ssl->session == NULL) {
