From 4a56a5a5e00043e684cd2c5b21d09842a1d2d027 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 13 Sep 2026 18:17:38 +0200 Subject: [PATCH 1/4] [refactor] assume X509CRL and cache hash-code certificate implementations already cache the code --- .../org/jruby/ext/openssl/x509store/CRL.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/x509store/CRL.java b/src/main/java/org/jruby/ext/openssl/x509store/CRL.java index 7effd1b81..8128c54e9 100644 --- a/src/main/java/org/jruby/ext/openssl/x509store/CRL.java +++ b/src/main/java/org/jruby/ext/openssl/x509store/CRL.java @@ -36,10 +36,10 @@ */ public class CRL extends X509Object { - public final java.security.cert.CRL crl; + public final java.security.cert.X509CRL crl; public CRL(java.security.cert.CRL crl) { - this.crl = crl; + this.crl = (X509CRL) crl; } @Override @@ -49,15 +49,13 @@ public int type() { @Override public boolean isName(final Name name) { - return name.equalTo( ((X509CRL) crl).getIssuerX500Principal() ); + return name.equalTo( crl.getIssuerX500Principal() ); } @Override public boolean matches(final X509Object other) { if (other instanceof CRL) { - final X509CRL thisCRL = (X509CRL) crl; - final X509CRL thatCRL = (X509CRL)((CRL) other).crl; - return thisCRL.getIssuerX500Principal().equals( thatCRL.getIssuerX500Principal() ); + return this.crl.getIssuerX500Principal().equals( ((CRL) other).crl.getIssuerX500Principal() ); } return false; } @@ -69,4 +67,14 @@ public int compareTo(final X509Object other) { return crl.equals( ((CRL) other).crl ) ? 0 : -1; } + private transient int hash = -1; + + @Override + public int hashCode() { + if (hash == -1) { + hash = crl.hashCode(); // X509CRL based on encoded bytes + } + return hash; + } + }// X509_OBJECT_CRL From 397016cd6b98f5a6a284c6ee33ad410c32b3675d Mon Sep 17 00:00:00 2001 From: kares Date: Wed, 9 Sep 2026 10:46:33 +0200 Subject: [PATCH 2/4] [fix] try all same-subject certificate issuers The local certificate store preserves insertion order rather than OpenSSL's subject index ordering. Continue scanning after nonmatching entries so rotated CAs with the same subject can be considered during verification. --- .../ext/openssl/x509store/StoreContext.java | 11 +- test/x509/test_x509store.rb | 115 ++++++++++++++++++ 2 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/x509store/StoreContext.java b/src/main/java/org/jruby/ext/openssl/x509store/StoreContext.java index f63c7687b..a53658b7d 100644 --- a/src/main/java/org/jruby/ext/openssl/x509store/StoreContext.java +++ b/src/main/java/org/jruby/ext/openssl/x509store/StoreContext.java @@ -30,7 +30,6 @@ import java.io.IOException; import java.security.GeneralSecurityException; import java.security.PublicKey; -import java.security.cert.CertificateException; import java.security.cert.CRLReason; import java.security.cert.X509CRL; import java.security.cert.X509CRLEntry; @@ -111,8 +110,6 @@ interface CheckPolicyFunction extends Function1 {} Store.LookupCerts lookup_certs; - //private boolean isValid; - private int num_untrusted; // last_untrusted (OpenSSL 1.0.2) in the chain private ArrayList chain; @@ -202,13 +199,13 @@ else if ( ok != X509Utils.X509_LU_FAIL ) { /* Look through all matching certificates for a suitable issuer */ for ( int i = idx; i < objects.size(); i++ ) { final X509Object pobj = objects.get(i); - /* See if we've run past the matches */ if (pobj.type() != X509_LU_X509) { - break; // return 0 + continue; } final X509AuxCertificate x509 = ((Certificate) pobj).cert; if ( ! xn.equalTo( x509.getSubjectX500Principal() ) ) { - break; // return 0 + // NOTE: unlike OpenSSL our store.objects aren't sorted by subject (insertion order) + continue; // same-DN certs may be separated by other entries - keep scanning } if ( checkIssued.call(this, x, x509) != 0 ) { _issuer[0] = x509; @@ -1556,7 +1553,7 @@ private int check_chain_extensions() throws Exception { } /* Return 1 is a certificate is self signed */ - private boolean cert_self_signed(X509AuxCertificate x) throws CertificateException, IOException { + private boolean cert_self_signed(X509AuxCertificate x) throws IOException { // Purpose.checkPurpose(x, -1, 0); if ((x.getExFlags() & EXFLAG_SI) != 0) { // TODO EXFLAG_SS return true; diff --git a/test/x509/test_x509store.rb b/test/x509/test_x509store.rb index e12a8ac62..ad2c5f4f8 100644 --- a/test/x509/test_x509store.rb +++ b/test/x509/test_x509store.rb @@ -1183,3 +1183,118 @@ def test_verify_at_exact_not_after_is_expired end end + +# GH#370: X509Store must try every CA matching the issuer DN +# (e.g. CA rotation where old and new chains share a subject) +class TestX509StoreMultiCASameDN < TestCase + + def setup + @old_root_cert, @old_root_key = make_root_ca('Test-Root', serial: 1) + @old_inter_cert, @old_inter_key = make_intermediate_ca('Test-Intermediate', @old_root_cert, @old_root_key, serial: 100) + @new_root_cert, @new_root_key = make_root_ca('Test-Root', serial: 2) + @new_inter_cert, @new_inter_key = make_intermediate_ca('Test-Intermediate', @new_root_cert, @new_root_key, serial: 200) + @server_cert, @server_key = make_leaf('localhost', @new_inter_cert, @new_inter_key, serial: 1000) + end + + private + + def make_root_ca(cn, serial:) + key = OpenSSL::PKey::RSA.new(2048) + name = OpenSSL::X509::Name.new([['CN', cn], ['O', 'TestOrg'], ['C', 'US']]) + + cert = OpenSSL::X509::Certificate.new + cert.version = 2 + cert.serial = serial + cert.subject = name + cert.issuer = name + cert.public_key = key.public_key + cert.not_before = Time.now - 3600 + cert.not_after = Time.now + 86400 * 365 + + ef = OpenSSL::X509::ExtensionFactory.new + ef.subject_certificate = cert + ef.issuer_certificate = cert + cert.add_extension(ef.create_extension('basicConstraints', 'CA:TRUE', true)) + cert.add_extension(ef.create_extension('keyUsage', 'keyCertSign,cRLSign', true)) + cert.add_extension(ef.create_extension('subjectKeyIdentifier', 'hash')) + cert.add_extension(ef.create_extension('authorityKeyIdentifier', 'keyid:always')) + + cert.sign(key, OpenSSL::Digest.new('SHA256')) + [cert, key] + end + + def make_intermediate_ca(cn, parent_cert, parent_key, serial:) + key = OpenSSL::PKey::RSA.new(2048) + name = OpenSSL::X509::Name.new([['CN', cn], ['O', 'TestOrg'], ['C', 'US']]) + + cert = OpenSSL::X509::Certificate.new + cert.version = 2 + cert.serial = serial + cert.subject = name + cert.issuer = parent_cert.subject + cert.public_key = key.public_key + cert.not_before = Time.now - 3600 + cert.not_after = Time.now + 86400 * 365 + + ef = OpenSSL::X509::ExtensionFactory.new + ef.subject_certificate = cert + ef.issuer_certificate = parent_cert + cert.add_extension(ef.create_extension('basicConstraints', 'CA:TRUE', true)) + cert.add_extension(ef.create_extension('keyUsage', 'keyCertSign,cRLSign', true)) + cert.add_extension(ef.create_extension('subjectKeyIdentifier', 'hash')) + cert.add_extension(ef.create_extension('authorityKeyIdentifier', 'keyid:always')) + + cert.sign(parent_key, OpenSSL::Digest.new('SHA256')) + [cert, key] + end + + def make_leaf(cn, parent_cert, parent_key, serial:) + key = OpenSSL::PKey::RSA.new(2048) + name = OpenSSL::X509::Name.new([['CN', cn], ['O', 'TestOrg'], ['C', 'US']]) + + cert = OpenSSL::X509::Certificate.new + cert.version = 2 + cert.serial = serial + cert.subject = name + cert.issuer = parent_cert.subject + cert.public_key = key.public_key + cert.not_before = Time.now - 3600 + cert.not_after = Time.now + 86400 * 365 + + ef = OpenSSL::X509::ExtensionFactory.new + ef.subject_certificate = cert + ef.issuer_certificate = parent_cert + cert.add_extension(ef.create_extension('basicConstraints', 'CA:FALSE')) + cert.add_extension(ef.create_extension('keyUsage', 'digitalSignature,keyEncipherment')) + cert.add_extension(ef.create_extension('extendedKeyUsage', 'serverAuth')) + cert.add_extension(ef.create_extension('subjectKeyIdentifier', 'hash')) + cert.add_extension(ef.create_extension('authorityKeyIdentifier', 'keyid:always')) + cert.add_extension(ef.create_extension('subjectAltName', "DNS:localhost,DNS:#{cn}")) + + cert.sign(parent_key, OpenSSL::Digest.new('SHA256')) + [cert, key] + end + + public + + def test_verify_with_old_chain_first + store = OpenSSL::X509::Store.new + store.add_cert(@old_inter_cert) + store.add_cert(@old_root_cert) + store.add_cert(@new_inter_cert) + store.add_cert(@new_root_cert) + + assert store.verify(@server_cert), "expected verify to pass, got: #{store.error_string} (error #{store.error})" + end + + def test_verify_with_new_chain_first + store = OpenSSL::X509::Store.new + store.add_cert(@new_inter_cert) + store.add_cert(@new_root_cert) + store.add_cert(@old_inter_cert) + store.add_cert(@old_root_cert) + + assert store.verify(@server_cert), "expected verify to pass, got: #{store.error_string} (error #{store.error})" + end + +end From b7312d94dea0fea1fbda61041ea4914900c0a42f Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 13 Sep 2026 19:01:02 +0200 Subject: [PATCH 3/4] [fix] CRL object matching (of same issuer) --- src/main/java/org/jruby/ext/openssl/x509store/CRL.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jruby/ext/openssl/x509store/CRL.java b/src/main/java/org/jruby/ext/openssl/x509store/CRL.java index 8128c54e9..99b7cb26d 100644 --- a/src/main/java/org/jruby/ext/openssl/x509store/CRL.java +++ b/src/main/java/org/jruby/ext/openssl/x509store/CRL.java @@ -55,7 +55,7 @@ public boolean isName(final Name name) { @Override public boolean matches(final X509Object other) { if (other instanceof CRL) { - return this.crl.getIssuerX500Principal().equals( ((CRL) other).crl.getIssuerX500Principal() ); + return this.hashCode() == other.hashCode() && this.crl.equals(((CRL) other).crl); } return false; } From 559de4a06103fc672ed2bc352485003456a85611 Mon Sep 17 00:00:00 2001 From: kares Date: Sun, 13 Sep 2026 19:01:37 +0200 Subject: [PATCH 4/4] [refactor] (safe) cert matching - full equality --- .../java/org/jruby/ext/openssl/x509store/Certificate.java | 5 ++--- .../org/jruby/ext/openssl/x509store/X509AuxCertificate.java | 5 ----- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/x509store/Certificate.java b/src/main/java/org/jruby/ext/openssl/x509store/Certificate.java index c06ac1f61..b132bef5d 100644 --- a/src/main/java/org/jruby/ext/openssl/x509store/Certificate.java +++ b/src/main/java/org/jruby/ext/openssl/x509store/Certificate.java @@ -54,9 +54,8 @@ public boolean isName(final Name name) { public boolean matches(final X509Object other) { if (other instanceof Certificate) { final Certificate that = (Certificate) other; - if (X509AuxCertificate.equalSubjects(this.cert, that.cert)) { - return this.cert.hashCode() == that.cert.hashCode(); - }; + return this.cert.cert.hashCode() == that.cert.cert.hashCode() + && this.cert.cert.equals(that.cert.cert); } return false; } diff --git a/src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java b/src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java index 27f99e12a..1ed7e1aa4 100644 --- a/src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java +++ b/src/main/java/org/jruby/ext/openssl/x509store/X509AuxCertificate.java @@ -368,9 +368,4 @@ public Integer getNsCertType() throws CertificateException { } } - static boolean equalSubjects(final X509AuxCertificate cert1, final X509AuxCertificate cert2) { - if ( cert1.cert == cert2.cert ) return true; - return cert1.getSubjectX500Principal().equals( cert2.getSubjectX500Principal() ); - } - }// X509AuxCertificate