Add experimental TLS 1.3 short record header extension. This extension will be used to test whether https://github.com/tlswg/tls13-spec/pull/762 is deployable against middleboxes. For simplicity, it is mutually exclusive with 0-RTT. If client and server agree on the extension, TLS 1.3 records will use the format in the PR rather than what is in draft 18. BUG=119 Change-Id: I1372ddf7b328ddf73d496df54ac03a95ede961e1 Reviewed-on: https://boringssl-review.googlesource.com/12684 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 84d0e28..1a0e052 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -3149,6 +3149,11 @@ * 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. */ @@ -4044,6 +4049,10 @@ * 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; + /* extra_certs is a dummy value included for compatibility. * TODO(agl): remove once node.js no longer references this. */ STACK_OF(X509)* extra_certs;
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index 54c2800..28828d1 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h
@@ -226,6 +226,9 @@ /* 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 a4ae83a..7a10936 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1483,6 +1483,10 @@ * 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_message is the current outgoing handshake message. */
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index f428104..40477a9 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c
@@ -2870,6 +2870,10 @@ 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 c27e84d..0681919 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c
@@ -2458,6 +2458,46 @@ } +/* 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 @@ -2688,6 +2728,14 @@ 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 4425ab0..8fd6d31 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc
@@ -1042,6 +1042,10 @@ return nullptr; } + if (config->enable_short_header) { + SSL_CTX_set_short_header_enabled(ssl_ctx.get(), 1); + } + return ssl_ctx; }
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index f5fedb0..dd185ce 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -99,6 +99,7 @@ extensionNextProtoNeg uint16 = 13172 // not IANA assigned extensionRenegotiationInfo uint16 = 0xff01 extensionChannelID uint16 = 30032 // not IANA assigned + extensionShortHeader uint16 = 27463 // not IANA assigned ) // TLS ticket extension numbers @@ -225,6 +226,7 @@ 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 @@ -1224,6 +1226,18 @@ // NoSignedCertificateTimestamps, if true, causes the client to not // 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 } func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index 80a5b06..510bcf7 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go
@@ -163,6 +163,8 @@ trafficSecret []byte + shortHeader bool + config *Config } @@ -301,10 +303,17 @@ 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 } @@ -608,6 +617,9 @@ 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 @@ -716,7 +728,7 @@ return c.dtlsDoReadRecord(want) } - recordHeaderLen := tlsRecordHeaderLen + recordHeaderLen := c.in.recordHeaderLen() if c.rawInput == nil { c.rawInput = c.in.newBlock() @@ -736,19 +748,36 @@ } return 0, nil, err } - 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")) + 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")) + } + + 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]) } - 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.) @@ -1007,7 +1036,7 @@ } func (c *Conn) doWriteRecord(typ recordType, data []byte) (n int, err error) { - recordHeaderLen := tlsRecordHeaderLen + recordHeaderLen := c.out.recordHeaderLen() b := c.out.newBlock() first := true isClientHello := typ == recordTypeHandshake && len(data) > 0 && data[0] == typeClientHello @@ -1049,32 +1078,36 @@ } } b.resize(recordHeaderLen + explicitIVLen + m) - 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) + // 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) + } } + 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 { @@ -1620,6 +1653,7 @@ state.SCTList = c.sctList state.PeerSignatureAlgorithm = c.peerSignatureAlgorithm state.CurveID = c.curveID + state.ShortHeader = c.in.shortHeader } return state @@ -1781,3 +1815,8 @@ _, 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 7fa7ea2..9691ae6 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -81,6 +81,7 @@ 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 @@ -684,6 +685,18 @@ 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() + } + // Switch to handshake traffic keys. clientHandshakeTrafficSecret := hs.finishedHash.deriveSecret(clientHandshakeTrafficLabel) c.out.useTrafficSecret(c.vers, hs.suite, clientHandshakeTrafficSecret, clientWrite) @@ -1291,6 +1304,10 @@ 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 8a338f0..1f87ad2 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -167,6 +167,7 @@ customExtension string hasGREASEExtension bool pskBinderFirst bool + shortHeaderSupported bool } func (m *clientHelloMsg) equal(i interface{}) bool { @@ -212,7 +213,8 @@ m.sctListSupported == m1.sctListSupported && m.customExtension == m1.customExtension && m.hasGREASEExtension == m1.hasGREASEExtension && - m.pskBinderFirst == m1.pskBinderFirst + m.pskBinderFirst == m1.pskBinderFirst && + m.shortHeaderSupported == m1.shortHeaderSupported } func (m *clientHelloMsg) marshal() []byte { @@ -430,6 +432,10 @@ 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) @@ -802,6 +808,11 @@ return false } m.sctListSupported = true + case extensionShortHeader: + if length != 0 { + return false + } + m.shortHeaderSupported = true case extensionCustom: m.customExtension = string(data[:length]) } @@ -831,6 +842,7 @@ compressionMethod uint8 customExtension string unencryptedALPN string + shortHeader bool extensions serverExtensions } @@ -868,6 +880,11 @@ extensions := hello.addU16LengthPrefixed() + if m.shortHeader { + extensions.addU16(extensionShortHeader) + extensions.addU16(0) // Length + } + if vers >= VersionTLS13 { if m.hasKeyShare { extensions.addU16(extensionKeyShare) @@ -996,6 +1013,11 @@ return false } m.earlyDataIndication = 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 57566c5..6499806 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -365,6 +365,15 @@ 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) @@ -972,6 +981,7 @@ 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 da1c90a..502e86e 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -383,6 +383,8 @@ // 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 @@ -611,6 +613,10 @@ } } + 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 { @@ -2563,11 +2569,57 @@ expectedError: expectedClientError, }) - if !shouldClientFail { - // Ensure the maximum record size is accepted. + if shouldClientFail { + return + } + + // Ensure the maximum record size is accepted. + testCases = append(testCases, testCase{ + protocol: protocol, + name: prefix + ver.name + "-" + suite.name + "-LargeRecord", + config: Config{ + MinVersion: ver.version, + MaxVersion: ver.version, + CipherSuites: []uint16{suite.id}, + Certificates: []Certificate{cert}, + PreSharedKey: []byte(psk), + PreSharedKeyIdentity: pskIdentity, + }, + flags: flags, + messageLen: maxPlaintext, + }) + + // Test bad records for all ciphers. Bad records are fatal in TLS + // and ignored in DTLS. + var shouldFail bool + var expectedError string + if protocol == tls { + shouldFail = true + expectedError = ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:" + } + + testCases = append(testCases, testCase{ + protocol: protocol, + name: prefix + ver.name + "-" + suite.name + "-BadRecord", + config: Config{ + MinVersion: ver.version, + MaxVersion: ver.version, + CipherSuites: []uint16{suite.id}, + Certificates: []Certificate{cert}, + PreSharedKey: []byte(psk), + PreSharedKeyIdentity: pskIdentity, + }, + flags: flags, + damageFirstWrite: true, + messageLen: maxPlaintext, + shouldFail: shouldFail, + expectedError: expectedError, + }) + + if ver.version >= VersionTLS13 { testCases = append(testCases, testCase{ protocol: protocol, - name: prefix + ver.name + "-" + suite.name + "-LargeRecord", + name: prefix + ver.name + "-" + suite.name + "-ShortHeader", config: Config{ MinVersion: ver.version, MaxVersion: ver.version, @@ -2575,36 +2627,13 @@ Certificates: []Certificate{cert}, PreSharedKey: []byte(psk), PreSharedKeyIdentity: pskIdentity, + Bugs: ProtocolBugs{ + EnableShortHeader: true, + }, }, - flags: flags, - messageLen: maxPlaintext, - }) - - // Test bad records for all ciphers. Bad records are fatal in TLS - // and ignored in DTLS. - var shouldFail bool - var expectedError string - if protocol == tls { - shouldFail = true - expectedError = ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:" - } - - testCases = append(testCases, testCase{ - protocol: protocol, - name: prefix + ver.name + "-" + suite.name + "-BadRecord", - config: Config{ - MinVersion: ver.version, - MaxVersion: ver.version, - CipherSuites: []uint16{suite.id}, - Certificates: []Certificate{cert}, - PreSharedKey: []byte(psk), - PreSharedKeyIdentity: pskIdentity, - }, - flags: flags, - damageFirstWrite: true, - messageLen: maxPlaintext, - shouldFail: shouldFail, - expectedError: expectedError, + flags: append([]string{"-enable-short-header"}, flags...), + resumeSession: true, + expectShortHeader: true, }) } } @@ -9767,6 +9796,150 @@ } } +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, + SendEarlyDataLength: 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() @@ -9893,6 +10066,7 @@ addCertificateTests() addRetainOnlySHA256ClientCertTests() addECDSAKeyUsageTests() + addShortHeaderTests() var wg sync.WaitGroup
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 2772831..0b11169 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -115,6 +115,7 @@ &TestConfig::expect_sha256_client_cert_initial }, { "-expect-sha256-client-cert-resume", &TestConfig::expect_sha256_client_cert_resume }, + { "-enable-short-header", &TestConfig::enable_short_header }, }; const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 2936477..9f3fbec 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -123,6 +123,7 @@ 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 ParseConfig(int argc, char **argv, TestConfig *out_config);
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index d19bc14..67807e8 100644 --- a/ssl/tls13_client.c +++ b/ssl/tls13_client.c
@@ -201,11 +201,12 @@ } /* Parse out the extensions. */ - int have_key_share = 0, have_pre_shared_key = 0; - CBS key_share, pre_shared_key; + int have_key_share = 0, have_pre_shared_key = 0, have_short_header = 0; + CBS key_share, pre_shared_key, short_header; 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; @@ -305,6 +306,23 @@ } 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 there was no HelloRetryRequest, the version negotiation logic has * already hashed the message. */ if (hs->received_hello_retry_request &&
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index e3606f1..a1bfe44 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c
@@ -127,6 +127,12 @@ return ssl_hs_error; } + /* 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; + } + hs->tls13_state = state_select_parameters; return ssl_hs_ok; } @@ -391,8 +397,18 @@ !CBB_add_u16(&body, ssl_cipher_get_value(ssl->s3->tmp.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) || - !ssl_complete_message(ssl, &cbb)) { + !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_complete_message(ssl, &cbb)) { goto err; }
diff --git a/ssl/tls_record.c b/ssl/tls_record.c index 9e04163..0039a02 100644 --- a/ssl/tls_record.c +++ b/ssl/tls_record.c
@@ -144,6 +144,19 @@ 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]; @@ -156,34 +169,46 @@ } size_t ssl_record_prefix_len(const SSL *ssl) { + size_t header_len; if (SSL_is_dtls(ssl)) { - return DTLS1_RT_HEADER_LENGTH + - SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_read_ctx); + header_len = DTLS1_RT_HEADER_LENGTH; + } else if (ssl_uses_short_header(ssl, evp_aead_open)) { + header_len = 2; } else { - return SSL3_RT_HEADER_LENGTH + - SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_read_ctx); + header_len = SSL3_RT_HEADER_LENGTH; } + + return header_len + SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_read_ctx); } size_t ssl_seal_align_prefix_len(const SSL *ssl) { if (SSL_is_dtls(ssl)) { return DTLS1_RT_HEADER_LENGTH + SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_write_ctx); - } else { - 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 += SSL3_RT_HEADER_LENGTH; - ret += ssl_cipher_get_record_split_len(ssl->s3->aead_write_ctx->cipher); - } - return ret; } + + 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); + if (ssl_needs_record_splitting(ssl)) { + ret += header_len; + ret += ssl_cipher_get_record_split_len(ssl->s3->aead_write_ctx->cipher); + } + return ret; } size_t SSL_max_seal_overhead(const SSL *ssl) { size_t ret = SSL_AEAD_CTX_max_overhead(ssl->s3->aead_write_ctx); if (SSL_is_dtls(ssl)) { ret += DTLS1_RT_HEADER_LENGTH; + } else if (ssl_uses_short_header(ssl, evp_aead_seal)) { + ret += 2; } else { ret += SSL3_RT_HEADER_LENGTH; } @@ -209,11 +234,31 @@ /* Decode the record header. */ uint8_t type; uint16_t version, ciphertext_len; - 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; + 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; } int version_ok; @@ -245,12 +290,11 @@ /* Extract the body. */ CBS body; if (!CBS_get_bytes(&cbs, &body, ciphertext_len)) { - *out_consumed = SSL3_RT_HEADER_LENGTH + (size_t)ciphertext_len; + *out_consumed = header_len + (size_t)ciphertext_len; return ssl_open_record_partial; } - ssl_do_msg_callback(ssl, 0 /* read */, SSL3_RT_HEADER, in, - SSL3_RT_HEADER_LENGTH); + ssl_do_msg_callback(ssl, 0 /* read */, SSL3_RT_HEADER, in, header_len); *out_consumed = in_len - CBS_len(&cbs); @@ -352,21 +396,32 @@ static int do_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out, uint8_t type, const uint8_t *in, size_t in_len) { - if (max_out < SSL3_RT_HEADER_LENGTH) { + 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) { + 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; + type = SSL3_RT_APPLICATION_DATA; + in_len++; + } + + if (max_out < header_len) { OPENSSL_PUT_ERROR(SSL, SSL_R_BUFFER_TOO_SMALL); return 0; } - /* Either |in| and |out| don't alias or |in| aligns with the - * ciphertext. |tls_seal_record| forbids aliasing, but TLS 1.3 aliases them - * internally. */ - assert(!buffers_alias(in, in_len, out, max_out) || - in == - out + SSL3_RT_HEADER_LENGTH + - SSL_AEAD_CTX_explicit_nonce_len(ssl->s3->aead_write_ctx)); - - out[0] = type; - /* The TLS record-layer version number is meaningless and, starting in * TLS 1.3, is frozen at TLS 1.0. But for historical reasons, SSL 3.0 * ClientHellos should use SSL 3.0 and pre-TLS-1.3 expects the version @@ -376,29 +431,39 @@ (ssl->s3->have_version && ssl3_protocol_version(ssl) < TLS1_3_VERSION)) { wire_version = ssl->version; } - out[1] = wire_version >> 8; - out[2] = wire_version & 0xff; + /* 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; + } + + /* Write the ciphertext, leaving two bytes for the length. */ size_t ciphertext_len; - if (!SSL_AEAD_CTX_seal(ssl->s3->aead_write_ctx, out + SSL3_RT_HEADER_LENGTH, - &ciphertext_len, max_out - SSL3_RT_HEADER_LENGTH, - type, wire_version, ssl->s3->write_sequence, in, - in_len) || + if (!SSL_AEAD_CTX_seal(ssl->s3->aead_write_ctx, out + 2, &ciphertext_len, + max_out - 2, type, wire_version, + ssl->s3->write_sequence, in, in_len) || !ssl_record_sequence_update(ssl->s3->write_sequence, 8)) { return 0; } - if (ciphertext_len >= 1 << 16) { + /* Fill in the length. */ + if (ciphertext_len >= 1 << 15) { OPENSSL_PUT_ERROR(SSL, ERR_R_OVERFLOW); return 0; } - out[3] = ciphertext_len >> 8; - out[4] = ciphertext_len & 0xff; + out[0] = ciphertext_len >> 8; + out[1] = ciphertext_len & 0xff; + if (short_header) { + out[0] |= 0x80; + } - *out_len = SSL3_RT_HEADER_LENGTH + ciphertext_len; + *out_len = header_len + ciphertext_len; - ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_HEADER, out, - SSL3_RT_HEADER_LENGTH); + ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_HEADER, out, header_len); return 1; } @@ -410,25 +475,6 @@ } size_t frag_len = 0; - - /* 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) { - size_t padding = SSL3_RT_HEADER_LENGTH + 1; - - if (in_len > in_len + padding || max_out < in_len + padding) { - OPENSSL_PUT_ERROR(SSL, SSL_R_BUFFER_TOO_SMALL); - return 0; - } - - OPENSSL_memmove(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 (type == SSL3_RT_APPLICATION_DATA && in_len > 1 && ssl_needs_record_splitting(ssl)) { if (!do_seal_record(ssl, out, &frag_len, max_out, type, in, 1)) { @@ -439,6 +485,7 @@ 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) ==