ssl: add SSLContext#add_certificate

Add a new method to add a certificate, a corresponding private key, and
extra CA certificates at once.

This has two advantages over the existing {cert,key,extra_cert_chain}
attributes:

  1. We can notice the problem with the certificate and/or the private
     key. Since the existing attributes are simple instance variables,
     they aren't set to the SSL_CTX until #setup which usually happens
     on the first connection.

  2. For the same reason, existing attributes allowed only one
     certificate for a context, even though OpenSSL itself is capable of
     handling multiple certificates and selecting the most appropriate
     one according to the cipher suite selected.

The documentation for the existing attributes are updated to recommend
using #add_certificate.

2 files changed, 199 insertions, 0 deletions

diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index 93fc497e..c6aa1e71 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1194,6 +1194,114 @@ ossl_sslctx_set_security_level(VALUE self, VALUE value)
 }
 
 /*
+ * call-seq:
+ *    ctx.add_certificate(certiticate, pkey [, extra_certs]) -> self
+ *
+ * Adds a certificate to the context. _pkey_ must be a corresponding private
+ * key with _certificate_.
+ *
+ * Multiple certificates with different public key type can be added by
+ * repeated calls of this method, and OpenSSL will choose the most appropriate
+ * certificate during the handshake.
+ *
+ * #cert=, #key=, and #extra_chain_cert= are old accessor methods for setting
+ * certificate and internally call this method.
+ *
+ * === Parameters
+ * _certificate_::
+ *   A certificate. An instance of OpenSSL::X509::Certificate.
+ * _pkey_::
+ *   The private key for _certificate_. An instance of OpenSSL::PKey::PKey.
+ * _extra_certs_::
+ *   Optional. An array of OpenSSL::X509::Certificate. When sending a
+ *   certificate chain, the certificates specified by this are sent following
+ *   _certificate_, in the order in the array.
+ *
+ * === Example
+ *   rsa_cert = OpenSSL::X509::Certificate.new(...)
+ *   rsa_pkey = OpenSSL::PKey.read(...)
+ *   ca_intermediate_cert = OpenSSL::X509::Certificate.new(...)
+ *   ctx.add_certificate(rsa_cert, rsa_pkey, [ca_intermediate_cert])
+ *
+ *   ecdsa_cert = ...
+ *   ecdsa_pkey = ...
+ *   another_ca_cert = ...
+ *   ctx.add_certificate(ecdsa_cert, ecdsa_pkey, [another_ca_cert])
+ *
+ * === Note
+ * OpenSSL before the version 1.0.2 could handle only one extra chain across
+ * all key types. Calling this method discards the chain set previously.
+ */
+static VALUE
+ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self)
+{
+    VALUE cert, key, extra_chain_ary;
+    SSL_CTX *ctx;
+    X509 *x509;
+    STACK_OF(X509) *extra_chain = NULL;
+    EVP_PKEY *pkey, *pub_pkey;
+
+    GetSSLCTX(self, ctx);
+    rb_scan_args(argc, argv, "21", &cert, &key, &extra_chain_ary);
+    rb_check_frozen(self);
+    x509 = GetX509CertPtr(cert);
+    pkey = GetPrivPKeyPtr(key);
+
+    /*
+     * The reference counter is bumped, and decremented immediately.
+     * X509_get0_pubkey() is only available in OpenSSL >= 1.1.0.
+     */
+    pub_pkey = X509_get_pubkey(x509);
+    EVP_PKEY_free(pub_pkey);
+    if (!pub_pkey)
+	rb_raise(rb_eArgError, "certificate does not contain public key");
+    if (EVP_PKEY_cmp(pub_pkey, pkey) != 1)
+	rb_raise(rb_eArgError, "public key mismatch");
+
+    if (argc >= 3)
+	extra_chain = ossl_x509_ary2sk(extra_chain_ary);
+
+    if (!SSL_CTX_use_certificate(ctx, x509)) {
+	sk_X509_pop_free(extra_chain, X509_free);
+	ossl_raise(eSSLError, "SSL_CTX_use_certificate");
+    }
+    if (!SSL_CTX_use_PrivateKey(ctx, pkey)) {
+	sk_X509_pop_free(extra_chain, X509_free);
+	ossl_raise(eSSLError, "SSL_CTX_use_PrivateKey");
+    }
+
+    if (extra_chain) {
+#if OPENSSL_VERSION_NUMBER >= 0x10002000 && !defined(LIBRESSL_VERSION_NUMBER)
+	if (!SSL_CTX_set0_chain(ctx, extra_chain)) {
+	    sk_X509_pop_free(extra_chain, X509_free);
+	    ossl_raise(eSSLError, "SSL_CTX_set0_chain");
+	}
+#else
+	STACK_OF(X509) *orig_extra_chain;
+	X509 *x509_tmp;
+
+	/* First, clear the existing chain */
+	SSL_CTX_get_extra_chain_certs(ctx, &orig_extra_chain);
+	if (orig_extra_chain && sk_X509_num(orig_extra_chain)) {
+	    rb_warning("SSL_CTX_set0_chain() is not available; " \
+		       "clearing previously set certificate chain");
+	    SSL_CTX_clear_extra_chain_certs(ctx);
+	}
+	while ((x509_tmp = sk_X509_shift(extra_chain))) {
+	    /* Transfers ownership */
+	    if (!SSL_CTX_add_extra_chain_cert(ctx, x509_tmp)) {
+		X509_free(x509_tmp);
+		sk_X509_pop_free(extra_chain, X509_free);
+		ossl_raise(eSSLError, "SSL_CTX_add_extra_chain_cert");
+	    }
+	}
+	sk_X509_free(extra_chain);
+#endif
+    }
+    return self;
+}
+
+/*
  *  call-seq:
  *     ctx.session_add(session) -> true | false
  *
@@ -2324,11 +2432,17 @@ Init_ossl_ssl(void)
 
     /*
      * Context certificate
+     *
+     * The _cert_, _key_, and _extra_chain_cert_ attributes are deprecated.
+     * It is recommended to use #add_certificate instead.
      */
     rb_attr(cSSLContext, rb_intern("cert"), 1, 1, Qfalse);
 
     /*
      * Context private key
+     *
+     * The _cert_, _key_, and _extra_chain_cert_ attributes are deprecated.
+     * It is recommended to use #add_certificate instead.
      */
     rb_attr(cSSLContext, rb_intern("key"), 1, 1, Qfalse);
 
