Don't bother accepting key_arg when parsing SSL_SESSION.
Doing some archeaology, since the initial OpenSSL commit, key_arg has been
omitted from the serialization if key_arg_length was 0. Since this is an
SSLv2-only field and resuming an SSLv2 session with SSLv3+ is not possible,
there is no need to support parsing those sessions.
Interestingly, it is actually not the case that key_arg_length was only ever
set in SSLv2, historically. In the initial commit of OpenSSL, SSLeay 0.8.1b,
key_arg was used to store what appears to be the IV. That was then removed in
the next commit, an import of SSLeay 0.9.0b, at which point key_arg was only
ever set in SSLv3. That is old enough that there is certainly no need to
parse pre-SSLeay-0.9.0b sessions...
Change-Id: Ia768a2d97ddbe60309be20e2efe488640c4776d9
Reviewed-on: https://boringssl-review.googlesource.com/2050
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/ssl_asn1.c b/ssl/ssl_asn1.c
index 53dc996..ee6eee5 100644
--- a/ssl/ssl_asn1.c
+++ b/ssl/ssl_asn1.c
@@ -98,8 +98,6 @@
* cipher OCTET STRING, -- two bytes long
* sessionID OCTET STRING,
* masterKey OCTET STRING,
- * keyArg [0] IMPLICIT OCTET STRING OPTIONAL,
- * -- ignored: legacy SSLv2-only field.
* time [1] INTEGER OPTIONAL, -- seconds since UNIX epoch
* timeout [2] INTEGER OPTIONAL, -- in seconds
* peer [3] Certificate OPTIONAL,
@@ -410,8 +408,8 @@
SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const uint8_t **pp, long length) {
SSL_SESSION *ret = NULL;
CBS cbs, session, cipher, session_id, master_key;
- CBS key_arg, peer, sid_ctx, peer_sha256, original_handshake_hash;
- int has_key_arg, has_peer, has_peer_sha256, extended_master_secret;
+ CBS peer, sid_ctx, peer_sha256, original_handshake_hash;
+ int has_peer, has_peer_sha256, extended_master_secret;
uint64_t version, ssl_version;
uint64_t session_time, timeout, verify_result, ticket_lifetime_hint;
@@ -431,7 +429,6 @@
!CBS_get_asn1(&session, &cipher, CBS_ASN1_OCTETSTRING) ||
!CBS_get_asn1(&session, &session_id, CBS_ASN1_OCTETSTRING) ||
!CBS_get_asn1(&session, &master_key, CBS_ASN1_OCTETSTRING) ||
- !CBS_get_optional_asn1(&session, &key_arg, &has_key_arg, kKeyArgTag) ||
!CBS_get_optional_asn1_uint64(&session, &session_time, kTimeTag,
time(NULL)) ||
!CBS_get_optional_asn1_uint64(&session, &timeout, kTimeoutTag, 3) ||
diff --git a/ssl/ssl_test.c b/ssl/ssl_test.c
index a202273..edf509e 100644
--- a/ssl/ssl_test.c
+++ b/ssl/ssl_test.c
@@ -307,20 +307,6 @@
* filling in missing fields from |kOpenSSLSession|. This includes
* providing |peer_sha256|, so |peer| is not serialized. */
static const char kCustomSession[] =
- "MIIBggIBAQICAwMEAsAvBCAG5Q1ndq4Yfmbeo1zwLkNRKmCXGdNgWvGT3cskV0yQ"
- "kAQwJlrlzkAWBOWiLj/jJ76D7l+UXoizP2KI2C7I2FccqMmIfFmmkUy32nIJ0mZH"
- "IWoJgAEBoQYCBFRDO46iBAICASykAwQBAqUDAgEUphAEDnd3dy5nb29nbGUuY29t"
- "pwcEBWhlbGxvqAcEBXdvcmxkqQUCAwGJwKqBpwSBpBwUQvoeOk0Kg36SYTcLEkXq"
- "KwOBfF9vE4KX0NxeLwjcDTpsuh3qXEaZ992r1N38VDcyS6P7I6HBYN9BsNHM362z"
- "ZnY27GpTw+Kwd751CLoXFPoaMOe57dbBpXoro6Pd3BTbf/Tzr88K06yEOTDKPNj3"
- "+inbMaVigtK4PLyPq+Topyzvx9USFgRvyuoxn0Hgb+R0A3j6SLRuyOdAi4gv7Y5o"
- "liynrSIEIAYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGrgMEAQevAwQB"
- "BLADBAEF";
-
-/* kCustomSession2 is kCustomSession with the old SSLv2-only key_arg
- * field removed. Encoding the decoded version of kCustomSession
- * should not preserve key_arg. */
-static const char kCustomSession2[] =
"MIIBfwIBAQICAwMEAsAvBCAG5Q1ndq4Yfmbeo1zwLkNRKmCXGdNgWvGT3cskV0yQ"
"kAQwJlrlzkAWBOWiLj/jJ76D7l+UXoizP2KI2C7I2FccqMmIfFmmkUy32nIJ0mZH"
"IWoJoQYCBFRDO46iBAICASykAwQBAqUDAgEUphAEDnd3dy5nb29nbGUuY29tpwcE"
@@ -355,18 +341,16 @@
return 1;
}
-static int test_ssl_session_asn1(const char *input_b64,
- const char *expected_b64) {
+static int test_ssl_session_asn1(const char *input_b64) {
int ret = 0, len;
- size_t input_len, expected_len;
- uint8_t *input = NULL, *expected = NULL, *encoded = NULL;
+ size_t input_len;
+ uint8_t *input = NULL, *encoded = NULL;
const uint8_t *cptr;
uint8_t *ptr;
SSL_SESSION *session = NULL;
/* Decode the input. */
- if (!decode_base64(&input, &input_len, input_b64) ||
- !decode_base64(&expected, &expected_len, expected_b64)) {
+ if (!decode_base64(&input, &input_len, input_b64)) {
goto done;
}
@@ -380,27 +364,27 @@
/* Verify the SSL_SESSION encoding round-trips. */
len = i2d_SSL_SESSION(session, NULL);
- if (len < 0 || (size_t)len != expected_len) {
+ if (len < 0 || (size_t)len != input_len) {
fprintf(stderr, "i2d_SSL_SESSION(NULL) returned invalid length\n");
goto done;
}
- encoded = OPENSSL_malloc(expected_len);
+ encoded = OPENSSL_malloc(input_len);
if (encoded == NULL) {
fprintf(stderr, "malloc failed\n");
goto done;
}
ptr = encoded;
len = i2d_SSL_SESSION(session, &ptr);
- if (len < 0 || (size_t)len != expected_len) {
+ if (len < 0 || (size_t)len != input_len) {
fprintf(stderr, "i2d_SSL_SESSION returned invalid length\n");
goto done;
}
- if (ptr != encoded + expected_len) {
+ if (ptr != encoded + input_len) {
fprintf(stderr, "i2d_SSL_SESSION did not advance ptr correctly\n");
goto done;
}
- if (memcmp(expected, encoded, expected_len) != 0) {
+ if (memcmp(input, encoded, input_len) != 0) {
fprintf(stderr, "i2d_SSL_SESSION did not round-trip\n");
goto done;
}
@@ -418,9 +402,6 @@
if (input) {
OPENSSL_free(input);
}
- if (expected) {
- OPENSSL_free(expected);
- }
if (encoded) {
OPENSSL_free(encoded);
}
@@ -431,9 +412,8 @@
SSL_library_init();
if (!test_cipher_rules() ||
- !test_ssl_session_asn1(kOpenSSLSession, kOpenSSLSession) ||
- !test_ssl_session_asn1(kCustomSession, kCustomSession2) ||
- !test_ssl_session_asn1(kCustomSession2, kCustomSession2)) {
+ !test_ssl_session_asn1(kOpenSSLSession) ||
+ !test_ssl_session_asn1(kCustomSession)) {
return 1;
}
