diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index ca1c2d5..cc330a0 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -1761,7 +1761,7 @@ // - `FIPS` is an alias for `HIGH`. // // - `SSLv3` and `TLSv1` match ciphers available in TLS 1.1 or earlier. -// `TLSv1_2` matches ciphers new in TLS 1.2. This is confusing and should not +// `TLSv1.2` matches ciphers new in TLS 1.2. This is confusing and should not // be used. // // Unknown rules are silently ignored by legacy APIs, and rejected by APIs with
diff --git a/pki/crl.cc b/pki/crl.cc index e143689..34b212e 100644 --- a/pki/crl.cc +++ b/pki/crl.cc
@@ -25,6 +25,7 @@ #include "parse_values.h" #include "parser.h" #include "revocation_util.h" +#include "simple_path_builder_delegate.h" #include "signature_algorithm.h" #include "verify_name_match.h" #include "verify_signed_data.h" @@ -429,6 +430,7 @@ std::optional<int64_t> max_age_seconds, VerifyCertificateChainDelegate *delegate) { BSSL_CHECK(target_cert_index < valid_chain.size()); + BSSL_CHECK(delegate); if (cert_dp.reasons) { // Reason codes are not supported. If the distribution point contains a @@ -470,8 +472,8 @@ } // Check the signature algorithm against policy. CertErrors unused_errors; - if (delegate && !delegate->IsSignatureAlgorithmAcceptable( - *signature_algorithm, &unused_errors)) { + if (!delegate->IsSignatureAlgorithmAcceptable(*signature_algorithm, + &unused_errors)) { return CRLRevocationStatus::UNKNOWN; } @@ -667,7 +669,7 @@ // key validated in step (f). if (!VerifySignedData(*signature_algorithm, tbs_cert_list_tlv, signature_value, issuer_cert->tbs().spki_tlv, - delegate ? delegate->GetVerifyCache() : nullptr)) { + delegate->GetVerifyCache())) { continue; } @@ -698,8 +700,16 @@ const ParsedDistributionPoint &cert_dp, int64_t verify_time_epoch_seconds, std::optional<int64_t> max_age_seconds) { + // Use a default delegate that matches the previous behavior of allowing + // SHA-1. (This doesn't actually need a PathBuilderDelegate, but there isn't + // a SimpleVerifyCertificateChainDelegate.) + // TODO(mattm): tighten the default allowed sigalgs? + SimplePathBuilderDelegate default_verify_delegate( + /*min_rsa_modulus_length_bits=*/1024, + SimplePathBuilderDelegate::DigestPolicy::kWeakAllowSha1); return CheckCRL(raw_crl, valid_chain, target_cert_index, cert_dp, - verify_time_epoch_seconds, max_age_seconds, nullptr); + verify_time_epoch_seconds, max_age_seconds, + &default_verify_delegate); } BSSL_NAMESPACE_END
diff --git a/pki/crl.h b/pki/crl.h index 01639cc..3035d1e 100644 --- a/pki/crl.h +++ b/pki/crl.h
@@ -221,10 +221,7 @@ // implemented as time since the `thisUpdate` field in the CRL // TBSCertList. Responses older than `max_age_seconds` will be // considered invalid. -// * `delegate`: If non-null, used to check the signature algorithm. -// -// TODO(mattm): Make the `delegate` non-optional. The other wrapper could -// configure a default one itself instead of passing in null. +// * `delegate`: Used to check the signature algorithm. [[nodiscard]] OPENSSL_EXPORT CRLRevocationStatus CheckCRL( std::string_view raw_crl, const ParsedCertificateList &valid_chain, size_t target_cert_index, const ParsedDistributionPoint &cert_dp,
diff --git a/pki/ocsp.cc b/pki/ocsp.cc index 8d8efe1..dbd570d 100644 --- a/pki/ocsp.cc +++ b/pki/ocsp.cc
@@ -663,16 +663,6 @@ const std::shared_ptr<const ParsedCertificate> &issuer_certificate, int64_t verify_time_epoch_seconds, VerifyCertificateChainDelegate *delegate) { - // If a delegate is not supplied, use a default delegate that matches the - // previous behavior of allowing SHA-1. (This doesn't actually need a - // PathBuilderDelegate, but there isn't a - // SimpleVerifyCertificateChainDelegate.) - SimplePathBuilderDelegate default_verify_delegate( - /*min_rsa_modulus_length_bits=*/1024, - SimplePathBuilderDelegate::DigestPolicy::kWeakAllowSha1); - if (!delegate) { - delegate = &default_verify_delegate; - } CertPathErrors errors; TrustAnchor issuer_trust(CertificateTrust::ForTrustAnchor()); der::GeneralizedTime verify_time; @@ -716,14 +706,14 @@ VerifyCertificateChainDelegate *delegate) { // Check the signature algorithm against policy. CertErrors unused_errors; - if (delegate && !delegate->IsSignatureAlgorithmAcceptable( - response.signature_algorithm, &unused_errors)) { + if (!delegate->IsSignatureAlgorithmAcceptable(response.signature_algorithm, + &unused_errors)) { return false; } return VerifySignedData(response.signature_algorithm, response.data, response.signature, cert->tbs().spki_tlv, - delegate ? delegate->GetVerifyCache() : nullptr); + delegate->GetVerifyCache()); } // Verifies that the OCSP response has a valid signature using @@ -1010,22 +1000,17 @@ OCSPParseCertificate(StringAsBytes(certificate_der)); std::shared_ptr<const ParsedCertificate> parsed_issuer_certificate = OCSPParseCertificate(StringAsBytes(issuer_certificate_der)); + // Use a default delegate that matches the previous behavior of allowing + // SHA-1. (This doesn't actually need a PathBuilderDelegate, but there isn't + // a SimpleVerifyCertificateChainDelegate.) + // TODO(mattm): tighten the default allowed sigalgs? + SimplePathBuilderDelegate default_verify_delegate( + /*min_rsa_modulus_length_bits=*/1024, + SimplePathBuilderDelegate::DigestPolicy::kWeakAllowSha1); return CheckOCSP(raw_response, parsed_certificate.get(), parsed_issuer_certificate, verify_time_epoch_seconds, - max_age_seconds, nullptr, response_details); -} - -OCSPRevocationStatus CheckOCSP( - std::string_view raw_response, const ParsedCertificate *certificate, - const ParsedCertificate *issuer_certificate, - int64_t verify_time_epoch_seconds, std::optional<int64_t> max_age_seconds, - OCSPVerifyResult::ResponseStatus *response_details) { - std::shared_ptr<const ParsedCertificate> parsed_issuer_certificate = - OCSPParseCertificate(issuer_certificate->der_cert()); - return CheckOCSP(raw_response, certificate, parsed_issuer_certificate, - verify_time_epoch_seconds, max_age_seconds, - nullptr, response_details); + max_age_seconds, &default_verify_delegate, response_details); } OCSPRevocationStatus CheckOCSP( @@ -1035,6 +1020,7 @@ int64_t verify_time_epoch_seconds, std::optional<int64_t> max_age_seconds, VerifyCertificateChainDelegate *delegate, OCSPVerifyResult::ResponseStatus *response_details) { + BSSL_CHECK(delegate); return CheckOCSP(raw_response, certificate.get(), issuer_certificate, verify_time_epoch_seconds, max_age_seconds, delegate, response_details);
diff --git a/pki/ocsp.h b/pki/ocsp.h index 4df1525..01d1853 100644 --- a/pki/ocsp.h +++ b/pki/ocsp.h
@@ -276,11 +276,7 @@ // // Arguments are the same as the CheckOCSP from the public ocsp.h, except that // it takes already parsed instances of the certificate and issuer certificate -// and takes a optional `delegate` for signature and certificate verification -// policy. -// -// TODO(mattm): Make the `delegate` non-optional. The other wrapper could -// configure a default one itself instead of passing in null. +// and takes a `delegate` for signature and certificate verification policy. [[nodiscard]] OPENSSL_EXPORT OCSPRevocationStatus CheckOCSP( std::string_view raw_response, const std::shared_ptr<const ParsedCertificate> &certificate, @@ -289,13 +285,6 @@ VerifyCertificateChainDelegate *delegate, OCSPVerifyResult::ResponseStatus *response_details); -// TODO(mattm): remove this after Chrome is updated to use the new signature. -[[nodiscard]] OPENSSL_EXPORT OCSPRevocationStatus CheckOCSP( - std::string_view raw_response, const ParsedCertificate *certificate, - const ParsedCertificate *issuer_certificate, - int64_t verify_time_epoch_seconds, std::optional<int64_t> max_age_seconds, - OCSPVerifyResult::ResponseStatus *response_details); - // Creates a DER-encoded OCSPRequest for `cert`. The request is fairly basic: // * No signature // * No requestorName
diff --git a/rust/bssl-tls/src/context.rs b/rust/bssl-tls/src/context.rs index 7858e68..5797286 100644 --- a/rust/bssl-tls/src/context.rs +++ b/rust/bssl-tls/src/context.rs
@@ -400,7 +400,7 @@ /// - `FIPS` is an alias for `HIGH`. /// /// - `SSLv3` and `TLSv1` match ciphers available in TLS 1.1 or earlier. - /// `TLSv1_2` matches ciphers new in TLS 1.2. This is confusing and should not + /// `TLSv1.2` matches ciphers new in TLS 1.2. This is confusing and should not /// be used. /// /// [cipher suite configuration]: <https://docs.openssl.org/3.0/man1/openssl-ciphers/#cipher-list-format>
diff --git a/ssl/ssl_cipher.cc b/ssl/ssl_cipher.cc index be2e725..60142fc 100644 --- a/ssl/ssl_cipher.cc +++ b/ssl/ssl_cipher.cc
@@ -30,7 +30,6 @@ #include <openssl/sha.h> #include <openssl/stack.h> -#include "../crypto/internal.h" #include "internal.h" @@ -450,8 +449,8 @@ // Legacy protocol minimum version aliases. "TLSv1" is intentionally the // same as "SSLv3". - {"SSLv3", ~0u, ~0u, ~0u, ~0u, SSL3_VERSION}, - {"TLSv1", ~0u, ~0u, ~0u, ~0u, SSL3_VERSION}, + {"SSLv3", ~0u, ~0u, ~0u, ~0u, TLS1_VERSION}, + {"TLSv1", ~0u, ~0u, ~0u, ~0u, TLS1_VERSION}, {"TLSv1.2", ~0u, ~0u, ~0u, ~0u, TLS1_2_VERSION}, // Legacy strength classes. @@ -1518,7 +1517,7 @@ // afterwards specify a particular hash. return TLS1_2_VERSION; } - return SSL3_VERSION; + return TLS1_VERSION; } uint16_t SSL_CIPHER_get_max_version(const SSL_CIPHER *cipher) {