Add |SSL[_CTX]_set_chain_and_key|.
This allows a caller to configure a serving chain without dealing with
crypto/x509.
Change-Id: Ib42bb2ab9227d32071cf13ab07f92d029643a9a6
Reviewed-on: https://boringssl-review.googlesource.com/14126
Commit-Queue: Adam Langley <alangley@gmail.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 5b9f3de..5013b20 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -248,18 +248,33 @@
c->cert_cb_arg = arg;
}
-int ssl_set_cert(CERT *cert, CRYPTO_BUFFER *buffer) {
+enum leaf_cert_and_privkey_result_t {
+ leaf_cert_and_privkey_error,
+ leaf_cert_and_privkey_ok,
+ leaf_cert_and_privkey_mismatch,
+};
+
+/* check_leaf_cert_and_privkey checks whether the certificate in |leaf_buffer|
+ * and the private key in |privkey| are suitable and coherent. It returns
+ * |leaf_cert_and_privkey_error| and pushes to the error queue if a problem is
+ * found. If the certificate and private key are valid, but incoherent, it
+ * returns |leaf_cert_and_privkey_mismatch|. Otherwise it returns
+ * |leaf_cert_and_privkey_ok|. */
+static enum leaf_cert_and_privkey_result_t check_leaf_cert_and_privkey(
+ CRYPTO_BUFFER *leaf_buffer, EVP_PKEY *privkey) {
+ enum leaf_cert_and_privkey_result_t ret = leaf_cert_and_privkey_error;
+
CBS cert_cbs;
- CRYPTO_BUFFER_init_CBS(buffer, &cert_cbs);
+ CRYPTO_BUFFER_init_CBS(leaf_buffer, &cert_cbs);
EVP_PKEY *pubkey = ssl_cert_parse_pubkey(&cert_cbs);
if (pubkey == NULL) {
- return 0;
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ goto out;
}
if (!ssl_is_key_type_supported(pubkey->type)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
- EVP_PKEY_free(pubkey);
- return 0;
+ goto out;
}
/* An ECC certificate may be usable for ECDH or ECDSA. We only support ECDSA
@@ -267,26 +282,102 @@
if (pubkey->type == EVP_PKEY_EC &&
!ssl_cert_check_digital_signature_key_usage(&cert_cbs)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CERTIFICATE_TYPE);
- EVP_PKEY_free(pubkey);
+ goto out;
+ }
+
+ if (privkey != NULL &&
+ /* Sanity-check that the private key and the certificate match. */
+ !ssl_compare_public_and_private_key(pubkey, privkey)) {
+ ERR_clear_error();
+ ret = leaf_cert_and_privkey_mismatch;
+ goto out;
+ }
+
+ ret = leaf_cert_and_privkey_ok;
+
+out:
+ EVP_PKEY_free(pubkey);
+ return ret;
+}
+
+static int cert_set_chain_and_key(
+ CERT *cert, CRYPTO_BUFFER *const *certs, size_t num_certs,
+ EVP_PKEY *privkey, const SSL_PRIVATE_KEY_METHOD *privkey_method) {
+ if (num_certs == 0 ||
+ (privkey == NULL && privkey_method == NULL)) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
- if (cert->privatekey != NULL) {
- /* Sanity-check that the private key and the certificate match, unless the
- * key is opaque (in case of, say, a smartcard). */
- if (!EVP_PKEY_is_opaque(cert->privatekey) &&
- !ssl_compare_public_and_private_key(pubkey, cert->privatekey)) {
- /* don't fail for a cert/key mismatch, just free current private key
- * (when switching to a different cert & key, first this function should
- * be used, then ssl_set_pkey */
- EVP_PKEY_free(cert->privatekey);
- cert->privatekey = NULL;
- /* clear error queue */
- ERR_clear_error();
- }
+ if (privkey != NULL && privkey_method != NULL) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_CANNOT_HAVE_BOTH_PRIVKEY_AND_METHOD);
+ return 0;
}
- EVP_PKEY_free(pubkey);
+ switch (check_leaf_cert_and_privkey(certs[0], privkey)) {
+ case leaf_cert_and_privkey_error:
+ return 0;
+ case leaf_cert_and_privkey_mismatch:
+ OPENSSL_PUT_ERROR(SSL, SSL_R_CERTIFICATE_AND_PRIVATE_KEY_MISMATCH);
+ return 0;
+ case leaf_cert_and_privkey_ok:
+ break;
+ }
+
+ STACK_OF(CRYPTO_BUFFER) *certs_sk = sk_CRYPTO_BUFFER_new_null();
+ if (certs_sk == NULL) {
+ return 0;
+ }
+
+ for (size_t i = 0; i < num_certs; i++) {
+ if (!sk_CRYPTO_BUFFER_push(certs_sk, certs[i])) {
+ sk_CRYPTO_BUFFER_pop_free(certs_sk, CRYPTO_BUFFER_free);
+ return 0;
+ }
+ CRYPTO_BUFFER_up_ref(certs[i]);
+ }
+
+ EVP_PKEY_free(cert->privatekey);
+ cert->privatekey = privkey;
+ if (privkey != NULL) {
+ EVP_PKEY_up_ref(privkey);
+ }
+ cert->key_method = privkey_method;
+
+ sk_CRYPTO_BUFFER_pop_free(cert->chain, CRYPTO_BUFFER_free);
+ cert->chain = certs_sk;
+
+ return 1;
+}
+
+int SSL_set_chain_and_key(SSL *ssl, CRYPTO_BUFFER *const *certs,
+ size_t num_certs, EVP_PKEY *privkey,
+ const SSL_PRIVATE_KEY_METHOD *privkey_method) {
+ return cert_set_chain_and_key(ssl->cert, certs, num_certs, privkey,
+ privkey_method);
+}
+
+int SSL_CTX_set_chain_and_key(SSL_CTX *ctx, CRYPTO_BUFFER *const *certs,
+ size_t num_certs, EVP_PKEY *privkey,
+ const SSL_PRIVATE_KEY_METHOD *privkey_method) {
+ return cert_set_chain_and_key(ctx->cert, certs, num_certs, privkey,
+ privkey_method);
+}
+
+int ssl_set_cert(CERT *cert, CRYPTO_BUFFER *buffer) {
+ switch (check_leaf_cert_and_privkey(buffer, cert->privatekey)) {
+ case leaf_cert_and_privkey_error:
+ return 0;
+ case leaf_cert_and_privkey_mismatch:
+ /* don't fail for a cert/key mismatch, just free current private key
+ * (when switching to a different cert & key, first this function should
+ * be used, then |ssl_set_pkey|. */
+ EVP_PKEY_free(cert->privatekey);
+ cert->privatekey = NULL;
+ break;
+ case leaf_cert_and_privkey_ok:
+ break;
+ }
cert->x509_method->cert_flush_cached_leaf(cert);
@@ -494,6 +585,12 @@
int ssl_compare_public_and_private_key(const EVP_PKEY *pubkey,
const EVP_PKEY *privkey) {
+ if (EVP_PKEY_is_opaque(privkey)) {
+ /* We cannot check an opaque private key and have to trust that it
+ * matches. */
+ return 1;
+ }
+
int ret = 0;
switch (EVP_PKEY_cmp(pubkey, privkey)) {
diff --git a/ssl/ssl_privkey.c b/ssl/ssl_privkey.c
index 70045da..e988827 100644
--- a/ssl/ssl_privkey.c
+++ b/ssl/ssl_privkey.c
@@ -80,9 +80,7 @@
if (cert->chain != NULL &&
sk_CRYPTO_BUFFER_value(cert->chain, 0) != NULL &&
- /* Sanity-check that the private key and the certificate match, unless
- * the key is opaque (in case of, say, a smartcard). */
- !EVP_PKEY_is_opaque(pkey) &&
+ /* Sanity-check that the private key and the certificate match. */
!ssl_cert_check_private_key(cert, pkey)) {
return 0;
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 4180463..0dc240a 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -1229,7 +1229,25 @@
PEM_read_bio_PrivateKey(bio.get(), nullptr, nullptr, nullptr));
}
-static bssl::UniquePtr<X509> GetChainTestCertificate() {
+static bssl::UniquePtr<CRYPTO_BUFFER> BufferFromPEM(const char *pem) {
+ bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(pem, strlen(pem)));
+ char *name, *header;
+ uint8_t *data;
+ long data_len;
+ if (!PEM_read_bio(bio.get(), &name, &header, &data,
+ &data_len)) {
+ return nullptr;
+ }
+ OPENSSL_free(name);
+ OPENSSL_free(header);
+
+ auto ret = bssl::UniquePtr<CRYPTO_BUFFER>(
+ CRYPTO_BUFFER_new(data, data_len, nullptr));
+ OPENSSL_free(data);
+ return ret;
+}
+
+static bssl::UniquePtr<CRYPTO_BUFFER> GetChainTestCertificateBuffer() {
static const char kCertPEM[] =
"-----BEGIN CERTIFICATE-----\n"
"MIIC0jCCAbqgAwIBAgICEAAwDQYJKoZIhvcNAQELBQAwDzENMAsGA1UEAwwEQiBD\n"
@@ -1249,12 +1267,24 @@
"MYgF91UDvVzvnYm6TfseM2+ewKirC00GOrZ7rEcFvtxnKSqYf4ckqfNdSU1Y+RRC\n"
"1ngWZ7Ih\n"
"-----END CERTIFICATE-----\n";
- bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(kCertPEM, strlen(kCertPEM)));
- return bssl::UniquePtr<X509>(
- PEM_read_bio_X509(bio.get(), nullptr, nullptr, nullptr));
+ return BufferFromPEM(kCertPEM);
}
-static bssl::UniquePtr<X509> GetChainTestIntermediate() {
+static bssl::UniquePtr<X509> X509FromBuffer(
+ bssl::UniquePtr<CRYPTO_BUFFER> buffer) {
+ if (!buffer) {
+ return nullptr;
+ }
+ const uint8_t *derp = CRYPTO_BUFFER_data(buffer.get());
+ return bssl::UniquePtr<X509>(
+ d2i_X509(NULL, &derp, CRYPTO_BUFFER_len(buffer.get())));
+}
+
+static bssl::UniquePtr<X509> GetChainTestCertificate() {
+ return X509FromBuffer(GetChainTestCertificateBuffer());
+}
+
+static bssl::UniquePtr<CRYPTO_BUFFER> GetChainTestIntermediateBuffer() {
static const char kCertPEM[] =
"-----BEGIN CERTIFICATE-----\n"
"MIICwjCCAaqgAwIBAgICEAEwDQYJKoZIhvcNAQELBQAwFDESMBAGA1UEAwwJQyBS\n"
@@ -1273,9 +1303,11 @@
"WhWwgM3P3X95fQ3d7oFPR/bVh0YV+Cf861INwplokXgXQ3/TCQ+HNXeAMWn3JLWv\n"
"XFwk8owk9dq/kQGdndGgy3KTEW4ctPX5GNhf3LJ9Q7dLji4ReQ4=\n"
"-----END CERTIFICATE-----\n";
- bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(kCertPEM, strlen(kCertPEM)));
- return bssl::UniquePtr<X509>(
- PEM_read_bio_X509(bio.get(), nullptr, nullptr, nullptr));
+ return BufferFromPEM(kCertPEM);
+}
+
+static bssl::UniquePtr<X509> GetChainTestIntermediate() {
+ return X509FromBuffer(GetChainTestIntermediateBuffer());
}
static bssl::UniquePtr<EVP_PKEY> GetChainTestKey() {
@@ -3152,6 +3184,51 @@
EXPECT_EQ(Bytes(der, der_len), Bytes(der3, der3_len));
}
+TEST(SSLTest, SetChainAndKeyMismatch) {
+ bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_with_buffers_method()));
+ ASSERT_TRUE(ctx);
+
+ bssl::UniquePtr<EVP_PKEY> key = GetTestKey();
+ ASSERT_TRUE(key);
+ bssl::UniquePtr<CRYPTO_BUFFER> leaf = GetChainTestCertificateBuffer();
+ ASSERT_TRUE(leaf);
+ std::vector<CRYPTO_BUFFER*> chain = {
+ leaf.get(),
+ };
+
+ // Should fail because |GetTestKey| doesn't match the chain-test certificate.
+ ASSERT_FALSE(SSL_CTX_set_chain_and_key(ctx.get(), &chain[0], chain.size(),
+ key.get(), nullptr));
+ ERR_clear_error();
+}
+
+TEST(SSLTest, SetChainAndKey) {
+ bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_with_buffers_method()));
+ ASSERT_TRUE(client_ctx);
+ bssl::UniquePtr<SSL_CTX> server_ctx(SSL_CTX_new(TLS_with_buffers_method()));
+ ASSERT_TRUE(server_ctx);
+
+ bssl::UniquePtr<EVP_PKEY> key = GetChainTestKey();
+ ASSERT_TRUE(key);
+ bssl::UniquePtr<CRYPTO_BUFFER> leaf = GetChainTestCertificateBuffer();
+ ASSERT_TRUE(leaf);
+ bssl::UniquePtr<CRYPTO_BUFFER> intermediate =
+ GetChainTestIntermediateBuffer();
+ ASSERT_TRUE(intermediate);
+ std::vector<CRYPTO_BUFFER*> chain = {
+ leaf.get(), intermediate.get(),
+ };
+ ASSERT_TRUE(SSL_CTX_set_chain_and_key(server_ctx.get(), &chain[0],
+ chain.size(), key.get(), nullptr));
+
+ SSL_CTX_i_promise_to_verify_certs_after_the_handshake(client_ctx.get());
+
+ bssl::UniquePtr<SSL> client, server;
+ ASSERT_TRUE(ConnectClientAndServer(&client, &server, client_ctx.get(),
+ server_ctx.get(),
+ nullptr /* no session */));
+}
+
// TODO(davidben): Convert this file to GTest properly.
TEST(SSLTest, AllTests) {
if (!TestCipherRules() ||