Skip to content

Fix X509::Certificate#dup dropping all extensions #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
sferik wants to merge 1 commit into
base: master
Choose a base branch
from
+193 −0
Open

Fix X509::Certificate#dup dropping all extensions #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/main/java/org/jruby/ext/openssl/X509Cert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
148 changes: 148 additions & 0 deletions src/test/ruby/x509/test_x509cert.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading