runner: Refactor BoGo clientHelloMsg extension marshalling. This CL replaces clientHelloMsg's npnAfterAlpn and pskBinderFirst fields with a new field: prefixExtensions. The extensions in prefixExtensions are tried first when marshalling clientHelloMsg. The ability to control extensions' marshalling order will make it simpler to implement the "outer_extensions" behavior defined in draft-ietf-tls-esni-07. Bug: 275 Change-Id: Ib6dcc1e6fa0281f312cb65a9e204415c3f3ef2c6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43064 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index d1ea08d..b7517e7 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -125,6 +125,7 @@ extensionQUICTransportParams uint16 = 0xffa5 // draft-ietf-quic-tls-13 extensionChannelID uint16 = 30032 // not IANA assigned extensionDelegatedCredentials uint16 = 0x22 // draft-ietf-tls-subcerts-06 + extensionDuplicate uint16 = 0xffff // not IANA assigned ) // TLS signaling cipher suite values
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 8548b78..241518f 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -100,6 +100,19 @@ return errors.New("tls: NextProtos values too large") } + // Translate the bugs that modify ClientHello extension order into a + // list of prefix extensions. The marshal function will try these + // extensions before any others, followed by any remaining extensions in + // the default order. + var prefixExtensions []uint16 + if c.config.Bugs.PSKBinderFirst && !c.config.Bugs.OnlyCorruptSecondPSKBinder { + prefixExtensions = append(prefixExtensions, extensionPreSharedKey) + } + if c.config.Bugs.SwapNPNAndALPN { + prefixExtensions = append(prefixExtensions, extensionALPN) + prefixExtensions = append(prefixExtensions, extensionNextProtoNeg) + } + minVersion := c.config.minVersion(c.isDTLS) maxVersion := c.config.maxVersion(c.isDTLS) hello := &clientHelloMsg{ @@ -119,15 +132,14 @@ channelIDSupported: c.config.ChannelID != nil, tokenBindingParams: c.config.TokenBindingParams, tokenBindingVersion: c.config.TokenBindingVersion, - npnAfterAlpn: c.config.Bugs.SwapNPNAndALPN, extendedMasterSecret: maxVersion >= VersionTLS10, srtpProtectionProfiles: c.config.SRTPProtectionProfiles, srtpMasterKeyIdentifier: c.config.Bugs.SRTPMasterKeyIdentifer, customExtension: c.config.Bugs.CustomExtension, - pskBinderFirst: c.config.Bugs.PSKBinderFirst && !c.config.Bugs.OnlyCorruptSecondPSKBinder, omitExtensions: c.config.Bugs.OmitExtensions, emptyExtensions: c.config.Bugs.EmptyExtensions, delegatedCredentials: !c.config.Bugs.DisableDelegatedCredentials, + prefixExtensions: prefixExtensions, } if maxVersion >= VersionTLS13 { @@ -608,7 +620,9 @@ hello.hasEarlyData = c.config.Bugs.SendEarlyDataOnSecondClientHello // The first ClientHello may have skipped this due to OnlyCorruptSecondPSKBinder. - hello.pskBinderFirst = c.config.Bugs.PSKBinderFirst + if c.config.Bugs.PSKBinderFirst && c.config.Bugs.OnlyCorruptSecondPSKBinder { + hello.prefixExtensions = append(hello.prefixExtensions, extensionPreSharedKey) + } if c.config.Bugs.OmitPSKsOnSecondClientHello { hello.pskIdentities = nil hello.pskBinders = nil
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 9d3b6bc..500e14e 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -285,19 +285,18 @@ channelIDSupported bool tokenBindingParams []byte tokenBindingVersion uint16 - npnAfterAlpn bool extendedMasterSecret bool srtpProtectionProfiles []uint16 srtpMasterKeyIdentifier string sctListSupported bool customExtension string hasGREASEExtension bool - pskBinderFirst bool omitExtensions bool emptyExtensions bool pad int compressedCertAlgs []uint16 delegatedCredentials bool + prefixExtensions []uint16 } func (m *clientHelloMsg) equal(i interface{}) bool { @@ -340,19 +339,18 @@ m.channelIDSupported == m1.channelIDSupported && bytes.Equal(m.tokenBindingParams, m1.tokenBindingParams) && m.tokenBindingVersion == m1.tokenBindingVersion && - m.npnAfterAlpn == m1.npnAfterAlpn && m.extendedMasterSecret == m1.extendedMasterSecret && eqUint16s(m.srtpProtectionProfiles, m1.srtpProtectionProfiles) && m.srtpMasterKeyIdentifier == m1.srtpMasterKeyIdentifier && m.sctListSupported == m1.sctListSupported && m.customExtension == m1.customExtension && m.hasGREASEExtension == m1.hasGREASEExtension && - m.pskBinderFirst == m1.pskBinderFirst && m.omitExtensions == m1.omitExtensions && m.emptyExtensions == m1.emptyExtensions && m.pad == m1.pad && eqUint16s(m.compressedCertAlgs, m1.compressedCertAlgs) && - m.delegatedCredentials == m1.delegatedCredentials + m.delegatedCredentials == m1.delegatedCredentials && + eqUint16s(m.prefixExtensions, m1.prefixExtensions) } func (m *clientHelloMsg) marshalKeyShares(bb *byteBuilder) { @@ -390,34 +388,20 @@ compressionMethods := hello.addU8LengthPrefixed() compressionMethods.addBytes(m.compressionMethods) - extensions := hello.addU16LengthPrefixed() - if len(m.pskIdentities) > 0 && m.pskBinderFirst { - extensions.addU16(extensionPreSharedKey) - pskExtension := extensions.addU16LengthPrefixed() - - pskIdentities := pskExtension.addU16LengthPrefixed() - for _, psk := range m.pskIdentities { - pskIdentities.addU16LengthPrefixed().addBytes(psk.ticket) - pskIdentities.addU32(psk.obfuscatedTicketAge) - } - pskBinders := pskExtension.addU16LengthPrefixed() - for _, binder := range m.pskBinders { - pskBinders.addU8LengthPrefixed().addBytes(binder) - } + type extension struct { + id uint16 + body []byte } + var extensions []extension + if m.duplicateExtension { // Add a duplicate bogus extension at the beginning and end. - extensions.addU16(0xffff) - extensions.addU16(0) // 0-length for empty extension + extensions = append(extensions, extension{id: extensionDuplicate}) } - if m.nextProtoNeg && !m.npnAfterAlpn { - extensions.addU16(extensionNextProtoNeg) - extensions.addU16(0) // The length is always 0 + if m.nextProtoNeg { + extensions = append(extensions, extension{id: extensionNextProtoNeg}) } if len(m.serverName) > 0 { - extensions.addU16(extensionServerName) - serverNameList := extensions.addU16LengthPrefixed() - // RFC 3546, section 3.1 // // struct { @@ -437,138 +421,170 @@ // ServerName server_name_list<1..2^16-1> // } ServerNameList; + serverNameList := newByteBuilder() serverName := serverNameList.addU16LengthPrefixed() serverName.addU8(0) // NameType host_name(0) hostName := serverName.addU16LengthPrefixed() hostName.addBytes([]byte(m.serverName)) + + extensions = append(extensions, extension{ + id: extensionServerName, + body: serverNameList.finish(), + }) } if m.ocspStapling { - extensions.addU16(extensionStatusRequest) - certificateStatusRequest := extensions.addU16LengthPrefixed() - + certificateStatusRequest := newByteBuilder() // RFC 4366, section 3.6 certificateStatusRequest.addU8(1) // OCSP type // Two zero valued uint16s for the two lengths. certificateStatusRequest.addU16(0) // ResponderID length certificateStatusRequest.addU16(0) // Extensions length + extensions = append(extensions, extension{ + id: extensionStatusRequest, + body: certificateStatusRequest.finish(), + }) } if len(m.supportedCurves) > 0 { // http://tools.ietf.org/html/rfc4492#section-5.1.1 - extensions.addU16(extensionSupportedCurves) - supportedCurvesList := extensions.addU16LengthPrefixed() + supportedCurvesList := newByteBuilder() supportedCurves := supportedCurvesList.addU16LengthPrefixed() for _, curve := range m.supportedCurves { supportedCurves.addU16(uint16(curve)) } + extensions = append(extensions, extension{ + id: extensionSupportedCurves, + body: supportedCurvesList.finish(), + }) } if len(m.supportedPoints) > 0 { // http://tools.ietf.org/html/rfc4492#section-5.1.2 - extensions.addU16(extensionSupportedPoints) - supportedPointsList := extensions.addU16LengthPrefixed() + supportedPointsList := newByteBuilder() supportedPoints := supportedPointsList.addU8LengthPrefixed() supportedPoints.addBytes(m.supportedPoints) + extensions = append(extensions, extension{ + id: extensionSupportedPoints, + body: supportedPointsList.finish(), + }) } if m.hasKeyShares { - extensions.addU16(extensionKeyShare) - keyShareList := extensions.addU16LengthPrefixed() + keyShareList := newByteBuilder() m.marshalKeyShares(keyShareList) + extensions = append(extensions, extension{ + id: extensionKeyShare, + body: keyShareList.finish(), + }) } if len(m.pskKEModes) > 0 { - extensions.addU16(extensionPSKKeyExchangeModes) - pskModesExtension := extensions.addU16LengthPrefixed() + pskModesExtension := newByteBuilder() pskModesExtension.addU8LengthPrefixed().addBytes(m.pskKEModes) + extensions = append(extensions, extension{ + id: extensionPSKKeyExchangeModes, + body: pskModesExtension.finish(), + }) } if m.hasEarlyData { - extensions.addU16(extensionEarlyData) - extensions.addU16(0) // The length is zero. + extensions = append(extensions, extension{id: extensionEarlyData}) } if len(m.tls13Cookie) > 0 { - extensions.addU16(extensionCookie) - body := extensions.addU16LengthPrefixed() + body := newByteBuilder() body.addU16LengthPrefixed().addBytes(m.tls13Cookie) + extensions = append(extensions, extension{ + id: extensionCookie, + body: body.finish(), + }) } if m.ticketSupported { // http://tools.ietf.org/html/rfc5077#section-3.2 - extensions.addU16(extensionSessionTicket) - sessionTicketExtension := extensions.addU16LengthPrefixed() - sessionTicketExtension.addBytes(m.sessionTicket) + extensions = append(extensions, extension{ + id: extensionSessionTicket, + body: m.sessionTicket, + }) } if len(m.signatureAlgorithms) > 0 { // https://tools.ietf.org/html/rfc5246#section-7.4.1.4.1 - extensions.addU16(extensionSignatureAlgorithms) - signatureAlgorithmsExtension := extensions.addU16LengthPrefixed() + signatureAlgorithmsExtension := newByteBuilder() signatureAlgorithms := signatureAlgorithmsExtension.addU16LengthPrefixed() for _, sigAlg := range m.signatureAlgorithms { signatureAlgorithms.addU16(uint16(sigAlg)) } + extensions = append(extensions, extension{ + id: extensionSignatureAlgorithms, + body: signatureAlgorithmsExtension.finish(), + }) } if len(m.signatureAlgorithmsCert) > 0 { - extensions.addU16(extensionSignatureAlgorithmsCert) - signatureAlgorithmsCertExtension := extensions.addU16LengthPrefixed() + signatureAlgorithmsCertExtension := newByteBuilder() signatureAlgorithmsCert := signatureAlgorithmsCertExtension.addU16LengthPrefixed() for _, sigAlg := range m.signatureAlgorithmsCert { signatureAlgorithmsCert.addU16(uint16(sigAlg)) } + extensions = append(extensions, extension{ + id: extensionSignatureAlgorithmsCert, + body: signatureAlgorithmsCertExtension.finish(), + }) } if len(m.supportedVersions) > 0 { - extensions.addU16(extensionSupportedVersions) - supportedVersionsExtension := extensions.addU16LengthPrefixed() + supportedVersionsExtension := newByteBuilder() supportedVersions := supportedVersionsExtension.addU8LengthPrefixed() for _, version := range m.supportedVersions { supportedVersions.addU16(uint16(version)) } + extensions = append(extensions, extension{ + id: extensionSupportedVersions, + body: supportedVersionsExtension.finish(), + }) } if m.secureRenegotiation != nil { - extensions.addU16(extensionRenegotiationInfo) - secureRenegoExt := extensions.addU16LengthPrefixed() - secureRenego := secureRenegoExt.addU8LengthPrefixed() - secureRenego.addBytes(m.secureRenegotiation) + secureRenegoExt := newByteBuilder() + secureRenegoExt.addU8LengthPrefixed().addBytes(m.secureRenegotiation) + extensions = append(extensions, extension{ + id: extensionRenegotiationInfo, + body: secureRenegoExt.finish(), + }) } if len(m.alpnProtocols) > 0 { // https://tools.ietf.org/html/rfc7301#section-3.1 - extensions.addU16(extensionALPN) - alpnExtension := extensions.addU16LengthPrefixed() - + alpnExtension := newByteBuilder() protocolNameList := alpnExtension.addU16LengthPrefixed() for _, s := range m.alpnProtocols { protocolName := protocolNameList.addU8LengthPrefixed() protocolName.addBytes([]byte(s)) } + extensions = append(extensions, extension{ + id: extensionALPN, + body: alpnExtension.finish(), + }) } if len(m.quicTransportParams) > 0 { - extensions.addU16(extensionQUICTransportParams) - params := extensions.addU16LengthPrefixed() - params.addBytes(m.quicTransportParams) + extensions = append(extensions, extension{ + id: extensionQUICTransportParams, + body: m.quicTransportParams, + }) } if m.channelIDSupported { - extensions.addU16(extensionChannelID) - extensions.addU16(0) // Length is always 0 + extensions = append(extensions, extension{id: extensionChannelID}) } if m.tokenBindingParams != nil { - extensions.addU16(extensionTokenBinding) - tokbindExtension := extensions.addU16LengthPrefixed() + tokbindExtension := newByteBuilder() tokbindExtension.addU16(m.tokenBindingVersion) tokbindParams := tokbindExtension.addU8LengthPrefixed() tokbindParams.addBytes(m.tokenBindingParams) - } - if m.nextProtoNeg && m.npnAfterAlpn { - extensions.addU16(extensionNextProtoNeg) - extensions.addU16(0) // Length is always 0 + extensions = append(extensions, extension{ + id: extensionTokenBinding, + body: tokbindExtension.finish(), + }) } if m.duplicateExtension { // Add a duplicate bogus extension at the beginning and end. - extensions.addU16(0xffff) - extensions.addU16(0) + extensions = append(extensions, extension{id: extensionDuplicate}) } if m.extendedMasterSecret { // https://tools.ietf.org/html/rfc7627 - extensions.addU16(extensionExtendedMasterSecret) - extensions.addU16(0) // Length is always 0 + extensions = append(extensions, extension{id: extensionExtendedMasterSecret}) } if len(m.srtpProtectionProfiles) > 0 { // https://tools.ietf.org/html/rfc5764#section-4.1.1 - extensions.addU16(extensionUseSRTP) - useSrtpExt := extensions.addU16LengthPrefixed() + useSrtpExt := newByteBuilder() srtpProtectionProfiles := useSrtpExt.addU16LengthPrefixed() for _, p := range m.srtpProtectionProfiles { @@ -576,38 +592,47 @@ } srtpMki := useSrtpExt.addU8LengthPrefixed() srtpMki.addBytes([]byte(m.srtpMasterKeyIdentifier)) + + extensions = append(extensions, extension{ + id: extensionUseSRTP, + body: useSrtpExt.finish(), + }) } if m.sctListSupported { - extensions.addU16(extensionSignedCertificateTimestamp) - extensions.addU16(0) // Length is always 0 + extensions = append(extensions, extension{id: extensionSignedCertificateTimestamp}) } if l := len(m.customExtension); l > 0 { - extensions.addU16(extensionCustom) - customExt := extensions.addU16LengthPrefixed() - customExt.addBytes([]byte(m.customExtension)) + extensions = append(extensions, extension{ + id: extensionCustom, + body: []byte(m.customExtension), + }) } if len(m.compressedCertAlgs) > 0 { - extensions.addU16(extensionCompressedCertAlgs) - body := extensions.addU16LengthPrefixed() + body := newByteBuilder() algIDs := body.addU8LengthPrefixed() for _, v := range m.compressedCertAlgs { algIDs.addU16(v) } + extensions = append(extensions, extension{ + id: extensionCompressedCertAlgs, + body: body.finish(), + }) } if m.delegatedCredentials { - extensions.addU16(extensionDelegatedCredentials) - body := extensions.addU16LengthPrefixed() + body := newByteBuilder() signatureSchemeList := body.addU16LengthPrefixed() for _, sigAlg := range m.signatureAlgorithms { signatureSchemeList.addU16(uint16(sigAlg)) } + extensions = append(extensions, extension{ + id: extensionDelegatedCredentials, + body: body.finish(), + }) } // The PSK extension must be last. See https://tools.ietf.org/html/rfc8446#section-4.2.11 - if len(m.pskIdentities) > 0 && !m.pskBinderFirst { - extensions.addU16(extensionPreSharedKey) - pskExtension := extensions.addU16LengthPrefixed() - + if len(m.pskIdentities) > 0 { + pskExtension := newByteBuilder() pskIdentities := pskExtension.addU16LengthPrefixed() for _, psk := range m.pskIdentities { pskIdentities.addU16LengthPrefixed().addBytes(psk.ticket) @@ -617,11 +642,43 @@ for _, binder := range m.pskBinders { pskBinders.addU8LengthPrefixed().addBytes(binder) } + extensions = append(extensions, extension{ + id: extensionPreSharedKey, + body: pskExtension.finish(), + }) + } + + // Write each extension in |extensions| to the |hello| message, hoisting + // the extensions named in |m.prefixExtensions| to the front. + extensionsBB := hello.addU16LengthPrefixed() + extMap := make(map[uint16][]byte) + for _, ext := range extensions { + extMap[ext.id] = ext.body + } + // Write each of the prefix extensions, if we have it. + for _, extID := range m.prefixExtensions { + if body, ok := extMap[extID]; ok { + extensionsBB.addU16(extID) + extensionsBB.addU16LengthPrefixed().addBytes(body) + } + } + // Forget each of the prefix extensions. This loop is separate from the + // extension-writing loop because |m.prefixExtensions| may contain + // duplicates. + for _, extID := range m.prefixExtensions { + delete(extMap, extID) + } + // Write each of the remaining extensions in their original order. + for _, ext := range extensions { + if _, ok := extMap[ext.id]; ok { + extensionsBB.addU16(ext.id) + extensionsBB.addU16LengthPrefixed().addBytes(ext.body) + } } if m.pad != 0 && hello.len()%m.pad != 0 { - extensions.addU16(extensionPadding) - padding := extensions.addU16LengthPrefixed() + extensionsBB.addU16(extensionPadding) + padding := extensionsBB.addU16LengthPrefixed() // Note hello.len() has changed at this point from the length // prefix. if l := hello.len() % m.pad; l != 0 { @@ -1236,7 +1293,7 @@ func (m *serverExtensions) marshal(extensions *byteBuilder) { if m.duplicateExtension { // Add a duplicate bogus extension at the beginning and end. - extensions.addU16(0xffff) + extensions.addU16(extensionDuplicate) extensions.addU16(0) // length = 0 for empty extension } if m.nextProtoNeg && !m.npnAfterAlpn { @@ -1286,7 +1343,7 @@ } if m.duplicateExtension { // Add a duplicate bogus extension at the beginning and end. - extensions.addU16(0xffff) + extensions.addU16(extensionDuplicate) extensions.addU16(0) } if m.extendedMasterSecret {