From 80aecbde2ce0a6a6188ee8a4de1a0acec2c1c0f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ronald=20Tschal=C3=A4r?= Date: Tue, 3 Oct 2017 01:36:25 -0700 Subject: [PATCH] Fix loading of Subject/Issuer-Alt-Name extensions. These were being treated specially and incorrectly when being loaded from encoded values. A given extension may not occur more than once in certificate or CRL, and hence this code could never be correct. Fixed the erroneous test for this too. --- .../org/jruby/ext/openssl/X509Extension.java | 16 ---------------- src/test/ruby/x509/test_x509cert.rb | 17 ++++++----------- 2 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/jruby/ext/openssl/X509Extension.java b/src/main/java/org/jruby/ext/openssl/X509Extension.java index 5f8bf8c8..199a2325 100644 --- a/src/main/java/org/jruby/ext/openssl/X509Extension.java +++ b/src/main/java/org/jruby/ext/openssl/X509Extension.java @@ -141,22 +141,6 @@ static X509Extension[] newExtension(final ThreadContext context, final ASN1ObjectIdentifier objectId = ASN1.getObjectID(runtime, oid); final ASN1Encodable value = ASN1.readObject(extValue); - if ( oid.equals("2.5.29.17") || oid.equals("2.5.29.18") ) { // subjectAltName || issuerAltName - if ( value instanceof ASN1OctetString ) { // DEROctetString - final ASN1Encodable oct = ASN1.readObject( ((ASN1OctetString) value).getOctets() ); - if ( oct instanceof ASN1Sequence ) { - final ASN1Sequence seq = (ASN1Sequence) oct; - final X509Extension[] ext = new X509Extension[ seq.size() ]; - for ( int i = 0; i < ext.length; i++ ) { - ext[i] = newExtension(runtime, objectId, seq.getObjectAt(i), critical); - } - return ext; - } - // NOTE need to unwrap ((ASN1TaggedObject) oct).getObject() - likely not ?!? - return new X509Extension[] { newExtension(runtime, objectId, oct, critical) }; - } - } - return new X509Extension[] { newExtension(runtime, objectId, value, critical) }; } diff --git a/src/test/ruby/x509/test_x509cert.rb b/src/test/ruby/x509/test_x509cert.rb index 1b2c2595..c813fba8 100644 --- a/src/test/ruby/x509/test_x509cert.rb +++ b/src/test/ruby/x509/test_x509cert.rb @@ -82,25 +82,23 @@ def test_resolve_extensions [ "keyUsage", "keyCertSign, cRLSign", true ], [ "subjectKeyIdentifier", "hash", false ], [ "authorityKeyIdentifier", "keyid:always", false ], - [ "subjectAltName", "email:self@jruby.org", false ], - [ "subjectAltName", "DNS:jruby.org", false ], + [ "subjectAltName", "email:self@jruby.org, DNS:jruby.org", false ], ] now = Time.now ca_cert = issue_cert(ca, rsa2048, 1, now, now + 3600, ca_exts, nil, nil, OpenSSL::Digest::SHA1.new) - assert_equal 6, ca_cert.extensions.size + assert_equal 5, ca_cert.extensions.size cert = OpenSSL::X509::Certificate.new ca_cert.to_der - assert_equal 6, cert.extensions.size + assert_equal 5, cert.extensions.size # Java 6/7 seems to maintain same order but Java 8 does definitely not : # TODO there must be something going on under - maybe not BC parsing ?!? if self.class.java6? || self.class.java7? assert_equal '97:39:9D:C3:FB:CD:BA:8F:54:0C:90:7B:46:3F:EA:D6:43:75:B1:CB', cert.extensions[2].value - assert_equal 'email:self@jruby.org', cert.extensions[4].value - assert_equal 'DNS:jruby.org', cert.extensions[5].value + assert_equal 'email:self@jruby.org, DNS:jruby.org', cert.extensions[4].value end exts = cert.extensions.dup @@ -118,10 +116,7 @@ def test_resolve_extensions assert ! ext.critical? assert ext = exts.find { |e| e.oid == 'subjectAltName' }, "missing 'subjectAltName' among: #{exts.join(', ')}" - assert_equal 'email:self@jruby.org', ext.value - exts.delete(ext) - assert ext = exts.find { |e| e.oid == 'subjectAltName' }, "missing 'subjectAltName' among: #{exts.join(', ')}" - assert_equal 'DNS:jruby.org', ext.value + assert_equal 'email:self@jruby.org, DNS:jruby.org', ext.value end def test_extensions @@ -367,4 +362,4 @@ def test_cert_loading_regression -----END RSA PRIVATE KEY----- _end_of_pem_ -end \ No newline at end of file +end