commit 338e06771c600108ba8b1e3df6a15a8920961c37
author David Benjamin <davidben@chromium.org> Thu May 28 20:00:08 2015 -0400
committer Adam Langley <agl@google.com> Mon Jun 01 20:30:02 2015 +0000
tree 33df0de1958de56f3a236b89971c092dfd799d33
parent f297e021f1403614f90c2f01e669022edfc1cfe6

Reject sessions with the wrong structure version.

The old upstream logic actually didn't do this, but 1.1.0's new code does.
Given that the version has never changed and even unknown fields were rejected
by the old code, this seems a safe and prudent thing to do.

Change-Id: I09071585e5183993b358c10ad36fc206f8bceeda
Reviewed-on: https://boringssl-review.googlesource.com/4942
Reviewed-by: Adam Langley <agl@google.com>

ssl/ssl_asn1.c

diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c
index b0ab622..21492dc 100644
--- a/ssl/ssl_asn1.c
+++ b/ssl/ssl_asn1.c
@@ -484,7 +484,10 @@
   }
   ret->extended_master_secret = extended_master_secret;
 
-  /* Ignore |version|. The structure version number is ignored. */
+  if (version != SSL_SESSION_ASN1_VERSION) {
+    OPENSSL_PUT_ERROR(SSL, d2i_SSL_SESSION, SSL_R_INVALID_SSL_SESSION);
+    goto err;
+  }
 
   /* Only support SSLv3/TLS and DTLS. */
   if ((ssl_version >> 8) != SSL3_VERSION_MAJOR &&

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index c0085aa..b29d28c 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -347,6 +347,18 @@
     "q+Topyzvx9USFgRvyuoxn0Hgb+R0A3j6SLRuyOdAi4gv7Y5oliynrSIEIAYGBgYG"
     "BgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGrgMEAQevAwQBBL4DBAEF";
 
+// kBadSessionVersion is a custom serialized SSL_SESSION generated by replacing
+// the version of |kCustomSession| with 2.
+static const char kBadSessionVersion[] =
+    "MIIBdgIBAgICAwMEAsAvBCAG5Q1ndq4Yfmbeo1zwLkNRKmCXGdNgWvGT3cskV0yQ"
+    "kAQwJlrlzkAWBOWiLj/jJ76D7l+UXoizP2KI2C7I2FccqMmIfFmmkUy32nIJ0mZH"
+    "IWoJoQYCBFRDO46iBAICASykAwQBAqUDAgEUphAEDnd3dy5nb29nbGUuY29tqAcE"
+    "BXdvcmxkqQUCAwGJwKqBpwSBpBwUQvoeOk0Kg36SYTcLEkXqKwOBfF9vE4KX0Nxe"
+    "LwjcDTpsuh3qXEaZ992r1N38VDcyS6P7I6HBYN9BsNHM362zZnY27GpTw+Kwd751"
+    "CLoXFPoaMOe57dbBpXoro6Pd3BTbf/Tzr88K06yEOTDKPNj3+inbMaVigtK4PLyP"
+    "q+Topyzvx9USFgRvyuoxn0Hgb+R0A3j6SLRuyOdAi4gv7Y5oliynrSIEIAYGBgYG"
+    "BgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGrgMEAQevAwQBBLADBAEF";
+
 static bool DecodeBase64(std::vector<uint8_t> *out, const char *in) {
   size_t len;
   if (!EVP_DecodedLength(&len, strlen(in))) {
@@ -524,6 +536,7 @@
       !TestSSL_SESSIONEncoding(kOpenSSLSession) ||
       !TestSSL_SESSIONEncoding(kCustomSession) ||
       !TestBadSSL_SESSIONEncoding(kBadSessionExtraField) ||
+      !TestBadSSL_SESSIONEncoding(kBadSessionVersion) ||
       !TestDefaultVersion(0, &TLS_method) ||
       !TestDefaultVersion(SSL3_VERSION, &SSLv3_method) ||
       !TestDefaultVersion(TLS1_VERSION, &TLSv1_method) ||