@@ -2402,6 +2516,9 @@ Init_ossl_ssl(void)
     /*
      * An Array of extra X509 certificates to be added to the certificate
      * chain.
+     *
+     * The _cert_, _key_, and _extra_chain_cert_ attributes are deprecated.
+     * It is recommended to use #add_certificate instead.
      */
     rb_attr(cSSLContext, rb_intern("extra_chain_cert"), 1, 1, Qfalse);
 
@@ -2557,6 +2674,7 @@ Init_ossl_ssl(void)
     rb_define_method(cSSLContext, "ecdh_curves=", ossl_sslctx_set_ecdh_curves, 1);
     rb_define_method(cSSLContext, "security_level", ossl_sslctx_get_security_level, 0);
     rb_define_method(cSSLContext, "security_level=", ossl_sslctx_set_security_level, 1);
+    rb_define_method(cSSLContext, "add_certificate", ossl_sslctx_add_certificate, -1);
 
     rb_define_method(cSSLContext, "setup", ossl_sslctx_setup, 0);
     rb_define_alias(cSSLContext, "freeze", "setup");
diff --git a/test/test_ssl.rb b/test/test_ssl.rb
index ab6382d7..1b665cc9 100644
--- a/test/test_ssl.rb
+++ b/test/test_ssl.rb
@@ -54,6 +54,87 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase
     }
   end
 
