Remove experimental TLS 1.3 short record header extension.
Due to middlebox and ecosystem intolerance, short record headers are going to
be unsustainable to deploy.
BUG=119
Change-Id: I20fee79dd85bff229eafc6aeb72e4f33cac96d82
Reviewed-on: https://boringssl-review.googlesource.com/14044
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/fuzz/client.cc b/fuzz/client.cc
index 2b91e7c..ee75a7c 100644
--- a/fuzz/client.cc
+++ b/fuzz/client.cc
@@ -247,8 +247,6 @@
assert(pkey != nullptr);
SSL_CTX_set1_tls_channel_id(ctx, pkey);
EVP_PKEY_free(pkey);
-
- SSL_CTX_set_short_header_enabled(ctx, 1);
}
~GlobalState() {
diff --git a/fuzz/server.cc b/fuzz/server.cc
index 9cdfad9..19417d2 100644
--- a/fuzz/server.cc
+++ b/fuzz/server.cc
@@ -240,8 +240,6 @@
SSL_CTX_set_alpn_select_cb(ctx, ALPNSelectCallback, nullptr);
SSL_CTX_set_next_protos_advertised_cb(ctx, NPNAdvertiseCallback, nullptr);
-
- SSL_CTX_set_short_header_enabled(ctx, 1);
}
~GlobalState() {
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index e1a8840..a2fea43 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3210,11 +3210,6 @@
* record with |ssl|. */
OPENSSL_EXPORT size_t SSL_max_seal_overhead(const SSL *ssl);
-/* SSL_CTX_set_short_header_enabled configures whether a short record header in
- * TLS 1.3 may be negotiated. This allows client and server to negotiate
- * https://github.com/tlswg/tls13-spec/pull/762 for testing. */
-OPENSSL_EXPORT void SSL_CTX_set_short_header_enabled(SSL_CTX *ctx, int enabled);
-
/* Deprecated functions. */
@@ -4136,10 +4131,6 @@
/* grease_enabled is one if draft-davidben-tls-grease-01 is enabled and zero
* otherwise. */
unsigned grease_enabled:1;
-
- /* short_header_enabled is one if a short record header in TLS 1.3 may
- * be negotiated and zero otherwise. */
- unsigned short_header_enabled:1;
};
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index 2601974..1842ee5 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -227,9 +227,6 @@
/* This is not an IANA defined extension number */
#define TLSEXT_TYPE_channel_id 30032
-/* This is not an IANA defined extension number */
-#define TLSEXT_TYPE_short_header 27463
-
/* status request value from RFC 3546 */
#define TLSEXT_STATUSTYPE_ocsp 1
diff --git a/ssl/internal.h b/ssl/internal.h
index dbeae92..caa9cd2 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1604,10 +1604,6 @@
* handshake. */
unsigned tlsext_channel_id_valid:1;
- /* short_header is one if https://github.com/tlswg/tls13-spec/pull/762 has
- * been negotiated. */
- unsigned short_header:1;
-
uint8_t send_alert[2];
/* pending_flight is the pending outgoing flight. This is used to flush each
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 75cac31..93d84f4 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2552,10 +2552,6 @@
ctx->grease_enabled = !!enabled;
}
-void SSL_CTX_set_short_header_enabled(SSL_CTX *ctx, int enabled) {
- ctx->short_header_enabled = !!enabled;
-}
-
int SSL_clear(SSL *ssl) {
/* In OpenSSL, reusing a client |SSL| with |SSL_clear| causes the previously
* established session to be offered the next time around. wpa_supplicant
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 2a9f945..adc7344 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2403,46 +2403,6 @@
}
-/* Short record headers
- *
- * This is a non-standard extension which negotiates
- * https://github.com/tlswg/tls13-spec/pull/762 for experimenting. */
-
-static int ext_short_header_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
- SSL *const ssl = hs->ssl;
- uint16_t min_version, max_version;
- if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
- return 0;
- }
-
- if (max_version < TLS1_3_VERSION ||
- !ssl->ctx->short_header_enabled) {
- return 1;
- }
-
- return CBB_add_u16(out, TLSEXT_TYPE_short_header) &&
- CBB_add_u16(out, 0 /* empty extension */);
-}
-
-static int ext_short_header_parse_clienthello(SSL_HANDSHAKE *hs,
- uint8_t *out_alert,
- CBS *contents) {
- SSL *const ssl = hs->ssl;
- if (contents == NULL ||
- !ssl->ctx->short_header_enabled ||
- ssl3_protocol_version(ssl) < TLS1_3_VERSION) {
- return 1;
- }
-
- if (CBS_len(contents) != 0) {
- return 0;
- }
-
- ssl->s3->short_header = 1;
- return 1;
-}
-
-
/* Negotiated Groups
*
* https://tools.ietf.org/html/rfc4492#section-5.1.2
@@ -2673,14 +2633,6 @@
ignore_parse_clienthello,
dont_add_serverhello,
},
- {
- TLSEXT_TYPE_short_header,
- NULL,
- ext_short_header_add_clienthello,
- forbid_parse_serverhello,
- ext_short_header_parse_clienthello,
- dont_add_serverhello,
- },
/* The final extension must be non-empty. WebSphere Application Server 7.0 is
* intolerant to the last extension being zero-length. See
* https://crbug.com/363583. */
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 02b7558..984136c 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1176,10 +1176,6 @@
return nullptr;
}
- if (config->enable_short_header) {
- SSL_CTX_set_short_header_enabled(ssl_ctx.get(), 1);
- }
-
if (config->enable_early_data) {
SSL_CTX_set_early_data_enabled(ssl_ctx.get(), 1);
}
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index e709a57..d316ddd 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -100,7 +100,6 @@
extensionNextProtoNeg uint16 = 13172 // not IANA assigned
extensionRenegotiationInfo uint16 = 0xff01
extensionChannelID uint16 = 30032 // not IANA assigned
- extensionShortHeader uint16 = 27463 // not IANA assigned
)
// TLS signaling cipher suite values
@@ -223,7 +222,6 @@
SCTList []byte // signed certificate timestamp list
PeerSignatureAlgorithm signatureAlgorithm // algorithm used by the peer in the handshake
CurveID CurveID // the curve used in ECDHE
- ShortHeader bool // whether the short header extension was negotiated
}
// ClientAuthType declares the policy the server will follow for
@@ -1274,18 +1272,6 @@
// request signed certificate timestamps.
NoSignedCertificateTimestamps bool
- // EnableShortHeader, if true, causes the TLS 1.3 short header extension
- // to be enabled.
- EnableShortHeader bool
-
- // AlwaysNegotiateShortHeader, if true, causes the server to always
- // negotiate the short header extension in ServerHello.
- AlwaysNegotiateShortHeader bool
-
- // ClearShortHeaderBit, if true, causes the server to send short headers
- // without the high bit set.
- ClearShortHeaderBit bool
-
// SendSupportedPointFormats, if not nil, is the list of supported point
// formats to send in ClientHello or ServerHello. If set to a non-nil
// empty slice, no extension will be sent.
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index a4cb573..1bdca84 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -167,8 +167,6 @@
trafficSecret []byte
- shortHeader bool
-
config *Config
}
@@ -315,17 +313,10 @@
copy(hc.outSeq[:], hc.seq[:])
}
-func (hc *halfConn) isShortHeader() bool {
- return hc.shortHeader && hc.cipher != nil
-}
-
func (hc *halfConn) recordHeaderLen() int {
if hc.isDTLS {
return dtlsRecordHeaderLen
}
- if hc.isShortHeader() {
- return 2
- }
return tlsRecordHeaderLen
}
@@ -629,9 +620,6 @@
n := len(b.data) - recordHeaderLen
b.data[recordHeaderLen-2] = byte(n >> 8)
b.data[recordHeaderLen-1] = byte(n)
- if hc.isShortHeader() && !hc.config.Bugs.ClearShortHeaderBit {
- b.data[0] |= 0x80
- }
hc.incSeq(true)
return true, 0
@@ -762,35 +750,20 @@
return 0, nil, err
}
- var typ recordType
- var vers uint16
- var n int
- if c.in.isShortHeader() {
- typ = recordTypeApplicationData
- vers = VersionTLS10
- n = int(b.data[0])<<8 | int(b.data[1])
- if n&0x8000 == 0 {
- c.sendAlert(alertDecodeError)
- return 0, nil, c.in.setErrorLocked(errors.New("tls: length did not have high bit set"))
- }
+ typ := recordType(b.data[0])
- n = n & 0x7fff
- } else {
- typ = recordType(b.data[0])
-
- // No valid TLS record has a type of 0x80, however SSLv2 handshakes
- // start with a uint16 length where the MSB is set and the first record
- // is always < 256 bytes long. Therefore typ == 0x80 strongly suggests
- // an SSLv2 client.
- if want == recordTypeHandshake && typ == 0x80 {
- c.sendAlert(alertProtocolVersion)
- return 0, nil, c.in.setErrorLocked(errors.New("tls: unsupported SSLv2 handshake received"))
- }
-
- vers = uint16(b.data[1])<<8 | uint16(b.data[2])
- n = int(b.data[3])<<8 | int(b.data[4])
+ // No valid TLS record has a type of 0x80, however SSLv2 handshakes
+ // start with a uint16 length where the MSB is set and the first record
+ // is always < 256 bytes long. Therefore typ == 0x80 strongly suggests
+ // an SSLv2 client.
+ if want == recordTypeHandshake && typ == 0x80 {
+ c.sendAlert(alertProtocolVersion)
+ return 0, nil, c.in.setErrorLocked(errors.New("tls: unsupported SSLv2 handshake received"))
}
+ vers := uint16(b.data[1])<<8 | uint16(b.data[2])
+ n := int(b.data[3])<<8 | int(b.data[4])
+
// Alerts sent near version negotiation do not have a well-defined
// record-layer version prior to TLS 1.3. (In TLS 1.3, the record-layer
// version is irrelevant.)
@@ -1118,36 +1091,32 @@
}
}
b.resize(recordHeaderLen + explicitIVLen + m)
- // If using a short record header, the length will be filled in
- // by encrypt.
- if !c.out.isShortHeader() {
- b.data[0] = byte(typ)
- if c.vers >= VersionTLS13 && c.out.cipher != nil {
- b.data[0] = byte(recordTypeApplicationData)
- if outerType := c.config.Bugs.OuterRecordType; outerType != 0 {
- b.data[0] = byte(outerType)
- }
+ b.data[0] = byte(typ)
+ if c.vers >= VersionTLS13 && c.out.cipher != nil {
+ b.data[0] = byte(recordTypeApplicationData)
+ if outerType := c.config.Bugs.OuterRecordType; outerType != 0 {
+ b.data[0] = byte(outerType)
}
- vers := c.vers
- if vers == 0 || vers >= VersionTLS13 {
- // Some TLS servers fail if the record version is
- // greater than TLS 1.0 for the initial ClientHello.
- //
- // TLS 1.3 fixes the version number in the record
- // layer to {3, 1}.
- vers = VersionTLS10
- }
- if c.config.Bugs.SendRecordVersion != 0 {
- vers = c.config.Bugs.SendRecordVersion
- }
- if c.vers == 0 && c.config.Bugs.SendInitialRecordVersion != 0 {
- vers = c.config.Bugs.SendInitialRecordVersion
- }
- b.data[1] = byte(vers >> 8)
- b.data[2] = byte(vers)
- b.data[3] = byte(m >> 8)
- b.data[4] = byte(m)
}
+ vers := c.vers
+ if vers == 0 || vers >= VersionTLS13 {
+ // Some TLS servers fail if the record version is
+ // greater than TLS 1.0 for the initial ClientHello.
+ //
+ // TLS 1.3 fixes the version number in the record
+ // layer to {3, 1}.
+ vers = VersionTLS10
+ }
+ if c.config.Bugs.SendRecordVersion != 0 {
+ vers = c.config.Bugs.SendRecordVersion
+ }
+ if c.vers == 0 && c.config.Bugs.SendInitialRecordVersion != 0 {
+ vers = c.config.Bugs.SendInitialRecordVersion
+ }
+ b.data[1] = byte(vers >> 8)
+ b.data[2] = byte(vers)
+ b.data[3] = byte(m >> 8)
+ b.data[4] = byte(m)
if explicitIVLen > 0 {
explicitIV := b.data[recordHeaderLen : recordHeaderLen+explicitIVLen]
if explicitIVIsSeq {
@@ -1705,7 +1674,6 @@
state.SCTList = c.sctList
state.PeerSignatureAlgorithm = c.peerSignatureAlgorithm
state.CurveID = c.curveID
- state.ShortHeader = c.in.shortHeader
}
return state
@@ -1873,8 +1841,3 @@
_, err := c.conn.Write(payload)
return err
}
-
-func (c *Conn) setShortHeader() {
- c.in.shortHeader = true
- c.out.shortHeader = true
-}
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index ba7b91e..eb072ad 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -81,7 +81,6 @@
srtpMasterKeyIdentifier: c.config.Bugs.SRTPMasterKeyIdentifer,
customExtension: c.config.Bugs.CustomExtension,
pskBinderFirst: c.config.Bugs.PSKBinderFirst,
- shortHeaderSupported: c.config.Bugs.EnableShortHeader,
}
disableEMS := c.config.Bugs.NoExtendedMasterSecret
@@ -717,18 +716,6 @@
hs.finishedHash.addEntropy(zeroSecret)
}
- if hs.serverHello.shortHeader && !hs.hello.shortHeaderSupported {
- return errors.New("tls: server sent unsolicited short header extension")
- }
-
- if hs.serverHello.shortHeader && hs.hello.hasEarlyData {
- return errors.New("tls: server sent short header extension in response to early data")
- }
-
- if hs.serverHello.shortHeader {
- c.setShortHeader()
- }
-
// Derive handshake traffic keys and switch read key to handshake
// traffic key.
clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(clientHandshakeTrafficLabel)
@@ -1363,10 +1350,6 @@
func (hs *clientHandshakeState) processServerHello() (bool, error) {
c := hs.c
- if hs.serverHello.shortHeader {
- return false, errors.New("tls: short header extension sent before TLS 1.3")
- }
-
if hs.serverResumedSession() {
// For test purposes, assert that the server never accepts the
// resumption offer on renegotiation.
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index 4ecb3cb..86e2821 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -167,7 +167,6 @@
customExtension string
hasGREASEExtension bool
pskBinderFirst bool
- shortHeaderSupported bool
}
func (m *clientHelloMsg) equal(i interface{}) bool {
@@ -213,8 +212,7 @@
m.sctListSupported == m1.sctListSupported &&
m.customExtension == m1.customExtension &&
m.hasGREASEExtension == m1.hasGREASEExtension &&
- m.pskBinderFirst == m1.pskBinderFirst &&
- m.shortHeaderSupported == m1.shortHeaderSupported
+ m.pskBinderFirst == m1.pskBinderFirst
}
func (m *clientHelloMsg) marshal() []byte {
@@ -430,10 +428,6 @@
customExt := extensions.addU16LengthPrefixed()
customExt.addBytes([]byte(m.customExtension))
}
- if m.shortHeaderSupported {
- extensions.addU16(extensionShortHeader)
- extensions.addU16(0) // Length is always 0
- }
// The PSK extension must be last (draft-ietf-tls-tls13-18 section 4.2.6).
if len(m.pskIdentities) > 0 && !m.pskBinderFirst {
extensions.addU16(extensionPreSharedKey)
@@ -805,11 +799,6 @@
return false
}
m.sctListSupported = true
- case extensionShortHeader:
- if length != 0 {
- return false
- }
- m.shortHeaderSupported = true
case extensionCustom:
m.customExtension = string(data[:length])
}
@@ -838,7 +827,6 @@
compressionMethod uint8
customExtension string
unencryptedALPN string
- shortHeader bool
extensions serverExtensions
}
@@ -876,11 +864,6 @@
extensions := hello.addU16LengthPrefixed()
- if m.shortHeader {
- extensions.addU16(extensionShortHeader)
- extensions.addU16(0) // Length
- }
-
if vers >= VersionTLS13 {
if m.hasKeyShare {
extensions.addU16(extensionKeyShare)
@@ -1000,11 +983,6 @@
}
m.pskIdentity = uint16(d[0])<<8 | uint16(d[1])
m.hasPSKIdentity = true
- case extensionShortHeader:
- if len(d) != 0 {
- return false
- }
- m.shortHeader = true
default:
// Only allow the 3 extensions that are sent in
// the clear in TLS 1.3.
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 1366837..9b4bff8 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -365,15 +365,6 @@
versOverride: config.Bugs.SendServerHelloVersion,
customExtension: config.Bugs.CustomUnencryptedExtension,
unencryptedALPN: config.Bugs.SendUnencryptedALPN,
- shortHeader: hs.clientHello.shortHeaderSupported && config.Bugs.EnableShortHeader,
- }
-
- if config.Bugs.AlwaysNegotiateShortHeader {
- hs.hello.shortHeader = true
- }
-
- if hs.hello.shortHeader {
- c.setShortHeader()
}
hs.hello.random = make([]byte, 32)
@@ -1025,7 +1016,6 @@
vers: versionToWire(c.vers, c.isDTLS),
versOverride: config.Bugs.SendServerHelloVersion,
compressionMethod: compressionNone,
- shortHeader: config.Bugs.AlwaysNegotiateShortHeader,
}
hs.hello.random = make([]byte, 32)
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index d039385..55685b0 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -407,8 +407,6 @@
// expectPeerCertificate, if not nil, is the certificate chain the peer
// is expected to send.
expectPeerCertificate *Certificate
- // expectShortHeader is whether the short header extension should be negotiated.
- expectShortHeader bool
}
var testCases []testCase
@@ -637,10 +635,6 @@
}
}
- if test.expectShortHeader != connState.ShortHeader {
- return fmt.Errorf("ShortHeader is %t, but we expected the opposite", connState.ShortHeader)
- }
-
if test.exportKeyingMaterial > 0 {
actual := make([]byte, test.exportKeyingMaterial)
if _, err := io.ReadFull(tlsConn, actual); err != nil {
@@ -2686,27 +2680,6 @@
shouldFail: shouldFail,
expectedError: expectedError,
})
-
- if ver.version >= VersionTLS13 {
- testCases = append(testCases, testCase{
- protocol: protocol,
- name: prefix + ver.name + "-" + suite.name + "-ShortHeader",
- config: Config{
- MinVersion: ver.version,
- MaxVersion: ver.version,
- CipherSuites: []uint16{suite.id},
- Certificates: []Certificate{cert},
- PreSharedKey: []byte(psk),
- PreSharedKeyIdentity: pskIdentity,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- },
- },
- flags: append([]string{"-enable-short-header"}, flags...),
- resumeSession: true,
- expectShortHeader: true,
- })
- }
}
func addCipherSuiteTests() {
@@ -10244,150 +10217,6 @@
}
}
-func addShortHeaderTests() {
- // The short header extension may be negotiated as either client or
- // server.
- testCases = append(testCases, testCase{
- name: "ShortHeader-Client",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- },
- },
- flags: []string{"-enable-short-header"},
- expectShortHeader: true,
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "ShortHeader-Server",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- },
- },
- flags: []string{"-enable-short-header"},
- expectShortHeader: true,
- })
-
- // If the peer doesn't support it, it will not be negotiated.
- testCases = append(testCases, testCase{
- name: "ShortHeader-No-Yes-Client",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- flags: []string{"-enable-short-header"},
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "ShortHeader-No-Yes-Server",
- config: Config{
- MaxVersion: VersionTLS13,
- },
- flags: []string{"-enable-short-header"},
- })
-
- // If we don't support it, it will not be negotiated.
- testCases = append(testCases, testCase{
- name: "ShortHeader-Yes-No-Client",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- },
- },
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "ShortHeader-Yes-No-Server",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- },
- },
- })
-
- // It will not be negotiated at TLS 1.2.
- testCases = append(testCases, testCase{
- name: "ShortHeader-TLS12-Client",
- config: Config{
- MaxVersion: VersionTLS12,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- },
- },
- flags: []string{"-enable-short-header"},
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "ShortHeader-TLS12-Server",
- config: Config{
- MaxVersion: VersionTLS12,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- },
- },
- flags: []string{"-enable-short-header"},
- })
-
- // Servers reject early data and short header sent together.
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "ShortHeader-EarlyData",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- SendFakeEarlyDataLength: 1,
- },
- },
- flags: []string{"-enable-short-header"},
- shouldFail: true,
- expectedError: ":UNEXPECTED_EXTENSION:",
- })
-
- // Clients reject unsolicited short header extensions.
- testCases = append(testCases, testCase{
- name: "ShortHeader-Unsolicited",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- AlwaysNegotiateShortHeader: true,
- },
- },
- shouldFail: true,
- expectedError: ":UNEXPECTED_EXTENSION:",
- })
- testCases = append(testCases, testCase{
- name: "ShortHeader-Unsolicited-TLS12",
- config: Config{
- MaxVersion: VersionTLS12,
- Bugs: ProtocolBugs{
- AlwaysNegotiateShortHeader: true,
- },
- },
- shouldFail: true,
- expectedError: ":UNEXPECTED_EXTENSION:",
- })
-
- // The high bit must be checked in short headers.
- testCases = append(testCases, testCase{
- name: "ShortHeader-ClearShortHeaderBit",
- config: Config{
- Bugs: ProtocolBugs{
- EnableShortHeader: true,
- ClearShortHeaderBit: true,
- },
- },
- flags: []string{"-enable-short-header"},
- shouldFail: true,
- expectedError: ":DECODE_ERROR:",
- expectedLocalError: "remote error: error decoding message",
- })
-}
-
func worker(statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) {
defer wg.Done()
@@ -10514,7 +10343,6 @@
addCertificateTests()
addRetainOnlySHA256ClientCertTests()
addECDSAKeyUsageTests()
- addShortHeaderTests()
var wg sync.WaitGroup
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index bec0421..fefe376 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -116,7 +116,6 @@
&TestConfig::expect_sha256_client_cert_initial },
{ "-expect-sha256-client-cert-resume",
&TestConfig::expect_sha256_client_cert_resume },
- { "-enable-short-header", &TestConfig::enable_short_header },
{ "-read-with-unfinished-write", &TestConfig::read_with_unfinished_write },
{ "-expect-secure-renegotiation",
&TestConfig::expect_secure_renegotiation },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index cbe4678..ee3d462 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -126,7 +126,6 @@
bool retain_only_sha256_client_cert_resume = false;
bool expect_sha256_client_cert_initial = false;
bool expect_sha256_client_cert_resume = false;
- bool enable_short_header = false;
bool read_with_unfinished_write = false;
bool expect_secure_renegotiation = false;
bool expect_no_secure_renegotiation = false;
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index 254c363..c0eb135 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -199,12 +199,11 @@
}
/* Parse out the extensions. */
- int have_key_share = 0, have_pre_shared_key = 0, have_short_header = 0;
- CBS key_share, pre_shared_key, short_header;
+ int have_key_share = 0, have_pre_shared_key = 0;
+ CBS key_share, pre_shared_key;
const SSL_EXTENSION_TYPE ext_types[] = {
{TLSEXT_TYPE_key_share, &have_key_share, &key_share},
{TLSEXT_TYPE_pre_shared_key, &have_pre_shared_key, &pre_shared_key},
- {TLSEXT_TYPE_short_header, &have_short_header, &short_header},
};
uint8_t alert = SSL_AD_DECODE_ERROR;
@@ -318,23 +317,6 @@
}
OPENSSL_free(dhe_secret);
- /* Negotiate short record headers. */
- if (have_short_header) {
- if (CBS_len(&short_header) != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
- return ssl_hs_error;
- }
-
- if (!ssl->ctx->short_header_enabled) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
- return ssl_hs_error;
- }
-
- ssl->s3->short_header = 1;
- }
-
if (!ssl_hash_current_message(hs) ||
!tls13_derive_handshake_secrets(hs) ||
!tls13_set_traffic_key(ssl, evp_aead_open, hs->server_handshake_secret,
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 402c234..e7cc296 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -135,11 +135,6 @@
static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
- /* The short record header extension is incompatible with early data. */
- if (ssl->s3->skip_early_data && ssl->s3->short_header) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- return ssl_hs_error;
- }
SSL_CLIENT_HELLO client_hello;
if (!ssl_client_hello_init(ssl, &client_hello, ssl->init_msg,
@@ -354,18 +349,8 @@
!CBB_add_u16(&body, ssl_cipher_get_value(hs->new_cipher)) ||
!CBB_add_u16_length_prefixed(&body, &extensions) ||
!ssl_ext_pre_shared_key_add_serverhello(hs, &extensions) ||
- !ssl_ext_key_share_add_serverhello(hs, &extensions)) {
- goto err;
- }
-
- if (ssl->s3->short_header) {
- if (!CBB_add_u16(&extensions, TLSEXT_TYPE_short_header) ||
- !CBB_add_u16(&extensions, 0 /* empty extension */)) {
- goto err;
- }
- }
-
- if (!ssl_add_message_cbb(ssl, &cbb)) {
+ !ssl_ext_key_share_add_serverhello(hs, &extensions) ||
+ !ssl_add_message_cbb(ssl, &cbb)) {
goto err;
}
diff --git a/ssl/tls_record.c b/ssl/tls_record.c
index 6ff79c4..bf9735c 100644
--- a/ssl/tls_record.c
+++ b/ssl/tls_record.c
@@ -145,19 +145,6 @@
SSL_CIPHER_is_block_cipher(ssl->s3->aead_write_ctx->cipher);
}
-static int ssl_uses_short_header(const SSL *ssl,
- enum evp_aead_direction_t dir) {
- if (!ssl->s3->short_header) {
- return 0;
- }
-
- if (dir == evp_aead_open) {
- return ssl->s3->aead_read_ctx != NULL;
- }
-
- return ssl->s3->aead_write_ctx != NULL;
-}
-
int ssl_record_sequence_update(uint8_t *seq, size_t seq_len) {
for (size_t i = seq_len - 1; i < seq_len; i--) {
++seq[i];
@@ -173,8 +160,6 @@
size_t header_len;
if (SSL_is_dtls(ssl)) {
header_len = DTLS1_RT_HEADER_LENGTH;
- } else if (ssl_uses_short_header(ssl, evp_aead_open)) {
- header_len = 2;
} else {
header_len = SSL3_RT_HEADER_LENGTH;
}
@@ -188,17 +173,10 @@
SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_write_ctx);
}
- size_t header_len;
- if (ssl_uses_short_header(ssl, evp_aead_seal)) {
- header_len = 2;
- } else {
- header_len = SSL3_RT_HEADER_LENGTH;
- }
-
- size_t ret =
- header_len + SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_write_ctx);
+ size_t ret = SSL3_RT_HEADER_LENGTH +
+ SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_write_ctx);
if (ssl_needs_record_splitting(ssl)) {
- ret += header_len;
+ ret += SSL3_RT_HEADER_LENGTH;
ret += ssl_cipher_get_record_split_len(ssl->s3->aead_write_ctx->cipher);
}
return ret;
@@ -209,8 +187,7 @@
return dtls_max_seal_overhead(ssl, dtls1_use_current_epoch);
}
- size_t ret =
- ssl_uses_short_header(ssl, evp_aead_seal) ? 2 : SSL3_RT_HEADER_LENGTH;
+ size_t ret = SSL3_RT_HEADER_LENGTH;
ret += SSL_AEAD_CTX_max_overhead(ssl->s3->aead_write_ctx);
/* TLS 1.3 needs an extra byte for the encrypted record type. */
if (ssl->s3->have_version &&
@@ -234,31 +211,11 @@
/* Decode the record header. */
uint8_t type;
uint16_t version, ciphertext_len;
- size_t header_len;
- if (ssl_uses_short_header(ssl, evp_aead_open)) {
- if (!CBS_get_u16(&cbs, &ciphertext_len)) {
- *out_consumed = 2;
- return ssl_open_record_partial;
- }
-
- if ((ciphertext_len & 0x8000) == 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- *out_alert = SSL_AD_DECODE_ERROR;
- return ssl_open_record_error;
- }
-
- ciphertext_len &= 0x7fff;
- type = SSL3_RT_APPLICATION_DATA;
- version = TLS1_VERSION;
- header_len = 2;
- } else {
- if (!CBS_get_u8(&cbs, &type) ||
- !CBS_get_u16(&cbs, &version) ||
- !CBS_get_u16(&cbs, &ciphertext_len)) {
- *out_consumed = SSL3_RT_HEADER_LENGTH;
- return ssl_open_record_partial;
- }
- header_len = SSL3_RT_HEADER_LENGTH;
+ if (!CBS_get_u8(&cbs, &type) ||
+ !CBS_get_u16(&cbs, &version) ||
+ !CBS_get_u16(&cbs, &ciphertext_len)) {
+ *out_consumed = SSL3_RT_HEADER_LENGTH;
+ return ssl_open_record_partial;
}
int version_ok;
@@ -290,11 +247,12 @@
/* Extract the body. */
CBS body;
if (!CBS_get_bytes(&cbs, &body, ciphertext_len)) {
- *out_consumed = header_len + (size_t)ciphertext_len;
+ *out_consumed = SSL3_RT_HEADER_LENGTH + (size_t)ciphertext_len;
return ssl_open_record_partial;
}
- ssl_do_msg_callback(ssl, 0 /* read */, SSL3_RT_HEADER, in, header_len);
+ ssl_do_msg_callback(ssl, 0 /* read */, SSL3_RT_HEADER, in,
+ SSL3_RT_HEADER_LENGTH);
*out_consumed = in_len - CBS_len(&cbs);
@@ -398,26 +356,24 @@
size_t in_len) {
assert(!buffers_alias(in, in_len, out, max_out));
- const int short_header = ssl_uses_short_header(ssl, evp_aead_seal);
- size_t header_len = short_header ? 2 : SSL3_RT_HEADER_LENGTH;
-
/* TLS 1.3 hides the actual record type inside the encrypted data. */
if (ssl->s3->have_version &&
ssl3_protocol_version(ssl) >= TLS1_3_VERSION &&
ssl->s3->aead_write_ctx != NULL) {
- if (in_len > in_len + header_len + 1 || max_out < in_len + header_len + 1) {
+ if (in_len > in_len + SSL3_RT_HEADER_LENGTH + 1 ||
+ max_out < in_len + SSL3_RT_HEADER_LENGTH + 1) {
OPENSSL_PUT_ERROR(SSL, SSL_R_BUFFER_TOO_SMALL);
return 0;
}
- OPENSSL_memcpy(out + header_len, in, in_len);
- out[header_len + in_len] = type;
- in = out + header_len;
+ OPENSSL_memcpy(out + SSL3_RT_HEADER_LENGTH, in, in_len);
+ out[SSL3_RT_HEADER_LENGTH + in_len] = type;
+ in = out + SSL3_RT_HEADER_LENGTH;
type = SSL3_RT_APPLICATION_DATA;
in_len++;
}
- if (max_out < header_len) {
+ if (max_out < SSL3_RT_HEADER_LENGTH) {
OPENSSL_PUT_ERROR(SSL, SSL_R_BUFFER_TOO_SMALL);
return 0;
}
@@ -433,13 +389,11 @@
}
/* Write the non-length portions of the header. */
- if (!short_header) {
- out[0] = type;
- out[1] = wire_version >> 8;
- out[2] = wire_version & 0xff;
- out += 3;
- max_out -= 3;
- }
+ out[0] = type;
+ out[1] = wire_version >> 8;
+ out[2] = wire_version & 0xff;
+ out += 3;
+ max_out -= 3;
/* Write the ciphertext, leaving two bytes for the length. */
size_t ciphertext_len;
@@ -457,13 +411,11 @@
}
out[0] = ciphertext_len >> 8;
out[1] = ciphertext_len & 0xff;
- if (short_header) {
- out[0] |= 0x80;
- }
- *out_len = header_len + ciphertext_len;
+ *out_len = SSL3_RT_HEADER_LENGTH + ciphertext_len;
- ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_HEADER, out, header_len);
+ ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_HEADER, out,
+ SSL3_RT_HEADER_LENGTH);
return 1;
}
@@ -485,7 +437,6 @@
out += frag_len;
max_out -= frag_len;
- assert(!ssl_uses_short_header(ssl, evp_aead_seal));
#if !defined(BORINGSSL_UNSAFE_FUZZER_MODE)
assert(SSL3_RT_HEADER_LENGTH + ssl_cipher_get_record_split_len(
ssl->s3->aead_write_ctx->cipher) ==
