Don't initialize enc_method before version negotiation.

Move it into ssl->s3 so it automatically behaves correctly on SSL_clear.
ssl->version is still a mess though.

Change-Id: I17a692a04a845886ec4f8de229fa6cf99fa7e24a
Reviewed-on: https://boringssl-review.googlesource.com/6844
Reviewed-by: Adam Langley <alangley@gmail.com>

7 files changed, 38 insertions, 40 deletions

diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 7f08b067..81079f48 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -110,11 +110,11 @@ int dtls1_new(SSL *ssl) {
 
   ssl->d1 = d1;
 
-  /* Set the version to the highest version for DTLS. This controls the initial
-   * state of |ssl->enc_method| and what the API reports as the version prior to
-   * negotiation.
+  /* Set the version to the highest supported version.
    *
-   * TODO(davidben): This is fragile and confusing. */
+   * TODO(davidben): Move this field into |s3|, have it store the normalized
+   * protocol version, and implement this pre-negotiation quirk in |SSL_version|
+   * at the API boundary rather than in internal state. */
   ssl->version = DTLS1_2_VERSION;
   return 1;
 }
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 1a91e0bd..725c4f53 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -163,8 +163,8 @@ int ssl3_send_finished(SSL *ssl, int a, int b) {
   if (ssl->state == a) {
     p = ssl_handshake_start(ssl);
 
-    n = ssl->enc_method->final_finish_mac(ssl, ssl->server,
-                                          ssl->s3->tmp.finish_md);
+    n = ssl->s3->enc_method->final_finish_mac(ssl, ssl->server,
+                                              ssl->s3->tmp.finish_md);
     if (n == 0) {
       return 0;
     }
@@ -208,7 +208,7 @@ static void ssl3_take_mac(SSL *ssl) {
     return;
   }
 
-  ssl->s3->tmp.peer_finish_md_len = ssl->enc_method->final_finish_mac(
+  ssl->s3->tmp.peer_finish_md_len = ssl->s3->enc_method->final_finish_mac(
       ssl, !ssl->server, ssl->s3->tmp.peer_finish_md);
 }
 
@@ -449,15 +449,15 @@ int ssl3_cert_verify_hash(SSL *ssl, uint8_t *out, size_t *out_len,
     }
     *out_len = len;
   } else if (pkey_type == EVP_PKEY_RSA) {
-    if (ssl->enc_method->cert_verify_mac(ssl, NID_md5, out) == 0 ||
-        ssl->enc_method->cert_verify_mac(ssl, NID_sha1,
-                                         out + MD5_DIGEST_LENGTH) == 0) {
+    if (ssl->s3->enc_method->cert_verify_mac(ssl, NID_md5, out) == 0 ||
+        ssl->s3->enc_method->cert_verify_mac(ssl, NID_sha1,
+                                             out + MD5_DIGEST_LENGTH) == 0) {
       return 0;
     }
     *out_len = MD5_DIGEST_LENGTH + SHA_DIGEST_LENGTH;
     *out_md = EVP_md5_sha1();
   } else if (pkey_type == EVP_PKEY_EC) {
-    if (ssl->enc_method->cert_verify_mac(ssl, NID_sha1, out) == 0) {
+    if (ssl->s3->enc_method->cert_verify_mac(ssl, NID_sha1, out) == 0) {
       return 0;
     }
     *out_len = SHA_DIGEST_LENGTH;
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index ec7a50b6..09e527a0 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -786,8 +786,8 @@ int ssl3_get_server_hello(SSL *ssl) {
       goto f_err;
     }
     ssl->version = server_version;
-    ssl->enc_method = ssl3_get_enc_method(server_version);
-    assert(ssl->enc_method != NULL);
+    ssl->s3->enc_method = ssl3_get_enc_method(server_version);
+    assert(ssl->s3->enc_method != NULL);
     /* At this point, the connection's version is known and ssl->version is
      * fixed. Begin enforcing the record-layer version. */
     ssl->s3->have_version = 1;
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 0d4a8219..fa255793 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -197,11 +197,11 @@ int ssl3_new(SSL *ssl) {
 
   ssl->s3 = s3;
 
-  /* Set the version to the highest supported version for TLS. This controls the
-   * initial state of |ssl->enc_method| and what the API reports as the version
-   * prior to negotiation.
+  /* Set the version to the highest supported version.
    *
-   * TODO(davidben): This is fragile and confusing. */
+   * TODO(davidben): Move this field into |s3|, have it store the normalized
+   * protocol version, and implement this pre-negotiation quirk in |SSL_version|
+   * at the API boundary rather than in internal state. */
   ssl->version = TLS1_2_VERSION;
   return 1;
 err:
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index bbe178fd..ae709dc3 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -871,8 +871,8 @@ int ssl3_get_client_hello(SSL *ssl) {
       goto f_err;
     }
     ssl->version = version;
-    ssl->enc_method = ssl3_get_enc_method(version);
-    assert(ssl->enc_method != NULL);
+    ssl->s3->enc_method = ssl3_get_enc_method(version);
+    assert(ssl->s3->enc_method != NULL);
     /* At this point, the connection's version is known and |ssl->version| is
      * fixed. Begin enforcing the record-layer version. */
     ssl->s3->have_version = 1;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 8eba906a..ff777499 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -417,8 +417,6 @@ SSL *SSL_new(SSL_CTX *ctx) {
   if (!ssl->method->ssl_new(ssl)) {
     goto err;
   }
-  ssl->enc_method = ssl3_get_enc_method(ssl->version);
-  assert(ssl->enc_method != NULL);
 
   ssl->rwstate = SSL_NOTHING;
 
@@ -2630,8 +2628,6 @@ int SSL_clear(SSL *ssl) {
   if (!ssl->method->ssl_new(ssl)) {
     return 0;
   }
-  ssl->enc_method = ssl3_get_enc_method(ssl->version);
-  assert(ssl->enc_method != NULL);
 
   if (SSL_IS_DTLS(ssl) && (SSL_get_options(ssl) & SSL_OP_NO_QUERY_MTU)) {
     ssl->d1->mtu = mtu;
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 39711d51..a64e9d82 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -328,7 +328,7 @@ size_t SSL_get_key_block_len(const SSL *ssl) {
 }
 
 int SSL_generate_key_block(const SSL *ssl, uint8_t *out, size_t out_len) {
-  return ssl->enc_method->prf(
+  return ssl->s3->enc_method->prf(
       ssl, out, out_len, ssl->session->master_key,
       ssl->session->master_key_length, TLS_MD_KEY_EXPANSION_CONST,
       TLS_MD_KEY_EXPANSION_CONST_SIZE, ssl->s3->server_random, SSL3_RANDOM_SIZE,
@@ -478,9 +478,10 @@ static int tls1_final_finish_mac(SSL *ssl, int from_server, uint8_t *out) {
   }
 
   static const size_t kFinishedLen = 12;
-  if (!ssl->enc_method->prf(ssl, out, kFinishedLen, ssl->session->master_key,
-                            ssl->session->master_key_length, label, label_len,
-                            buf, digests_len, NULL, 0)) {
+  if (!ssl->s3->enc_method->prf(ssl, out, kFinishedLen,
+                                ssl->session->master_key,
+                                ssl->session->master_key_length, label,
+                                label_len, buf, digests_len, NULL, 0)) {
     return 0;
   }
 
@@ -497,19 +498,19 @@ int tls1_generate_master_secret(SSL *ssl, uint8_t *out,
       return 0;
     }
 
-    if (!ssl->enc_method->prf(ssl, out, SSL3_MASTER_SECRET_SIZE, premaster,
-                              premaster_len,
-                              TLS_MD_EXTENDED_MASTER_SECRET_CONST,
-                              TLS_MD_EXTENDED_MASTER_SECRET_CONST_SIZE, digests,
-                              digests_len, NULL, 0)) {
+    if (!ssl->s3->enc_method->prf(ssl, out, SSL3_MASTER_SECRET_SIZE, premaster,
+                                  premaster_len,
+                                  TLS_MD_EXTENDED_MASTER_SECRET_CONST,
+                                  TLS_MD_EXTENDED_MASTER_SECRET_CONST_SIZE,
+                                  digests, digests_len, NULL, 0)) {
       return 0;
     }
   } else {
-    if (!ssl->enc_method->prf(ssl, out, SSL3_MASTER_SECRET_SIZE, premaster,
-                              premaster_len, TLS_MD_MASTER_SECRET_CONST,
-                              TLS_MD_MASTER_SECRET_CONST_SIZE,
-                              ssl->s3->client_random, SSL3_RANDOM_SIZE,
-                              ssl->s3->server_random, SSL3_RANDOM_SIZE)) {
+    if (!ssl->s3->enc_method->prf(ssl, out, SSL3_MASTER_SECRET_SIZE, premaster,
+                                  premaster_len, TLS_MD_MASTER_SECRET_CONST,
+                                  TLS_MD_MASTER_SECRET_CONST_SIZE,
+                                  ssl->s3->client_random, SSL3_RANDOM_SIZE,
+                                  ssl->s3->server_random, SSL3_RANDOM_SIZE)) {
       return 0;
     }
   }
@@ -547,9 +548,10 @@ int SSL_export_keying_material(SSL *ssl, uint8_t *out, size_t out_len,
     memcpy(seed + 2 * SSL3_RANDOM_SIZE + 2, context, context_len);
   }
 
-  int ret = ssl->enc_method->prf(ssl, out, out_len, ssl->session->master_key,
-                                 ssl->session->master_key_length, label,
-                                 label_len, seed, seed_len, NULL, 0);
+  int ret =
+      ssl->s3->enc_method->prf(ssl, out, out_len, ssl->session->master_key,
+                               ssl->session->master_key_length, label,
+                               label_len, seed, seed_len, NULL, 0);
   OPENSSL_free(seed);
   return ret;
 }