Add SSL_set0_CA_names to configure the CA extension Add tests to ssl_test.cc to check basic combinations of credential matching with the CA extension, and with credentials both marked to match against issuers, and a default credential that will match anything. Change-Id: Ic41a2dd0ecbbdc4c1be05e440d4673c087b52775 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71613 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 6503de5..7f733ac 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -1505,6 +1505,25 @@ OPENSSL_EXPORT int SSL_CREDENTIAL_set_private_key_method( SSL_CREDENTIAL *cred, const SSL_PRIVATE_KEY_METHOD *key_method); +// SSL_CREDENTIAL_set_must_match_issuer sets the flag that this credential +// should be considered only when it matches a peer request for a particular +// issuer via a negotiation mechanism (such as the certificate_authorities +// extension). +OPENSSL_EXPORT void SSL_CREDENTIAL_set_must_match_issuer(SSL_CREDENTIAL *cred); + +// SSL_CREDENTIAL_clear_must_match_issuer clears the flag requiring issuer +// matching, indicating this credential should be considered regardless of peer +// issuer matching requests. (This is the default). +OPENSSL_EXPORT void SSL_CREDENTIAL_clear_must_match_issuer( + SSL_CREDENTIAL *cred); + +// SSL_CREDENTIAL_must_match_issuer returns the value of the flag indicating +// that this credential should be considered only when it matches a peer request +// for a particular issuer via a negotiation mechanism (such as the +// certificate_authorities extension). +OPENSSL_EXPORT int SSL_CREDENTIAL_must_match_issuer( + const SSL_CREDENTIAL *cred); + // SSL_can_release_private_key returns one if |ssl| will no longer call into the // private key and zero otherwise. If the function returns one, the caller can // release state associated with the private key. @@ -2981,6 +3000,12 @@ OPENSSL_EXPORT void SSL_set0_client_CAs(SSL *ssl, STACK_OF(CRYPTO_BUFFER) *name_list); +// SSL_set0_CA_names sets |ssl|'s CA name list for the certificate authorities +// extension to |name_list|, which should contain DER-encoded distinguished names +// (RFC 5280). It takes ownership of |name_list|. +OPENSSL_EXPORT void SSL_set0_CA_names(SSL *ssl, + STACK_OF(CRYPTO_BUFFER) *name_list); + // SSL_CTX_set0_client_CAs sets |ctx|'s client certificate CA list to // |name_list|, which should contain DER-encoded distinguished names (RFC 5280). // It takes ownership of |name_list|.
diff --git a/ssl/ssl_cert.cc b/ssl/ssl_cert.cc index 541097c..8cfb04a 100644 --- a/ssl/ssl_cert.cc +++ b/ssl/ssl_cert.cc
@@ -736,3 +736,10 @@ ssl->ctx->x509_method->ssl_flush_cached_client_CA(ssl->config.get()); ssl->config->client_CA.reset(name_list); } + +void SSL_set0_CA_names(SSL *ssl, STACK_OF(CRYPTO_BUFFER) *name_list) { + if (!ssl->config) { + return; + } + ssl->config->CA_names.reset(name_list); +}
diff --git a/ssl/ssl_credential.cc b/ssl/ssl_credential.cc index 5f13ea5..d5df82e 100644 --- a/ssl/ssl_credential.cc +++ b/ssl/ssl_credential.cc
@@ -468,3 +468,15 @@ void *SSL_CREDENTIAL_get_ex_data(const SSL_CREDENTIAL *cred, int idx) { return CRYPTO_get_ex_data(&cred->ex_data, idx); } + +void SSL_CREDENTIAL_set_must_match_issuer(SSL_CREDENTIAL *cred) { + cred->must_match_issuer = true; +} + +void SSL_CREDENTIAL_clear_must_match_issuer(SSL_CREDENTIAL *cred) { + cred->must_match_issuer = false; +} + +int SSL_CREDENTIAL_must_match_issuer(const SSL_CREDENTIAL *cred) { + return cred->must_match_issuer ? 1 : 0; +}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index c84c507..f0b3872 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -571,10 +571,12 @@ ret->cert = MakeUnique<CERT>(method->x509_method); ret->sessions = lh_SSL_SESSION_new(ssl_session_hash, ssl_session_cmp); ret->client_CA.reset(sk_CRYPTO_BUFFER_new_null()); + ret->CA_names.reset(sk_CRYPTO_BUFFER_new_null()); if (ret->cert == nullptr || // !ret->cert->is_valid() || // ret->sessions == nullptr || // ret->client_CA == nullptr || // + ret->CA_names == nullptr || // !ret->x509_method->ssl_ctx_new(ret.get())) { return nullptr; }
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 0056779..963f2ee 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc
@@ -1709,6 +1709,25 @@ "-----END CERTIFICATE-----\n"; return CertFromPEM(kCertPEM); } +static bssl::UniquePtr<CRYPTO_BUFFER> GetTestCertificateBuffer() { + static const char kCertPEM[] = + "-----BEGIN CERTIFICATE-----\n" + "MIICWDCCAcGgAwIBAgIJAPuwTC6rEJsMMA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV\n" + "BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX\n" + "aWRnaXRzIFB0eSBMdGQwHhcNMTQwNDIzMjA1MDQwWhcNMTcwNDIyMjA1MDQwWjBF\n" + "MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50\n" + "ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKB\n" + "gQDYK8imMuRi/03z0K1Zi0WnvfFHvwlYeyK9Na6XJYaUoIDAtB92kWdGMdAQhLci\n" + "HnAjkXLI6W15OoV3gA/ElRZ1xUpxTMhjP6PyY5wqT5r6y8FxbiiFKKAnHmUcrgfV\n" + "W28tQ+0rkLGMryRtrukXOgXBv7gcrmU7G1jC2a7WqmeI8QIDAQABo1AwTjAdBgNV\n" + "HQ4EFgQUi3XVrMsIvg4fZbf6Vr5sp3Xaha8wHwYDVR0jBBgwFoAUi3XVrMsIvg4f\n" + "Zbf6Vr5sp3Xaha8wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQUFAAOBgQA76Hht\n" + "ldY9avcTGSwbwoiuIqv0jTL1fHFnzy3RHMLDh+Lpvolc5DSrSJHCP5WuK0eeJXhr\n" + "T5oQpHL9z/cCDLAKCKRa4uV0fhEdOWBqyR9p8y5jJtye72t6CuFUV5iqcpF4BH4f\n" + "j2VNHwsSrJwkD4QUGlUtH7vwnQmyCFxZMmWAJg==\n" + "-----END CERTIFICATE-----\n"; + return BufferFromPEM(kCertPEM); +} static bssl::UniquePtr<EVP_PKEY> GetTestKey() { static const char kKeyPEM[] = @@ -1832,7 +1851,16 @@ return BufferFromPEM(kSubjectPEM); } -static bssl::UniquePtr<CRYPTO_BUFFER> GetChainTestUnmatchingIssuerBuffer() { +static bssl::UniquePtr<CRYPTO_BUFFER> GetTestCertIssuerBuffer() { + static const char kSubjectPEM[] = + "-----BEGIN SUBJECT-----\n" + "MEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJ\n" + "bnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQ=\n" + "-----END SUBJECT-----\n"; + return BufferFromPEM(kSubjectPEM); +} + +static bssl::UniquePtr<CRYPTO_BUFFER> GetBogusIssuerBuffer() { static const char kSubjectPEM[] = "-----BEGIN SUBJECT-----\n" "MBYxFDASBgNVBAMMC0RpZ2lOb3RBRm94\n" @@ -1967,6 +1995,7 @@ struct ClientConfig { SSL_SESSION *session = nullptr; + STACK_OF(CRYPTO_BUFFER) *ca_names = nullptr; std::string servername; std::string verify_hostname; unsigned hostflags = 0; @@ -1999,6 +2028,10 @@ SSL_set_hostflags(client.get(), config.hostflags); } + if (config.ca_names) { + SSL_set0_CA_names(client.get(), config.ca_names); + } + SSL_set_shed_handshake_config(client.get(), shed_handshake_config); SSL_set_shed_handshake_config(server.get(), shed_handshake_config); @@ -5185,10 +5218,13 @@ bssl::UniquePtr<CRYPTO_BUFFER> ca_subject = GetChainTestIntermediateIssuerBuffer(); ASSERT_TRUE(ca_subject); - bssl::UniquePtr<CRYPTO_BUFFER> bogus_subject = - GetChainTestUnmatchingIssuerBuffer(); - ASSERT_TRUE(bogus_subject); + bssl::UniquePtr<CRYPTO_BUFFER> testcert = GetTestCertificateBuffer(); + ASSERT_TRUE(testcert); + bssl::UniquePtr<EVP_PKEY> testkey = GetTestKey(); + ASSERT_TRUE(testkey); + + std::vector<CRYPTO_BUFFER *> test_chain = {testcert.get()}; std::vector<CRYPTO_BUFFER *> chain = {leaf.get(), ca.get()}; std::vector<CRYPTO_BUFFER *> wrong_chain = {leaf.get(), leaf.get(), leaf.get()}; @@ -5197,6 +5233,8 @@ ASSERT_TRUE(ctx); bssl::UniquePtr<SSL_CREDENTIAL> cred(SSL_CREDENTIAL_new_x509()); ASSERT_TRUE(cred); + bssl::UniquePtr<SSL_CREDENTIAL> cred2(SSL_CREDENTIAL_new_x509()); + ASSERT_TRUE(cred2); // Configure one chain (including the leaf), then replace it with another. ASSERT_TRUE(SSL_CREDENTIAL_set1_cert_chain(cred.get(), wrong_chain.data(), @@ -5225,13 +5263,88 @@ CRYPTO_BUFFER_len(subject_buf.get())))); #endif + ASSERT_TRUE(SSL_CREDENTIAL_set1_cert_chain(cred2.get(), test_chain.data(), + test_chain.size())); + ASSERT_TRUE(SSL_CREDENTIAL_set1_private_key(cred.get(), key.get())); + ASSERT_TRUE(SSL_CREDENTIAL_set1_private_key(cred2.get(), testkey.get())); + SSL_CREDENTIAL_set_must_match_issuer(cred.get()); + SSL_CREDENTIAL_set_must_match_issuer(cred2.get()); ASSERT_TRUE(SSL_CTX_add1_credential(ctx.get(), cred.get())); + ASSERT_TRUE(SSL_CTX_add1_credential(ctx.get(), cred2.get())); bssl::UniquePtr<SSL> client, server; - ASSERT_TRUE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get())); - EXPECT_TRUE(BuffersEqual(SSL_get0_peer_certificates(client.get()), + + // With no CA requested by client, we should fail with only cred1 and cred2 + ASSERT_FALSE(ConnectClientAndServer(&client, &server, ctx.get(), ctx.get())); + + //EXPECT_TRUE(BuffersEqual(SSL_get0_peer_certificates(client.get()), + // {leaf.get(), ca.get()})); + + // Have the client request a bogus name that will not match + bssl::UniquePtr<CRYPTO_BUFFER> bogus_subject = GetBogusIssuerBuffer(); + ASSERT_TRUE(bogus_subject); + bssl::UniquePtr<SSL> client2, server2; + ClientConfig bogus_subject_config; + bssl::UniquePtr<STACK_OF(CRYPTO_BUFFER)> bogus_subjects( + sk_CRYPTO_BUFFER_new_null()); + ASSERT_TRUE(bogus_subjects); + ASSERT_TRUE(PushToStack(bogus_subjects.get(), std::move(bogus_subject))); + bogus_subject_config.ca_names = bogus_subjects.get(); + bogus_subjects.release(); + // A bogus issuer that does not match should fail + ASSERT_FALSE(ConnectClientAndServer(&client2, &server2, ctx.get(), ctx.get(), + bogus_subject_config)); + + // Have the client request the name of the chain ca. + bssl::UniquePtr<CRYPTO_BUFFER> chain_subject = + GetChainTestIntermediateIssuerBuffer(); + ASSERT_TRUE(chain_subject); + bssl::UniquePtr<SSL> client3, server3; + ClientConfig chain_subject_config; + bssl::UniquePtr<STACK_OF(CRYPTO_BUFFER)> chain_subjects( + sk_CRYPTO_BUFFER_new_null()); + ASSERT_TRUE(chain_subjects); + ASSERT_TRUE(PushToStack(chain_subjects.get(), std::move(chain_subject))); + chain_subject_config.ca_names = chain_subjects.get(); + chain_subjects.release(); + // If we ask for the chain ca subject, we should get it + ASSERT_TRUE(ConnectClientAndServer(&client3, &server3, ctx.get(), ctx.get(), + chain_subject_config)); + EXPECT_TRUE(BuffersEqual(SSL_get0_peer_certificates(client3.get()), {leaf.get(), ca.get()})); + + // Have the client request the name of the test ca. + bssl::UniquePtr<CRYPTO_BUFFER> test_subject = GetTestCertIssuerBuffer(); + ASSERT_TRUE(test_subject); + bssl::UniquePtr<SSL> client4, server4; + ClientConfig test_subject_config; + bssl::UniquePtr<STACK_OF(CRYPTO_BUFFER)> test_subjects( + sk_CRYPTO_BUFFER_new_null()); + ASSERT_TRUE(test_subjects); + ASSERT_TRUE(PushToStack(test_subjects.get(), std::move(test_subject))); + test_subject_config.ca_names = test_subjects.get(); + test_subjects.release(); + // If we ask for the test ca subject, we should get it + ASSERT_TRUE(ConnectClientAndServer(&client4, &server4, ctx.get(), ctx.get(), + test_subject_config)); + EXPECT_TRUE(BuffersEqual(SSL_get0_peer_certificates(client4.get()), + {testcert.get()})); + + // Add cred3 to the CTX so we have an ubiquitous credential + bssl::UniquePtr<SSL_CREDENTIAL> cred3(SSL_CREDENTIAL_new_x509()); + ASSERT_TRUE(cred3); + ASSERT_TRUE( + SSL_CREDENTIAL_set1_cert_chain(cred3.get(), chain.data(), chain.size())); + ASSERT_TRUE(SSL_CREDENTIAL_set1_private_key(cred3.get(), key.get())); + ASSERT_TRUE(SSL_CTX_add1_credential(ctx.get(), cred3.get())); + + // With no CA sent, we should now succeed. + bssl::UniquePtr<SSL> client5, server5; + ASSERT_TRUE(ConnectClientAndServer(&client5, &server5, ctx.get(), ctx.get())); + EXPECT_TRUE(BuffersEqual(SSL_get0_peer_certificates(client5.get()), + {leaf.get(), ca.get()})); + } TEST(SSLTest, SetChainAndKeyCtx) {