Adding HelloRetryRequest.
[Tests added by davidben.]
Change-Id: I0d54a4f8b8fe91b348ff22658d95340cdb48b089
Reviewed-on: https://boringssl-review.googlesource.com/8850
Reviewed-by: Steven Valdez <svaldez@google.com>
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/ssl/test/runner/common.go b/ssl/test/runner/common.go
index e20b2d5..b2d31dc 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -206,6 +206,7 @@
TLSUnique []byte // the tls-unique channel binding
SCTList []byte // signed certificate timestamp list
PeerSignatureAlgorithm signatureAlgorithm // algorithm used by the peer in the handshake
+ CurveID CurveID // the curve used in ECDHE
}
// ClientAuthType declares the policy the server will follow for
@@ -438,10 +439,16 @@
// or CertificateVerify message should be invalid.
InvalidSignature bool
- // SendCurve, if non-zero, causes the ServerKeyExchange message to use
- // the specified curve ID rather than the negotiated one.
+ // SendCurve, if non-zero, causes the server to send the specified curve
+ // ID in ServerKeyExchange (TLS 1.2) or ServerHello (TLS 1.3) rather
+ // than the negotiated one.
SendCurve CurveID
+ // SendHelloRetryRequestCurve, if non-zero, causes the server to send
+ // the specified curve in HelloRetryRequest rather than the negotiated
+ // one.
+ SendHelloRetryRequestCurve CurveID
+
// InvalidECDHPoint, if true, causes the ECC points in
// ServerKeyExchange or ClientKeyExchange messages to be invalid.
InvalidECDHPoint bool
@@ -953,6 +960,16 @@
// instead.
MissingKeyShare bool
+ // SecondClientHelloMissingKeyShare, if true, causes the second TLS 1.3
+ // ClientHello to skip sending a key_share extension and use the zero
+ // ECDHE secret instead.
+ SecondClientHelloMissingKeyShare bool
+
+ // MisinterpretHelloRetryRequestCurve, if non-zero, causes the TLS 1.3
+ // client to pretend the server requested a HelloRetryRequest with the
+ // given curve rather than the actual one.
+ MisinterpretHelloRetryRequestCurve CurveID
+
// DuplicateKeyShares, if true, causes the TLS 1.3 client to send two
// copies of each KeyShareEntry.
DuplicateKeyShares bool
@@ -964,6 +981,22 @@
// EncryptedExtensionsWithKeyShare, if true, causes the TLS 1.3 server to
// include the KeyShare extension in the EncryptedExtensions block.
EncryptedExtensionsWithKeyShare bool
+
+ // UnnecessaryHelloRetryRequest, if true, causes the TLS 1.3 server to
+ // send a HelloRetryRequest regardless of whether it needs to.
+ UnnecessaryHelloRetryRequest bool
+
+ // SecondHelloRetryRequest, if true, causes the TLS 1.3 server to send
+ // two HelloRetryRequests instead of one.
+ SecondHelloRetryRequest bool
+
+ // SendServerHelloVersion, if non-zero, causes the server to send the
+ // specified version in ServerHello rather than the true version.
+ SendServerHelloVersion uint16
+
+ // SkipHelloRetryRequest, if true, causes the TLS 1.3 server to not send
+ // HelloRetryRequest.
+ SkipHelloRetryRequest bool
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 3789b28..fbd501a 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -53,6 +53,9 @@
// peerSignatureAlgorithm contains the signature algorithm that was used
// by the peer in the handshake, or zero if not applicable.
peerSignatureAlgorithm signatureAlgorithm
+ // curveID contains the curve that was used in the handshake, or zero if
+ // not applicable.
+ curveID CurveID
clientRandom, serverRandom [32]byte
exporterSecret []byte
@@ -1500,6 +1503,7 @@
state.TLSUnique = c.firstFinished[:]
state.SCTList = c.sctList
state.PeerSignatureAlgorithm = c.peerSignatureAlgorithm
+ state.CurveID = c.curveID
}
return state
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 291fb75..50f3fa8 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -140,7 +140,7 @@
}
if c.config.Bugs.MissingKeyShare {
- hello.keyShares = nil
+ hello.hasKeyShares = false
}
}
@@ -337,6 +337,9 @@
var secondHelloBytes []byte
if haveHelloRetryRequest {
var hrrCurveFound bool
+ if c.config.Bugs.MisinterpretHelloRetryRequestCurve != 0 {
+ helloRetryRequest.selectedGroup = c.config.Bugs.MisinterpretHelloRetryRequestCurve
+ }
group := helloRetryRequest.selectedGroup
for _, curveID := range hello.supportedCurves {
if group == curveID {
@@ -362,6 +365,10 @@
keyExchange: publicKey,
})
+ if c.config.Bugs.SecondClientHelloMissingKeyShare {
+ hello.hasKeyShares = false
+ }
+
hello.hasEarlyData = false
hello.earlyDataContext = nil
hello.raw = nil
@@ -553,7 +560,7 @@
// Resolve ECDHE and compute the handshake secret.
var ecdheSecret []byte
- if hs.suite.flags&suiteECDHE != 0 && !c.config.Bugs.MissingKeyShare {
+ if hs.suite.flags&suiteECDHE != 0 && !c.config.Bugs.MissingKeyShare && !c.config.Bugs.SecondClientHelloMissingKeyShare {
if !hs.serverHello.hasKeyShare {
c.sendAlert(alertMissingExtension)
return errors.New("tls: server omitted the key share extension")
@@ -564,6 +571,7 @@
c.sendAlert(alertHandshakeFailure)
return errors.New("tls: server selected an unsupported group")
}
+ c.curveID = hs.serverHello.keyShare.group
var err error
ecdheSecret, err = curve.finish(hs.serverHello.keyShare.keyExchange)
@@ -821,6 +829,9 @@
c.sendAlert(alertUnexpectedMessage)
return err
}
+ if ecdhe, ok := keyAgreement.(*ecdheKeyAgreement); ok {
+ c.curveID = ecdhe.curveID
+ }
c.peerSignatureAlgorithm = keyAgreement.peerSignatureAlgorithm()
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 300ab50..f8b5dee 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -266,6 +266,10 @@
vers: c.vers,
}
+ if config.Bugs.SendServerHelloVersion != 0 {
+ hs.hello.vers = config.Bugs.SendServerHelloVersion
+ }
+
hs.hello.random = make([]byte, 32)
if _, err := io.ReadFull(config.rand(), hs.hello.random); err != nil {
c.sendAlert(alertInternalError)
@@ -352,13 +356,25 @@
}
}
- if selectedKeyShare == nil {
+ sendHelloRetryRequest := selectedKeyShare == nil
+ if config.Bugs.UnnecessaryHelloRetryRequest {
+ sendHelloRetryRequest = true
+ }
+ if config.Bugs.SkipHelloRetryRequest {
+ sendHelloRetryRequest = false
+ }
+ if sendHelloRetryRequest {
+ firstTime := true
+ ResendHelloRetryRequest:
// Send HelloRetryRequest.
helloRetryRequestMsg := helloRetryRequestMsg{
vers: c.vers,
cipherSuite: hs.hello.cipherSuite,
selectedGroup: selectedCurve,
}
+ if config.Bugs.SendHelloRetryRequestCurve != 0 {
+ helloRetryRequestMsg.selectedGroup = config.Bugs.SendHelloRetryRequestCurve
+ }
hs.writeServerHash(helloRetryRequestMsg.marshal())
c.writeRecord(recordTypeHandshake, helloRetryRequestMsg.marshal())
@@ -392,26 +408,47 @@
return errors.New("tls: new ClientHello does not match")
}
+ if firstTime && config.Bugs.SecondHelloRetryRequest {
+ firstTime = false
+ goto ResendHelloRetryRequest
+ }
+
selectedKeyShare = &newKeyShares[len(newKeyShares)-1]
}
// Once a curve has been selected and a key share identified,
// the server needs to generate a public value and send it in
// the ServerHello.
- curve, ok := curveForCurveID(selectedKeyShare.group)
+ curve, ok := curveForCurveID(selectedCurve)
if !ok {
panic("tls: server failed to look up curve ID")
}
+ c.curveID = selectedCurve
+
+ var peerKey []byte
+ if config.Bugs.SkipHelloRetryRequest {
+ // If skipping HelloRetryRequest, use a random key to
+ // avoid crashing.
+ curve2, _ := curveForCurveID(selectedCurve)
+ var err error
+ peerKey, err = curve2.offer(config.rand())
+ if err != nil {
+ return err
+ }
+ } else {
+ peerKey = selectedKeyShare.keyExchange
+ }
+
var publicKey []byte
var err error
- publicKey, ecdheSecret, err = curve.accept(config.rand(), selectedKeyShare.keyExchange)
+ publicKey, ecdheSecret, err = curve.accept(config.rand(), peerKey)
if err != nil {
c.sendAlert(alertHandshakeFailure)
return err
}
hs.hello.hasKeyShare = true
- curveID := selectedKeyShare.group
+ curveID := selectedCurve
if c.config.Bugs.SendCurve != 0 {
curveID = config.Bugs.SendCurve
}
@@ -648,6 +685,10 @@
compressionMethod: compressionNone,
}
+ if config.Bugs.SendServerHelloVersion != 0 {
+ hs.hello.vers = config.Bugs.SendServerHelloVersion
+ }
+
hs.hello.random = make([]byte, 32)
_, err = io.ReadFull(config.rand(), hs.hello.random)
if err != nil {
@@ -1009,6 +1050,9 @@
c.sendAlert(alertHandshakeFailure)
return err
}
+ if ecdhe, ok := keyAgreement.(*ecdheKeyAgreement); ok {
+ c.curveID = ecdhe.curveID
+ }
if skx != nil && !config.Bugs.SkipServerKeyExchange {
hs.writeServerHash(skx.marshal())
c.writeRecord(recordTypeHandshake, skx.marshal())
diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go
index e4a349f..ebfb93d 100644
--- a/ssl/test/runner/key_agreement.go
+++ b/ssl/test/runner/key_agreement.go
@@ -484,13 +484,14 @@
return verifyMessage(ka.version, cert.PublicKey, config, sigAlg, msg, sig)
}
-// ecdheRSAKeyAgreement implements a TLS key agreement where the server
+// ecdheKeyAgreement implements a TLS key agreement where the server
// generates a ephemeral EC public/private key pair and signs it. The
// pre-master secret is then calculated using ECDH. The signature may
// either be ECDSA or RSA.
type ecdheKeyAgreement struct {
auth keyAgreementAuthentication
curve ecdhCurve
+ curveID CurveID
peerKey []byte
}
@@ -516,6 +517,7 @@
if ka.curve, ok = curveForCurveID(curveid); !ok {
return nil, errors.New("tls: preferredCurves includes unsupported curve")
}
+ ka.curveID = curveid
publicKey, err := ka.curve.offer(config.rand())
if err != nil {
@@ -554,6 +556,7 @@
return errors.New("tls: server selected unsupported curve")
}
curveid := CurveID(skx.key[1])<<8 | CurveID(skx.key[2])
+ ka.curveID = curveid
var ok bool
if ka.curve, ok = curveForCurveID(curveid); !ok {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 92a2b6a..4997836 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -254,6 +254,9 @@
// expectedPeerSignatureAlgorithm, if not zero, is the signature
// algorithm that the peer should have used in the handshake.
expectedPeerSignatureAlgorithm signatureAlgorithm
+ // expectedCurveID, if not zero, is the curve that the handshake should
+ // have used.
+ expectedCurveID CurveID
// messageLen is the length, in bytes, of the test message that will be
// sent.
messageLen int
@@ -518,6 +521,10 @@
return fmt.Errorf("expected peer to use signature algorithm %04x, but got %04x", expected, connState.PeerSignatureAlgorithm)
}
+ if expected := test.expectedCurveID; expected != 0 && expected != connState.CurveID {
+ return fmt.Errorf("expected peer to use curve %04x, but got %04x", expected, connState.CurveID)
+ }
+
if test.exportKeyingMaterial > 0 {
actual := make([]byte, test.exportKeyingMaterial)
if _, err := io.ReadFull(tlsConn, actual); err != nil {
@@ -6304,6 +6311,8 @@
{"X25519", CurveX25519},
}
+const bogusCurve = 0x1234
+
func addCurveTests() {
for _, curve := range testCurves {
testCases = append(testCases, testCase{
@@ -6313,7 +6322,8 @@
CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
CurvePreferences: []CurveID{curve.id},
},
- flags: []string{"-enable-all-curves"},
+ flags: []string{"-enable-all-curves"},
+ expectedCurveID: curve.id,
})
testCases = append(testCases, testCase{
name: "CurveTest-Client-" + curve.name + "-TLS13",
@@ -6322,7 +6332,8 @@
CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
CurvePreferences: []CurveID{curve.id},
},
- flags: []string{"-enable-all-curves"},
+ flags: []string{"-enable-all-curves"},
+ expectedCurveID: curve.id,
})
testCases = append(testCases, testCase{
testType: serverTest,
@@ -6332,7 +6343,8 @@
CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
CurvePreferences: []CurveID{curve.id},
},
- flags: []string{"-enable-all-curves"},
+ flags: []string{"-enable-all-curves"},
+ expectedCurveID: curve.id,
})
testCases = append(testCases, testCase{
testType: serverTest,
@@ -6342,12 +6354,12 @@
CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
CurvePreferences: []CurveID{curve.id},
},
- flags: []string{"-enable-all-curves"},
+ flags: []string{"-enable-all-curves"},
+ expectedCurveID: curve.id,
})
}
// The server must be tolerant to bogus curves.
- const bogusCurve = 0x1234
testCases = append(testCases, testCase{
testType: serverTest,
name: "UnknownCurve",
@@ -7359,7 +7371,8 @@
})
testCases = append(testCases, testCase{
- name: "MissingKeyShare-Server",
+ testType: serverTest,
+ name: "MissingKeyShare-Server",
config: Config{
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
@@ -7432,6 +7445,158 @@
shouldFail: true,
expectedLocalError: "remote error: unsupported extension",
})
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SendHelloRetryRequest",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // Require a HelloRetryRequest for every curve.
+ DefaultCurves: []CurveID{},
+ },
+ expectedCurveID: CurveX25519,
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SendHelloRetryRequest-2",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ DefaultCurves: []CurveID{CurveP384},
+ },
+ // Although the ClientHello did not predict our preferred curve,
+ // we always select it whether it is predicted or not.
+ expectedCurveID: CurveX25519,
+ })
+
+ testCases = append(testCases, testCase{
+ name: "UnknownCurve-HelloRetryRequest",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // P-384 requires HelloRetryRequest in BoringSSL.
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ SendHelloRetryRequestCurve: bogusCurve,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":WRONG_CURVE:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "DisabledCurve-HelloRetryRequest",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ CurvePreferences: []CurveID{CurveP256},
+ Bugs: ProtocolBugs{
+ IgnorePeerCurvePreferences: true,
+ },
+ },
+ flags: []string{"-p384-only"},
+ shouldFail: true,
+ expectedError: ":WRONG_CURVE:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "UnnecessaryHelloRetryRequest",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ Bugs: ProtocolBugs{
+ UnnecessaryHelloRetryRequest: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":WRONG_CURVE:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "SecondHelloRetryRequest",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // P-384 requires HelloRetryRequest in BoringSSL.
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ SecondHelloRetryRequest: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":UNEXPECTED_MESSAGE:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SecondClientHelloMissingKeyShare",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ DefaultCurves: []CurveID{},
+ Bugs: ProtocolBugs{
+ SecondClientHelloMissingKeyShare: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":MISSING_KEY_SHARE:",
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "SecondClientHelloWrongCurve",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ DefaultCurves: []CurveID{},
+ Bugs: ProtocolBugs{
+ MisinterpretHelloRetryRequestCurve: CurveP521,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":WRONG_CURVE:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "HelloRetryRequestVersionMismatch",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // P-384 requires HelloRetryRequest in BoringSSL.
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ SendServerHelloVersion: 0x0305,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":WRONG_VERSION_NUMBER:",
+ })
+
+ testCases = append(testCases, testCase{
+ name: "HelloRetryRequestCurveMismatch",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // P-384 requires HelloRetryRequest in BoringSSL.
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ // Send P-384 (correct) in the HelloRetryRequest.
+ SendHelloRetryRequestCurve: CurveP384,
+ // But send P-256 in the ServerHello.
+ SendCurve: CurveP256,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":WRONG_CURVE:",
+ })
+
+ // Test the server selecting a curve that requires a HelloRetryRequest
+ // without sending it.
+ testCases = append(testCases, testCase{
+ name: "SkipHelloRetryRequest",
+ config: Config{
+ MaxVersion: VersionTLS13,
+ // P-384 requires HelloRetryRequest in BoringSSL.
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ SkipHelloRetryRequest: true,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":WRONG_CURVE:",
+ })
}
func worker(statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) {
