Fix X509::Certificate#dup dropping all extensions

Author: sferik

src/main/java/org/jruby/ext/openssl/X509Cert.java

@@ -292,9 +292,53 @@ public IRubyObject initialize_copy(IRubyObject obj) {
         if ( this == obj ) return this;
 
         checkFrozen();
+        super.initialize_copy(obj);
+
+        copyState(getRuntime().getCurrentContext(), (X509Cert) obj);
         return this;
     }
 
+    private void copyState(final ThreadContext context, final X509Cert that) {
+        final Ruby runtime = context.runtime;
+
+        this.subject = copyName(context, that.subject);
+        this.issuer = copyName(context, that.issuer);
+        this.serial = that.serial;
+        this.not_before = copyTime(runtime, that.not_before);
+        this.not_after = copyTime(runtime, that.not_after);
+        this.sig_alg = that.sig_alg == null ? null : that.sig_alg.dup();
+        this.version = that.version;
+        this.cert = copyCertificate(context, that.cert);
+        this.public_key = that.public_key == null ? null : (PKey) that.public_key.dup();
+
+        this.extensions.clear();
+        for ( X509Extension ext : that.extensions ) {
+            this.extensions.add( (X509Extension) ext.dup() );
+        }
+
+        this.changed = that.changed;
+    }
+
+    private static IRubyObject copyName(final ThreadContext context, final IRubyObject name) {
+        if ( name == null || name.isNil() ) return name;
+        return X509Name.newName(context.runtime, ((X509Name) name).getX500Name());
+    }
+
+    private static RubyTime copyTime(final Ruby runtime, final RubyTime time) {
+        return time == null ? null : RubyTime.newTime(runtime, time.getJavaDate().getTime());
+    }
+
+    private static X509Certificate copyCertificate(final ThreadContext context, final X509Certificate cert) {
+        if ( cert == null ) return null;
+        try {
+            final ByteArrayInputStream bis = new ByteArrayInputStream(cert.getEncoded());
+            return (X509Certificate) SecurityHelper.getCertificateFactory("X.509").generateCertificate(bis);
+        }
+        catch (CertificateException e) {
+            throw newCertificateError(context.runtime, e);
+        }
+    }
+
     @JRubyMethod
     public IRubyObject to_der() {
         try {
@@ -724,6 +768,7 @@ public RubyArray extensions() {
     @SuppressWarnings("unchecked")
     @JRubyMethod(name = "extensions=")
     public IRubyObject set_extensions(final IRubyObject array) {
+        changed = true;
         extensions.clear(); // RubyArray is a List :
         extensions.addAll( (List<X509Extension>) array );
         return array;

src/test/ruby/x509/test_x509cert.rb

@@ -796,4 +796,152 @@ def test_authority_info_access_ocsp_uris # GH-210
     assert_equal ['http://cacerts.digicert.com/DigiCertSHA2SecureServerCA.crt'], cert.ca_issuer_uris
     assert_not_match(/#<OpenSSL::ASN1::/, aia_ext.value)
   end
+
+  def test_dup_preserves_extensions
+    rsa2048 = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+    ca = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=CA")
+
+    ca_exts = [
+      [ "basicConstraints", "CA:TRUE", true ],
+      [ "keyUsage", "keyCertSign, cRLSign", true ],
+      [ "subjectKeyIdentifier", "hash", false ],
+    ]
+
+    now = Time.now
+    cert = issue_cert(ca, rsa2048, 1, ca_exts, nil, nil,
+                      not_before: now, not_after: now + 3600)
+
+    duped = cert.dup
+
+    assert_equal cert.extensions.size, duped.extensions.size,
+      "dup should preserve extensions"
+    assert_equal cert.extensions.map(&:oid).sort, duped.extensions.map(&:oid).sort
+  end
+
+  def test_dup_preserves_subject_and_issuer
+    rsa2048 = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+    ca = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=CA")
+
+    now = Time.now
+    cert = issue_cert(ca, rsa2048, 42, [], nil, nil,
+                      not_before: now, not_after: now + 3600)
+
+    duped = cert.dup
+
+    assert_equal cert.subject.to_s, duped.subject.to_s
+    assert_equal cert.issuer.to_s, duped.issuer.to_s
+    assert_equal cert.serial, duped.serial
+    assert_equal cert.version, duped.version
+    assert_equal cert.not_before.to_i, duped.not_before.to_i
+    assert_equal cert.not_after.to_i, duped.not_after.to_i
+  end
+
+  def test_dup_produces_independent_copy
+    rsa2048 = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+    ca = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=CA")
+
+    ca_exts = [
+      [ "basicConstraints", "CA:TRUE", true ],
+      [ "subjectKeyIdentifier", "hash", false ],
+    ]
+
+    now = Time.now
+    cert = issue_cert(ca, rsa2048, 1, ca_exts, nil, nil,
+                      not_before: now, not_after: now + 3600)
+
+    duped = cert.dup
+
+    # Modifying the dup's extensions should not affect the original
+    duped.extensions = []
+    assert_equal 2, cert.extensions.size,
+      "modifying duped cert's extensions should not affect original"
+  end
+
+  def test_dup_to_der_matches_original
+    rsa2048 = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+    ca = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=CA")
+
+    ca_exts = [
+      [ "basicConstraints", "CA:TRUE", true ],
+      [ "keyUsage", "keyCertSign, cRLSign", true ],
+      [ "subjectKeyIdentifier", "hash", false ],
+    ]
+
+    now = Time.now
+    cert = issue_cert(ca, rsa2048, 1, ca_exts, nil, nil,
+                      not_before: now, not_after: now + 3600)
+
+    duped = cert.dup
+
+    assert_equal cert.to_der, duped.to_der
+  end
+
+  def test_dup_preserves_live_subject_after_signed_cert_name_is_mutated
+    rsa2048 = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+    ca = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=CA")
+
+    now = Time.now
+    cert = issue_cert(ca, rsa2048, 1, [], nil, nil,
+                      not_before: now, not_after: now + 3600)
+
+    cert.subject.add_entry("O", "mutated")
+    duped = cert.dup
+
+    assert_equal cert.subject.to_a, duped.subject.to_a
+  end
+
+  def test_dup_preserves_live_extensions_after_signed_cert_is_modified
+    rsa2048 = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+    ca = OpenSSL::X509::Name.parse("/DC=org/DC=ruby-lang/CN=CA")
+
+    ca_exts = [
+      [ "basicConstraints", "CA:TRUE", true ],
+      [ "subjectKeyIdentifier", "hash", false ],
+    ]
+
+    now = Time.now
+    cert = issue_cert(ca, rsa2048, 1, ca_exts, nil, nil,
+                      not_before: now, not_after: now + 3600)
+
+    cert.extensions = []
+    duped = cert.dup
+
+    assert_equal [], duped.extensions.map(&:oid)
+  end
+
+  def test_dup_unsigned_cert_preserves_fields
+    rsa2048 = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+
+    cert = OpenSSL::X509::Certificate.new
+    cert.version = 2
+    cert.serial = 99
+    cert.subject = OpenSSL::X509::Name.parse("/CN=unsigned")
+    cert.issuer = OpenSSL::X509::Name.parse("/CN=issuer")
+    cert.not_before = Time.now
+    cert.not_after = Time.now + 3600
+    cert.public_key = rsa2048.public_key
+
+    duped = cert.dup
+
+    assert_equal cert.subject.to_s, duped.subject.to_s
+    assert_equal cert.issuer.to_s, duped.issuer.to_s
+    assert_equal cert.serial, duped.serial
+    assert_equal cert.version, duped.version
+    assert_equal cert.public_key.to_der, duped.public_key.to_der
+  end
+
+  def test_dup_unsigned_cert_deep_copies_names
+    cert = OpenSSL::X509::Certificate.new
+    cert.subject = OpenSSL::X509::Name.parse("/CN=unsigned")
+    cert.issuer = OpenSSL::X509::Name.parse("/CN=issuer")
+
+    duped = cert.dup
+    duped.subject.add_entry("O", "mutated")
+    duped.issuer.add_entry("O", "mutated")
+
+    assert_equal "/CN=unsigned", cert.subject.to_s
+    assert_equal "/CN=issuer", cert.issuer.to_s
+    assert_equal "/CN=unsigned/O=mutated", duped.subject.to_s
+    assert_equal "/CN=issuer/O=mutated", duped.issuer.to_s
+  end
 end