runner: Ensure helloBytes is always the same as hello.marshal().
The client handshake currently defers creating the finishedHash and
writing things into the transcript, which is a little annoying for ECH.
In preparation for simplifying that, one nuisance is that we retain both
hello and helloBytes, across a long span of code. helloBytes is *almost*
the same as hello.marshal() except:
- When we send a V2ClientHello, helloBytes records that we serialized
the ClientHello completely differently.
- For the JDK11 workaround tests, helloBytes records that we swapped out
the ClientHello entirely.
- By the time we finally write helloBytes into the transcript, hello may
have been updated to the second ClientHello.
This CL resolves the first two issues. It replaces the v2ClientHelloMsg
with an option when serializing the clientHelloMsg, and it has the
ClientHello replacement function return a clientHelloMsg instead of a
[]byte. (This is a little weird because we're conflating parsed and
constructed ClientHellos, but ah well.)
A follow-up CL will remove the differed transcript bits and we'll
actually be able to drop helloBytes.
Change-Id: Ib82ac216604e2c4bf421277e57aa5fd3b4cef161
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46629
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 116e2be..6ba4e4b 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -52,30 +52,33 @@
panic("Unknown ClientHello version.")
}
-func fixClientHellos(hello *clientHelloMsg, in []byte) ([]byte, error) {
- ret := append([]byte{}, in...)
+// replaceClientHello returns a new clientHelloMsg which serializes to |in|, but
+// with key shares copied from |hello|. This allows sending an exact
+// externally-specified ClientHello in tests. However, we use |hello|'s key
+// shares. This ensures we have the private keys to complete the handshake. Note
+// this function does not update internal handshake state, so the test must be
+// configured compatibly with |in|.
+func replaceClientHello(hello *clientHelloMsg, in []byte) (*clientHelloMsg, error) {
+ copied := append([]byte{}, in...)
newHello := new(clientHelloMsg)
- if !newHello.unmarshal(ret) {
+ if !newHello.unmarshal(copied) {
return nil, errors.New("tls: invalid ClientHello")
}
- hello.random = newHello.random
- hello.sessionID = newHello.sessionID
-
- // Replace |ret|'s key shares with those of |hello|. For simplicity, we
- // require their lengths match, which is satisfied by matching the
- // DefaultCurves setting to the selection in the replacement
- // ClientHello.
+ // Replace |newHellos|'s key shares with those of |hello|. For simplicity,
+ // we require their lengths match, which is satisfied by matching the
+ // DefaultCurves setting to the selection in the replacement ClientHello.
bb := newByteBuilder()
hello.marshalKeyShares(bb)
keyShares := bb.finish()
if len(keyShares) != len(newHello.keySharesRaw) {
return nil, errors.New("tls: ClientHello key share length is inconsistent with DefaultCurves setting")
}
- // |newHello.keySharesRaw| aliases |ret|.
+ // |newHello.keySharesRaw| aliases |copied|.
copy(newHello.keySharesRaw, keyShares)
+ newHello.keyShares = hello.keyShares
- return ret, nil
+ return newHello, nil
}
func (c *Conn) clientHandshake() error {
@@ -446,38 +449,37 @@
hello.sessionID = c.config.Bugs.SendClientHelloSessionID
}
+ // PSK binders must be computed after the rest of the ClientHello is
+ // constructed.
+ if len(hello.pskIdentities) > 0 {
+ version := session.wireVersion
+ // We may have a pre-1.3 session if SendBothTickets is
+ // set.
+ if session.vers < VersionTLS13 {
+ version = VersionTLS13
+ }
+ generatePSKBinders(version, hello, session, nil, nil, c.config)
+ }
+
+ if c.config.Bugs.SendClientHelloWithFixes != nil {
+ hello, err = replaceClientHello(hello, c.config.Bugs.SendClientHelloWithFixes)
+ if err != nil {
+ return err
+ }
+ }
+
var helloBytes []byte
if c.config.Bugs.SendV2ClientHello {
- // Test that the peer left-pads random.
+ hello.isV2ClientHello = true
+ // The V2ClientHello "challenge" field is variable-length and is
+ // left-padded to become the SSL3/TLS random. Test this behavior by
+ // forcing the left padding.
hello.random[0] = 0
- v2Hello := &v2ClientHelloMsg{
- vers: hello.vers,
- cipherSuites: hello.cipherSuites,
- // No session resumption for V2ClientHello.
- sessionID: nil,
- challenge: hello.random[1:],
- }
- helloBytes = v2Hello.marshal()
+ hello.v2Challenge = hello.random[1:]
+ helloBytes = hello.marshal()
c.writeV2Record(helloBytes)
} else {
- if len(hello.pskIdentities) > 0 {
- version := session.wireVersion
- // We may have a pre-1.3 session if SendBothTickets is
- // set.
- if session.vers < VersionTLS13 {
- version = VersionTLS13
- }
- generatePSKBinders(version, hello, session, []byte{}, []byte{}, c.config)
- }
- if c.config.Bugs.SendClientHelloWithFixes != nil {
- helloBytes, err = fixClientHellos(hello, c.config.Bugs.SendClientHelloWithFixes)
- if err != nil {
- return err
- }
- } else {
- helloBytes = hello.marshal()
- }
-
+ helloBytes = hello.marshal()
var appendToHello byte
if c.config.Bugs.PartialClientFinishedWithClientHello {
appendToHello = typeFinished
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index 121244c..158f7c7 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -294,8 +294,10 @@
type clientHelloMsg struct {
raw []byte
isDTLS bool
+ isV2ClientHello bool
vers uint16
random []byte
+ v2Challenge []byte
sessionID []byte
cookie []byte
cipherSuites []uint16
@@ -361,6 +363,22 @@
return m.raw
}
+ if m.isV2ClientHello {
+ v2Msg := newByteBuilder()
+ v2Msg.addU8(1)
+ v2Msg.addU16(m.vers)
+ v2Msg.addU16(uint16(len(m.cipherSuites) * 3))
+ v2Msg.addU16(uint16(len(m.sessionID)))
+ v2Msg.addU16(uint16(len(m.v2Challenge)))
+ for _, spec := range m.cipherSuites {
+ v2Msg.addU24(int(spec))
+ }
+ v2Msg.addBytes(m.sessionID)
+ v2Msg.addBytes(m.v2Challenge)
+ m.raw = v2Msg.finish()
+ return m.raw
+ }
+
handshakeMsg := newByteBuilder()
handshakeMsg.addU8(typeClientHello)
hello := handshakeMsg.addU24LengthPrefixed()
@@ -2567,47 +2585,6 @@
return true
}
-type v2ClientHelloMsg struct {
- raw []byte
- vers uint16
- cipherSuites []uint16
- sessionID []byte
- challenge []byte
-}
-
-func (m *v2ClientHelloMsg) marshal() []byte {
- if m.raw != nil {
- return m.raw
- }
-
- length := 1 + 2 + 2 + 2 + 2 + len(m.cipherSuites)*3 + len(m.sessionID) + len(m.challenge)
-
- x := make([]byte, length)
- x[0] = 1
- x[1] = uint8(m.vers >> 8)
- x[2] = uint8(m.vers)
- x[3] = uint8((len(m.cipherSuites) * 3) >> 8)
- x[4] = uint8(len(m.cipherSuites) * 3)
- x[5] = uint8(len(m.sessionID) >> 8)
- x[6] = uint8(len(m.sessionID))
- x[7] = uint8(len(m.challenge) >> 8)
- x[8] = uint8(len(m.challenge))
- y := x[9:]
- for i, spec := range m.cipherSuites {
- y[i*3] = 0
- y[i*3+1] = uint8(spec >> 8)
- y[i*3+2] = uint8(spec)
- }
- y = y[len(m.cipherSuites)*3:]
- copy(y, m.sessionID)
- y = y[len(m.sessionID):]
- copy(y, m.challenge)
-
- m.raw = x
-
- return x
-}
-
type helloVerifyRequestMsg struct {
raw []byte
vers uint16
