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) ==