Add out_alert to ProofVerifier::VerifyCertChain
This change will allow a ProofVerifier in the future to provide more detail
to the server when closing a connection due to a certificate verification
failure. Right now, this CL only plumbs through a new (currently unused)
argument to ProofVerifier::VerifyCertChain.
PiperOrigin-RevId: 342087110
Change-Id: I4d04b61d8c89b18556a7a6ef08289081087f694b
diff --git a/quic/core/crypto/proof_verifier.h b/quic/core/crypto/proof_verifier.h
index 7576620..ab09811 100644
--- a/quic/core/crypto/proof_verifier.h
+++ b/quic/core/crypto/proof_verifier.h
@@ -97,6 +97,11 @@
// for some implementations) that provides useful information for the
// verifier, e.g. logging handles.
//
+ // If certificate verification fails, a TLS alert will be sent when closing
+ // the connection. This alert defaults to certificate_unknown. By setting
+ // |*out_alert|, a different alert can be sent to provide a more specific
+ // reason why verification failed.
+ //
// This function may also return QUIC_PENDING, in which case the ProofVerifier
// will call back, on the original thread, via |callback| when complete.
// In this case, the ProofVerifier will take ownership of |callback|.
@@ -109,6 +114,7 @@
const ProofVerifyContext* context,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) = 0;
// Returns a ProofVerifyContext instance which can be use for subsequent
diff --git a/quic/core/quic_crypto_client_handshaker_test.cc b/quic/core/quic_crypto_client_handshaker_test.cc
index cfae506..2d6db7f 100644
--- a/quic/core/quic_crypto_client_handshaker_test.cc
+++ b/quic/core/quic_crypto_client_handshaker_test.cc
@@ -54,6 +54,7 @@
const ProofVerifyContext* /*context*/,
std::string* /*error_details*/,
std::unique_ptr<ProofVerifyDetails>* /*details*/,
+ uint8_t* /*out_alert*/,
std::unique_ptr<ProofVerifierCallback> /*callback*/) override {
return QUIC_SUCCESS;
}
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index fc69ad6..6ea06b8 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -376,6 +376,7 @@
const std::vector<std::string>& certs,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) {
const uint8_t* ocsp_response_raw;
size_t ocsp_response_len;
@@ -390,7 +391,8 @@
return proof_verifier_->VerifyCertChain(
server_id_.host(), server_id_.port(), certs, ocsp_response, sct_list,
- verify_context_.get(), error_details, details, std::move(callback));
+ verify_context_.get(), error_details, details, out_alert,
+ std::move(callback));
}
void TlsClientHandshaker::OnProofVerifyDetailsAvailable(
diff --git a/quic/core/tls_client_handshaker.h b/quic/core/tls_client_handshaker.h
index c2707c7..aadfa0b 100644
--- a/quic/core/tls_client_handshaker.h
+++ b/quic/core/tls_client_handshaker.h
@@ -92,6 +92,7 @@
const std::vector<std::string>& certs,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) override;
void OnProofVerifyDetailsAvailable(
const ProofVerifyDetails& verify_details) override;
diff --git a/quic/core/tls_client_handshaker_test.cc b/quic/core/tls_client_handshaker_test.cc
index 7cd7d45..2488426 100644
--- a/quic/core/tls_client_handshaker_test.cc
+++ b/quic/core/tls_client_handshaker_test.cc
@@ -74,15 +74,16 @@
const ProofVerifyContext* context,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) override {
if (!active_) {
- return verifier_->VerifyCertChain(hostname, port, certs, ocsp_response,
- cert_sct, context, error_details,
- details, std::move(callback));
+ return verifier_->VerifyCertChain(
+ hostname, port, certs, ocsp_response, cert_sct, context,
+ error_details, details, out_alert, std::move(callback));
}
pending_ops_.push_back(std::make_unique<VerifyChainPendingOp>(
hostname, port, certs, ocsp_response, cert_sct, context, error_details,
- details, std::move(callback), verifier_.get()));
+ details, out_alert, std::move(callback), verifier_.get()));
return QUIC_PENDING;
}
@@ -123,6 +124,7 @@
const ProofVerifyContext* context,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback,
ProofVerifier* delegate)
: hostname_(hostname),
@@ -133,6 +135,7 @@
context_(context),
error_details_(error_details),
details_(details),
+ out_alert_(out_alert),
callback_(std::move(callback)),
delegate_(delegate) {}
@@ -143,7 +146,7 @@
// synchronously.
QuicAsyncStatus status = delegate_->VerifyCertChain(
hostname_, port_, certs_, ocsp_response_, cert_sct_, context_,
- error_details_, details_,
+ error_details_, details_, out_alert_,
std::make_unique<FailingProofVerifierCallback>());
ASSERT_NE(status, QUIC_PENDING);
callback_->Run(status == QUIC_SUCCESS, *error_details_, details_);
@@ -158,6 +161,7 @@
const ProofVerifyContext* context_;
std::string* error_details_;
std::unique_ptr<ProofVerifyDetails>* details_;
+ uint8_t* out_alert_;
std::unique_ptr<ProofVerifierCallback> callback_;
ProofVerifier* delegate_;
};
diff --git a/quic/core/tls_handshaker.cc b/quic/core/tls_handshaker.cc
index 495312d..bfe3545 100644
--- a/quic/core/tls_handshaker.cc
+++ b/quic/core/tls_handshaker.cc
@@ -146,6 +146,7 @@
expected_ssl_error() == SSL_ERROR_WANT_CERTIFICATE_VERIFY) {
enum ssl_verify_result_t result = verify_result_;
verify_result_ = ssl_verify_retry;
+ *out_alert = cert_verify_tls_alert_;
return result;
}
const STACK_OF(CRYPTO_BUFFER)* cert_chain = SSL_get0_peer_certificates(ssl());
@@ -164,8 +165,10 @@
ProofVerifierCallbackImpl* proof_verify_callback =
new ProofVerifierCallbackImpl(this);
+ cert_verify_tls_alert_ = *out_alert;
QuicAsyncStatus verify_result = VerifyCertChain(
certs, &cert_verify_error_details_, &verify_details_,
+ &cert_verify_tls_alert_,
std::unique_ptr<ProofVerifierCallback>(proof_verify_callback));
switch (verify_result) {
case QUIC_SUCCESS:
@@ -179,6 +182,7 @@
return ssl_verify_retry;
case QUIC_FAILURE:
default:
+ *out_alert = cert_verify_tls_alert_;
QUIC_LOG(INFO) << "Cert chain verification failed: "
<< cert_verify_error_details_;
return ssl_verify_invalid;
diff --git a/quic/core/tls_handshaker.h b/quic/core/tls_handshaker.h
index 657c9c7..12b27f1 100644
--- a/quic/core/tls_handshaker.h
+++ b/quic/core/tls_handshaker.h
@@ -90,10 +90,16 @@
// non-owning pointer to |callback|; the callback must live until this
// function returns QUIC_SUCCESS or QUIC_FAILURE, or until the callback is
// run.
+ //
+ // If certificate verification fails, |*out_alert| may be set to a TLS alert
+ // that will be sent when closing the connection; it defaults to
+ // certificate_unknown. Implementations of VerifyCertChain may retain the
+ // |out_alert| pointer while performing an async operation.
virtual QuicAsyncStatus VerifyCertChain(
const std::vector<std::string>& certs,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) = 0;
// Called when certificate verification is completed.
virtual void OnProofVerifyDetailsAvailable(
@@ -173,6 +179,7 @@
ProofVerifierCallbackImpl* proof_verify_callback_ = nullptr;
std::unique_ptr<ProofVerifyDetails> verify_details_;
enum ssl_verify_result_t verify_result_ = ssl_verify_retry;
+ uint8_t cert_verify_tls_alert_ = SSL_AD_CERTIFICATE_UNKNOWN;
std::string cert_verify_error_details_;
int expected_ssl_error_ = SSL_ERROR_WANT_READ;
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index aa49597..e0fe84d 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -397,6 +397,7 @@
const std::vector<std::string>& /*certs*/,
std::string* /*error_details*/,
std::unique_ptr<ProofVerifyDetails>* /*details*/,
+ uint8_t* /*out_alert*/,
std::unique_ptr<ProofVerifierCallback> /*callback*/) {
QUIC_BUG << "Client certificates are not yet supported on the server";
return QUIC_FAILURE;
diff --git a/quic/core/tls_server_handshaker.h b/quic/core/tls_server_handshaker.h
index 2fba611..4d12bf3 100644
--- a/quic/core/tls_server_handshaker.h
+++ b/quic/core/tls_server_handshaker.h
@@ -97,6 +97,7 @@
const std::vector<std::string>& certs,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) override;
void OnProofVerifyDetailsAvailable(
const ProofVerifyDetails& verify_details) override;
diff --git a/quic/qbone/qbone_session_test.cc b/quic/qbone/qbone_session_test.cc
index 92f8fa7..5059274 100644
--- a/quic/qbone/qbone_session_test.cc
+++ b/quic/qbone/qbone_session_test.cc
@@ -167,13 +167,14 @@
const ProofVerifyContext* context,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) override {
if (!proof_verifier_) {
return QUIC_FAILURE;
}
return proof_verifier_->VerifyCertChain(
hostname, port, certs, ocsp_response, cert_sct, context, error_details,
- details, std::move(callback));
+ details, out_alert, std::move(callback));
}
std::unique_ptr<ProofVerifyContext> CreateDefaultContext() override {
diff --git a/quic/quic_transport/web_transport_fingerprint_proof_verifier.cc b/quic/quic_transport/web_transport_fingerprint_proof_verifier.cc
index 2ea1e44..5932b9c 100644
--- a/quic/quic_transport/web_transport_fingerprint_proof_verifier.cc
+++ b/quic/quic_transport/web_transport_fingerprint_proof_verifier.cc
@@ -139,6 +139,7 @@
const ProofVerifyContext* /*context*/,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* /*out_alert*/,
std::unique_ptr<ProofVerifierCallback> /*callback*/) {
if (certs.empty()) {
*details = std::make_unique<Details>(Status::kInternalError);
diff --git a/quic/quic_transport/web_transport_fingerprint_proof_verifier.h b/quic/quic_transport/web_transport_fingerprint_proof_verifier.h
index fba1f83..ad83788 100644
--- a/quic/quic_transport/web_transport_fingerprint_proof_verifier.h
+++ b/quic/quic_transport/web_transport_fingerprint_proof_verifier.h
@@ -100,6 +100,7 @@
const ProofVerifyContext* context,
std::string* error_details,
std::unique_ptr<ProofVerifyDetails>* details,
+ uint8_t* out_alert,
std::unique_ptr<ProofVerifierCallback> callback) override;
std::unique_ptr<ProofVerifyContext> CreateDefaultContext() override;
diff --git a/quic/quic_transport/web_transport_fingerprint_proof_verifier_test.cc b/quic/quic_transport/web_transport_fingerprint_proof_verifier_test.cc
index b527ae6..a84a8a5 100644
--- a/quic/quic_transport/web_transport_fingerprint_proof_verifier_test.cc
+++ b/quic/quic_transport/web_transport_fingerprint_proof_verifier_test.cc
@@ -40,12 +40,13 @@
VerifyResult Verify(absl::string_view certificate) {
VerifyResult result;
std::unique_ptr<ProofVerifyDetails> details;
+ uint8_t tls_alert;
result.status = verifier_->VerifyCertChain(
/*hostname=*/"", /*port=*/0,
std::vector<std::string>{std::string(certificate)},
/*ocsp_response=*/"",
/*cert_sct=*/"",
- /*context=*/nullptr, &result.error, &details,
+ /*context=*/nullptr, &result.error, &details, &tls_alert,
/*callback=*/nullptr);
result.detailed_status =
static_cast<WebTransportFingerprintProofVerifier::Details*>(
diff --git a/quic/test_tools/quic_test_client.cc b/quic/test_tools/quic_test_client.cc
index d7f2033..5b93de5 100644
--- a/quic/test_tools/quic_test_client.cc
+++ b/quic/test_tools/quic_test_client.cc
@@ -82,6 +82,7 @@
const ProofVerifyContext* /*context*/,
std::string* /*error_details*/,
std::unique_ptr<ProofVerifyDetails>* /*details*/,
+ uint8_t* /*out_alert*/,
std::unique_ptr<ProofVerifierCallback> /*callback*/) override {
return ProcessCerts(certs, cert_sct);
}
diff --git a/quic/tools/fake_proof_verifier.h b/quic/tools/fake_proof_verifier.h
index 2d7d7f4..8707879 100644
--- a/quic/tools/fake_proof_verifier.h
+++ b/quic/tools/fake_proof_verifier.h
@@ -38,6 +38,7 @@
const ProofVerifyContext* /*context*/,
std::string* /*error_details*/,
std::unique_ptr<ProofVerifyDetails>* /*details*/,
+ uint8_t* /*out_alert*/,
std::unique_ptr<ProofVerifierCallback> /*callback*/) override {
return QUIC_SUCCESS;
}
