modify ossl_sslctx_add_certificate_chain_file() to raise Error and to return self

add test_add_certificate_chain_file_multiple_certs

2 files changed, 88 insertions, 11 deletions

diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index 4ee698cd..ee56edc0 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1324,7 +1324,7 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self)
 
 /*
  * call-seq:
- *    ctx.add_certificate_chain_file(certs_path, pkey_path) -> true | false
+ *    ctx.add_certificate_chain_file(certs_path, pkey_path) -> self
  *
  * Loads chain certificates from _certs_path_ and a private key from
  * _pkey_path_.
@@ -1336,11 +1336,19 @@ ossl_sslctx_add_certificate(int argc, VALUE *argv, VALUE self)
  * _pkey_path_::
  *   A path to a private key file. An instance of String.
  *
+ * === Example
+ *   ctx.add_certificate_chain(rsa_cert_path, rsa_key_path)
+ *
+ *   ctx.add_certificate_chain(ecdsa_cert_path, ecdsa_key_path)
+ *
  * === Note
  * The file format of the certificate and private key must be PEM.
  *
  * The certificate file must be starting with the subject's certificate and
- * followed by intermediate CA certificates (and root CA certificate).
+ * followed by intermediate CA certificate(s).
+ *
+ * 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_chain_file(VALUE self, VALUE certs_path, VALUE pkey_path)
@@ -1348,20 +1356,15 @@ ossl_sslctx_add_certificate_chain_file(VALUE self, VALUE certs_path, VALUE pkey_
     SSL_CTX *ctx;
 
     GetSSLCTX(self, ctx);
-    if (NIL_P(certs_path))
-        ossl_raise(rb_eArgError, "certs_path must be the path to certificates");
-
-    if (NIL_P(pkey_path))
-        ossl_raise(rb_eArgError, "pkey_path must be the path to private key");
 
     /* SSL_CTX_use_certificate_chain_file() loads PEM format file. */
     if (SSL_CTX_use_certificate_chain_file(ctx, StringValueCStr(certs_path)) != 1)
-        return Qfalse;
+        ossl_raise(eSSLError, "SSL_CTX_use_certificate_chain_file");
 
     if (SSL_CTX_use_PrivateKey_file(ctx, StringValueCStr(pkey_path), SSL_FILETYPE_PEM) != 1)
-        return Qfalse;
+        ossl_raise(eSSLError, "SSL_CTX_use_PrivateKey_file");
 
-    return Qtrue;
+    return self;
 }
 
 /*
diff --git a/test/test_ssl.rb b/test/test_ssl.rb
index 07484769..cc89ab6e 100644
--- a/test/test_ssl.rb
+++ b/test/test_ssl.rb
@@ -194,7 +194,7 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase
     ctx_proc = -> ctx {
       # Unset values set by start_server
       ctx.cert = ctx.key = ctx.extra_chain_cert = nil
-      assert ctx.add_certificate_chain_file(certs.path, pkey.path)
+      assert_nothing_raised { ctx.add_certificate_chain_file(certs.path, pkey.path) }
     }
 
     start_server(ctx_proc: ctx_proc) { |port|
@@ -211,6 +211,80 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase
     pkey&.close
   end
 
+  def test_add_certificate_chain_file_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("rsa2048")
+      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)
+
+    # Create chain certificates file
+    GC.disable # for tempfile
+    certs1 = Tempfile.open { |f| f << @svr_cert.to_pem << @ca_cert.to_pem; f }
+    pkey1 = Tempfile.open { |f| f << @svr_key.to_pem; f }
+    certs2 = Tempfile.open { |f| f << ecdsa_cert.to_pem << ca2_cert.to_pem; f }
+    pkey2 = Tempfile.open { |f| f << ecdsa_key.to_pem; f }
+
+    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)
+      assert_nothing_raised {
+        ctx.add_certificate_chain_file(certs1.path, pkey1.path) # RSA
+        ctx.add_certificate_chain_file(certs2.path, pkey2.path) # ECDSA
+      }
+    }
+
+    start_server(ctx_proc: ctx_proc) do |port|
+      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)
+
+        ssl.puts "abc"; assert_equal "abc\n", ssl.gets
+      }
+
+      ctx = OpenSSL::SSL::SSLContext.new
+      ctx.max_version = :TLS1_2
+      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)
+
+        ssl.puts "123"; assert_equal "123\n", ssl.gets
+      }
+    end
+  ensure
+    certs1&.close
+    pkey1&.close
+    certs2&.close
+    pkey2&.close
+  end
+
   def test_sysread_and_syswrite
     start_server { |port|
       server_connect(port) { |ssl|