Add tests for the old client cert callback.

Also add no-certificate cases to the state machine coverage tests.

Change-Id: I88a80df6f3ea69aabc978dd356abcb9e309e156f
Reviewed-on: https://boringssl-review.googlesource.com/7417
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index f6c9d6a..7da64bb 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -138,13 +138,20 @@
   return (TestState *)SSL_get_ex_data(ssl, g_state_index);
 }
 
+static ScopedX509 LoadCertificate(const std::string &file) {
+  ScopedBIO bio(BIO_new(BIO_s_file()));
+  if (!bio || !BIO_read_filename(bio.get(), file.c_str())) {
+    return nullptr;
+  }
+  return ScopedX509(PEM_read_bio_X509(bio.get(), NULL, NULL, NULL));
+}
+
 static ScopedEVP_PKEY LoadPrivateKey(const std::string &file) {
   ScopedBIO bio(BIO_new(BIO_s_file()));
   if (!bio || !BIO_read_filename(bio.get(), file.c_str())) {
     return nullptr;
   }
-  ScopedEVP_PKEY pkey(PEM_read_bio_PrivateKey(bio.get(), NULL, NULL, NULL));
-  return pkey;
+  return ScopedEVP_PKEY(PEM_read_bio_PrivateKey(bio.get(), NULL, NULL, NULL));
 }
 
 static int AsyncPrivateKeyType(SSL *ssl) {
@@ -291,9 +298,9 @@
   }
 };
 
