From cfd83744d93dbf7a6932c66e88cc457f35082598 Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Fri, 9 Aug 2024 11:56:33 -0700 Subject: [PATCH] Misc OpenSSL fixes improving the error message from PEM parsing and adding a few DCHECKs Tested locally with OpenSSL 3.x BUG=webrtc:42225468 Change-Id: Ia2ff1e5826f486060db73bee979e2703fc6c5823 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/358441 Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Reviewed-by: David Benjamin Cr-Commit-Position: refs/heads/main@{#42776} --- rtc_base/openssl_certificate.cc | 5 +++-- rtc_base/openssl_identity.cc | 8 ++++++-- rtc_base/rtc_certificate.cc | 2 +- rtc_base/ssl_stream_adapter_unittest.cc | 2 ++ 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/rtc_base/openssl_certificate.cc b/rtc_base/openssl_certificate.cc index 46ff576124..fcc2946dda 100644 --- a/rtc_base/openssl_certificate.cc +++ b/rtc_base/openssl_certificate.cc @@ -56,6 +56,7 @@ static void PrintCert(X509* x509) { // Generate a self-signed certificate, with the public key from the // given key pair. Caller is responsible for freeing the returned object. static X509* MakeCertificate(EVP_PKEY* pkey, const SSLIdentityParams& params) { + RTC_DCHECK(pkey != nullptr); RTC_LOG(LS_INFO) << "Making certificate for " << params.common_name; ASN1_INTEGER* asn1_serial_number = nullptr; @@ -95,8 +96,8 @@ static X509* MakeCertificate(EVP_PKEY* pkey, const SSLIdentityParams& params) { name.reset(X509_NAME_new()); if (name == nullptr || !X509_NAME_add_entry_by_NID(name.get(), NID_commonName, MBSTRING_UTF8, - (unsigned char*)params.common_name.c_str(), - -1, -1, 0) || + (unsigned char*)params.common_name.data(), -1, + -1, 0) || !X509_set_subject_name(x509.get(), name.get()) || !X509_set_issuer_name(x509.get(), name.get())) { return nullptr; diff --git a/rtc_base/openssl_identity.cc b/rtc_base/openssl_identity.cc index 186497836d..9ddd1786b1 100644 --- a/rtc_base/openssl_identity.cc +++ b/rtc_base/openssl_identity.cc @@ -79,8 +79,11 @@ std::unique_ptr OpenSSLIdentity::CreateWithExpiration( time_t now = time(nullptr); params.not_before = now + kCertificateWindowInSeconds; params.not_after = now + certificate_lifetime; - if (params.not_before > params.not_after) + if (params.not_before > params.not_after) { + RTC_LOG(LS_ERROR) + << "Іdentity generated failed, not_before is after not_after."; return nullptr; + } return CreateInternal(params); } @@ -127,7 +130,8 @@ std::unique_ptr OpenSSLIdentity::CreateFromPEMChainStrings( ERR_GET_REASON(err) == PEM_R_NO_START_LINE) { break; } - RTC_LOG(LS_ERROR) << "Failed to parse certificate from PEM string."; + RTC_LOG(LS_ERROR) << "Failed to parse certificate from PEM string: " + << ERR_reason_error_string(err); BIO_free(bio); return nullptr; } diff --git a/rtc_base/rtc_certificate.cc b/rtc_base/rtc_certificate.cc index e0b6b3258e..93e5f15148 100644 --- a/rtc_base/rtc_certificate.cc +++ b/rtc_base/rtc_certificate.cc @@ -21,7 +21,7 @@ namespace rtc { scoped_refptr RTCCertificate::Create( std::unique_ptr identity) { - // Explicit new to access proteced constructor. + // Explicit new to access protected constructor. return rtc::scoped_refptr( new RTCCertificate(identity.release())); } diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc index 1e45a57b4d..12780d3a07 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/rtc_base/ssl_stream_adapter_unittest.cc @@ -493,6 +493,8 @@ class SSLStreamAdapterTestBase : public ::testing::Test, : rtc::SSLPeerCertificateDigestError::VERIFICATION_FAILED; RTC_LOG(LS_INFO) << "Setting peer identities by digest"; + RTC_DCHECK(server_identity()); + RTC_DCHECK(client_identity()); rv = server_identity()->certificate().ComputeDigest( digest_algorithm_, server_digest, digest_length_, &server_digest_len);