Revise version negotiation logic on the C side.
This is in preparation for upcoming experiments which will require
supporting multiple experimental versions of TLS 1.3 with, on the
server, the ability to enable multiple variants at once. This means the
version <-> wire bijection no longer exists, even when limiting to a
single SSL*. Thus version_to_wire is removed and instead we treat the
wire version as the canonical version value.
There is a mapping from valid wire versions to protocol versions which
describe the high-level handshake protocol in use. This mapping is not
injective, so uses of version_from_wire are rewritten differently.
All the version-munging logic is moved to ssl_versions.c with a master
preference list of all TLS and DTLS versions. The legacy version
negotiation is converted to the new scheme. The version lists and
negotiation are driven by the preference lists and a
ssl_supports_version API.
To simplify the mess around SSL_SESSION and versions, version_from_wire
is now DTLS/TLS-agnostic, with any filtering being done by
ssl_supports_version. This is screwy but allows parsing SSL_SESSIONs to
sanity-check it and reject all bogus versions in SSL_SESSION. This
reduces a mess of error cases.
As part of this, the weird logic where ssl->version is set early when
sending the ClientHello is removed. The one place where we were relying
on this behavior is tweaked to query hs->max_version instead.
Change-Id: Ic91b348481ceba94d9ae06d6781187c11adc15b0
Reviewed-on: https://boringssl-review.googlesource.com/17524
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 1b14371..02f5a30 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -970,14 +970,11 @@
* advertise the extension to avoid potentially breaking servers which carry
* over the state from the previous handshake, such as OpenSSL servers
* without upstream's 3c3f0259238594d77264a78944d409f2127642c4. */
- uint16_t session_version;
if (!ssl->s3->initial_handshake_complete &&
ssl->session != NULL &&
ssl->session->tlsext_tick != NULL &&
/* Don't send TLS 1.3 session tickets in the ticket extension. */
- ssl->method->version_from_wire(&session_version,
- ssl->session->ssl_version) &&
- session_version < TLS1_3_VERSION) {
+ SSL_SESSION_protocol_version(ssl->session) < TLS1_3_VERSION) {
ticket_data = ssl->session->tlsext_tick;
ticket_len = ssl->session->tlsext_ticklen;
}
@@ -1863,31 +1860,19 @@
static size_t ext_pre_shared_key_clienthello_length(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- uint16_t session_version;
if (hs->max_version < TLS1_3_VERSION || ssl->session == NULL ||
- !ssl->method->version_from_wire(&session_version,
- ssl->session->ssl_version) ||
- session_version < TLS1_3_VERSION) {
+ SSL_SESSION_protocol_version(ssl->session) < TLS1_3_VERSION) {
return 0;
}
- const EVP_MD *digest = SSL_SESSION_get_digest(ssl->session, ssl);
- if (digest == NULL) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return 0;
- }
-
- size_t binder_len = EVP_MD_size(digest);
+ size_t binder_len = EVP_MD_size(SSL_SESSION_get_digest(ssl->session));
return 15 + ssl->session->tlsext_ticklen + binder_len;
}
static int ext_pre_shared_key_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
SSL *const ssl = hs->ssl;
- uint16_t session_version;
if (hs->max_version < TLS1_3_VERSION || ssl->session == NULL ||
- !ssl->method->version_from_wire(&session_version,
- ssl->session->ssl_version) ||
- session_version < TLS1_3_VERSION) {
+ SSL_SESSION_protocol_version(ssl->session) < TLS1_3_VERSION) {
return 1;
}
@@ -1899,14 +1884,7 @@
/* Fill in a placeholder zero binder of the appropriate length. It will be
* computed and filled in later after length prefixes are computed. */
uint8_t zero_binder[EVP_MAX_MD_SIZE] = {0};
-
- const EVP_MD *digest = SSL_SESSION_get_digest(ssl->session, ssl);
- if (digest == NULL) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return 0;
- }
-
- size_t binder_len = EVP_MD_size(digest);
+ size_t binder_len = EVP_MD_size(SSL_SESSION_get_digest(ssl->session));
CBB contents, identity, ticket, binders, binder;
if (!CBB_add_u16(out, TLSEXT_TYPE_pre_shared_key) ||
@@ -2071,11 +2049,8 @@
static int ext_early_data_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
SSL *const ssl = hs->ssl;
- uint16_t session_version;
if (ssl->session == NULL ||
- !ssl->method->version_from_wire(&session_version,
- ssl->session->ssl_version) ||
- session_version < TLS1_3_VERSION ||
+ SSL_SESSION_protocol_version(ssl->session) < TLS1_3_VERSION ||
ssl->session->ticket_max_early_data == 0 ||
hs->received_hello_retry_request ||
!ssl->cert->enable_early_data) {
@@ -2375,14 +2350,8 @@
return 0;
}
- for (uint16_t version = hs->max_version; version >= hs->min_version;
- version--) {
- if (!CBB_add_u16(&versions, ssl->method->version_to_wire(version))) {
- return 0;
- }
- }
-
- if (!CBB_flush(out)) {
+ if (!ssl_add_supported_versions(hs, &versions) ||
+ !CBB_flush(out)) {
return 0;
}
