runner: Implement a more complete ClientHello consistency check.

The Go TLS implementation, at the time runner forked, had custom
testing-only equal methods on all the handshake messages. We've since
removed all of them except for ClientHello, where we repurposed the
function to check ClientHello consistency on HelloVerifyRequest and
HelloRetryRequest.

These are tedious to update. Upstream has since replaced them with
reflect.DeepEqual, but the comparison we want is even tighter. Even
unknown extensions aren't allowed to change. Replace the check with a
custom one that works on the byte serialization and remove
clientHelloMsg.equal.

Along the way, I've fixed the HRR PSK identity logic to match the spec a
bit more and check binders more consistently.

Change-Id: Ib39e8791201c42d37e304ae5110c7aeed62c8b3f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/43364
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
chromium-5359
David Benjamin 4 years ago committed by CQ bot account: commit-bot@chromium.org
parent f2b2ef8407
commit 974ac218e7
  1. 142
      ssl/test/runner/handshake_messages.go
  2. 250
      ssl/test/runner/handshake_server.go

@ -5,7 +5,6 @@
package runner
import (
"bytes"
"encoding/binary"
"fmt"
)
@ -299,60 +298,6 @@ type clientHelloMsg struct {
prefixExtensions []uint16
}
func (m *clientHelloMsg) equal(i interface{}) bool {
m1, ok := i.(*clientHelloMsg)
if !ok {
return false
}
return bytes.Equal(m.raw, m1.raw) &&
m.isDTLS == m1.isDTLS &&
m.vers == m1.vers &&
bytes.Equal(m.random, m1.random) &&
bytes.Equal(m.sessionId, m1.sessionId) &&
bytes.Equal(m.cookie, m1.cookie) &&
eqUint16s(m.cipherSuites, m1.cipherSuites) &&
bytes.Equal(m.compressionMethods, m1.compressionMethods) &&
m.nextProtoNeg == m1.nextProtoNeg &&
m.serverName == m1.serverName &&
m.ocspStapling == m1.ocspStapling &&
eqCurveIDs(m.supportedCurves, m1.supportedCurves) &&
bytes.Equal(m.supportedPoints, m1.supportedPoints) &&
m.hasKeyShares == m1.hasKeyShares &&
eqKeyShareEntryLists(m.keyShares, m1.keyShares) &&
m.trailingKeyShareData == m1.trailingKeyShareData &&
eqPSKIdentityLists(m.pskIdentities, m1.pskIdentities) &&
bytes.Equal(m.pskKEModes, m1.pskKEModes) &&
eqByteSlices(m.pskBinders, m1.pskBinders) &&
m.hasEarlyData == m1.hasEarlyData &&
bytes.Equal(m.tls13Cookie, m1.tls13Cookie) &&
m.ticketSupported == m1.ticketSupported &&
bytes.Equal(m.sessionTicket, m1.sessionTicket) &&
eqSignatureAlgorithms(m.signatureAlgorithms, m1.signatureAlgorithms) &&
eqSignatureAlgorithms(m.signatureAlgorithmsCert, m1.signatureAlgorithmsCert) &&
eqUint16s(m.supportedVersions, m1.supportedVersions) &&
bytes.Equal(m.secureRenegotiation, m1.secureRenegotiation) &&
(m.secureRenegotiation == nil) == (m1.secureRenegotiation == nil) &&
eqStrings(m.alpnProtocols, m1.alpnProtocols) &&
bytes.Equal(m.quicTransportParams, m1.quicTransportParams) &&
m.duplicateExtension == m1.duplicateExtension &&
m.channelIDSupported == m1.channelIDSupported &&
bytes.Equal(m.tokenBindingParams, m1.tokenBindingParams) &&
m.tokenBindingVersion == m1.tokenBindingVersion &&
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.omitExtensions == m1.omitExtensions &&
m.emptyExtensions == m1.emptyExtensions &&
m.pad == m1.pad &&
eqUint16s(m.compressedCertAlgs, m1.compressedCertAlgs) &&
m.delegatedCredentials == m1.delegatedCredentials &&
eqUint16s(m.prefixExtensions, m1.prefixExtensions)
}
func (m *clientHelloMsg) marshalKeyShares(bb *byteBuilder) {
keyShares := bb.addU16LengthPrefixed()
for _, keyShare := range m.keyShares {
@ -2591,90 +2536,3 @@ func (*endOfEarlyDataMsg) unmarshal(data []byte) bool {
// ssl3NoCertificateMsg is a dummy message to handle SSL 3.0 using a warning
// alert in the handshake.
type ssl3NoCertificateMsg struct{}
func eqUint16s(x, y []uint16) bool {
if len(x) != len(y) {
return false
}
for i, v := range x {
if y[i] != v {
return false
}
}
return true
}
func eqCurveIDs(x, y []CurveID) bool {
if len(x) != len(y) {
return false
}
for i, v := range x {
if y[i] != v {
return false
}
}
return true
}
func eqStrings(x, y []string) bool {
if len(x) != len(y) {
return false
}
for i, v := range x {
if y[i] != v {
return false
}
}
return true
}
func eqByteSlices(x, y [][]byte) bool {
if len(x) != len(y) {
return false
}
for i, v := range x {
if !bytes.Equal(v, y[i]) {
return false
}
}
return true
}
func eqSignatureAlgorithms(x, y []signatureAlgorithm) bool {
if len(x) != len(y) {
return false
}
for i, v := range x {
v2 := y[i]
if v != v2 {
return false
}
}
return true
}
func eqKeyShareEntryLists(x, y []keyShareEntry) bool {
if len(x) != len(y) {
return false
}
for i, v := range x {
if y[i].group != v.group || !bytes.Equal(y[i].keyExchange, v.keyExchange) {
return false
}
}
return true
}
func eqPSKIdentityLists(x, y []pskIdentity) bool {
if len(x) != len(y) {
return false
}
for i, v := range x {
if !bytes.Equal(y[i].ticket, v.ticket) || y[i].obfuscatedTicketAge != v.obfuscatedTicketAge {
return false
}
}
return true
}

@ -185,18 +185,8 @@ func (hs *serverHandshakeState) readClientHello() error {
if !bytes.Equal(newClientHello.cookie, helloVerifyRequest.cookie) {
return errors.New("dtls: invalid cookie")
}
// Apart from the cookie, the two ClientHellos must
// match. Note that clientHello.equal compares the
// serialization, so we make a copy.
oldClientHelloCopy := *hs.clientHello
oldClientHelloCopy.raw = nil
oldClientHelloCopy.cookie = nil
newClientHelloCopy := *newClientHello
newClientHelloCopy.raw = nil
newClientHelloCopy.cookie = nil
if !oldClientHelloCopy.equal(&newClientHelloCopy) {
return errors.New("dtls: retransmitted ClientHello does not match")
if err := checkClientHellosEqual(hs.clientHello.raw, newClientHello.raw, c.isDTLS, nil); err != nil {
return err
}
hs.clientHello = newClientHello
}
@ -465,12 +455,16 @@ Curves:
pskIdentities := hs.clientHello.pskIdentities
pskKEModes := hs.clientHello.pskKEModes
var replacedPSKIdentities bool
if len(pskIdentities) == 0 && len(hs.clientHello.sessionTicket) > 0 && c.config.Bugs.AcceptAnySession {
// Pick up the ticket from the TLS 1.2 extension, to test the
// client does not get in a mixed up state.
psk := pskIdentity{
ticket: hs.clientHello.sessionTicket,
}
pskIdentities = []pskIdentity{psk}
pskKEModes = []byte{pskDHEKEMode}
replacedPSKIdentities = true
}
var pskIndex int
@ -502,6 +496,13 @@ Curves:
return errors.New("tls: invalid ticket age")
}
if !replacedPSKIdentities {
binderToVerify := hs.clientHello.pskBinders[i]
if err := verifyPSKBinder(c.wireVersion, hs.clientHello, sessionState, binderToVerify, []byte{}, []byte{}); err != nil {
return err
}
}
hs.sessionState = sessionState
hs.hello.hasPSKIdentity = true
hs.hello.pskIdentity = uint16(i)
@ -519,15 +520,6 @@ Curves:
hs.hello.pskIdentity = 0
}
// Verify the PSK binder. Note there may not be a PSK binder if
// AcceptAnyBinder is set. See https://crbug.com/boringssl/115.
if hs.sessionState != nil && !config.Bugs.AcceptAnySession {
binderToVerify := hs.clientHello.pskBinders[pskIndex]
if err := verifyPSKBinder(c.wireVersion, hs.clientHello, hs.sessionState, binderToVerify, []byte{}, []byte{}); err != nil {
return err
}
}
// Resolve PSK and compute the early secret.
if hs.sessionState != nil {
hs.finishedHash.addEntropy(hs.sessionState.masterSecret)
@ -643,70 +635,84 @@ ResendHelloRetryRequest:
return fmt.Errorf("tls: client offered unexpected PSK identities after HelloRetryRequest")
}
if newClientHello.hasEarlyData {
return errors.New("tls: EarlyData sent in new ClientHello")
}
applyBugsToClientHello(newClientHello, config)
// Check that the new ClientHello matches the old ClientHello,
// except for relevant modifications.
//
// TODO(davidben): Make this check more precise.
oldClientHelloCopy := *hs.clientHello
oldClientHelloCopy.raw = nil
oldClientHelloCopy.hasEarlyData = false
newClientHelloCopy := *newClientHello
newClientHelloCopy.raw = nil
// except for relevant modifications. See RFC 8446, section 4.1.2.
ignoreExtensions := []uint16{extensionPadding}
if helloRetryRequest.hasSelectedGroup {
newKeyShares := newClientHelloCopy.keyShares
newKeyShares := newClientHello.keyShares
if len(newKeyShares) != 1 || newKeyShares[0].group != helloRetryRequest.selectedGroup {
return errors.New("tls: KeyShare from HelloRetryRequest not in new ClientHello")
}
selectedKeyShare = &newKeyShares[0]
newClientHelloCopy.keyShares = oldClientHelloCopy.keyShares
ignoreExtensions = append(ignoreExtensions, extensionKeyShare)
}
if len(helloRetryRequest.cookie) > 0 {
if !bytes.Equal(newClientHelloCopy.tls13Cookie, helloRetryRequest.cookie) {
if !bytes.Equal(newClientHello.tls13Cookie, helloRetryRequest.cookie) {
return errors.New("tls: cookie from HelloRetryRequest not present in new ClientHello")
}
newClientHelloCopy.tls13Cookie = nil
ignoreExtensions = append(ignoreExtensions, extensionCookie)
}
// PSK binders and obfuscated ticket age are both updated in the
// second ClientHello.
if len(oldClientHelloCopy.pskIdentities) != len(newClientHelloCopy.pskIdentities) {
newClientHelloCopy.pskIdentities = oldClientHelloCopy.pskIdentities
} else {
if len(oldClientHelloCopy.pskIdentities) != len(newClientHelloCopy.pskIdentities) {
return errors.New("tls: PSK identity count from old and new ClientHello do not match")
// The second ClientHello refreshes binders, and may drop PSK identities
// that are no longer consistent with the cipher suite.
oldPSKIdentities := hs.clientHello.pskIdentities
for _, identity := range newClientHello.pskIdentities {
// Skip to the matching PSK identity in oldPSKIdentities.
for len(oldPSKIdentities) > 0 && !bytes.Equal(oldPSKIdentities[0].ticket, identity.ticket) {
oldPSKIdentities = oldPSKIdentities[1:]
}
// The identity now either matches, or oldPSKIdentities is empty.
if len(oldPSKIdentities) == 0 {
return errors.New("tls: unexpected PSK identity in second ClientHello")
}
oldPSKIdentities = oldPSKIdentities[1:]
}
ignoreExtensions = append(ignoreExtensions, extensionPreSharedKey)
// Update the index for the identity we resumed. The client may have
// dropped some entries.
if hs.sessionState != nil {
var found bool
ticket := hs.clientHello.pskIdentities[pskIndex].ticket
for i, identity := range newClientHello.pskIdentities {
if bytes.Equal(identity.ticket, ticket) {
found = true
pskIndex = i
break
}
}
for i, identity := range oldClientHelloCopy.pskIdentities {
newClientHelloCopy.pskIdentities[i].obfuscatedTicketAge = identity.obfuscatedTicketAge
if found {
binderToVerify := newClientHello.pskBinders[pskIndex]
if err := verifyPSKBinder(c.wireVersion, newClientHello, hs.sessionState, binderToVerify, oldClientHelloBytes, helloRetryRequest.marshal()); err != nil {
return err
}
} else if !config.Bugs.AcceptAnySession {
// If AcceptAnySession is set, the client may have already noticed
// the selected session is incompatible with the HelloRetryRequest
// and correctly dropped the PSK identity. We may also have
// attempted to resume a session from the TLS 1.2 extension.
return errors.New("tls: second ClientHello is missing selected session")
}
}
newClientHelloCopy.pskBinders = oldClientHelloCopy.pskBinders
newClientHelloCopy.hasEarlyData = oldClientHelloCopy.hasEarlyData
if !oldClientHelloCopy.equal(&newClientHelloCopy) {
return errors.New("tls: new ClientHello does not match")
// The second ClientHello must stop offering early data.
if newClientHello.hasEarlyData {
return errors.New("tls: EarlyData sent in new ClientHello")
}
ignoreExtensions = append(ignoreExtensions, extensionEarlyData)
if err := checkClientHellosEqual(hs.clientHello.raw, newClientHello.raw, c.isDTLS, ignoreExtensions); err != nil {
return err
}
if firstHelloRetryRequest && config.Bugs.SecondHelloRetryRequest {
firstHelloRetryRequest = false
goto ResendHelloRetryRequest
}
// Verify the PSK binder. Note there may not be a PSK binder if
// AcceptAnyBinder is set. See https://crbug.com/115.
if hs.sessionState != nil && !config.Bugs.AcceptAnySession {
binderToVerify := newClientHello.pskBinders[pskIndex]
if err := verifyPSKBinder(c.wireVersion, newClientHello, hs.sessionState, binderToVerify, oldClientHelloBytes, helloRetryRequest.marshal()); err != nil {
return err
}
}
}
// Decide whether or not to accept early data.
@ -2211,3 +2217,129 @@ func verifyPSKBinder(version uint16, clientHello *clientHelloMsg, sessionState *
return nil
}
// checkClientHellosEqual checks whether a and b are equal ClientHello
// messages. If isDTLS is true, the ClientHellos are parsed as DTLS and any
// differences in the cookie field are ignored. Extensions listed in
// ignoreExtensions may change or be removed between the two ClientHellos.
func checkClientHellosEqual(a, b []byte, isDTLS bool, ignoreExtensions []uint16) error {
ignoreExtensionsSet := make(map[uint16]struct{})
for _, ext := range ignoreExtensions {
ignoreExtensionsSet[ext] = struct{}{}
}
// Skip the handshake message header.
aReader := byteReader(a[4:])
bReader := byteReader(b[4:])
var aVers, bVers uint16
var aRandom, bRandom []byte
var aSessionID, bSessionID []byte
if !aReader.readU16(&aVers) ||
!bReader.readU16(&bVers) ||
!aReader.readBytes(&aRandom, 32) ||
!bReader.readBytes(&bRandom, 32) ||
!aReader.readU8LengthPrefixedBytes(&aSessionID) ||
!bReader.readU8LengthPrefixedBytes(&bSessionID) {
return errors.New("tls: could not parse ClientHello")
}
if aVers != bVers {
return errors.New("tls: second ClientHello version did not match")
}
if !bytes.Equal(aRandom, bRandom) {
return errors.New("tls: second ClientHello random did not match")
}
if !bytes.Equal(aSessionID, bSessionID) {
return errors.New("tls: second ClientHello session ID did not match")
}
if isDTLS {
// DTLS 1.2 checks two ClientHellos match after a HelloVerifyRequest,
// where we expect the cookies to change. DTLS 1.3 forbids the legacy
// cookie altogether. If we implement DTLS 1.3, we'll need to ensure
// that parsing logic above this function rejects this cookie.
var aCookie, bCookie []byte
if !aReader.readU8LengthPrefixedBytes(&aCookie) ||
!bReader.readU8LengthPrefixedBytes(&bCookie) {
return errors.New("tls: could not parse ClientHello")
}
}
var aCipherSuites, bCipherSuites, aCompressionMethods, bCompressionMethods []byte
if !aReader.readU16LengthPrefixedBytes(&aCipherSuites) ||
!bReader.readU16LengthPrefixedBytes(&bCipherSuites) ||
!aReader.readU8LengthPrefixedBytes(&aCompressionMethods) ||
!bReader.readU8LengthPrefixedBytes(&bCompressionMethods) {
return errors.New("tls: could not parse ClientHello")
}
if !bytes.Equal(aCipherSuites, bCipherSuites) {
return errors.New("tls: second ClientHello cipher suites did not match")
}
if !bytes.Equal(aCompressionMethods, bCompressionMethods) {
return errors.New("tls: second ClientHello compression methods did not match")
}
if len(aReader) == 0 && len(bReader) == 0 {
// Both ClientHellos omit the extensions block.
return nil
}
var aExtensions, bExtensions byteReader
if !aReader.readU16LengthPrefixed(&aExtensions) ||
!bReader.readU16LengthPrefixed(&bExtensions) ||
len(aReader) != 0 ||
len(bReader) != 0 {
return errors.New("tls: could not parse ClientHello")
}
for len(aExtensions) != 0 {
var aID uint16
var aBody []byte
if !aExtensions.readU16(&aID) ||
!aExtensions.readU16LengthPrefixedBytes(&aBody) {
return errors.New("tls: could not parse ClientHello")
}
if _, ok := ignoreExtensionsSet[aID]; ok {
continue
}
for {
if len(bExtensions) == 0 {
return fmt.Errorf("tls: second ClientHello missing extension %d", aID)
}
var bID uint16
var bBody []byte
if !bExtensions.readU16(&bID) ||
!bExtensions.readU16LengthPrefixedBytes(&bBody) {
return errors.New("tls: could not parse ClientHello")
}
if _, ok := ignoreExtensionsSet[bID]; ok {
continue
}
if aID != bID {
return fmt.Errorf("tls: unexpected extension %d in second ClientHello (wanted %d)", bID, aID)
}
if !bytes.Equal(aBody, bBody) {
return fmt.Errorf("tls: extension %d in second ClientHello unexpectedly changed", aID)
}
break
}
}
// Any remaining extensions in the second ClientHello must be in the
// ignored set.
for len(bExtensions) != 0 {
var id uint16
var body []byte
if !bExtensions.readU16(&id) ||
!bExtensions.readU16LengthPrefixedBytes(&body) {
return errors.New("tls: could not parse ClientHello")
}
if _, ok := ignoreExtensionsSet[id]; !ok {
return fmt.Errorf("tls: unexpected extension %d in second ClientHello", id)
}
}
return nil
}

Loading…
Cancel
Save