Add tests for SSL_VERIFY_PEER_IF_NO_OBC and fix TLS 1.3.
Also mirror the structure of the TLS 1.2 and TLS 1.3 code a bit.
Change-Id: I7b34bf30de63fa0bd47a39a90570846fb2314ad5
Reviewed-on: https://boringssl-review.googlesource.com/17539
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 8143d1b..f3cbec1 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1739,12 +1739,20 @@
SSL_set_cert_cb(ssl.get(), CertCallback, nullptr);
}
if (config->require_any_client_certificate) {
- SSL_set_verify(ssl.get(), SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
+ SSL_set_verify(ssl.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
NULL);
}
if (config->verify_peer) {
SSL_set_verify(ssl.get(), SSL_VERIFY_PEER, NULL);
}
+ if (config->verify_peer_if_no_obc) {
+ // Set SSL_VERIFY_FAIL_IF_NO_PEER_CERT so testing whether client
+ // certificates were requested is easy.
+ SSL_set_verify(ssl.get(),
+ SSL_VERIFY_PEER | SSL_VERIFY_PEER_IF_NO_OBC |
+ SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
+ NULL);
+ }
if (config->false_start) {
SSL_set_mode(ssl.get(), SSL_MODE_ENABLE_FALSE_START);
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index cc5aa3c..496f9a7 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -3381,6 +3381,37 @@
shouldFail: true,
expectedError: ":UNEXPECTED_MESSAGE:",
})
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "VerifyPeerIfNoOBC-NoChannelID-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{
+ "-enable-channel-id",
+ "-verify-peer-if-no-obc",
+ },
+ shouldFail: true,
+ expectedError: ":PEER_DID_NOT_RETURN_A_CERTIFICATE:",
+ expectedLocalError: certificateRequired,
+ })
+
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "VerifyPeerIfNoOBC-ChannelID-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ ChannelID: channelIDKey,
+ },
+ expectChannelID: true,
+ flags: []string{
+ "-enable-channel-id",
+ "-verify-peer-if-no-obc",
+ },
+ })
}
testCases = append(testCases, testCase{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 4fe09b2..2004565 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -95,6 +95,7 @@
{ "-shim-shuts-down", &TestConfig::shim_shuts_down },
{ "-verify-fail", &TestConfig::verify_fail },
{ "-verify-peer", &TestConfig::verify_peer },
+ { "-verify-peer-if-no-obc", &TestConfig::verify_peer_if_no_obc },
{ "-expect-verify-result", &TestConfig::expect_verify_result },
{ "-renegotiate-once", &TestConfig::renegotiate_once },
{ "-renegotiate-freely", &TestConfig::renegotiate_freely },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 64271bf..8ad53d5 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -101,6 +101,7 @@
bool shim_shuts_down = false;
bool verify_fail = false;
bool verify_peer = false;
+ bool verify_peer_if_no_obc = false;
bool expect_verify_result = false;
std::string signed_cert_timestamps;
int expect_total_renegotiations = 0;
diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c
index 0a5e1a2..fe2463b 100644
--- a/ssl/tls13_server.c
+++ b/ssl/tls13_server.c
@@ -537,11 +537,14 @@
goto err;
}
- /* Determine whether to request a client certificate. */
- hs->cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER);
- /* CertificateRequest may only be sent in non-resumption handshakes. */
- if (ssl->s3->session_reused) {
- hs->cert_request = 0;
+ if (!ssl->s3->session_reused) {
+ /* Determine whether to request a client certificate. */
+ hs->cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER);
+ /* Only request a certificate if Channel ID isn't negotiated. */
+ if ((ssl->verify_mode & SSL_VERIFY_PEER_IF_NO_OBC) &&
+ ssl->s3->tlsext_channel_id_valid) {
+ hs->cert_request = 0;
+ }
}
/* Send a CertificateRequest, if necessary. */