+  def test_add_certificate
+    ctx_proc = -> ctx {
+      # Unset values set by start_server
+      ctx.cert = ctx.key = ctx.extra_chain_cert = nil
+      ctx.add_certificate(@svr_cert, @svr_key, [@ca_cert]) # RSA
+    }
+    start_server(ctx_proc: ctx_proc) do |port|
+      server_connect(port) { |ssl|
+        assert_equal @svr_cert.subject, ssl.peer_cert.subject
+        assert_equal [@svr_cert.subject, @ca_cert.subject],
+          ssl.peer_cert_chain.map(&:subject)
+      }
+    end
+  end
+
+  def test_add_certificate_multiple_certs
+    pend "EC is not supported" unless defined?(OpenSSL::PKey::EC)
+    pend "TLS 1.2 is not supported" unless tls12_supported?
+
+    # SSL_CTX_set0_chain() is needed for setting multiple certificate chains
+    add0_chain_supported = openssl?(1, 0, 2)
+
+    if add0_chain_supported
+      ca2_key = Fixtures.pkey("rsa1024")
+      ca2_exts = [
+        ["basicConstraints", "CA:TRUE", true],
+        ["keyUsage", "cRLSign, keyCertSign", true],
+      ]
+      ca2_dn = OpenSSL::X509::Name.parse_rfc2253("CN=CA2")
+      ca2_cert = issue_cert(ca2_dn, ca2_key, 123, ca2_exts, nil, nil)
+    else
+      # Use the same CA as @svr_cert
+      ca2_key = @ca_key; ca2_cert = @ca_cert
+    end
+
+    ecdsa_key = Fixtures.pkey("p256")
+    exts = [
+      ["keyUsage", "digitalSignature", false],
+    ]
+    ecdsa_dn = OpenSSL::X509::Name.parse_rfc2253("CN=localhost2")
+    ecdsa_cert = issue_cert(ecdsa_dn, ecdsa_key, 456, exts, ca2_cert, ca2_key)
+
+    if !add0_chain_supported
+      # Testing the warning emitted when 'extra' chain is replaced
+      tctx = OpenSSL::SSL::SSLContext.new
+      tctx.add_certificate(@svr_cert, @svr_key, [@ca_cert])
+      assert_warning(/set0_chain/) {
+        tctx.add_certificate(ecdsa_cert, ecdsa_key, [ca2_cert])
+      }
+    end
+
+    ctx_proc = -> ctx {
+      # Unset values set by start_server
+      ctx.cert = ctx.key = ctx.extra_chain_cert = nil
+      ctx.ecdh_curves = "P-256" unless openssl?(1, 0, 2)
+      ctx.add_certificate(@svr_cert, @svr_key, [@ca_cert]) # RSA
+      EnvUtil.suppress_warning do # !add0_chain_supported
+        ctx.add_certificate(ecdsa_cert, ecdsa_key, [ca2_cert])
+      end
+    }
+    start_server(ctx_proc: ctx_proc) do |port|
+      ctx = OpenSSL::SSL::SSLContext.new
+      ctx.max_version = :TLS1_2 # TODO: We need this to force certificate type
+      ctx.ciphers = "aECDSA"
+      server_connect(port, ctx) { |ssl|
+        assert_equal ecdsa_cert.subject, ssl.peer_cert.subject
+        assert_equal [ecdsa_cert.subject, ca2_cert.subject],
+          ssl.peer_cert_chain.map(&:subject)
+      }
+
+      ctx = OpenSSL::SSL::SSLContext.new
+      ctx.max_version = :TLS1_2
+      ctx.ciphers = "aRSA"
+      server_connect(port, ctx) { |ssl|
+        assert_equal @svr_cert.subject, ssl.peer_cert.subject
+        assert_equal [@svr_cert.subject, @ca_cert.subject],
+          ssl.peer_cert_chain.map(&:subject)
+      }
+    end
+  end
+
   def test_sysread_and_syswrite
     start_server { |port|
       server_connect(port) { |ssl|