diff --git a/include/openssl/prefix_symbols.h b/include/openssl/prefix_symbols.h index aa82789..8ae8d28 100644 --- a/include/openssl/prefix_symbols.h +++ b/include/openssl/prefix_symbols.h
@@ -2023,6 +2023,7 @@ #pragma redefine_extname SSL_CTX_set_alpn_protos BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set_alpn_protos) #pragma redefine_extname SSL_CTX_set_alpn_select_cb BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set_alpn_select_cb) #pragma redefine_extname SSL_CTX_set_cert_cb BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set_cert_cb) +#pragma redefine_extname SSL_CTX_set_cert_cb_ex BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set_cert_cb_ex) #pragma redefine_extname SSL_CTX_set_cert_store BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set_cert_store) #pragma redefine_extname SSL_CTX_set_cert_verify_callback BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set_cert_verify_callback) #pragma redefine_extname SSL_CTX_set_chain_and_key BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_CTX_set_chain_and_key) @@ -2374,6 +2375,7 @@ #pragma redefine_extname SSL_set_alps_use_new_codepoint BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_alps_use_new_codepoint) #pragma redefine_extname SSL_set_bio BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_bio) #pragma redefine_extname SSL_set_cert_cb BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_cert_cb) +#pragma redefine_extname SSL_set_cert_cb_ex BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_cert_cb_ex) #pragma redefine_extname SSL_set_chain_and_key BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_chain_and_key) #pragma redefine_extname SSL_set_cipher_list BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_cipher_list) #pragma redefine_extname SSL_set_client_CA_list BORINGSSL_ADD_USER_LABEL_AND_PREFIX(SSL_set_client_CA_list) @@ -5168,6 +5170,7 @@ #define SSL_CTX_set_alpn_protos BORINGSSL_ADD_PREFIX(SSL_CTX_set_alpn_protos) #define SSL_CTX_set_alpn_select_cb BORINGSSL_ADD_PREFIX(SSL_CTX_set_alpn_select_cb) #define SSL_CTX_set_cert_cb BORINGSSL_ADD_PREFIX(SSL_CTX_set_cert_cb) +#define SSL_CTX_set_cert_cb_ex BORINGSSL_ADD_PREFIX(SSL_CTX_set_cert_cb_ex) #define SSL_CTX_set_cert_store BORINGSSL_ADD_PREFIX(SSL_CTX_set_cert_store) #define SSL_CTX_set_cert_verify_callback BORINGSSL_ADD_PREFIX(SSL_CTX_set_cert_verify_callback) #define SSL_CTX_set_chain_and_key BORINGSSL_ADD_PREFIX(SSL_CTX_set_chain_and_key) @@ -5519,6 +5522,7 @@ #define SSL_set_alps_use_new_codepoint BORINGSSL_ADD_PREFIX(SSL_set_alps_use_new_codepoint) #define SSL_set_bio BORINGSSL_ADD_PREFIX(SSL_set_bio) #define SSL_set_cert_cb BORINGSSL_ADD_PREFIX(SSL_set_cert_cb) +#define SSL_set_cert_cb_ex BORINGSSL_ADD_PREFIX(SSL_set_cert_cb_ex) #define SSL_set_chain_and_key BORINGSSL_ADD_PREFIX(SSL_set_chain_and_key) #define SSL_set_cipher_list BORINGSSL_ADD_PREFIX(SSL_set_cipher_list) #define SSL_set_client_CA_list BORINGSSL_ADD_PREFIX(SSL_set_client_CA_list)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index bfc7f9e..0d8241c 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -1091,6 +1091,18 @@ int (*cb)(SSL *ssl, void *arg), void *arg); +// SSL_CTX_set_cert_cb_ex sets a callback that is called to select a certificate +// like `SSL_CTX_set_cert_cb` with an additional argument to allow the callback +// to select the fatal alert to send. +// If `cb` returns zero, it should set `*out_alert` to one of `SSL_AD_*` to +// specify the alert. If unset, it defaults to `SSL_AD_INTERNAL_ERROR`. +// `SSL_AD_HANDSHAKE_FAILURE` is an appropriate alert if the caller and peer do +// not have parameters in common. +OPENSSL_EXPORT void SSL_CTX_set_cert_cb_ex(SSL_CTX *ctx, + int (*cb)(SSL *ssl, void *arg, + uint8_t *out_alert), + void *arg); + // SSL_set_cert_cb sets a callback that is called to select a certificate. The // callback returns one on success, zero on internal error, and a negative // number on failure or to pause the handshake. If the handshake is paused, @@ -1106,6 +1118,16 @@ OPENSSL_EXPORT void SSL_set_cert_cb(SSL *ssl, int (*cb)(SSL *ssl, void *arg), void *arg); +// SSL_set_cert_cb_ex sets a callback that is called to select a certificate +// like `SSL_CTX_set_cert_cb` with an additional argument to allow the callback +// to select the fatal alert to send. +// If `cb` returns zero, it should set `*out_alert` to one of `SSL_AD_*` to +// specify the alert. If unset, it defaults to `SSL_AD_INTERNAL_ERROR`. +// `SSL_AD_HANDSHAKE_FAILURE` is an appropriate alert if the caller and peer do +// not have parameters in common. +OPENSSL_EXPORT void SSL_set_cert_cb_ex( + SSL *ssl, int (*cb)(SSL *ssl, void *arg, uint8_t *out_alert), void *arg); + // SSL_get0_certificate_types, for a client, sets `*out_types` to an array // containing the client certificate types requested by a server. It returns the // length of the array. Note this list is always empty in TLS 1.3. The server
diff --git a/pki/common_cert_errors.cc b/pki/common_cert_errors.cc index 99d493f..64c4bc8 100644 --- a/pki/common_cert_errors.cc +++ b/pki/common_cert_errors.cc
@@ -103,6 +103,9 @@ DEFINE_CERT_ERROR_ID(kDeadlineExceeded, "Deadline exceeded"); DEFINE_CERT_ERROR_ID(kIterationLimitExceeded, "Iteration limit exceeded"); DEFINE_CERT_ERROR_ID(kDepthLimitExceeded, "Depth limit exceeded"); +DEFINE_CERT_ERROR_ID(kMtcLandmarkNotRecognized, "Landmark not recognized"); +DEFINE_CERT_ERROR_ID(kMtcUnacceptableCosignatureVerificationResult, + "Unacceptable cosignature verification result"); } // namespace cert_errors BSSL_NAMESPACE_END
diff --git a/pki/common_cert_errors.h b/pki/common_cert_errors.h index a1486fe..c49c538 100644 --- a/pki/common_cert_errors.h +++ b/pki/common_cert_errors.h
@@ -179,6 +179,18 @@ // Depth limit was reached during path building. OPENSSL_EXPORT extern const CertErrorId kDepthLimitExceeded; +// The certificate is a landmark relative MTC (no signatures) but the subtree +// was not present in `mtc_anchor`'s configured trusted subtrees. +// (Note that this error does not include the case where the subtree is present +// in the trusted subtrees but has the incorrect hash.) +OPENSSL_EXPORT extern const CertErrorId kMtcLandmarkNotRecognized; + +// The certificate is an MTC which was verified as a standalone and had a valid +// CA signature, but `delegate->IsCosignatureVerificationResultAcceptable` +// returned false. +OPENSSL_EXPORT extern const CertErrorId + kMtcUnacceptableCosignatureVerificationResult; + } // namespace cert_errors BSSL_NAMESPACE_END
diff --git a/pki/path_builder.cc b/pki/path_builder.cc index 1baaa9d..388b7b1 100644 --- a/pki/path_builder.cc +++ b/pki/path_builder.cc
@@ -820,7 +820,10 @@ return VerifyError(VerifyError::StatusCode::PATH_NOT_FOUND, depth, std::move(diagnostic)); } - if (single_error.value() == cert_errors::kVerifySignedDataFailed) { + if (single_error.value() == cert_errors::kVerifySignedDataFailed || + single_error.value() == cert_errors::kMtcLandmarkNotRecognized || + single_error.value() == + cert_errors::kMtcUnacceptableCosignatureVerificationResult) { return VerifyError(VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE, depth, std::move(diagnostic)); }
diff --git a/pki/path_builder_unittest.cc b/pki/path_builder_unittest.cc index 518fc3b..6a3a9f9 100644 --- a/pki/path_builder_unittest.cc +++ b/pki/path_builder_unittest.cc
@@ -2664,10 +2664,16 @@ result = RunPathBuilder(signatureless_leaf, &trust_store_no_subtrees, nullptr, &mtc_cosigner_not_called_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kMtcLandmarkNotRecognized)); result = RunPathBuilder(signatureless_leaf, &trust_store_no_subtrees_wrong_key, nullptr, &mtc_cosigner_not_called_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kMtcLandmarkNotRecognized)); // Standalone cert should be valid when verified against the anchor // configured with subtrees (regardless of what key the anchor is configured @@ -2688,15 +2694,24 @@ result = RunPathBuilder(standalone_leaf, &trust_store_no_subtrees_wrong_key, nullptr, &no_cosigners_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); // Both certs should fail when verified against the anchor with wrong subtree // hash. result = RunPathBuilder(signatureless_leaf, &trust_store_wrong_subtreehash, nullptr, &mtc_cosigner_not_called_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); result = RunPathBuilder(standalone_leaf, &trust_store_wrong_subtreehash, nullptr, &mtc_cosigner_not_called_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); // Cert with multiple cosigners (including valid CA cosigner) should validate // successfully, ignoring the unknown cosigners. @@ -2712,6 +2727,9 @@ &trust_store_no_subtrees_wrong_key, nullptr, &no_cosigners_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); // Cert with a cosigner but no CA cosigner should fail: std::shared_ptr<const ParsedCertificate> standalone_leaf_no_ca_signer; @@ -2721,6 +2739,9 @@ RunPathBuilder(standalone_leaf_no_ca_signer, &trust_store_no_subtrees, nullptr, &no_cosigners_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); // Cert with a duplicate CA cosigner should fail: std::shared_ptr<const ParsedCertificate> standalone_leaf_duplicate_ca_signer; @@ -2731,6 +2752,9 @@ RunPathBuilder(standalone_leaf_duplicate_ca_signer, &trust_store_no_subtrees, nullptr, &no_cosigners_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); // Cert with a cosigners in non-sorted order should fail: std::shared_ptr<const ParsedCertificate> standalone_leaf_cosigner_wrong_order; @@ -2741,6 +2765,9 @@ RunPathBuilder(standalone_leaf_cosigner_wrong_order, &trust_store_no_subtrees, nullptr, &no_cosigners_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); } TEST_F(PathBuilderMTCPlants04Test, CosignatureVerification) { @@ -2815,6 +2842,9 @@ &trust_store_no_subtrees_wrong_key, nullptr, &cosigners_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kVerifySignedDataFailed)); } { @@ -2891,6 +2921,9 @@ RunPathBuilder(standalone_leaf_3_cosigners, &trust_store_no_subtrees, nullptr, &rejecting_delegate); EXPECT_FALSE(result.HasValidPath()); + ASSERT_TRUE(result.GetBestPathPossiblyInvalid()); + EXPECT_TRUE(result.GetBestPathPossiblyInvalid()->errors.ContainsError( + cert_errors::kMtcUnacceptableCosignatureVerificationResult)); } } @@ -2969,6 +3002,7 @@ EXPECT_EQ(leaf, path.certs[0]); EXPECT_EQ(ica, path.certs[1]); EXPECT_EQ(mtc_anchor_->AsCert(), path.certs[2]); + EXPECT_TRUE(path.errors.ContainsError(cert_errors::kMaxPathLengthViolated)); } }
diff --git a/pki/verify_certificate_chain.cc b/pki/verify_certificate_chain.cc index 36edb1b..eb651ab 100644 --- a/pki/verify_certificate_chain.cc +++ b/pki/verify_certificate_chain.cc
@@ -1220,18 +1220,35 @@ Span(CRYPTO_BUFFER_data(key_bytes), CRYPTO_BUFFER_len(key_bytes)), cache); } +enum class VerifyMTCResult { + // Failed for any reason not covered by the more specific cases. + kFailed, + + // The MTC was landmark relative (had no signatures) but the subtree range + // was not present in `mtc_anchor`'s configured trusted subtrees. + // (Note that this does not include the case where the range was present in + // the trusted subtrees but had the incorrect hash.) + kLandmarkNotRecognized, + + // The MTC was verified as a standalone MTC and had a valid CA signature, but + // `delegate->IsCosignatureVerificationResultAcceptable` returned false. + kUnacceptableCosignatureVerificationResult, + + // The MTC verified successfully. + kSuccess, +}; // This function implements draft-ietf-plants-merkle-tree-certs-04 section 7.2: // Verifying Certificate Signatures. -static bool VerifyMTC(const ParsedCertificate &cert, - const MTCAnchor *mtc_anchor, - VerifyCertificateChainDelegate *delegate) { +static VerifyMTCResult VerifyMTC(const ParsedCertificate &cert, + const MTCAnchor *mtc_anchor, + VerifyCertificateChainDelegate *delegate) { // Step 1: Check that the TBSCertificate's signature field is id-alg-mtcProof // (kMtcProofDraftPlants04) with omitted parameters. if (cert.signature_algorithm() != SignatureAlgorithm::kMtcProofDraftPlants04) { // When we parse the signature algorithm, we check that the parameters are // omitted. - return false; + return VerifyMTCResult::kFailed; } // Step 2: Decode the signatureValue as an MTCProof. @@ -1244,14 +1261,14 @@ !CBS_get_u16_length_prefixed(&mtc_proof, &inclusion_proof) || !CBS_get_u16_length_prefixed(&mtc_proof, &signatures) || CBS_len(&mtc_proof) != 0) { - return false; + return VerifyMTCResult::kFailed; } // Step 3: Let serial be the certificate's serial number. If serial is // negative or greater than 2^64-1, abort this process and fail verification. uint64_t serial; if (!der::ParseUint64(cert.tbs().serial_number, &serial)) { - return false; + return VerifyMTCResult::kFailed; } // Step 4's revocation check is not performed in this function. The caller is @@ -1263,7 +1280,7 @@ uint64_t index = serial & ((1ull << 48) - 1); uint16_t log_number = serial >> 48; if (log_number == 0) { - return false; + return VerifyMTCResult::kFailed; } // Step 6 is only relevant for standalone certificate verification, so we put @@ -1275,7 +1292,7 @@ // 1. Initialize a hash instance. ScopedEVP_MD_CTX entry_hash_ctx; if (!EVP_DigestInit(entry_hash_ctx.get(), EVP_sha256())) { - return false; + return VerifyMTCResult::kFailed; } // Write the octet 0x00 to the hash. This is the domain separator for leaf @@ -1284,7 +1301,7 @@ static constexpr uint8_t kDomainSeparator[] = {0x00}; if (!EVP_DigestUpdate(entry_hash_ctx.get(), kDomainSeparator, sizeof(kDomainSeparator))) { - return false; + return VerifyMTCResult::kFailed; } // 2. Write the extensions field from the MTCProof to the hash. @@ -1295,14 +1312,14 @@ sizeof(extensions_length)) || !EVP_DigestUpdate(entry_hash_ctx.get(), CBS_data(&extensions), CBS_len(&extensions))) { - return false; + return VerifyMTCResult::kFailed; } // 3. Write the big-endian, two-byte tbs_cert_entry value to the hash. static constexpr uint8_t kTbsCertEntry[] = {0, 1}; if (!EVP_DigestUpdate(entry_hash_ctx.get(), kTbsCertEntry, sizeof(kTbsCertEntry))) { - return false; + return VerifyMTCResult::kFailed; } // 4. Write the TBSCertificate's `version`, `issuer`, `validity`, and @@ -1319,7 +1336,7 @@ !CBB_flush(version_outer.get()) || !EVP_DigestUpdate(entry_hash_ctx.get(), CBB_data(version_outer.get()), CBB_len(version_outer.get()))) { - return false; + return VerifyMTCResult::kFailed; } } if (!EVP_DigestUpdate(entry_hash_ctx.get(), cert.tbs().issuer_tlv.data(), @@ -1328,7 +1345,7 @@ cert.tbs().validity_tlv.size()) || !EVP_DigestUpdate(entry_hash_ctx.get(), cert.tbs().subject_tlv.data(), cert.tbs().subject_tlv.size())) { - return false; + return VerifyMTCResult::kFailed; } // 5. Write the subjectPublicKeyInfo's algorithm field to the hash. @@ -1339,7 +1356,7 @@ CBS_ASN1_SEQUENCE) || !EVP_DigestUpdate(entry_hash_ctx.get(), CBS_data(&spki_algorithm_tlv), CBS_len(&spki_algorithm_tlv))) { - return false; + return VerifyMTCResult::kFailed; } // 6. Write the octet 0x04 to the hash. This is an OCTET STRING identifier. @@ -1349,7 +1366,7 @@ SHA256_DIGEST_LENGTH}; if (!EVP_DigestUpdate(entry_hash_ctx.get(), kSpkiHashTagAndLength, sizeof(kSpkiHashTagAndLength))) { - return false; + return VerifyMTCResult::kFailed; } // 8. Write H to the hash, where H is the hash of the entire @@ -1357,7 +1374,7 @@ uint8_t spki_hash[SHA256_DIGEST_LENGTH]; SHA256(cert.tbs().spki_tlv.data(), cert.tbs().spki_tlv.size(), spki_hash); if (!EVP_DigestUpdate(entry_hash_ctx.get(), spki_hash, sizeof(spki_hash))) { - return false; + return VerifyMTCResult::kFailed; } // 9. Write the remainder of the TBSCertificate contents octets to the hash, @@ -1365,13 +1382,13 @@ if (!EVP_DigestUpdate(entry_hash_ctx.get(), cert.tbs().bytes_after_spki.data(), cert.tbs().bytes_after_spki.size())) { - return false; + return VerifyMTCResult::kFailed; } // 10. Finalize the hash and set entry_hash to the result. TreeHash entry_hash; if (!EVP_DigestFinal(entry_hash_ctx.get(), entry_hash.data(), nullptr)) { - return false; + return VerifyMTCResult::kFailed; } // Step 10. Let expected_subtree_hash be the result of evaluating the @@ -1381,14 +1398,14 @@ EvaluateMerkleSubtreeInclusionProof(inclusion_proof, index, entry_hash, range); if (!expected_subtree_hash) { - return false; + return VerifyMTCResult::kFailed; } // Step 11. If log_number, start, and end matches a trusted subtree (Section // 7.4) for the CA, check that expected_subtree_hash is equal to the trusted // subtree's hash. if (!mtc_anchor) { - return false; + return VerifyMTCResult::kFailed; } std::optional<TreeHashConstSpan> trusted_subtree_hash = mtc_anchor->SubtreeHash(log_number, range); @@ -1398,7 +1415,7 @@ CBS ca_id(mtc_anchor->ca_id()); UniquePtr<char> ca_id_text(CBS_asn1_relative_oid_to_text(&ca_id)); if (!ca_id_text) { - return false; + return VerifyMTCResult::kFailed; } if (delegate->IsDebugLogEnabled()) { std::string trusted_subtree_hash_string = @@ -1421,9 +1438,13 @@ ); } if (trusted_subtree_hash) { - return CRYPTO_memcmp(expected_subtree_hash->data(), - trusted_subtree_hash->data(), - expected_subtree_hash->size()) == 0; + return (CRYPTO_memcmp(expected_subtree_hash->data(), + trusted_subtree_hash->data(), + expected_subtree_hash->size()) == 0) + ? VerifyMTCResult::kSuccess + : VerifyMTCResult::kFailed; + } else if (CBS_len(&signatures) == 0) { + return VerifyMTCResult::kLandmarkNotRecognized; } // Step 6: Let log_id be the log ID constructed from the CA ID in issuer and @@ -1449,7 +1470,7 @@ if (!CBS_get_u8_length_prefixed(&signatures, &cbs_cosigner_id) || CBS_len(&cbs_cosigner_id) == 0 || !CBS_get_u16_length_prefixed(&signatures, &signature)) { - return false; + return VerifyMTCResult::kFailed; } Span<const uint8_t> cosigner_id(cbs_cosigner_id); // Section 6.1: Each element of the signatures field MUST have a unique @@ -1460,14 +1481,14 @@ if (!prev_cosigner_id.empty()) { // Shorter byte strings are ordered before longer byte strings if (prev_cosigner_id.size() > cosigner_id.size()) { - return false; + return VerifyMTCResult::kFailed; } // Byte strings of the same length are ordered lexicographically if (prev_cosigner_id.size() == cosigner_id.size() && !std::lexicographical_compare( prev_cosigner_id.begin(), prev_cosigner_id.end(), cosigner_id.begin(), cosigner_id.end())) { - return false; + return VerifyMTCResult::kFailed; } } @@ -1517,9 +1538,11 @@ if (found_valid_ca_signature) { return delegate->IsCosignatureVerificationResultAcceptable( - mtc_anchor, std::move(valid_additional_cosigners)); + mtc_anchor, std::move(valid_additional_cosigners)) + ? VerifyMTCResult::kSuccess + : VerifyMTCResult::kUnacceptableCosignatureVerificationResult; } - return false; + return VerifyMTCResult::kFailed; } void PathVerifier::BasicCertificateProcessing( @@ -1559,9 +1582,21 @@ if (!is_target_cert) { *shortcircuit_chain_validation = true; errors->AddError(cert_errors::kMaxPathLengthViolated); - } else if (!VerifyMTC(cert, working_mtc_anchor_, delegate_)) { + } else if (VerifyMTCResult result = + VerifyMTC(cert, working_mtc_anchor_, delegate_); + result != VerifyMTCResult::kSuccess) { *shortcircuit_chain_validation = true; - errors->AddError(cert_errors::kVerifySignedDataFailed); + switch (result) { + case VerifyMTCResult::kLandmarkNotRecognized: + errors->AddError(cert_errors::kMtcLandmarkNotRecognized); + break; + case VerifyMTCResult::kUnacceptableCosignatureVerificationResult: + errors->AddError( + cert_errors::kMtcUnacceptableCosignatureVerificationResult); + break; + default: + errors->AddError(cert_errors::kVerifySignedDataFailed); + } } } else { // If `working_public_key_` is null, that indicates the SPKI of the issuer
diff --git a/pki/verify_unittest.cc b/pki/verify_unittest.cc index e4d7c84..ccaf4b0 100644 --- a/pki/verify_unittest.cc +++ b/pki/verify_unittest.cc
@@ -497,6 +497,8 @@ VerifyError error; ASSERT_TRUE(PrepareOptsForVerify(leaf_b_, trust_store_a.get(), &opts)); EXPECT_FALSE(CertificateVerify(opts, &error)) << error.DiagnosticString(); + EXPECT_EQ(error.Code(), + VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE); } std::unique_ptr<VerifyTrustStore> trust_store_b = EmptyTrustStore(); @@ -513,6 +515,8 @@ ASSERT_TRUE( PrepareOptsForVerify(generic_cert_, trust_store_b.get(), &opts)); EXPECT_FALSE(CertificateVerify(opts, &error)) << error.DiagnosticString(); + EXPECT_EQ(error.Code(), + VerifyError::StatusCode::CERTIFICATE_INVALID_SIGNATURE); } }
diff --git a/rust/bssl-tls/src/alerts.rs b/rust/bssl-tls/src/alerts.rs index d7d59d1..1ee12cb 100644 --- a/rust/bssl-tls/src/alerts.rs +++ b/rust/bssl-tls/src/alerts.rs
@@ -27,8 +27,11 @@ /// TLS alert description /// /// These alert variants have [IANA entries]. + /// The precise meaning of each alert is as of this release documented in [RFC 9846] §6.2. + /// It is reproduced here for your convenience. /// /// [IANA entries]: <https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-6> + /// [RFC 9846]: <https://datatracker.ietf.org/doc/html/rfc9846#section-6.2> pub enum AlertDescription : u8 { /// `close_notify` ///
diff --git a/rust/bssl-tls/src/connection/io/stdio.rs b/rust/bssl-tls/src/connection/io/stdio.rs index e048bcc..43448ae 100644 --- a/rust/bssl-tls/src/connection/io/stdio.rs +++ b/rust/bssl-tls/src/connection/io/stdio.rs
@@ -18,19 +18,12 @@ use crate::{ ReceiveBuffer, connection::TlsConnection, - context::{ - DtlsMode, // - TlsMode, - }, + context::TlsMode, errors::{ IoError, TlsRetryReason, // }, - io::{ - AbstractSocketResult, - IoStatus, - stdio::DatagramSocket, // - }, // + io::IoStatus, // }; fn translate_res_for_stdio(res: Result<IoStatus, Error>) -> Result<usize, io::Error> { @@ -79,29 +72,3 @@ translate_res_for_stdio(self.flush()).map(|_| ()) } } - -fn translate_result_for_datagram(res: Result<IoStatus, Error>) -> AbstractSocketResult { - match res { - Ok(IoStatus::Ok(bytes)) => AbstractSocketResult::Ok(bytes), - Ok(IoStatus::EndOfStream) | Err(Error::Io(IoError::EndOfStream)) => { - AbstractSocketResult::EndOfStream - } - Ok(IoStatus::Retry(_)) => AbstractSocketResult::Retry, - Ok(IoStatus::Empty | IoStatus::Err) => AbstractSocketResult::Err(Box::new(io::Error::new( - io::ErrorKind::Other, - "transport failed or empty", - ))), - Err(e) => AbstractSocketResult::Err(Box::new(io::Error::new(io::ErrorKind::Other, e))), - } -} - -impl<R> DatagramSocket for TlsConnection<R, DtlsMode> { - fn send(&mut self, datagram: &[u8]) -> AbstractSocketResult { - translate_result_for_datagram(self.sync_send(datagram)) - } - - fn recv(&mut self, datagram: &mut [u8]) -> AbstractSocketResult { - let mut datagram = ReceiveBuffer::new(datagram); - translate_result_for_datagram(self.sync_recv(&mut datagram)) - } -}
diff --git a/rust/bssl-tls/src/io.rs b/rust/bssl-tls/src/io.rs index ca5a9ea..176c78f 100644 --- a/rust/bssl-tls/src/io.rs +++ b/rust/bssl-tls/src/io.rs
@@ -348,6 +348,10 @@ buffer: *mut c_char, buf_len: c_int, ) -> c_int { + unsafe { + // Safety: `bio` is valid as witnessed by the C callback contract. + bssl_sys::BIO_clear_retry_flags(bio); + } let rust_bio = unsafe { // Safety: `bio` is still valid and so is the `RustBio` which we have exclusive access to. rust_bio_data_mut(bio) @@ -408,6 +412,10 @@ buffer: *const c_char, buf_len: c_int, ) -> c_int { + unsafe { + // Safety: `bio` is valid as witnessed by the C callback contract. + bssl_sys::BIO_clear_retry_flags(bio); + } let rust_bio = unsafe { // Safety: `bio` is still valid and so is the `RustBio` which we have exclusive access to. rust_bio_data_mut(bio) @@ -463,6 +471,10 @@ } unsafe fn rust_bio_flush(bio: *mut bssl_sys::BIO) -> c_long { + unsafe { + // Safety: `bio` is valid as witnessed by the caller contract. + bssl_sys::BIO_clear_retry_flags(bio); + } let rust_bio = unsafe { // Safety: `bio` is still valid rust_bio_data_mut(bio)
diff --git a/rust/bssl-tls/src/io/stdio.rs b/rust/bssl-tls/src/io/stdio.rs index 8da2071..535d5b1 100644 --- a/rust/bssl-tls/src/io/stdio.rs +++ b/rust/bssl-tls/src/io/stdio.rs
@@ -21,7 +21,7 @@ use super::AbstractSocketResult; -/// A datagram socket protocol +/// A datagram socket protocol as backing transport of TLS connection. pub trait DatagramSocket: Send { /// Send a complete datagram through the socket. ///
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index 62d5a0b..3d57f34 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -1283,11 +1283,13 @@ // Do not send client certificates on ECH reject. We have not authenticated // the server for the name that can learn the certificate. SSL_certs_clear(ssl); - } else if (hs->config->cert->cert_cb != nullptr) { + } else if (hs->config->cert->cert_cb) { + uint8_t alert = SSL_AD_INTERNAL_ERROR; // Call cert_cb to update the certificate. - int rv = hs->config->cert->cert_cb(ssl, hs->config->cert->cert_cb_arg); + int rv = + hs->config->cert->cert_cb(ssl, hs->config->cert->cert_cb_arg, &alert); if (rv == 0) { - ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + ssl_send_alert(ssl, SSL3_AL_FATAL, alert); OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_CB_ERROR); return ssl_hs_error; }
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index d47af74..1fdefbb 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -78,6 +78,7 @@ return false; } } else { + // clang-format off // Convert the ClientHello version to an equivalent supported_versions // extension. static const uint8_t kTLSVersions[] = { @@ -90,6 +91,7 @@ 0xfe, 0xfd, // DTLS 1.2 0xfe, 0xff, // DTLS 1.0 }; + // clang-format on size_t versions_len = 0; if (SSL_is_dtls(ssl)) { @@ -595,11 +597,13 @@ SSLImpl *const ssl = hs->ssl; // Call `cert_cb` to update server certificates if required. - if (hs->config->cert->cert_cb != nullptr) { - int rv = hs->config->cert->cert_cb(ssl, hs->config->cert->cert_cb_arg); + if (hs->config->cert->cert_cb) { + uint8_t alert = SSL_AD_INTERNAL_ERROR; + int rv = + hs->config->cert->cert_cb(ssl, hs->config->cert->cert_cb_arg, &alert); if (rv == 0) { OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_CB_ERROR); - ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + ssl_send_alert(ssl, SSL3_AL_FATAL, alert); return ssl_hs_error; } if (rv < 0) {
diff --git a/ssl/internal.h b/ssl/internal.h index 60ee51b..39c132d 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -26,11 +26,8 @@ #include <bitset> #include <cstdint> #include <initializer_list> -#include <limits> -#include <new> #include <optional> #include <string_view> -#include <type_traits> #include <utility> #include <variant> @@ -2589,6 +2586,29 @@ // From RFC 4492, used in encoding the curve type in ECParameters #define NAMED_CURVE_TYPE 3 +struct CertCb { + using OldCallback = int (*)(SSL *ssl, void *arg); + using NewCallback = int (*)(SSL *ssl, void *arg, uint8_t *out_alert); + + std::variant<std::monostate, OldCallback, NewCallback> cb; + + explicit operator bool() const { + return !std::holds_alternative<std::monostate>(cb); + } + + int operator()(SSL *ssl, void *arg, uint8_t *out_alert) const { + switch (cb.index()) { + default: + case 0: + return 0; + case 1: + return std::get<1>(cb)(ssl, arg); + case 2: + return std::get<2>(cb)(ssl, arg, out_alert); + } + } +}; + struct CERT { static constexpr bool kAllowUniquePtr = true; @@ -2636,7 +2656,7 @@ // certificates required. This allows advanced applications // to select certificates on the fly: for example based on // supported signature algorithms or curves. - int (*cert_cb)(SSL *ssl, void *arg) = nullptr; + CertCb cert_cb = {}; void *cert_cb_arg = nullptr; // Optional X509_STORE for certificate validation. If NULL the parent SSL_CTX
diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc index e5c1b0a..dcb6226 100644 --- a/ssl/ssl_cert.cc +++ b/ssl/ssl_cert.cc
@@ -77,7 +77,13 @@ static void ssl_cert_set_cert_cb(CERT *cert, int (*cb)(SSL *ssl, void *arg), void *arg) { - cert->cert_cb = cb; + cert->cert_cb.cb = cb; + cert->cert_cb_arg = arg; +} + +static void ssl_cert_set_cert_cb( + CERT *cert, int (*cb)(SSL *ssl, void *arg, uint8_t *out_alert), void *arg) { + cert->cert_cb.cb = cb; cert->cert_cb_arg = arg; } @@ -606,6 +612,12 @@ ssl_cert_set_cert_cb(FromOpaque(ctx)->cert.get(), cb, arg); } +void SSL_CTX_set_cert_cb_ex(SSL_CTX *ctx, + int (*cb)(SSL *ssl, void *arg, uint8_t *out_alert), + void *arg) { + ssl_cert_set_cert_cb(FromOpaque(ctx)->cert.get(), cb, arg); +} + void SSL_set_cert_cb(SSL *ssl, int (*cb)(SSL *ssl, void *arg), void *arg) { auto *ssl_impl = FromOpaque(ssl); if (!ssl_impl->config) { @@ -614,6 +626,16 @@ ssl_cert_set_cert_cb(ssl_impl->config->cert.get(), cb, arg); } +void SSL_set_cert_cb_ex(SSL *ssl, + int (*cb)(SSL *ssl, void *arg, uint8_t *out_alert), + void *arg) { + auto *ssl_impl = FromOpaque(ssl); + if (!ssl_impl->config) { + return; + } + ssl_cert_set_cert_cb(ssl_impl->config->cert.get(), cb, arg); +} + const STACK_OF(CRYPTO_BUFFER) *SSL_get0_peer_certificates(const SSL *ssl) { SSL_SESSION *session = SSL_get_session(ssl); if (session == nullptr) {
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 216ab42..5137d3a 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -6197,6 +6197,207 @@ EXPECT_TRUE(cert_cb_called); } +TEST(SSLTest, CertCallbackExServerAlert) { + for (uint16_t version : {TLS1_2_VERSION, TLS1_3_VERSION}) { + SCOPED_TRACE(version); + + // Test with a custom alert. + { + bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(client_ctx && server_ctx); + ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), version)); + ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), version)); + + SSL_CTX_set_cert_cb_ex( + server_ctx.get(), + [](SSL *ssl, void *arg, uint8_t *out_alert) -> int { + *out_alert = SSL_AD_UNRECOGNIZED_NAME; + return 0; + }, + nullptr); + + bssl::UniquePtr<SSL> client, server; + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + + int client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_WANT_READ); + + int server_ret = SSL_do_handshake(server.get()); + EXPECT_EQ(server_ret, -1); + EXPECT_EQ(SSL_get_error(server.get(), server_ret), SSL_ERROR_SSL); + EXPECT_TRUE(ErrorsAreAndClear({{ERR_LIB_SSL, SSL_R_CERT_CB_ERROR}})); + + client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(client_ret, -1); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_SSL); + EXPECT_TRUE(ErrorsAreAndClear( + {{ERR_LIB_SSL, SSL_R_TLSV1_ALERT_UNRECOGNIZED_NAME}})); + } + + // Test that the default alert (internal error) is used if untouched. + { + bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(client_ctx && server_ctx); + ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), version)); + ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), version)); + + SSL_CTX_set_cert_cb_ex( + server_ctx.get(), + [](SSL *ssl, void *arg, uint8_t *out_alert) -> int { return 0; }, + nullptr); + + bssl::UniquePtr<SSL> client, server; + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + + int client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_WANT_READ); + + int server_ret = SSL_do_handshake(server.get()); + EXPECT_EQ(server_ret, -1); + EXPECT_EQ(SSL_get_error(server.get(), server_ret), SSL_ERROR_SSL); + EXPECT_TRUE(ErrorsAreAndClear({{ERR_LIB_SSL, SSL_R_CERT_CB_ERROR}})); + + client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(client_ret, -1); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_SSL); + EXPECT_TRUE( + ErrorsAreAndClear({{ERR_LIB_SSL, SSL_R_TLSV1_ALERT_INTERNAL_ERROR}})); + } + } +} + +TEST(SSLTest, CertCallbackExClientAlert) { + for (uint16_t version : {TLS1_2_VERSION, TLS1_3_VERSION}) { + SCOPED_TRACE(version); + + bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr<SSL_CTX> server_ctx( + CreateContextWithTestCertificate(TLS_method())); + ASSERT_TRUE(client_ctx && server_ctx); + ASSERT_TRUE(SSL_CTX_set_max_proto_version(client_ctx.get(), version)); + ASSERT_TRUE(SSL_CTX_set_max_proto_version(server_ctx.get(), version)); + + SSL_CTX_set_custom_verify(server_ctx.get(), + SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, + AcceptAnyCertificate); + SSL_CTX_set_custom_verify(client_ctx.get(), SSL_VERIFY_PEER, + AcceptAnyCertificate); + + SSL_CTX_set_cert_cb_ex( + client_ctx.get(), + [](SSL *ssl, void *arg, uint8_t *out_alert) -> int { + *out_alert = SSL_AD_ACCESS_DENIED; + return 0; + }, + nullptr); + + bssl::UniquePtr<SSL> client, server; + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + + // Step handshakes until the client cert callback fails. + int client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_WANT_READ); + + int server_ret = SSL_do_handshake(server.get()); + EXPECT_EQ(SSL_get_error(server.get(), server_ret), SSL_ERROR_WANT_READ); + + client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(client_ret, -1); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_SSL); + EXPECT_TRUE(ErrorsAreAndClear({{ERR_LIB_SSL, SSL_R_CERT_CB_ERROR}})); + + server_ret = SSL_do_handshake(server.get()); + EXPECT_EQ(server_ret, -1); + EXPECT_EQ(SSL_get_error(server.get(), server_ret), SSL_ERROR_SSL); + EXPECT_TRUE( + ErrorsAreAndClear({{ERR_LIB_SSL, SSL_R_TLSV1_ALERT_ACCESS_DENIED}})); + } +} + +TEST(SSLTest, CertCallbackExPauseAndResume) { + bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(client_ctx && server_ctx); + + SSL_CTX_set_custom_verify(client_ctx.get(), SSL_VERIFY_PEER, + AcceptAnyCertificate); + + bool cert_ready = false; + SSL_CTX_set_cert_cb_ex( + server_ctx.get(), + [](SSL *ssl, void *arg, uint8_t *out_alert) -> int { + bool *ready = reinterpret_cast<bool *>(arg); + if (!*ready) { + return -1; + } + bssl::UniquePtr<X509> cert = GetTestCertificate(); + bssl::UniquePtr<EVP_PKEY> key = GetTestKey(); + if (!SSL_use_certificate(ssl, cert.get()) || + !SSL_use_PrivateKey(ssl, key.get())) { + return 0; + } + return 1; + }, + &cert_ready); + + bssl::UniquePtr<SSL> client, server; + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + + int client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_WANT_READ); + + int server_ret = SSL_do_handshake(server.get()); + EXPECT_EQ(server_ret, -1); + EXPECT_EQ(SSL_get_error(server.get(), server_ret), + SSL_ERROR_WANT_X509_LOOKUP); + + cert_ready = true; + ASSERT_TRUE(CompleteHandshakes(client.get(), server.get())); +} + +TEST(SSLTest, CertCallbackExSSLOverride) { + bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method())); + bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(client_ctx && server_ctx); + + // Set CTX-level callback to fail with default alert. + SSL_CTX_set_cert_cb( + server_ctx.get(), [](SSL *ssl, void *arg) -> int { return 0; }, nullptr); + + bssl::UniquePtr<SSL> client, server; + ASSERT_TRUE(CreateClientAndServer(&client, &server, client_ctx.get(), + server_ctx.get())); + + // Simulate rejection of peer certificate *somehow*. + SSL_set_cert_cb_ex( + server.get(), + [](SSL *ssl, void *arg, uint8_t *out_alert) -> int { + *out_alert = SSL_AD_HANDSHAKE_FAILURE; + return 0; + }, + nullptr); + + int client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_WANT_READ); + + int server_ret = SSL_do_handshake(server.get()); + EXPECT_EQ(server_ret, -1); + EXPECT_EQ(SSL_get_error(server.get(), server_ret), SSL_ERROR_SSL); + EXPECT_TRUE(ErrorsAreAndClear({{ERR_LIB_SSL, SSL_R_CERT_CB_ERROR}})); + + client_ret = SSL_do_handshake(client.get()); + EXPECT_EQ(client_ret, -1); + EXPECT_EQ(SSL_get_error(client.get(), client_ret), SSL_ERROR_SSL); + EXPECT_TRUE( + ErrorsAreAndClear({{ERR_LIB_SSL, SSL_R_SSLV3_ALERT_HANDSHAKE_FAILURE}})); +} + // Configuring the empty cipher list, though an error, should still modify the // configuration. TEST(SSLTest, EmptyCipherList) {
diff --git a/ssl/test/runner/basic_tests.go b/ssl/test/runner/basic_tests.go index 46f44bc..884d623 100644 --- a/ssl/test/runner/basic_tests.go +++ b/ssl/test/runner/basic_tests.go
@@ -761,6 +761,21 @@ expectedLocalError: "remote error: internal error", }, { + name: "FailCertCallback-Client-TLS12-CustomAlert", + config: Config{ + MaxVersion: VersionTLS12, + ClientAuth: RequestClientCert, + }, + flags: []string{ + "-fail-cert-callback", + "-fail-cert-callback-alert", + strconv.Itoa(int(alertAccessDenied)), + }, + shouldFail: true, + expectedError: ":CERT_CB_ERROR:", + expectedLocalError: "remote error: access denied", + }, + { testType: serverTest, name: "FailCertCallback-Server-TLS12", config: Config{ @@ -772,6 +787,21 @@ expectedLocalError: "remote error: internal error", }, { + testType: serverTest, + name: "FailCertCallback-Server-TLS12-CustomAlert", + config: Config{ + MaxVersion: VersionTLS12, + }, + flags: []string{ + "-fail-cert-callback", + "-fail-cert-callback-alert", + strconv.Itoa(int(alertUnrecognizedName)), + }, + shouldFail: true, + expectedError: ":CERT_CB_ERROR:", + expectedLocalError: "remote error: unrecognized name", + }, + { name: "FailCertCallback-Client-TLS13", config: Config{ MaxVersion: VersionTLS13, @@ -783,6 +813,21 @@ expectedLocalError: "remote error: internal error", }, { + name: "FailCertCallback-Client-TLS13-CustomAlert", + config: Config{ + MaxVersion: VersionTLS13, + ClientAuth: RequestClientCert, + }, + flags: []string{ + "-fail-cert-callback", + "-fail-cert-callback-alert", + strconv.Itoa(int(alertAccessDenied)), + }, + shouldFail: true, + expectedError: ":CERT_CB_ERROR:", + expectedLocalError: "remote error: access denied", + }, + { testType: serverTest, name: "FailCertCallback-Server-TLS13", config: Config{ @@ -794,6 +839,21 @@ expectedLocalError: "remote error: internal error", }, { + testType: serverTest, + name: "FailCertCallback-Server-TLS13-CustomAlert", + config: Config{ + MaxVersion: VersionTLS13, + }, + flags: []string{ + "-fail-cert-callback", + "-fail-cert-callback-alert", + strconv.Itoa(int(alertUnrecognizedName)), + }, + shouldFail: true, + expectedError: ":CERT_CB_ERROR:", + expectedLocalError: "remote error: unrecognized name", + }, + { protocol: dtls, name: "FragmentMessageTypeMismatch-DTLS", config: Config{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 4f10104..591856d 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -443,6 +443,8 @@ BoolFlag("-install-ddos-callback", &TestConfig::install_ddos_callback), BoolFlag("-fail-ddos-callback", &TestConfig::fail_ddos_callback), BoolFlag("-fail-cert-callback", &TestConfig::fail_cert_callback), + IntFlag("-fail-cert-callback-alert", + &TestConfig::fail_cert_callback_alert), StringFlag("-cipher", &TestConfig::cipher), BoolFlag("-handshake-never-done", &TestConfig::handshake_never_done), IntFlag("-export-keying-material", &TestConfig::export_keying_material), @@ -2361,7 +2363,7 @@ return ssl_verify_ok; } -static int CertCallback(SSL *ssl, void *arg) { +static int CertCallback(SSL *ssl, void *arg, uint8_t *out_alert) { const TestConfig *config = GetTestConfig(ssl); // Check the peer certificate metadata is as expected. @@ -2371,6 +2373,9 @@ } if (config->fail_cert_callback) { + if (config->fail_cert_callback_alert != 0) { + *out_alert = static_cast<uint8_t>(config->fail_cert_callback_alert); + } return 0; } @@ -2414,7 +2419,7 @@ return nullptr; } if (!use_old_client_cert_callback) { - SSL_set_cert_cb(ssl.get(), CertCallback, nullptr); + SSL_set_cert_cb_ex(ssl.get(), CertCallback, nullptr); } int mode = SSL_VERIFY_NONE; if (require_any_client_certificate) {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 491dfca..632c1d6 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -153,6 +153,7 @@ bool install_ddos_callback = false; bool fail_ddos_callback = false; bool fail_cert_callback = false; + int fail_cert_callback_alert = 0; std::string cipher; bool handshake_never_done = false; int export_keying_material = 0;
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index f0caf83..6ad8604 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc
@@ -993,11 +993,13 @@ // Do not send client certificates on ECH reject. We have not authenticated // the server for the name that can learn the certificate. SSL_certs_clear(ssl); - } else if (hs->config->cert->cert_cb != nullptr) { + } else if (hs->config->cert->cert_cb) { + uint8_t alert = SSL_AD_INTERNAL_ERROR; // Call cert_cb to update the certificate. - int rv = hs->config->cert->cert_cb(ssl, hs->config->cert->cert_cb_arg); + int rv = + hs->config->cert->cert_cb(ssl, hs->config->cert->cert_cb_arg, &alert); if (rv == 0) { - ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + ssl_send_alert(ssl, SSL3_AL_FATAL, alert); OPENSSL_PUT_ERROR(SSL, SSL_R_CERT_CB_ERROR); return ssl_hs_error; }