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>
diff --git a/ssl/d1_lib.c b/ssl/d1_lib.c
index 7f08b06..81079f4 100644
--- a/ssl/d1_lib.c
+++ b/ssl/d1_lib.c
@@ -110,11 +110,11 @@
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 1a91e0b..725c4f5 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -163,8 +163,8 @@
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 @@
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 @@
}
*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 ec7a50b..09e527a 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -786,8 +786,8 @@
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 0d4a821..fa25579 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -197,11 +197,11 @@
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 bbe178f..ae709dc 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -871,8 +871,8 @@
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 8eba906..ff77749 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -417,8 +417,6 @@
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 @@
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 39711d5..a64e9d8 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -328,7 +328,7 @@
}
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 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 @@
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 @@
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;
}
