Slightly refactor SSL_SESSION parsing.

Rather than parse the fields in two passes, group the code relating to
one field together. Somewhat less annoying to add new fields. To keep
this from getting too unwieldy, add a few more helper functions for the
common field types.

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

ssl/ssl_asn1.c

@@ -404,7 +404,7 @@ static int SSL_SESSION_parse_string(CBS *cbs, char **out, unsigned tag) {
  * |*out_ptr| to NULL. It returns one on success, whether or not the
  * element was found, and zero on decode error. */
 static int SSL_SESSION_parse_octet_string(CBS *cbs, uint8_t **out_ptr,
-                                            size_t *out_len, unsigned tag) {
+                                          size_t *out_len, unsigned tag) {
   CBS value;
   if (!CBS_get_optional_asn1_octet_string(cbs, &value, NULL, tag)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
@@ -417,91 +417,72 @@ static int SSL_SESSION_parse_octet_string(CBS *cbs, uint8_t **out_ptr,
   return 1;
 }
 
-static SSL_SESSION *SSL_SESSION_parse(CBS *cbs) {
-  SSL_SESSION *ret = NULL;
-  CBS session, cipher, session_id, master_key;
-  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,
-      key_exchange_info;
-
-  ret = SSL_SESSION_new();
-  if (ret == NULL) {
-    goto err;
-  }
-
-  if (!CBS_get_asn1(cbs, &session, CBS_ASN1_SEQUENCE) ||
-      !CBS_get_asn1_uint64(&session, &version) ||
-      !CBS_get_asn1_uint64(&session, &ssl_version) ||
-      !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_uint64(&session, &session_time, kTimeTag,
-                                    time(NULL)) ||
-      !CBS_get_optional_asn1_uint64(&session, &timeout, kTimeoutTag, 3) ||
-      !CBS_get_optional_asn1(&session, &peer, &has_peer, kPeerTag) ||
-      !CBS_get_optional_asn1_octet_string(&session, &sid_ctx, NULL,
-                                          kSessionIDContextTag) ||
-      !CBS_get_optional_asn1_uint64(&session, &verify_result, kVerifyResultTag,
-                                    X509_V_OK)) {
+/* SSL_SESSION_parse_bounded_octet_string parses an optional ASN.1 OCTET STRING
+ * explicitly tagged with |tag| of size at most |max_out|. */
+static int SSL_SESSION_parse_bounded_octet_string(
+    CBS *cbs, uint8_t *out, unsigned *out_len, unsigned max_out, unsigned tag) {
+  CBS value;
+  if (!CBS_get_optional_asn1_octet_string(cbs, &value, NULL, tag) ||
+      CBS_len(&value) > max_out) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
-    goto err;
-  }
-  if (!SSL_SESSION_parse_string(&session, &ret->tlsext_hostname,
-                                kHostNameTag) ||
-      !SSL_SESSION_parse_string(&session, &ret->psk_identity,
-                                kPSKIdentityTag)) {
-    goto err;
+    return 0;
   }
-  if (!CBS_get_optional_asn1_uint64(&session, &ticket_lifetime_hint,
-                                    kTicketLifetimeHintTag, 0)) {
+  memcpy(out, CBS_data(&value), CBS_len(&value));
+  *out_len = (unsigned)CBS_len(&value);
+  return 1;
+}
+
+static int SSL_SESSION_parse_long(CBS *cbs, long *out, unsigned tag,
+                                  long default_value) {
+  uint64_t value;
+  if (!CBS_get_optional_asn1_uint64(cbs, &value, tag,
+                                    (uint64_t)default_value) ||
+      value > LONG_MAX) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
-    goto err;
-  }
-  if (!SSL_SESSION_parse_octet_string(&session, &ret->tlsext_tick,
-                                        &ret->tlsext_ticklen, kTicketTag)) {
-    goto err;
+    return 0;
   }
-  if (!CBS_get_optional_asn1_octet_string(&session, &peer_sha256,
-                                          &has_peer_sha256, kPeerSHA256Tag) ||
-      !CBS_get_optional_asn1_octet_string(&session, &original_handshake_hash,
-                                          NULL, kOriginalHandshakeHashTag)) {
+  *out = (long)value;
+  return 1;
+}
+
+static int SSL_SESSION_parse_u32(CBS *cbs, uint32_t *out, unsigned tag,
+                                 uint32_t default_value) {
+  uint64_t value;
+  if (!CBS_get_optional_asn1_uint64(cbs, &value, tag,
+                                    (uint64_t)default_value) ||
+      value > 0xffffffff) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
-    goto err;
-  }
-  if (!SSL_SESSION_parse_octet_string(
-          &session, &ret->tlsext_signed_cert_timestamp_list,
-          &ret->tlsext_signed_cert_timestamp_list_length,
-          kSignedCertTimestampListTag) ||
-      !SSL_SESSION_parse_octet_string(
-          &session, &ret->ocsp_response, &ret->ocsp_response_length,
-          kOCSPResponseTag)) {
-    goto err;
+    return 0;
   }
-  if (!CBS_get_optional_asn1_bool(&session, &extended_master_secret,
-                                  kExtendedMasterSecretTag,
-                                  0 /* default to false */)) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
-    goto err;
+  *out = (uint32_t)value;
+  return 1;
+}
+
+static X509 *parse_x509(CBS *cbs) {
+  const uint8_t *ptr = CBS_data(cbs);
+  X509 *ret = d2i_X509(NULL, &ptr, CBS_len(cbs));
+  if (ret == NULL) {
+    return NULL;
   }
-  ret->extended_master_secret = extended_master_secret;
+  CBS_skip(cbs, ptr - CBS_data(cbs));
+  return ret;
+}
 
-  if (!CBS_get_optional_asn1_uint64(&session, &key_exchange_info,
-                                    kKeyExchangeInfoTag, 0) ||
-      CBS_len(&session) != 0) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+static SSL_SESSION *SSL_SESSION_parse(CBS *cbs) {
+  SSL_SESSION *ret = SSL_SESSION_new();
+  if (ret == NULL) {
     goto err;
   }
-  if (key_exchange_info <= 0xffffffff) {
-    ret->key_exchange_info = key_exchange_info;
-  }
 
-  if (version != SSL_SESSION_ASN1_VERSION) {
+  CBS session;
+  uint64_t version, ssl_version;
+  if (!CBS_get_asn1(cbs, &session, CBS_ASN1_SEQUENCE) ||
+      !CBS_get_asn1_uint64(&session, &version) ||
+      version != SSL_SESSION_ASN1_VERSION ||
+      !CBS_get_asn1_uint64(&session, &ssl_version)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
     goto err;
   }
-
   /* Only support SSLv3/TLS and DTLS. */
   if ((ssl_version >> 8) != SSL3_VERSION_MAJOR &&
       (ssl_version >> 8) != (DTLS1_VERSION >> 8)) {
@@ -510,9 +491,12 @@ static SSL_SESSION *SSL_SESSION_parse(CBS *cbs) {
   }
   ret->ssl_version = ssl_version;
 
+  CBS cipher;
   uint16_t cipher_value;
-  if (!CBS_get_u16(&cipher, &cipher_value) || CBS_len(&cipher) != 0) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_CIPHER_CODE_WRONG_LENGTH);
+  if (!CBS_get_asn1(&session, &cipher, CBS_ASN1_OCTETSTRING) ||
+      !CBS_get_u16(&cipher, &cipher_value) ||
+      CBS_len(&cipher) != 0) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
     goto err;
   }
   ret->cipher = SSL_get_cipher_by_value(cipher_value);
@@ -521,60 +505,66 @@ static SSL_SESSION *SSL_SESSION_parse(CBS *cbs) {
     goto err;
   }
 
-  if (CBS_len(&session_id) > SSL3_MAX_SSL_SESSION_ID_LENGTH) {
+  CBS session_id, master_key;
+  if (!CBS_get_asn1(&session, &session_id, CBS_ASN1_OCTETSTRING) ||
+      CBS_len(&session_id) > SSL3_MAX_SSL_SESSION_ID_LENGTH ||
+      !CBS_get_asn1(&session, &master_key, CBS_ASN1_OCTETSTRING) ||
+      CBS_len(&master_key) > SSL_MAX_MASTER_KEY_LENGTH) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
     goto err;
   }
   memcpy(ret->session_id, CBS_data(&session_id), CBS_len(&session_id));
   ret->session_id_length = CBS_len(&session_id);
+  memcpy(ret->master_key, CBS_data(&master_key), CBS_len(&master_key));
+  ret->master_key_length = CBS_len(&master_key);
 
-  if (CBS_len(&master_key) > SSL_MAX_MASTER_KEY_LENGTH) {
+  if (!SSL_SESSION_parse_long(&session, &ret->time, kTimeTag, time(NULL)) ||
+      !SSL_SESSION_parse_long(&session, &ret->timeout, kTimeoutTag, 3)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
     goto err;
   }
-  memcpy(ret->master_key, CBS_data(&master_key), CBS_len(&master_key));
-  ret->master_key_length = CBS_len(&master_key);
 
-  if (session_time > LONG_MAX ||
-      timeout > LONG_MAX) {
+  CBS peer;
+  int has_peer;
+  if (!CBS_get_optional_asn1(&session, &peer, &has_peer, kPeerTag)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
     goto err;
   }
-  ret->time = session_time;
-  ret->timeout = timeout;
-
   X509_free(ret->peer);
   ret->peer = NULL;
   if (has_peer) {
-    const uint8_t *ptr;
-    ptr = CBS_data(&peer);
-    ret->peer = d2i_X509(NULL, &ptr, CBS_len(&peer));
+    ret->peer = parse_x509(&peer);
     if (ret->peer == NULL) {
       goto err;
     }
-    if (ptr != CBS_data(&peer) + CBS_len(&peer)) {
+    if (CBS_len(&peer) != 0) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
       goto err;
     }
   }
 
-  if (CBS_len(&sid_ctx) > sizeof(ret->sid_ctx)) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
-    goto err;
-  }
-  memcpy(ret->sid_ctx, CBS_data(&sid_ctx), CBS_len(&sid_ctx));
-  ret->sid_ctx_length = CBS_len(&sid_ctx);
-
-  if (verify_result > LONG_MAX ||
-      ticket_lifetime_hint > 0xffffffff) {
-    OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+  if (!SSL_SESSION_parse_bounded_octet_string(
+          &session, ret->sid_ctx, &ret->sid_ctx_length, sizeof(ret->sid_ctx),
+          kSessionIDContextTag) ||
+      !SSL_SESSION_parse_long(&session, &ret->verify_result, kVerifyResultTag,
+                              X509_V_OK) ||
+      !SSL_SESSION_parse_string(&session, &ret->tlsext_hostname,
+                                kHostNameTag) ||
+      !SSL_SESSION_parse_string(&session, &ret->psk_identity,
+                                kPSKIdentityTag) ||
+      !SSL_SESSION_parse_u32(&session, &ret->tlsext_tick_lifetime_hint,
+                             kTicketLifetimeHintTag, 0) ||
+      !SSL_SESSION_parse_octet_string(&session, &ret->tlsext_tick,
+                                      &ret->tlsext_ticklen, kTicketTag)) {
     goto err;
   }
-  ret->verify_result = verify_result;
-  ret->tlsext_tick_lifetime_hint = ticket_lifetime_hint;
 
-  if (has_peer_sha256) {
-    if (CBS_len(&peer_sha256) != sizeof(ret->peer_sha256)) {
+  if (CBS_peek_asn1_tag(&session, kPeerSHA256Tag)) {
+    CBS child, peer_sha256;
+    if (!CBS_get_asn1(&session, &child, kPeerSHA256Tag) ||
+        !CBS_get_asn1(&child, &peer_sha256, CBS_ASN1_OCTETSTRING) ||
+        CBS_len(&peer_sha256) != sizeof(ret->peer_sha256) ||
+        CBS_len(&child) != 0) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
       goto err;
     }
@@ -584,14 +574,35 @@ static SSL_SESSION *SSL_SESSION_parse(CBS *cbs) {
     ret->peer_sha256_valid = 0;
   }
 
-  if (CBS_len(&original_handshake_hash) >
-      sizeof(ret->original_handshake_hash)) {
+  if (!SSL_SESSION_parse_bounded_octet_string(
+          &session, ret->original_handshake_hash,
+          &ret->original_handshake_hash_len,
+          sizeof(ret->original_handshake_hash), kOriginalHandshakeHashTag) ||
+      !SSL_SESSION_parse_octet_string(
+          &session, &ret->tlsext_signed_cert_timestamp_list,
+          &ret->tlsext_signed_cert_timestamp_list_length,
+          kSignedCertTimestampListTag) ||
+      !SSL_SESSION_parse_octet_string(
+          &session, &ret->ocsp_response, &ret->ocsp_response_length,
+          kOCSPResponseTag)) {
+    goto err;
+  }
+
+  int extended_master_secret;
+  if (!CBS_get_optional_asn1_bool(&session, &extended_master_secret,
+                                  kExtendedMasterSecretTag,
+                                  0 /* default to false */)) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
+    goto err;
+  }
+  ret->extended_master_secret = !!extended_master_secret;
+
+  if (!SSL_SESSION_parse_u32(&session, &ret->key_exchange_info,
+                             kKeyExchangeInfoTag, 0) ||
+      CBS_len(&session) != 0) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_SSL_SESSION);
     goto err;
   }
-  memcpy(ret->original_handshake_hash, CBS_data(&original_handshake_hash),
-         CBS_len(&original_handshake_hash));
-  ret->original_handshake_hash_len = CBS_len(&original_handshake_hash);
 
   return ret;