kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Don't bother accepting key_arg when parsing SSL_SESSION.

Browse Source 
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>
This commit is contained in:
David Benjamin 2014-10-26 00:03:08 -04:00 committed by Adam Langley
parent 3b73c18fca
commit 7001a7fce6
2 changed files with 13 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
ssl/ssl_asn1.c
View File

@ -98,8 +98,6 @@
* cipher OCTET STRING, -- two bytes long * cipher OCTET STRING, -- two bytes long
* sessionID OCTET STRING, * sessionID OCTET STRING,
* masterKey OCTET STRING, * masterKey OCTET STRING,
* keyArg [0] IMPLICIT OCTET STRING OPTIONAL,
* -- ignored: legacy SSLv2-only field.
* time [1] INTEGER OPTIONAL, -- seconds since UNIX epoch * time [1] INTEGER OPTIONAL, -- seconds since UNIX epoch
* timeout [2] INTEGER OPTIONAL, -- in seconds * timeout [2] INTEGER OPTIONAL, -- in seconds
* peer [3] Certificate OPTIONAL, * peer [3] Certificate OPTIONAL,
@ -410,8 +408,8 @@ static int d2i_SSL_SESSION_get_octet_string(CBS *cbs, uint8_t **out_ptr,
SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const uint8_t **pp, long length) { SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const uint8_t **pp, long length) {
SSL_SESSION *ret = NULL; SSL_SESSION *ret = NULL;
CBS cbs, session, cipher, session_id, master_key; CBS cbs, session, cipher, session_id, master_key;
CBS key_arg, peer, sid_ctx, peer_sha256, original_handshake_hash; CBS peer, sid_ctx, peer_sha256, original_handshake_hash;
int has_key_arg, has_peer, has_peer_sha256, extended_master_secret; int has_peer, has_peer_sha256, extended_master_secret;
uint64_t version, ssl_version; uint64_t version, ssl_version;
uint64_t session_time, timeout, verify_result, ticket_lifetime_hint; uint64_t session_time, timeout, verify_result, ticket_lifetime_hint;
@ -431,7 +429,6 @@ SSL_SESSION *d2i_SSL_SESSION(SSL_SESSION **a, const uint8_t **pp, long length) {
!CBS_get_asn1(&session, &cipher, CBS_ASN1_OCTETSTRING) || !CBS_get_asn1(&session, &cipher, CBS_ASN1_OCTETSTRING) ||
!CBS_get_asn1(&session, &session_id, CBS_ASN1_OCTETSTRING) || !CBS_get_asn1(&session, &session_id, CBS_ASN1_OCTETSTRING) ||
!CBS_get_asn1(&session, &master_key, 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, !CBS_get_optional_asn1_uint64(&session, &session_time, kTimeTag,
time(NULL)) || time(NULL)) ||
!CBS_get_optional_asn1_uint64(&session, &timeout, kTimeoutTag, 3) || !CBS_get_optional_asn1_uint64(&session, &timeout, kTimeoutTag, 3) ||

42
ssl/ssl_test.c
View File

@ -307,20 +307,6 @@ static const char kOpenSSLSession[] =
* filling in missing fields from |kOpenSSLSession|. This includes * filling in missing fields from |kOpenSSLSession|. This includes
* providing |peer_sha256|, so |peer| is not serialized. */ * providing |peer_sha256|, so |peer| is not serialized. */
static const char kCustomSession[] = 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" "MIIBfwIBAQICAwMEAsAvBCAG5Q1ndq4Yfmbeo1zwLkNRKmCXGdNgWvGT3cskV0yQ"
"kAQwJlrlzkAWBOWiLj/jJ76D7l+UXoizP2KI2C7I2FccqMmIfFmmkUy32nIJ0mZH" "kAQwJlrlzkAWBOWiLj/jJ76D7l+UXoizP2KI2C7I2FccqMmIfFmmkUy32nIJ0mZH"
"IWoJoQYCBFRDO46iBAICASykAwQBAqUDAgEUphAEDnd3dy5nb29nbGUuY29tpwcE" "IWoJoQYCBFRDO46iBAICASykAwQBAqUDAgEUphAEDnd3dy5nb29nbGUuY29tpwcE"
@ -355,18 +341,16 @@ static int decode_base64(uint8_t **out, size_t *out_len, const char *in) {
return 1; return 1;
} }
static int test_ssl_session_asn1(const char *input_b64, static int test_ssl_session_asn1(const char *input_b64) {
const char *expected_b64) {
int ret = 0, len; int ret = 0, len;
size_t input_len, expected_len; size_t input_len;
uint8_t *input = NULL, *expected = NULL, *encoded = NULL; uint8_t *input = NULL, *encoded = NULL;
const uint8_t *cptr; const uint8_t *cptr;
uint8_t *ptr; uint8_t *ptr;
SSL_SESSION *session = NULL; SSL_SESSION *session = NULL;
/* Decode the input. */ /* Decode the input. */
if (!decode_base64(&input, &input_len, input_b64) || if (!decode_base64(&input, &input_len, input_b64)) {
!decode_base64(&expected, &expected_len, expected_b64)) {
goto done; goto done;
} }
@ -380,27 +364,27 @@ static int test_ssl_session_asn1(const char *input_b64,
/* Verify the SSL_SESSION encoding round-trips. */ /* Verify the SSL_SESSION encoding round-trips. */
len = i2d_SSL_SESSION(session, NULL); 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"); fprintf(stderr, "i2d_SSL_SESSION(NULL) returned invalid length\n");
goto done; goto done;
} }
encoded = OPENSSL_malloc(expected_len); encoded = OPENSSL_malloc(input_len);
if (encoded == NULL) { if (encoded == NULL) {
fprintf(stderr, "malloc failed\n"); fprintf(stderr, "malloc failed\n");
goto done; goto done;
} }
ptr = encoded; ptr = encoded;
len = i2d_SSL_SESSION(session, &ptr); 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"); fprintf(stderr, "i2d_SSL_SESSION returned invalid length\n");
goto done; goto done;
} }
if (ptr != encoded + expected_len) { if (ptr != encoded + input_len) {
fprintf(stderr, "i2d_SSL_SESSION did not advance ptr correctly\n"); fprintf(stderr, "i2d_SSL_SESSION did not advance ptr correctly\n");
goto done; 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"); fprintf(stderr, "i2d_SSL_SESSION did not round-trip\n");
goto done; goto done;
} }
@ -418,9 +402,6 @@ static int test_ssl_session_asn1(const char *input_b64,
if (input) { if (input) {
OPENSSL_free(input); OPENSSL_free(input);
} }
if (expected) {
OPENSSL_free(expected);
}
if (encoded) { if (encoded) {
OPENSSL_free(encoded); OPENSSL_free(encoded);
} }
@ -431,9 +412,8 @@ int main(void) {
SSL_library_init(); SSL_library_init();
if (!test_cipher_rules() || if (!test_cipher_rules() ||
!test_ssl_session_asn1(kOpenSSLSession, kOpenSSLSession) || !test_ssl_session_asn1(kOpenSSLSession) ||
!test_ssl_session_asn1(kCustomSession, kCustomSession2) || !test_ssl_session_asn1(kCustomSession)) {
!test_ssl_session_asn1(kCustomSession2, kCustomSession2)) {
return 1; return 1;
} }