-static bool InstallCertificate(SSL *ssl) {
+static bool GetCertificate(SSL *ssl, ScopedX509 *out_x509,
+                           ScopedEVP_PKEY *out_pkey) {
   const TestConfig *config = GetConfigPtr(ssl);
-  TestState *test_state = GetTestState(ssl);
 
   if (!config->digest_prefs.empty()) {
     std::unique_ptr<char, Free<char>> digest_prefs(
@@ -317,21 +324,16 @@
   }
 
   if (!config->key_file.empty()) {
-    if (config->async) {
-      test_state->private_key = LoadPrivateKey(config->key_file.c_str());
-      if (!test_state->private_key) {
-        return false;
-      }
-      SSL_set_private_key_method(ssl, &g_async_private_key_method);
-    } else if (!SSL_use_PrivateKey_file(ssl, config->key_file.c_str(),
-                                        SSL_FILETYPE_PEM)) {
+    *out_pkey = LoadPrivateKey(config->key_file.c_str());
+    if (!*out_pkey) {
       return false;
     }
   }
-  if (!config->cert_file.empty() &&
-      !SSL_use_certificate_file(ssl, config->cert_file.c_str(),
-                                SSL_FILETYPE_PEM)) {
-    return false;
+  if (!config->cert_file.empty()) {
+    *out_x509 = LoadCertificate(config->cert_file.c_str());
+    if (!*out_x509) {
+      return false;
+    }
   }
   if (!config->ocsp_response.empty() &&
       !SSL_CTX_set_ocsp_response(ssl->ctx,
@@ -342,6 +344,31 @@
   return true;
 }
 
+static bool InstallCertificate(SSL *ssl) {
+  ScopedX509 x509;
+  ScopedEVP_PKEY pkey;
+  if (!GetCertificate(ssl, &x509, &pkey)) {
+    return false;
+  }
+
+  if (pkey) {
+    TestState *test_state = GetTestState(ssl);
+    const TestConfig *config = GetConfigPtr(ssl);
+    if (config->async) {
+      test_state->private_key = std::move(pkey);
+      SSL_set_private_key_method(ssl, &g_async_private_key_method);
+    } else if (!SSL_use_PrivateKey(ssl, pkey.get())) {
+      return false;
+    }
+  }
+
+  if (x509 && !SSL_use_certificate(ssl, x509.get())) {
+    return false;
+  }
+
+  return true;
+}
+
 static int SelectCertificateCallback(const struct ssl_early_callback_ctx *ctx) {
   const TestConfig *config = GetConfigPtr(ctx->ssl);
   GetTestState(ctx->ssl)->early_callback_called = true;
@@ -394,6 +421,28 @@
   return 1;
 }
 
+static int ClientCertCallback(SSL *ssl, X509 **out_x509, EVP_PKEY **out_pkey) {
+  if (GetConfigPtr(ssl)->async && !GetTestState(ssl)->cert_ready) {
+    return -1;
+  }
+
+  ScopedX509 x509;
+  ScopedEVP_PKEY pkey;
+  if (!GetCertificate(ssl, &x509, &pkey)) {
+    return -1;
+  }
+
+  // Return zero for no certificate.
+  if (!x509) {
+    return 0;
+  }
+
+  // Asynchronous private keys are not supported with client_cert_cb.
+  *out_x509 = x509.release();
+  *out_pkey = pkey.release();
+  return 1;
+}
+
 static int VerifySucceed(X509_STORE_CTX *store_ctx, void *arg) {
   SSL* ssl = (SSL*)X509_STORE_CTX_get_ex_data(store_ctx,
       SSL_get_ex_data_X509_STORE_CTX_idx());
@@ -768,6 +817,10 @@
 
   SSL_CTX_set_select_certificate_cb(ssl_ctx.get(), SelectCertificateCallback);
 
+  if (config->use_old_client_cert_callback) {
+    SSL_CTX_set_client_cert_cb(ssl_ctx.get(), ClientCertCallback);
+  }
+
   SSL_CTX_set_next_protos_advertised_cb(
       ssl_ctx.get(), NextProtosAdvertisedCallback, NULL);
   if (!config->select_next_proto.empty()) {
@@ -1140,9 +1193,8 @@
       !SSL_set_mode(ssl.get(), SSL_MODE_SEND_FALLBACK_SCSV)) {
     return false;
   }
-  if (!config->use_early_callback) {
+  if (!config->use_early_callback && !config->use_old_client_cert_callback) {
     if (config->async) {
-      // TODO(davidben): Also test |s->ctx->client_cert_cb| on the client.
       SSL_set_cert_cb(ssl.get(), CertCallback, NULL);
     } else if (!InstallCertificate(ssl.get())) {
       return false;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index fe53b5f..4c46639 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -2859,6 +2859,21 @@
 	// TLS client auth.
 	tests = append(tests, testCase{
 		testType: clientTest,
+		name:     "ClientAuth-NoCertificate",
+		config: Config{
+			ClientAuth: RequestClientCert,
+		},
+	})
+	tests = append(tests, testCase{
+		testType: clientTest,
+		name:     "ClientAuth-NoCertificate-OldCallback",
+		config: Config{
+			ClientAuth: RequestClientCert,
+		},
+		flags: []string{"-use-old-client-cert-callback"},
+	})
+	tests = append(tests, testCase{
+		testType: clientTest,
 		name:     "ClientAuth-RSA-Client",
 		config: Config{
 			ClientAuth: RequireAnyClientCert,
@@ -2879,6 +2894,19 @@
 			"-key-file", path.Join(*resourceDir, ecdsaKeyFile),
 		},
 	})
+	tests = append(tests, testCase{
+		testType: clientTest,
+		name:     "ClientAuth-OldCallback",
+		config: Config{
+			ClientAuth: RequireAnyClientCert,
+		},
+		flags: []string{
+			"-cert-file", path.Join(*resourceDir, rsaCertificateFile),
+			"-key-file", path.Join(*resourceDir, rsaKeyFile),
+			"-use-old-client-cert-callback",
+		},
+	})
+
 	if async {
 		// Test async keys against each key exchange.
 		tests = append(tests, testCase{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 95c85a8..dfbd380 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -98,6 +98,8 @@
   { "-p384-only", &TestConfig::p384_only },
   { "-enable-all-curves", &TestConfig::enable_all_curves },
   { "-use-sparse-dh-prime", &TestConfig::use_sparse_dh_prime },
+  { "-use-old-client-cert-callback",
+    &TestConfig::use_old_client_cert_callback },
 };
 
 const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 4e0a46a..e04fdd4 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -102,6 +102,7 @@
   bool enable_all_curves = false;
   bool use_sparse_dh_prime = false;
   int expect_key_exchange_info = 0;
+  bool use_old_client_cert_callback = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_config);