Skip to content

Commit

Permalink
Remove deprecated TLS 1.3 variants.
Browse files Browse the repository at this point in the history
Upgrade-Note: SSL_CTX_set_tls13_variant(tls13_experiment) on the server
should switch to SSL_CTX_set_tls13_variant(tls13_experiment2).
(Configuring any TLS 1.3 variants on the server enables all variants,
so this is a no-op. We're just retiring some old experiments.)
Change-Id: I60f0ca3f96ff84bdf59e1a282a46e51d99047462
Reviewed-on: https://boringssl-review.googlesource.com/23784
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
  • Loading branch information
dvorak42 authored and CQ bot account: commit-bot@chromium.org committed Dec 18, 2017
1 parent ea52ec9 commit 64cc121
Show file tree
Hide file tree
Showing 20 changed files with 189 additions and 422 deletions.
10 changes: 1 addition & 9 deletions include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -591,12 +591,8 @@ OPENSSL_EXPORT int DTLSv1_handle_timeout(SSL *ssl);
#define DTLS1_VERSION 0xfeff
#define DTLS1_2_VERSION 0xfefd

#define TLS1_3_DRAFT_VERSION 0x7f12
#define TLS1_3_DRAFT21_VERSION 0x7f15
#define TLS1_3_DRAFT22_VERSION 0x7f16
#define TLS1_3_EXPERIMENT_VERSION 0x7e01
#define TLS1_3_EXPERIMENT2_VERSION 0x7e02
#define TLS1_3_EXPERIMENT3_VERSION 0x7e03

// SSL_CTX_set_min_proto_version sets the minimum protocol version for |ctx| to
// |version|. If |version| is zero, the default minimum version is used. It
Expand Down Expand Up @@ -3226,11 +3222,7 @@ OPENSSL_EXPORT int SSL_total_renegotiations(const SSL *ssl);

enum tls13_variant_t {
tls13_default = 0,
tls13_experiment = 1,
tls13_experiment2 = 2,
tls13_experiment3 = 3,
tls13_draft21 = 4,
tls13_draft22 = 5,
tls13_experiment2 = 1,
};

// SSL_CTX_set_tls13_variant sets which variant of TLS 1.3 we negotiate. On the
Expand Down
2 changes: 1 addition & 1 deletion ssl/handshake_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,7 @@ static enum ssl_hs_wait_t do_start_connect(SSL_HANDSHAKE *hs) {
hs->session_id_len = ssl->session->session_id_length;
OPENSSL_memcpy(hs->session_id, ssl->session->session_id,
hs->session_id_len);
} else if (ssl_is_resumption_variant(hs->max_version, ssl->tls13_variant)) {
} else if (hs->max_version >= TLS1_3_VERSION) {
hs->session_id_len = sizeof(hs->session_id);
if (!RAND_bytes(hs->session_id, hs->session_id_len)) {
return ssl_hs_error;
Expand Down
4 changes: 2 additions & 2 deletions ssl/s3_pkt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ ssl_open_record_t ssl3_open_app_data(SSL *ssl, Span<uint8_t> *out,
if (type == SSL3_RT_HANDSHAKE) {
// If reading 0-RTT data, reject handshake data. 0-RTT data is terminated
// by an alert.
if (!ssl_is_draft21(ssl->version) && is_early_data_read) {
if (!ssl_is_draft22(ssl->version) && is_early_data_read) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_RECORD);
*out_alert = SSL_AD_UNEXPECTED_MESSAGE;
return ssl_open_record_error;
Expand Down Expand Up @@ -335,7 +335,7 @@ ssl_open_record_t ssl3_open_app_data(SSL *ssl, Span<uint8_t> *out,
// Handle the end_of_early_data alert.
static const uint8_t kEndOfEarlyData[2] = {SSL3_AL_WARNING,
TLS1_AD_END_OF_EARLY_DATA};
if (!ssl_is_draft21(ssl->version) && is_early_data_read &&
if (!ssl_is_draft22(ssl->version) && is_early_data_read &&
type == SSL3_RT_ALERT && body == kEndOfEarlyData) {
// Stop accepting early data.
ssl->s3->hs->can_early_read = false;
Expand Down
5 changes: 1 addition & 4 deletions ssl/ssl_aead_ctx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,7 @@ uint16_t SSLAEADContext::RecordVersion() const {
return version_;
}

if (ssl_is_resumption_record_version_experiment(version_)) {
return TLS1_2_VERSION;
}
return TLS1_VERSION;
return TLS1_2_VERSION;
}

size_t SSLAEADContext::ExplicitNonceLen() const {
Expand Down
9 changes: 4 additions & 5 deletions ssl/ssl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2617,7 +2617,8 @@ TEST(SSLTest, SetVersion) {
EXPECT_EQ(TLS1_3_VERSION, ctx->conf_max_version);

// TLS1_3_DRAFT_VERSION is not an API-level version.
EXPECT_FALSE(SSL_CTX_set_max_proto_version(ctx.get(), TLS1_3_DRAFT_VERSION));
EXPECT_FALSE(
SSL_CTX_set_max_proto_version(ctx.get(), TLS1_3_DRAFT22_VERSION));
ERR_clear_error();

ctx.reset(SSL_CTX_new(DTLS_method()));
Expand Down Expand Up @@ -2960,9 +2961,7 @@ TEST_P(SSLVersionTest, RecordCallback) {
uint16_t record_version, length;
ASSERT_TRUE(CBS_get_u8(&cbs, &type));
ASSERT_TRUE(CBS_get_u16(&cbs, &record_version));
EXPECT_TRUE(record_version == version() ||
record_version == (is_dtls() ? DTLS1_VERSION : TLS1_VERSION))
<< "Invalid record version: " << record_version;
EXPECT_EQ(record_version & 0xff00, version() & 0xff00);
if (is_dtls()) {
uint16_t epoch;
ASSERT_TRUE(CBS_get_u16(&cbs, &epoch));
Expand Down Expand Up @@ -3862,7 +3861,7 @@ TEST(SSLTest, AllTests) {
!TestPaddingExtension(TLS1_3_VERSION, TLS1_2_VERSION) ||
// Test the padding extension at TLS 1.3 with a TLS 1.3 session, so there
// will be a PSK binder after the padding extension.
!TestPaddingExtension(TLS1_3_VERSION, TLS1_3_DRAFT_VERSION)) {
!TestPaddingExtension(TLS1_3_VERSION, TLS1_3_DRAFT22_VERSION)) {
ADD_FAILURE() << "Tests failed";
}
}
Expand Down
80 changes: 6 additions & 74 deletions ssl/ssl_versions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,8 @@ bool ssl_protocol_version_from_wire(uint16_t *out, uint16_t version) {
*out = version;
return true;

case TLS1_3_DRAFT_VERSION:
case TLS1_3_DRAFT21_VERSION:
case TLS1_3_DRAFT22_VERSION:
case TLS1_3_EXPERIMENT_VERSION:
case TLS1_3_EXPERIMENT2_VERSION:
case TLS1_3_EXPERIMENT3_VERSION:
*out = TLS1_3_VERSION;
return true;

Expand All @@ -62,11 +58,7 @@ bool ssl_protocol_version_from_wire(uint16_t *out, uint16_t version) {

static const uint16_t kTLSVersions[] = {
TLS1_3_DRAFT22_VERSION,
TLS1_3_EXPERIMENT3_VERSION,
TLS1_3_EXPERIMENT2_VERSION,
TLS1_3_EXPERIMENT_VERSION,
TLS1_3_DRAFT_VERSION,
TLS1_3_DRAFT21_VERSION,
TLS1_2_VERSION,
TLS1_1_VERSION,
TLS1_VERSION,
Expand Down Expand Up @@ -109,12 +101,8 @@ static bool method_supports_version(const SSL_PROTOCOL_METHOD *method,

static const char *ssl_version_to_string(uint16_t version) {
switch (version) {
case TLS1_3_DRAFT_VERSION:
case TLS1_3_DRAFT21_VERSION:
case TLS1_3_DRAFT22_VERSION:
case TLS1_3_EXPERIMENT_VERSION:
case TLS1_3_EXPERIMENT2_VERSION:
case TLS1_3_EXPERIMENT3_VERSION:
return "TLSv1.3";

case TLS1_2_VERSION:
Expand Down Expand Up @@ -143,12 +131,8 @@ static const char *ssl_version_to_string(uint16_t version) {
static uint16_t wire_version_to_api(uint16_t version) {
switch (version) {
// Report TLS 1.3 draft versions as TLS 1.3 in the public API.
case TLS1_3_DRAFT_VERSION:
case TLS1_3_DRAFT21_VERSION:
case TLS1_3_DRAFT22_VERSION:
case TLS1_3_EXPERIMENT_VERSION:
case TLS1_3_EXPERIMENT2_VERSION:
case TLS1_3_EXPERIMENT3_VERSION:
return TLS1_3_VERSION;
default:
return version;
Expand All @@ -159,16 +143,12 @@ static uint16_t wire_version_to_api(uint16_t version) {
// particular, it picks an arbitrary TLS 1.3 representative. This should only be
// used in context where that does not matter.
static bool api_version_to_wire(uint16_t *out, uint16_t version) {
if (version == TLS1_3_DRAFT_VERSION ||
version == TLS1_3_DRAFT21_VERSION ||
version == TLS1_3_DRAFT22_VERSION ||
version == TLS1_3_EXPERIMENT_VERSION ||
version == TLS1_3_EXPERIMENT2_VERSION ||
version == TLS1_3_EXPERIMENT3_VERSION) {
if (version == TLS1_3_DRAFT22_VERSION ||
version == TLS1_3_EXPERIMENT2_VERSION) {
return false;
}
if (version == TLS1_3_VERSION) {
version = TLS1_3_DRAFT_VERSION;
version = TLS1_3_DRAFT22_VERSION;
}

// Check it is a real protocol version.
Expand Down Expand Up @@ -321,32 +301,16 @@ bool ssl_supports_version(SSL_HANDSHAKE *hs, uint16_t version) {

// TLS 1.3 variants must additionally match |tls13_variant|.
if (protocol_version != TLS1_3_VERSION ||
(ssl->tls13_variant == tls13_experiment &&
version == TLS1_3_EXPERIMENT_VERSION) ||
(ssl->tls13_variant == tls13_experiment2 &&
version == TLS1_3_EXPERIMENT2_VERSION) ||
(ssl->tls13_variant == tls13_experiment3 &&
version == TLS1_3_EXPERIMENT3_VERSION) ||
(ssl->tls13_variant == tls13_draft21 &&
version == TLS1_3_DRAFT21_VERSION) ||
(ssl->tls13_variant == tls13_draft22 &&
version == TLS1_3_DRAFT22_VERSION) ||
(ssl->tls13_variant == tls13_default &&
version == TLS1_3_DRAFT_VERSION)) {
version == TLS1_3_DRAFT22_VERSION)) {
return true;
}

// The server, when not configured at |tls13_default|, should additionally
// enable all variants, except draft-21 which is implemented solely for QUIC
// interop testing and will not be deployed, and draft-22 which will be
// enabled once the draft is finalized and ready to be deployed in Chrome.
// Currently, this is to implement the draft-18 vs. experiments field trials.
// In the future, this will be to transition cleanly to a final draft-22
// which hopefully includes the deployability fixes.
if (ssl->server &&
ssl->tls13_variant != tls13_default &&
version != TLS1_3_DRAFT21_VERSION &&
version != TLS1_3_DRAFT22_VERSION) {
// enable all variants.
if (ssl->server && ssl->tls13_variant != tls13_default) {
return true;
}

Expand Down Expand Up @@ -397,42 +361,10 @@ bool ssl_negotiate_version(SSL_HANDSHAKE *hs, uint8_t *out_alert,
return false;
}

bool ssl_is_draft21(uint16_t version) {
return version == TLS1_3_DRAFT21_VERSION || version == TLS1_3_DRAFT22_VERSION;
}

bool ssl_is_draft22(uint16_t version) {
return version == TLS1_3_DRAFT22_VERSION;
}

bool ssl_is_resumption_experiment(uint16_t version) {
return version == TLS1_3_EXPERIMENT_VERSION ||
version == TLS1_3_EXPERIMENT2_VERSION ||
version == TLS1_3_EXPERIMENT3_VERSION ||
version == TLS1_3_DRAFT22_VERSION;
}

bool ssl_is_resumption_variant(uint16_t max_version,
enum tls13_variant_t variant) {
if (max_version < TLS1_3_VERSION) {
return false;
}
return variant == tls13_experiment || variant == tls13_experiment2 ||
variant == tls13_experiment3 || variant == tls13_draft22;
}

bool ssl_is_resumption_client_ccs_experiment(uint16_t version) {
return version == TLS1_3_EXPERIMENT_VERSION ||
version == TLS1_3_EXPERIMENT2_VERSION ||
version == TLS1_3_DRAFT22_VERSION;
}

bool ssl_is_resumption_record_version_experiment(uint16_t version) {
return version == TLS1_3_EXPERIMENT2_VERSION ||
version == TLS1_3_EXPERIMENT3_VERSION ||
version == TLS1_3_DRAFT22_VERSION;
}

} // namespace bssl

using namespace bssl;
Expand Down
2 changes: 1 addition & 1 deletion ssl/t1_lib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1810,7 +1810,7 @@ static bool ext_pre_shared_key_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
// selected cipher in HelloRetryRequest does not match. This avoids performing
// the transcript hash transformation for multiple hashes.
if (hs->received_hello_retry_request &&
ssl_is_draft21(ssl->version) &&
ssl_is_draft22(ssl->version) &&
ssl->session->cipher->algorithm_prf != hs->new_cipher->algorithm_prf) {
return true;
}
Expand Down
46 changes: 5 additions & 41 deletions ssl/test/runner/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,30 +33,18 @@ const (

// A draft version of TLS 1.3 that is sent over the wire for the current draft.
const (
tls13DraftVersion = 0x7f12
tls13Draft21Version = 0x7f15
tls13ExperimentVersion = 0x7e01
tls13Experiment2Version = 0x7e02
tls13Experiment3Version = 0x7e03
tls13Draft22Version = 0x7f16
)

const (
TLS13Default = 0
TLS13Experiment = 1
TLS13Experiment2 = 2
TLS13Experiment3 = 3
TLS13Draft21 = 4
TLS13Draft22 = 5
TLS13Draft22 = 0
TLS13Experiment2 = 1
)

var allTLSWireVersions = []uint16{
tls13DraftVersion,
tls13Draft22Version,
tls13Draft21Version,
tls13Experiment3Version,
tls13Experiment2Version,
tls13ExperimentVersion,
VersionTLS12,
VersionTLS11,
VersionTLS10,
Expand Down Expand Up @@ -1637,48 +1625,24 @@ func wireToVersion(vers uint16, isDTLS bool) (uint16, bool) {
switch vers {
case VersionSSL30, VersionTLS10, VersionTLS11, VersionTLS12:
return vers, true
case tls13DraftVersion, tls13Draft22Version, tls13Draft21Version, tls13ExperimentVersion, tls13Experiment2Version, tls13Experiment3Version:
case tls13Draft22Version, tls13Experiment2Version:
return VersionTLS13, true
}
}

return 0, false
}

func isDraft21(vers uint16) bool {
return vers == tls13Draft21Version || vers == tls13Draft22Version
}

func isDraft22(vers uint16) bool {
return vers == tls13Draft22Version
}

func isResumptionExperiment(vers uint16) bool {
return vers == tls13ExperimentVersion || vers == tls13Experiment2Version || vers == tls13Experiment3Version || vers == tls13Draft22Version
}

func isResumptionClientCCSExperiment(vers uint16) bool {
return vers == tls13ExperimentVersion || vers == tls13Experiment2Version || vers == tls13Draft22Version
}

func isResumptionRecordVersionExperiment(vers uint16) bool {
return vers == tls13Experiment2Version || vers == tls13Experiment3Version || vers == tls13Draft22Version
}

func isResumptionRecordVersionVariant(variant int) bool {
return variant == TLS13Experiment2 || variant == TLS13Experiment3 || variant == TLS13Draft22
}

// isSupportedVersion checks if the specified wire version is acceptable. If so,
// it returns true and the corresponding protocol version. Otherwise, it returns
// false.
func (c *Config) isSupportedVersion(wireVers uint16, isDTLS bool) (uint16, bool) {
if (c.TLS13Variant != TLS13Experiment && wireVers == tls13ExperimentVersion) ||
(c.TLS13Variant != TLS13Experiment2 && wireVers == tls13Experiment2Version) ||
(c.TLS13Variant != TLS13Experiment3 && wireVers == tls13Experiment3Version) ||
(c.TLS13Variant != TLS13Draft22 && wireVers == tls13Draft22Version) ||
(c.TLS13Variant != TLS13Draft21 && wireVers == tls13Draft21Version) ||
(c.TLS13Variant != TLS13Default && wireVers == tls13DraftVersion) {
if (c.TLS13Variant != TLS13Experiment2 && wireVers == tls13Experiment2Version) ||
(c.TLS13Variant != TLS13Draft22 && wireVers == tls13Draft22Version) {
return 0, false
}

Expand Down
Loading

0 comments on commit 64cc121

Please sign in to comment.