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/handshake_client.c b/ssl/handshake_client.c
index 8afc289..52ff212 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -548,9 +548,8 @@
return ret;
}
-static int ssl3_write_client_cipher_list(SSL *ssl, CBB *out,
- uint16_t min_version,
- uint16_t max_version) {
+int ssl_write_client_cipher_list(SSL *ssl, CBB *out, uint16_t min_version,
+ uint16_t max_version) {
/* Prepare disabled cipher masks. */
ssl_set_client_disabled(ssl);
@@ -605,6 +604,45 @@
return CBB_flush(out);
}
+int ssl_add_client_hello_body(SSL *ssl, CBB *body) {
+ uint16_t min_version, max_version;
+ if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
+ return 0;
+ }
+
+ /* Renegotiations do not participate in session resumption. */
+ int has_session = ssl->session != NULL &&
+ !ssl->s3->initial_handshake_complete;
+
+ CBB child;
+ if (!CBB_add_u16(body, ssl->client_version) ||
+ !CBB_add_bytes(body, ssl->s3->client_random, SSL3_RANDOM_SIZE) ||
+ !CBB_add_u8_length_prefixed(body, &child) ||
+ (has_session &&
+ !CBB_add_bytes(&child, ssl->session->session_id,
+ ssl->session->session_id_length))) {
+ return 0;
+ }
+
+ if (SSL_IS_DTLS(ssl)) {
+ if (!CBB_add_u8_length_prefixed(body, &child) ||
+ !CBB_add_bytes(&child, ssl->d1->cookie, ssl->d1->cookie_len)) {
+ return 0;
+ }
+ }
+
+ size_t header_len =
+ SSL_IS_DTLS(ssl) ? DTLS1_HM_HEADER_LENGTH : SSL3_HM_HEADER_LENGTH;
+ if (!ssl_write_client_cipher_list(ssl, body, min_version, max_version) ||
+ !CBB_add_u8(body, 1 /* one compression method */) ||
+ !CBB_add_u8(body, 0 /* null compression */) ||
+ !ssl_add_clienthello_tlsext(ssl, body, header_len + CBB_len(body))) {
+ return 0;
+ }
+
+ return 1;
+}
+
static int ssl3_send_client_hello(SSL *ssl) {
if (ssl->state == SSL3_ST_CW_CLNT_HELLO_B) {
return ssl->method->write_message(ssl);
@@ -654,34 +692,9 @@
goto err;
}
- /* Renegotiations do not participate in session resumption. */
- int has_session = ssl->session != NULL &&
- !ssl->s3->initial_handshake_complete;
-
- CBB body, child;
+ CBB body;
if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CLIENT_HELLO) ||
- !CBB_add_u16(&body, ssl->client_version) ||
- !CBB_add_bytes(&body, ssl->s3->client_random, SSL3_RANDOM_SIZE) ||
- !CBB_add_u8_length_prefixed(&body, &child) ||
- (has_session &&
- !CBB_add_bytes(&child, ssl->session->session_id,
- ssl->session->session_id_length))) {
- goto err;
- }
-
- if (SSL_IS_DTLS(ssl)) {
- if (!CBB_add_u8_length_prefixed(&body, &child) ||
- !CBB_add_bytes(&child, ssl->d1->cookie, ssl->d1->cookie_len)) {
- goto err;
- }
- }
-
- size_t header_len =
- SSL_IS_DTLS(ssl) ? DTLS1_HM_HEADER_LENGTH : SSL3_HM_HEADER_LENGTH;
- if (!ssl3_write_client_cipher_list(ssl, &body, min_version, max_version) ||
- !CBB_add_u8(&body, 1 /* one compression method */) ||
- !CBB_add_u8(&body, 0 /* null compression */) ||
- !ssl_add_clienthello_tlsext(ssl, &body, header_len + CBB_len(&body)) ||
+ !ssl_add_client_hello_body(ssl, &body) ||
!ssl->method->finish_message(ssl, &cbb)) {
goto err;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index eb8b879..0401791 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -873,6 +873,11 @@
SSL_ECDH_CTX *groups;
size_t groups_len;
+ /* retry_group is the group ID selected by the server in HelloRetryRequest. */
+ uint16_t retry_group;
+ /* key_share_bytes is the value of the previously sent KeyShare extension. */
+ uint8_t *key_share_bytes;
+ size_t key_share_bytes_len;
uint8_t *public_key;
size_t public_key_len;
@@ -882,6 +887,8 @@
SSL_HANDSHAKE *ssl_handshake_new(enum ssl_hs_wait_t (*do_handshake)(SSL *ssl));
+void ssl_handshake_clear_groups(SSL_HANDSHAKE *hs);
+
/* ssl_handshake_free releases all memory associated with |hs|. */
void ssl_handshake_free(SSL_HANDSHAKE *hs);
@@ -910,11 +917,14 @@
int ext_key_share_parse_serverhello(SSL *ssl, uint8_t **out_secret,
size_t *out_secret_len, uint8_t *out_alert,
CBS *contents);
-int ext_key_share_parse_clienthello(SSL *ssl, uint8_t **out_secret,
+int ext_key_share_parse_clienthello(SSL *ssl,
+ int *out_found, uint8_t **out_secret,
size_t *out_secret_len, uint8_t *out_alert,
CBS *contents);
int ext_key_share_add_serverhello(SSL *ssl, CBB *out);
+int ssl_add_client_hello_body(SSL *ssl, CBB *body);
+
/* SSLKEYLOGFILE functions. */
@@ -1228,6 +1238,9 @@
STACK_OF(SSL_CIPHER) *ssl_get_ciphers_by_id(SSL *ssl);
int ssl_verify_alarm_type(long type);
+int ssl_write_client_cipher_list(SSL *ssl, CBB *out, uint16_t min_version,
+ uint16_t max_version);
+
int ssl3_get_finished(SSL *ssl);
int ssl3_send_change_cipher_spec(SSL *ssl);
void ssl3_cleanup_key_block(SSL *ssl);
@@ -1331,6 +1344,13 @@
char ssl_early_callback_init(struct ssl_early_callback_ctx *ctx);
+/* tls1_get_grouplist sets |*out_group_ids| and |*out_group_ids_len| to the
+ * list of allowed group IDs. If |get_peer_groups| is non-zero, return the
+ * peer's group list. Otherwise, return the preferred list. */
+void tls1_get_grouplist(SSL *ssl, int get_peer_groups,
+ const uint16_t **out_group_ids,
+ size_t *out_group_ids_len);
+
/* tls1_check_group_id returns one if |group_id| is consistent with both our
* and the peer's group preferences. Note: if called as the client, only our
* preferences are checked; the peer (the server) does not send preferences. */
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index f76d9f0..7549240 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -300,12 +300,9 @@
#endif
};
-/* tls1_get_grouplist sets |*out_group_ids| and |*out_group_ids_len| to the
- * list of allowed group IDs. If |get_peer_groups| is non-zero, return the
- * peer's group list. Otherwise, return the preferred list. */
-static void tls1_get_grouplist(SSL *ssl, int get_peer_groups,
- const uint16_t **out_group_ids,
- size_t *out_group_ids_len) {
+void tls1_get_grouplist(SSL *ssl, int get_peer_groups,
+ const uint16_t **out_group_ids,
+ size_t *out_group_ids_len) {
if (get_peer_groups) {
/* Only clients send a supported group list, so this function is only
* called on the server. */
@@ -1973,7 +1970,24 @@
const uint16_t *groups;
size_t groups_len;
- tls1_get_grouplist(ssl, 0 /* local groups */, &groups, &groups_len);
+ if (ssl->s3->hs->retry_group) {
+ /* Append the new key share to the old list. */
+ if (!CBB_add_bytes(&kse_bytes, ssl->s3->hs->key_share_bytes,
+ ssl->s3->hs->key_share_bytes_len)) {
+ return 0;
+ }
+ OPENSSL_free(ssl->s3->hs->key_share_bytes);
+ ssl->s3->hs->key_share_bytes = NULL;
+
+ groups = &ssl->s3->hs->retry_group;
+ groups_len = 1;
+ } else {
+ tls1_get_grouplist(ssl, 0 /* local groups */, &groups, &groups_len);
+ /* Only send the top two preferred key shares. */
+ if (groups_len > 2) {
+ groups_len = 2;
+ }
+ }
ssl->s3->hs->groups = OPENSSL_malloc(groups_len * sizeof(SSL_ECDH_CTX));
if (ssl->s3->hs->groups == NULL) {
@@ -1996,6 +2010,17 @@
}
}
+ if (!ssl->s3->hs->retry_group) {
+ /* Save the contents of the extension to repeat it in the second
+ * ClientHello. */
+ ssl->s3->hs->key_share_bytes_len = CBB_len(&kse_bytes);
+ ssl->s3->hs->key_share_bytes = BUF_memdup(CBB_data(&kse_bytes),
+ CBB_len(&kse_bytes));
+ if (ssl->s3->hs->key_share_bytes == NULL) {
+ return 0;
+ }
+ }
+
return CBB_flush(out);
}
@@ -2030,16 +2055,12 @@
return 0;
}
- for (size_t i = 0; i < ssl->s3->hs->groups_len; i++) {
- SSL_ECDH_CTX_cleanup(&ssl->s3->hs->groups[i]);
- }
- OPENSSL_free(ssl->s3->hs->groups);
- ssl->s3->hs->groups = NULL;
-
+ ssl_handshake_clear_groups(ssl->s3->hs);
return 1;
}
-int ext_key_share_parse_clienthello(SSL *ssl, uint8_t **out_secret,
+int ext_key_share_parse_clienthello(SSL *ssl, int *out_found,
+ uint8_t **out_secret,
size_t *out_secret_len, uint8_t *out_alert,
CBS *contents) {
uint16_t group_id;
@@ -2049,7 +2070,7 @@
return 0;
}
- int found = 0;
+ *out_found = 0;
while (CBS_len(&key_shares) > 0) {
uint16_t id;
CBS peer_key;
@@ -2058,7 +2079,7 @@
return 0;
}
- if (id != group_id || found) {
+ if (id != group_id || *out_found) {
continue;
}
@@ -2078,10 +2099,10 @@
}
SSL_ECDH_CTX_cleanup(&group);
- found = 1;
+ *out_found = 1;
}
- return found;
+ return 1;
}
int ext_key_share_add_serverhello(SSL *ssl, CBB *out) {
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) {
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index 023dc0d..3cebdd3 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -40,6 +40,19 @@
return hs;
}
+void ssl_handshake_clear_groups(SSL_HANDSHAKE *hs) {
+ if (hs->groups == NULL) {
+ return;
+ }
+
+ for (size_t i = 0; i < hs->groups_len; i++) {
+ SSL_ECDH_CTX_cleanup(&hs->groups[i]);
+ }
+ OPENSSL_free(hs->groups);
+ hs->groups = NULL;
+ hs->groups_len = 0;
+}
+
void ssl_handshake_free(SSL_HANDSHAKE *hs) {
if (hs == NULL) {
return;
@@ -47,12 +60,8 @@
OPENSSL_cleanse(hs->secret, sizeof(hs->secret));
OPENSSL_cleanse(hs->traffic_secret_0, sizeof(hs->traffic_secret_0));
- if (hs->groups != NULL) {
- for (size_t i = 0; i < hs->groups_len; i++) {
- SSL_ECDH_CTX_cleanup(&hs->groups[i]);
- }
- OPENSSL_free(hs->groups);
- }
+ ssl_handshake_clear_groups(hs);
+ OPENSSL_free(hs->key_share_bytes);
OPENSSL_free(hs->public_key);
OPENSSL_free(hs->cert_context);
OPENSSL_free(hs);
diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c
index 1b5573b..ead5b82 100644
--- a/ssl/tls13_client.c
+++ b/ssl/tls13_client.c
@@ -28,7 +28,10 @@
enum client_hs_state_t {
- state_process_server_hello = 0,
+ state_process_hello_retry_request = 0,
+ state_send_second_client_hello,
+ state_flush_second_client_hello,
+ state_process_server_hello,
state_process_encrypted_extensions,
state_process_certificate_request,
state_process_server_certificate,
@@ -43,6 +46,85 @@
state_done,
};
+static enum ssl_hs_wait_t do_process_hello_retry_request(SSL *ssl,
+ SSL_HANDSHAKE *hs) {
+ if (ssl->s3->tmp.message_type != SSL3_MT_HELLO_RETRY_REQUEST) {
+ hs->state = state_process_server_hello;
+ return ssl_hs_ok;
+ }
+
+ CBS cbs, extensions;
+ uint16_t server_wire_version, cipher_suite, group_id;
+ CBS_init(&cbs, ssl->init_msg, ssl->init_num);
+ if (!CBS_get_u16(&cbs, &server_wire_version) ||
+ !CBS_get_u16(&cbs, &cipher_suite) ||
+ !CBS_get_u16(&cbs, &group_id) ||
+ /* We do not currently parse any HelloRetryRequest extensions. */
+ !CBS_get_u16_length_prefixed(&cbs, &extensions) ||
+ CBS_len(&cbs) != 0) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+
+ /* TODO(svaldez): Don't do early_data on HelloRetryRequest. */
+
+ const uint16_t *groups;
+ size_t groups_len;
+ tls1_get_grouplist(ssl, 0 /* local groups */, &groups, &groups_len);
+ int found = 0;
+ for (size_t i = 0; i < groups_len; i++) {
+ if (groups[i] == group_id) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (!found) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
+ return ssl_hs_error;
+ }
+
+ for (size_t i = 0; i < ssl->s3->hs->groups_len; i++) {
+ /* Check that the HelloRetryRequest does not request a key share that was
+ * provided in the initial ClientHello.
+ *
+ * TODO(svaldez): Don't enforce this check when the HelloRetryRequest is due
+ * to a cookie. */
+ if (SSL_ECDH_CTX_get_id(&ssl->s3->hs->groups[i]) == group_id) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
+ return ssl_hs_error;
+ }
+ }
+
+ ssl_handshake_clear_groups(ssl->s3->hs);
+ ssl->s3->hs->retry_group = group_id;
+
+ hs->state = state_send_second_client_hello;
+ return ssl_hs_ok;
+}
+
+static enum ssl_hs_wait_t do_send_second_client_hello(SSL *ssl,
+ SSL_HANDSHAKE *hs) {
+ CBB cbb, body;
+ if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CLIENT_HELLO) ||
+ !ssl_add_client_hello_body(ssl, &body) ||
+ !ssl->method->finish_message(ssl, &cbb)) {
+ CBB_cleanup(&cbb);
+ return ssl_hs_error;
+ }
+
+ hs->state = state_flush_second_client_hello;
+ return ssl_hs_write_message;
+}
+
+static enum ssl_hs_wait_t do_flush_second_client_hello(SSL *ssl,
+ SSL_HANDSHAKE *hs) {
+ hs->state = state_process_server_hello;
+ return ssl_hs_flush_and_read_message;
+}
+
static enum ssl_hs_wait_t do_process_server_hello(SSL *ssl, SSL_HANDSHAKE *hs) {
if (!tls13_check_message_type(ssl, SSL3_MT_SERVER_HELLO)) {
return ssl_hs_error;
@@ -58,6 +140,13 @@
!CBS_get_u16_length_prefixed(&cbs, &extensions) ||
CBS_len(&cbs) != 0) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+
+ if (server_wire_version != ssl->version) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_VERSION_NUMBER);
return ssl_hs_error;
}
@@ -169,6 +258,12 @@
if (!tls13_advance_key_schedule(ssl, kZeroes, hash_len)) {
return ssl_hs_error;
}
+ }
+
+ /* If there was no HelloRetryRequest, the version negotiation logic has
+ * already hashed the message. */
+ if (ssl->s3->hs->retry_group != 0 &&
+ !ssl->method->hash_current_message(ssl)) {
return ssl_hs_error;
}
@@ -403,6 +498,15 @@
enum ssl_hs_wait_t ret = ssl_hs_error;
enum client_hs_state_t state = hs->state;
switch (state) {
+ case state_process_hello_retry_request:
+ ret = do_process_hello_retry_request(ssl, hs);
+ break;
+ case state_send_second_client_hello:
+ ret = do_send_second_client_hello(ssl, hs);
+ break;
+ case state_flush_second_client_hello:
+ ret = do_flush_second_client_hello(ssl, hs);
+ break;
case state_process_server_hello:
ret = do_process_server_hello(ssl, hs);
break;
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 1a1e52a..22392f0 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -29,6 +29,9 @@
enum server_hs_state_t {
state_process_client_hello = 0,
+ state_send_hello_retry_request,
+ state_flush_hello_retry_request,
+ state_process_second_client_hello,
state_send_server_hello,
state_send_encrypted_extensions,
state_send_certificate_request,
@@ -43,6 +46,60 @@
state_done,
};
+static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0};
+
+static int resolve_psk_secret(SSL *ssl) {
+ SSL_HANDSHAKE *hs = ssl->s3->hs;
+
+ if (ssl->s3->tmp.new_cipher->algorithm_auth != SSL_aPSK) {
+ return tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len);
+ }
+
+ /* TODO(davidben): Support PSK. */
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return 0;
+}
+
+static int resolve_ecdhe_secret(SSL *ssl, int *out_need_retry,
+ struct ssl_early_callback_ctx *early_ctx) {
+ *out_need_retry = 0;
+ SSL_HANDSHAKE *hs = ssl->s3->hs;
+
+ if (ssl->s3->tmp.new_cipher->algorithm_mkey != SSL_kECDHE) {
+ return tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len);
+ }
+
+ const uint8_t *key_share_buf = NULL;
+ size_t key_share_len = 0;
+ CBS key_share;
+ if (!SSL_early_callback_ctx_extension_get(early_ctx, TLSEXT_TYPE_key_share,
+ &key_share_buf, &key_share_len)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_KEY_SHARE);
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_MISSING_EXTENSION);
+ return ssl_hs_error;
+ }
+
+ CBS_init(&key_share, key_share_buf, key_share_len);
+ int found_key_share;
+ uint8_t *dhe_secret;
+ size_t dhe_secret_len;
+ uint8_t alert;
+ if (!ext_key_share_parse_clienthello(ssl, &found_key_share, &dhe_secret,
+ &dhe_secret_len, &alert, &key_share)) {
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return 0;
+ }
+
+ if (!found_key_share) {
+ *out_need_retry = 1;
+ return 0;
+ }
+
+ int ok = tls13_advance_key_schedule(ssl, dhe_secret, dhe_secret_len);
+ OPENSSL_free(dhe_secret);
+ return ok;
+}
+
static enum ssl_hs_wait_t do_process_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) {
if (!tls13_check_message_type(ssl, SSL3_MT_CLIENT_HELLO)) {
return ssl_hs_error;
@@ -165,7 +222,6 @@
/* The PRF hash is now known. Set up the key schedule and hash the
* ClientHello. */
- static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0};
size_t hash_len =
EVP_MD_size(ssl_get_handshake_digest(ssl_get_algorithm_prf(ssl)));
if (!tls13_init_key_schedule(ssl, kZeroes, hash_len)) {
@@ -173,42 +229,79 @@
}
/* Resolve PSK and incorporate it into the secret. */
- if (cipher->algorithm_auth == SSL_aPSK) {
- /* TODO(davidben): Support PSK. */
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return ssl_hs_error;
- } else if (!tls13_advance_key_schedule(ssl, kZeroes, hash_len)) {
+ if (!resolve_psk_secret(ssl)) {
return ssl_hs_error;
}
/* Resolve ECDHE and incorporate it into the secret. */
- if (cipher->algorithm_mkey == SSL_kECDHE) {
- const uint8_t *key_share_buf = NULL;
- size_t key_share_len = 0;
- CBS key_share;
- if (!SSL_early_callback_ctx_extension_get(&early_ctx, TLSEXT_TYPE_key_share,
- &key_share_buf, &key_share_len)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_KEY_SHARE);
- ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_MISSING_EXTENSION);
- return ssl_hs_error;
+ int need_retry;
+ if (!resolve_ecdhe_secret(ssl, &need_retry, &early_ctx)) {
+ if (need_retry) {
+ hs->state = state_send_hello_retry_request;
+ return ssl_hs_ok;
}
+ return ssl_hs_error;
+ }
- CBS_init(&key_share, key_share_buf, key_share_len);
- uint8_t *dhe_secret;
- size_t dhe_secret_len;
- uint8_t alert;
- if (!ext_key_share_parse_clienthello(ssl, &dhe_secret, &dhe_secret_len,
- &alert, &key_share)) {
- ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
- return ssl_hs_error;
- }
+ hs->state = state_send_server_hello;
+ return ssl_hs_ok;
+}
- int ok = tls13_advance_key_schedule(ssl, dhe_secret, dhe_secret_len);
- OPENSSL_free(dhe_secret);
- if (!ok) {
- return ssl_hs_error;
+static enum ssl_hs_wait_t do_send_hello_retry_request(SSL *ssl,
+ SSL_HANDSHAKE *hs) {
+ CBB cbb, body, extensions;
+ uint16_t group_id;
+ if (!ssl->method->init_message(ssl, &cbb, &body,
+ SSL3_MT_HELLO_RETRY_REQUEST) ||
+ !CBB_add_u16(&body, ssl->version) ||
+ !CBB_add_u16(&body, ssl_cipher_get_value(ssl->s3->tmp.new_cipher)) ||
+ !tls1_get_shared_group(ssl, &group_id) ||
+ !CBB_add_u16(&body, group_id) ||
+ !CBB_add_u16_length_prefixed(&body, &extensions) ||
+ !ssl->method->finish_message(ssl, &cbb)) {
+ CBB_cleanup(&cbb);
+ return ssl_hs_error;
+ }
+
+ hs->state = state_flush_hello_retry_request;
+ return ssl_hs_write_message;
+}
+
+static enum ssl_hs_wait_t do_flush_hello_retry_request(SSL *ssl,
+ SSL_HANDSHAKE *hs) {
+ hs->state = state_process_second_client_hello;
+ return ssl_hs_flush_and_read_message;
+}
+
+static enum ssl_hs_wait_t do_process_second_client_hello(SSL *ssl,
+ SSL_HANDSHAKE *hs) {
+ if (!tls13_check_message_type(ssl, SSL3_MT_CLIENT_HELLO)) {
+ return ssl_hs_error;
+ }
+
+ struct ssl_early_callback_ctx early_ctx;
+
+ memset(&early_ctx, 0, sizeof(early_ctx));
+ early_ctx.ssl = ssl;
+ early_ctx.client_hello = ssl->init_msg;
+ early_ctx.client_hello_len = ssl->init_num;
+ if (!ssl_early_callback_init(&early_ctx)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED);
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+
+ int need_retry;
+ if (!resolve_ecdhe_secret(ssl, &need_retry, &early_ctx)) {
+ if (need_retry) {
+ /* Only send one HelloRetryRequest. */
+ ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
}
- } else if (!tls13_advance_key_schedule(ssl, kZeroes, hash_len)) {
+ return ssl_hs_error;
+ }
+
+ if (!ssl->method->hash_current_message(ssl)) {
return ssl_hs_error;
}
@@ -352,7 +445,6 @@
static enum ssl_hs_wait_t do_flush(SSL *ssl, SSL_HANDSHAKE *hs) {
/* Update the secret to the master secret and derive traffic keys. */
- static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0};
if (!tls13_advance_key_schedule(ssl, kZeroes, hs->hash_len) ||
!tls13_derive_traffic_secret_0(ssl) ||
!tls13_set_traffic_key(ssl, type_data, evp_aead_seal,
@@ -426,6 +518,15 @@
case state_process_client_hello:
ret = do_process_client_hello(ssl, hs);
break;
+ case state_send_hello_retry_request:
+ ret = do_send_hello_retry_request(ssl, hs);
+ break;
+ case state_flush_hello_retry_request:
+ ret = do_flush_hello_retry_request(ssl, hs);
+ break;
+ case state_process_second_client_hello:
+ ret = do_process_second_client_hello(ssl, hs);
+ break;
case state_send_server_hello:
ret = do_send_server_hello(ssl, hs);
break;
