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>

ssl/test/bssl_shim.cc

@@ -1739,12 +1739,20 @@ static bool DoConnection(bssl::UniquePtr<SSL_SESSION> *out_session,
     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);
   }

ssl/test/runner/runner.go

@@ -3381,6 +3381,37 @@ func addClientAuthTests() {
 				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{

ssl/test/test_config.cc

@@ -95,6 +95,7 @@ const Flag<bool> kBoolFlags[] = {
   { "-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 },

ssl/test/test_config.h

@@ -101,6 +101,7 @@ struct TestConfig {
   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;

ssl/tls13_server.c

@@ -537,11 +537,14 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) {
     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. */