Update key share extension number for draft23.
Change-Id: I7561fc7e04d726ea9e26f645da10e45b62a20627
Reviewed-on: https://boringssl-review.googlesource.com/24704
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>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 6ab1682..92a502d 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -592,6 +592,7 @@
#define DTLS1_2_VERSION 0xfefd
#define TLS1_3_DRAFT22_VERSION 0x7f16
+#define TLS1_3_DRAFT23_VERSION 0x7f17
#define TLS1_3_EXPERIMENT2_VERSION 0x7e02
// SSL_CTX_set_min_proto_version sets the minimum protocol version for |ctx| to
@@ -3228,6 +3229,7 @@
enum tls13_variant_t {
tls13_default = 0,
tls13_experiment2 = 1,
+ tls13_draft22 = 2,
};
// SSL_CTX_set_tls13_variant sets which variant of TLS 1.3 we negotiate. On the
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index f62ee1f..2238043 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -207,7 +207,7 @@
// ExtensionType values from draft-ietf-tls-tls13-18
#define TLSEXT_TYPE_supported_groups 10
-#define TLSEXT_TYPE_key_share 40
+#define TLSEXT_TYPE_old_key_share 40
#define TLSEXT_TYPE_pre_shared_key 41
#define TLSEXT_TYPE_early_data 42
#define TLSEXT_TYPE_supported_versions 43
@@ -215,6 +215,7 @@
#define TLSEXT_TYPE_psk_key_exchange_modes 45
#define TLSEXT_TYPE_ticket_early_data_info 46
#define TLSEXT_TYPE_certificate_authorities 47
+#define TLSEXT_TYPE_new_key_share 51
// ExtensionType value from RFC5746
#define TLSEXT_TYPE_renegotiate 0xff01
diff --git a/ssl/internal.h b/ssl/internal.h
index d59ecc4..bbefd48 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -392,32 +392,17 @@
// call this function before the version is determined.
uint16_t ssl_protocol_version(const SSL *ssl);
-// ssl_is_draft21 returns whether the version corresponds to a draft21 TLS 1.3
-// variant.
-bool ssl_is_draft21(uint16_t version);
-
// ssl_is_draft22 returns whether the version corresponds to a draft22 TLS 1.3
// variant.
bool ssl_is_draft22(uint16_t version);
-// ssl_is_resumption_experiment returns whether the version corresponds to a
-// TLS 1.3 resumption experiment.
-bool ssl_is_resumption_experiment(uint16_t version);
+// ssl_is_draft23 returns whether the version corresponds to a draft23 TLS 1.3
+// variant.
+bool ssl_is_draft23(uint16_t version);
-// ssl_is_resumption_variant returns whether the variant corresponds to a
-// TLS 1.3 resumption experiment.
-bool ssl_is_resumption_variant(uint16_t max_version,
- enum tls13_variant_t variant);
-
-// ssl_is_resumption_client_ccs_experiment returns whether the version
-// corresponds to a TLS 1.3 resumption experiment that sends a client CCS.
-bool ssl_is_resumption_client_ccs_experiment(uint16_t version);
-
-// ssl_is_resumption_record_version_experiment returns whether the version
-// corresponds to a TLS 1.3 resumption experiment that modifies the record
-// version.
-bool ssl_is_resumption_record_version_experiment(uint16_t version);
-
+// ssl_is_draft23_variant returns whether the variant corresponds to a
+// draft23 TLS 1.3 variant.
+ bool ssl_is_draft23_variant(enum tls13_variant_t variant);
// Cipher suites.
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 5d37448..a2a53f6 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -3861,7 +3861,7 @@
!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_DRAFT22_VERSION)) {
+ !TestPaddingExtension(TLS1_3_VERSION, TLS1_3_DRAFT23_VERSION)) {
ADD_FAILURE() << "Tests failed";
}
}
diff --git a/ssl/ssl_versions.cc b/ssl/ssl_versions.cc
index 4ef54da..15012dc 100644
--- a/ssl/ssl_versions.cc
+++ b/ssl/ssl_versions.cc
@@ -35,6 +35,7 @@
return true;
case TLS1_3_DRAFT22_VERSION:
+ case TLS1_3_DRAFT23_VERSION:
case TLS1_3_EXPERIMENT2_VERSION:
*out = TLS1_3_VERSION;
return true;
@@ -57,6 +58,7 @@
// decreasing preference.
static const uint16_t kTLSVersions[] = {
+ TLS1_3_DRAFT23_VERSION,
TLS1_3_DRAFT22_VERSION,
TLS1_3_EXPERIMENT2_VERSION,
TLS1_2_VERSION,
@@ -102,6 +104,7 @@
static const char *ssl_version_to_string(uint16_t version) {
switch (version) {
case TLS1_3_DRAFT22_VERSION:
+ case TLS1_3_DRAFT23_VERSION:
case TLS1_3_EXPERIMENT2_VERSION:
return "TLSv1.3";
@@ -132,6 +135,7 @@
switch (version) {
// Report TLS 1.3 draft versions as TLS 1.3 in the public API.
case TLS1_3_DRAFT22_VERSION:
+ case TLS1_3_DRAFT23_VERSION:
case TLS1_3_EXPERIMENT2_VERSION:
return TLS1_3_VERSION;
default:
@@ -144,6 +148,7 @@
// used in context where that does not matter.
static bool api_version_to_wire(uint16_t *out, uint16_t version) {
if (version == TLS1_3_DRAFT22_VERSION ||
+ version == TLS1_3_DRAFT23_VERSION ||
version == TLS1_3_EXPERIMENT2_VERSION) {
return false;
}
@@ -303,8 +308,10 @@
if (protocol_version != TLS1_3_VERSION ||
(ssl->tls13_variant == tls13_experiment2 &&
version == TLS1_3_EXPERIMENT2_VERSION) ||
+ (ssl->tls13_variant == tls13_draft22 &&
+ version == TLS1_3_DRAFT22_VERSION) ||
(ssl->tls13_variant == tls13_default &&
- version == TLS1_3_DRAFT22_VERSION)) {
+ version == TLS1_3_DRAFT23_VERSION)) {
return true;
}
@@ -362,7 +369,15 @@
}
bool ssl_is_draft22(uint16_t version) {
- return version == TLS1_3_DRAFT22_VERSION;
+ return version == TLS1_3_DRAFT22_VERSION || version == TLS1_3_DRAFT23_VERSION;
+}
+
+bool ssl_is_draft23(uint16_t version) {
+ return version == TLS1_3_DRAFT23_VERSION;
+}
+
+bool ssl_is_draft23_variant(tls13_variant_t variant) {
+ return variant == tls13_default;
}
} // namespace bssl
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 814cce1..4b3de43 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -549,6 +549,10 @@
return true;
}
+static bool dont_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
+ return true;
+}
+
static bool ignore_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
CBS *contents) {
// This extension from the client is handled elsewhere.
@@ -2080,7 +2084,9 @@
}
CBB contents, kse_bytes;
- if (!CBB_add_u16(out, TLSEXT_TYPE_key_share) ||
+ if (!CBB_add_u16(out, ssl_is_draft23_variant(ssl->tls13_variant)
+ ? TLSEXT_TYPE_new_key_share
+ : TLSEXT_TYPE_old_key_share) ||
!CBB_add_u16_length_prefixed(out, &contents) ||
!CBB_add_u16_length_prefixed(&contents, &kse_bytes)) {
return false;
@@ -2237,7 +2243,9 @@
uint16_t group_id;
CBB kse_bytes, public_key;
if (!tls1_get_shared_group(hs, &group_id) ||
- !CBB_add_u16(out, TLSEXT_TYPE_key_share) ||
+ !CBB_add_u16(out, ssl_is_draft23(hs->ssl->version)
+ ? TLSEXT_TYPE_new_key_share
+ : TLSEXT_TYPE_old_key_share) ||
!CBB_add_u16_length_prefixed(out, &kse_bytes) ||
!CBB_add_u16(&kse_bytes, group_id) ||
!CBB_add_u16_length_prefixed(&kse_bytes, &public_key) ||
@@ -2491,7 +2499,16 @@
ext_ec_point_add_serverhello,
},
{
- TLSEXT_TYPE_key_share,
+ TLSEXT_TYPE_old_key_share,
+ // This is added by TLSEXT_TYPE_new_key_share's callback.
+ NULL,
+ dont_add_clienthello,
+ forbid_parse_serverhello,
+ ignore_parse_clienthello,
+ dont_add_serverhello,
+ },
+ {
+ TLSEXT_TYPE_new_key_share,
NULL,
ext_key_share_add_clienthello,
forbid_parse_serverhello,
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 51effed..cd04acf 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -35,14 +35,17 @@
const (
tls13Experiment2Version = 0x7e02
tls13Draft22Version = 0x7f16
+ tls13Draft23Version = 0x7f17
)
const (
- TLS13Draft22 = 0
+ TLS13Draft23 = 0
TLS13Experiment2 = 1
+ TLS13Draft22 = 2
)
var allTLSWireVersions = []uint16{
+ tls13Draft23Version,
tls13Draft22Version,
tls13Experiment2Version,
VersionTLS12,
@@ -120,7 +123,7 @@
extensionPadding uint16 = 21
extensionExtendedMasterSecret uint16 = 23
extensionSessionTicket uint16 = 35
- extensionKeyShare uint16 = 40 // draft-ietf-tls-tls13-16
+ extensionOldKeyShare uint16 = 40 // draft-ietf-tls-tls13-16
extensionPreSharedKey uint16 = 41 // draft-ietf-tls-tls13-16
extensionEarlyData uint16 = 42 // draft-ietf-tls-tls13-16
extensionSupportedVersions uint16 = 43 // draft-ietf-tls-tls13-16
@@ -128,6 +131,7 @@
extensionPSKKeyExchangeModes uint16 = 45 // draft-ietf-tls-tls13-18
extensionTicketEarlyDataInfo uint16 = 46 // draft-ietf-tls-tls13-18
extensionCertificateAuthorities uint16 = 47 // draft-ietf-tls-tls13-21
+ extensionNewKeyShare uint16 = 51 // draft-ietf-tls-tls13-23
extensionCustom uint16 = 1234 // not IANA assigned
extensionNextProtoNeg uint16 = 13172 // not IANA assigned
extensionRenegotiationInfo uint16 = 0xff01
@@ -1633,7 +1637,7 @@
switch vers {
case VersionSSL30, VersionTLS10, VersionTLS11, VersionTLS12:
return vers, true
- case tls13Draft22Version, tls13Experiment2Version:
+ case tls13Draft23Version, tls13Draft22Version, tls13Experiment2Version:
return VersionTLS13, true
}
}
@@ -1642,7 +1646,11 @@
}
func isDraft22(vers uint16) bool {
- return vers == tls13Draft22Version
+ return vers == tls13Draft22Version || vers == tls13Draft23Version
+}
+
+func isDraft23(vers uint16) bool {
+ return vers == tls13Draft23Version
}
// isSupportedVersion checks if the specified wire version is acceptable. If so,
@@ -1650,6 +1658,7 @@
// false.
func (c *Config) isSupportedVersion(wireVers uint16, isDTLS bool) (uint16, bool) {
if (c.TLS13Variant != TLS13Experiment2 && wireVers == tls13Experiment2Version) ||
+ (c.TLS13Variant != TLS13Draft23 && wireVers == tls13Draft23Version) ||
(c.TLS13Variant != TLS13Draft22 && wireVers == tls13Draft22Version) {
return 0, false
}
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 0f4a714..e477edf 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -159,6 +159,11 @@
if maxVersion >= VersionTLS13 {
keyShares = make(map[CurveID]ecdhCurve)
hello.hasKeyShares = true
+ if c.config.TLS13Variant == TLS13Draft23 {
+ hello.keyShareExtension = extensionNewKeyShare
+ } else {
+ hello.keyShareExtension = extensionOldKeyShare
+ }
hello.trailingKeyShareData = c.config.Bugs.TrailingKeyShareData
curvesToSend := c.config.defaultCurves()
for _, curveID := range hello.supportedCurves {
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index c4a6e16..b650393 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -265,6 +265,7 @@
supportedCurves []CurveID
supportedPoints []uint8
hasKeyShares bool
+ keyShareExtension uint16
keyShares []keyShareEntry
trailingKeyShareData bool
pskIdentities []pskIdentity
@@ -444,7 +445,7 @@
supportedPoints.addBytes(m.supportedPoints)
}
if m.hasKeyShares {
- extensions.addU16(extensionKeyShare)
+ extensions.addU16(m.keyShareExtension)
keyShareList := extensions.addU16LengthPrefixed()
keyShares := keyShareList.addU16LengthPrefixed()
@@ -719,7 +720,12 @@
// http://tools.ietf.org/html/rfc5077#section-3.2
m.ticketSupported = true
m.sessionTicket = []byte(body)
- case extensionKeyShare:
+ case extensionOldKeyShare, extensionNewKeyShare:
+ // We assume the client only supports one of draft-22 or draft-23.
+ if m.keyShareExtension != 0 {
+ return false
+ }
+ m.keyShareExtension = extension
// draft-ietf-tls-tls13 section 6.3.2.3
var keyShares byteReader
if !body.readU16LengthPrefixed(&keyShares) || len(body) != 0 {
@@ -912,7 +918,11 @@
if vers >= VersionTLS13 {
if m.hasKeyShare {
- extensions.addU16(extensionKeyShare)
+ if isDraft23(m.vers) {
+ extensions.addU16(extensionNewKeyShare)
+ } else {
+ extensions.addU16(extensionOldKeyShare)
+ }
keyShare := extensions.addU16LengthPrefixed()
keyShare.addU16(uint16(m.keyShare.group))
keyExchange := keyShare.addU16LengthPrefixed()
@@ -1015,6 +1025,10 @@
}
if vers >= VersionTLS13 {
+ extensionKeyShare := extensionOldKeyShare
+ if isDraft23(m.vers) {
+ extensionKeyShare = extensionNewKeyShare
+ }
for len(extensions) > 0 {
var extension uint16
var body byteReader
@@ -1195,7 +1209,7 @@
}
}
if m.hasKeyShare {
- extensions.addU16(extensionKeyShare)
+ extensions.addU16(extensionOldKeyShare)
keyShare := extensions.addU16LengthPrefixed()
keyShare.addU16(uint16(m.keyShare.group))
keyExchange := keyShare.addU16LengthPrefixed()
@@ -1388,7 +1402,11 @@
extensions.addU16(m.vers)
}
if m.hasSelectedGroup {
- extensions.addU16(extensionKeyShare)
+ if isDraft23(m.vers) {
+ extensions.addU16(extensionNewKeyShare)
+ } else {
+ extensions.addU16(extensionOldKeyShare)
+ }
extensions.addU16(2) // length
extensions.addU16(uint16(m.selectedGroup))
}
@@ -1431,11 +1449,12 @@
if !reader.readU16LengthPrefixed(&extensions) || len(reader) != 0 {
return false
}
- for len(extensions) > 0 {
+ extensionsCopy := extensions
+ for len(extensionsCopy) > 0 {
var extension uint16
var body byteReader
- if !extensions.readU16(&extension) ||
- !extensions.readU16LengthPrefixed(&body) {
+ if !extensionsCopy.readU16(&extension) ||
+ !extensionsCopy.readU16LengthPrefixed(&body) {
return false
}
switch extension {
@@ -1445,6 +1464,23 @@
len(body) != 0 {
return false
}
+ default:
+ }
+ }
+ extensionKeyShare := extensionOldKeyShare
+ if isDraft23(m.vers) {
+ extensionKeyShare = extensionNewKeyShare
+ }
+ for len(extensions) > 0 {
+ var extension uint16
+ var body byteReader
+ if !extensions.readU16(&extension) ||
+ !extensions.readU16LengthPrefixed(&body) {
+ return false
+ }
+ switch extension {
+ case extensionSupportedVersions:
+ // Parsed above.
case extensionKeyShare:
var v uint16
if !body.readU16(&v) || len(body) != 0 {
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 6b7daee..abfab91 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -280,6 +280,14 @@
}
}
+ // Check that we received the expected version of the key_share extension.
+ if c.vers >= VersionTLS13 {
+ if (isDraft23(c.wireVersion) && hs.clientHello.keyShareExtension != extensionNewKeyShare) ||
+ (!isDraft23(c.wireVersion) && hs.clientHello.keyShareExtension != extensionOldKeyShare) {
+ return fmt.Errorf("tls: client offered wrong key_share extension")
+ }
+ }
+
if config.Bugs.ExpectNoTLS12Session {
if len(hs.clientHello.sessionId) > 0 && c.vers >= VersionTLS13 {
return fmt.Errorf("tls: client offered an unexpected session ID")
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 7506280..74e060c 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5306,7 +5306,7 @@
expectedClientVersion := expectedVersion
if expectedVersion == VersionTLS13 && runnerVers.tls13Variant != shimVers.tls13Variant {
expectedClientVersion = VersionTLS12
- if shimVers.tls13Variant == TLS13Draft22 {
+ if shimVers.tls13Variant == TLS13Draft23 {
expectedServerVersion = VersionTLS12
}
}
@@ -5516,7 +5516,7 @@
name: "IgnoreClientVersionOrder",
config: Config{
Bugs: ProtocolBugs{
- SendSupportedVersions: []uint16{VersionTLS12, tls13Draft22Version},
+ SendSupportedVersions: []uint16{VersionTLS12, tls13Draft23Version},
},
},
expectedVersion: VersionTLS13,
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 8d46d9f..f437519 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -136,8 +136,10 @@
bool have_cookie, have_key_share, have_supported_versions;
CBS cookie, key_share, supported_versions;
- const SSL_EXTENSION_TYPE ext_types[] = {
- {TLSEXT_TYPE_key_share, &have_key_share, &key_share},
+ SSL_EXTENSION_TYPE ext_types[] = {
+ {ssl_is_draft23(ssl->version) ? (uint16_t)TLSEXT_TYPE_new_key_share
+ : (uint16_t)TLSEXT_TYPE_old_key_share,
+ &have_key_share, &key_share},
{TLSEXT_TYPE_cookie, &have_cookie, &cookie},
{TLSEXT_TYPE_supported_versions, &have_supported_versions,
&supported_versions},
@@ -305,8 +307,10 @@
bool have_key_share = false, have_pre_shared_key = false,
have_supported_versions = false;
CBS key_share, pre_shared_key, supported_versions;
- const SSL_EXTENSION_TYPE ext_types[] = {
- {TLSEXT_TYPE_key_share, &have_key_share, &key_share},
+ SSL_EXTENSION_TYPE ext_types[] = {
+ {ssl_is_draft23(ssl->version) ? (uint16_t)TLSEXT_TYPE_new_key_share
+ : (uint16_t)TLSEXT_TYPE_old_key_share,
+ &have_key_share, &key_share},
{TLSEXT_TYPE_pre_shared_key, &have_pre_shared_key, &pre_shared_key},
{TLSEXT_TYPE_supported_versions, &have_supported_versions,
&supported_versions},
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 4651459..72273da 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -63,10 +63,15 @@
SSL *const ssl = hs->ssl;
*out_need_retry = false;
+ uint16_t key_share_ext = TLSEXT_TYPE_old_key_share;
+ if (ssl_is_draft23(ssl->version)) {
+ key_share_ext = TLSEXT_TYPE_new_key_share;
+ }
+
// We only support connections that include an ECDHE key exchange.
CBS key_share;
if (!ssl_client_hello_get_extension(client_hello, &key_share,
- TLSEXT_TYPE_key_share)) {
+ key_share_ext)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_KEY_SHARE);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_MISSING_EXTENSION);
return 0;
@@ -508,7 +513,9 @@
!CBB_add_u16(&extensions, TLSEXT_TYPE_supported_versions) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
!CBB_add_u16(&extensions, ssl->version) ||
- !CBB_add_u16(&extensions, TLSEXT_TYPE_key_share) ||
+ !CBB_add_u16(&extensions, ssl_is_draft23(ssl->version)
+ ? TLSEXT_TYPE_new_key_share
+ : TLSEXT_TYPE_old_key_share) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
!CBB_add_u16(&extensions, group_id) ||
!ssl_add_message_cbb(ssl, cbb.get())) {
@@ -529,7 +536,7 @@
!CBB_add_u16(&body, ssl_cipher_get_value(hs->new_cipher))) ||
!tls1_get_shared_group(hs, &group_id) ||
!CBB_add_u16_length_prefixed(&body, &extensions) ||
- !CBB_add_u16(&extensions, TLSEXT_TYPE_key_share) ||
+ !CBB_add_u16(&extensions, TLSEXT_TYPE_old_key_share) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
!CBB_add_u16(&extensions, group_id) ||
!ssl_add_message_cbb(ssl, cbb.get())) {
