Don't invoke custom certificate verifier unless there is an error.
The custom callback is intended to override errors, so there's no point in calling it if the status is ok. Calling it during an otherwise successful verification was an unintentional change from: https://webrtc-review.googlesource.com/c/src/+/196941 This is misleading as the return value isn't even used. Bug: chromium:1247577 Change-Id: Id74411f7364537a3225021e7631bc9ab962889ee Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/231881 Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#34994}
This commit is contained in:
committed by
WebRTC LUCI CQ
parent
bc503c95b5
commit
bd917a13fb
@ -868,11 +868,11 @@ int OpenSSLAdapter::SSLVerifyCallback(int status, X509_STORE_CTX* store) {
|
||||
return status;
|
||||
}
|
||||
|
||||
int OpenSSLAdapter::SSLVerifyInternal(int status_on_failure,
|
||||
int OpenSSLAdapter::SSLVerifyInternal(int previous_status,
|
||||
SSL* ssl,
|
||||
X509_STORE_CTX* store) {
|
||||
#if !defined(NDEBUG)
|
||||
if (!status_on_failure) {
|
||||
if (!previous_status) {
|
||||
char data[256];
|
||||
X509* cert = X509_STORE_CTX_get_current_cert(store);
|
||||
int depth = X509_STORE_CTX_get_error_depth(store);
|
||||
@ -887,8 +887,10 @@ int OpenSSLAdapter::SSLVerifyInternal(int status_on_failure,
|
||||
<< X509_verify_cert_error_string(err);
|
||||
}
|
||||
#endif
|
||||
if (ssl_cert_verifier_ == nullptr) {
|
||||
return status_on_failure;
|
||||
// `ssl_cert_verifier_` is used to override errors; if there is no error
|
||||
// there is no reason to call it.
|
||||
if (previous_status || ssl_cert_verifier_ == nullptr) {
|
||||
return previous_status;
|
||||
}
|
||||
|
||||
RTC_LOG(LS_INFO) << "Invoking SSL Verify Callback.";
|
||||
@ -898,14 +900,14 @@ int OpenSSLAdapter::SSLVerifyInternal(int status_on_failure,
|
||||
int length = i2d_X509(X509_STORE_CTX_get_current_cert(store), &data);
|
||||
if (length < 0) {
|
||||
RTC_LOG(LS_ERROR) << "Failed to encode X509.";
|
||||
return status_on_failure;
|
||||
return previous_status;
|
||||
}
|
||||
bssl::UniquePtr<uint8_t> owned_data(data);
|
||||
bssl::UniquePtr<CRYPTO_BUFFER> crypto_buffer(
|
||||
CRYPTO_BUFFER_new(data, length, openssl::GetBufferPool()));
|
||||
if (!crypto_buffer) {
|
||||
RTC_LOG(LS_ERROR) << "Failed to allocate CRYPTO_BUFFER.";
|
||||
return status_on_failure;
|
||||
return previous_status;
|
||||
}
|
||||
const BoringSSLCertificate cert(std::move(crypto_buffer));
|
||||
#else
|
||||
@ -913,7 +915,7 @@ int OpenSSLAdapter::SSLVerifyInternal(int status_on_failure,
|
||||
#endif
|
||||
if (!ssl_cert_verifier_->Verify(cert)) {
|
||||
RTC_LOG(LS_INFO) << "Failed to verify certificate using custom callback";
|
||||
return status_on_failure;
|
||||
return previous_status;
|
||||
}
|
||||
|
||||
custom_cert_verifier_status_ = true;
|
||||
|
||||
Reference in New Issue
Block a user