[fix] authorityKeyIdentifier ext (general-name) value

we've aligned formatting of ext.value with C OpenSSL

Author: kares

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

@@ -420,41 +420,50 @@ public RubyString value(final ThreadContext context) {
             if ( oid.equals("2.5.29.35") ) { // authorityKeyIdentifier
                 ASN1Encodable value = getRealValue();
 
-                if ( value instanceof ASN1OctetString ) {
+                if (value instanceof ASN1OctetString) {
                     value = ASN1.readObject( ((ASN1OctetString) value).getOctets() );
                 }
 
-                final ByteList val = new ByteList(72); val.append(keyid_);
+                final ByteList val = new ByteList(72);
 
-                if ( value instanceof ASN1Sequence ) {
+                if (value instanceof ASN1Sequence) {
                     final ASN1Sequence seq = (ASN1Sequence) value;
                     final int size = seq.size();
                     if ( size == 0 ) return RubyString.newEmptyString(runtime);
 
-                    ASN1Primitive keyid = seq.getObjectAt(0).toASN1Primitive();
-                    hexBytes( keyidBytes(keyid), val ).append('\n');
-
-                    for ( int i = 1; i < size; i++ ) {
-                        final ASN1Encodable issuer = seq.getObjectAt(i);
-                        // NOTE: blindly got OpenSSL tests passing (likely in-complete) :
-                        if ( issuer instanceof ASN1TaggedObject ) {
-                            ASN1Primitive obj = ((ASN1TaggedObject) issuer).getObject();
-                            switch( ((ASN1TaggedObject) issuer).getTagNo() ) {
+                    for ( int i = 0; i < size; i++ ) {
+                        final ASN1Encodable enc = seq.getObjectAt(i);
+                        if (enc instanceof ASN1TaggedObject) {
+                            ASN1Primitive obj = ((ASN1TaggedObject) enc).getObject();
+                            switch( ((ASN1TaggedObject) enc).getTagNo() ) {
+                                case 0 :
+                                    ASN1Primitive keyid = obj;
+                                    val.append(keyid_);
+                                    hexBytes( keyidBytes(keyid), val );
+                                    break;
                                 case 1 :
-                                    if ( obj instanceof ASN1TaggedObject ) {
-                                        formatGeneralName(GeneralName.getInstance(obj), val, true);
+                                    GeneralName name;
+                                    if (obj instanceof ASN1Sequence) { // GeneralNames -> toASN1Primitive()
+                                        GeneralName[] names = GeneralNames.getInstance(obj).getNames();
+                                        name = names.length > 0 ? names[0] : null;
+                                    } else {
+                                        name = GeneralName.getInstance(obj);
                                     }
+                                    if (name != null) formatGeneralName(name, val, true);
                                     break;
                                 case 2 : // serial
                                     val.append(new byte[] { 's','e','r','i','a','l',':' });
                                     if (obj instanceof ASN1Integer) {
                                         hexBytes( ((ASN1Integer) obj).getValue().toByteArray(), val);
                                     }
                                     else {
-                                        hexBytes( ((ASN1OctetString) obj ).getOctets(), val );                                        
+                                        hexBytes( ((ASN1OctetString) obj ).getOctets(), val );
                                     }
                                     break;
                             }
+                        } else if (size == 1) {
+                            ASN1Primitive keyid = enc.toASN1Primitive();
+                            hexBytes( keyidBytes(keyid), val );
                         }
                         val.append('\n');
                     }

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

@@ -403,12 +403,16 @@ private static DLSequence parseBasicConstrains(final String valuex) {
     private ASN1Sequence parseAuthorityKeyIdentifier(final ThreadContext context, final String valuex) {
         final ASN1EncodableVector vec = new ASN1EncodableVector();
 
-        for ( String value : valuex.split(",") ) { // e.g. "keyid:always,issuer:always"
+        final String[] values = valuex.split(","); // e.g. "keyid:always,issuer:always"
+        for ( int i = 0; i<values.length; i++ ) {
+            final String value = values[i];
             if ( value.startsWith("keyid") ) { // keyid[:always]
                 ASN1Encodable publicKeyIdentifier = new DEROctetString(issuerPublicKeyIdentifier(context));
                 vec.add(new DERTaggedObject(false, 0, publicKeyIdentifier));
+
+                if ( i < values.length - 1 && !"issuer:always".equals(values[i + 1]) ) break;
             }
-            else if ( value.startsWith("issuer") ) { // issuer[:always]
+            if ( value.startsWith("issuer") ) { // issuer[:always]
                 GeneralName issuerName = new GeneralName(authorityCertIssuer(context));
                 vec.add(new DERTaggedObject(false, 1, new GeneralNames(issuerName)));
 
@@ -505,7 +509,7 @@ private static byte[] getSHA1Digest(Ruby runtime, ByteList bytes) {
     private ASN1Encodable parseIssuerAltName(final ThreadContext context, final String valuex)
         throws IOException {
         if ( valuex.startsWith("issuer:copy") ) {
-            RubyArray exts = (RubyArray) getInstanceVariable("@issuer_certificate").callMethod(context, "extensions");
+            RubyArray exts = (RubyArray) issuer_cert().callMethod(context, "extensions");
             for ( int i = 0; i < exts.size(); i++ ) {
                 X509Extension ext = (X509Extension) exts.entry(i);
                 final String oid = ext.getRealObjectID().getId();

src/test/ruby/test_asn1.rb

@@ -451,7 +451,7 @@ def test_decode
 
     assert OpenSSL::X509::Certificate.new( cert_der ).verify key
     # running the same in MRI also fails
-    #calulated_sig = key.sign(OpenSSL::Digest::SHA1.new, cert_der)
+    calulated_sig = key.sign(OpenSSL::Digest::SHA1.new, cert_der)
     #assert_equal calulated_sig, sig_val.value
   end
 

src/test/ruby/x509/test_x509cert.rb

@@ -74,18 +74,16 @@ def test_cert_extensions # JRUBY-3468
   end
 
   def test_aki_extension_to_text
-    cert = create_self_signed_cert [ %w[CN localhost] ], __method__
+    cert = create_self_signed_cert [ %w[CN localhost] ], __method__.to_s
     keyid = "97:39:9D:C3:FB:CD:BA:8F:54:0C:90:7B:46:3F:EA:D6:43:75:B1:CB"
 
     assert cert.extensions.size > 0
     value = cert.extensions.last.value
-    # assert_equal "keyid:#{keyid}\nDirName:/CN=localhost\nserial:01\n", value
-    assert value.start_with?("keyid:#{keyid}\n")
-    assert value.end_with?("\nserial:01\n")
+    assert_equal "keyid:#{keyid}\nDirName:/CN=localhost\nserial:01\n", value
   end
 
   def create_self_signed_cert(cn, comment) # cert generation ripped from WEBrick
-    rsa = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
+    key = OpenSSL::PKey::RSA.new TEST_KEY_RSA2048
     cert = OpenSSL::X509::Certificate.new
     cert.version = 2
     cert.serial = 1
@@ -94,7 +92,7 @@ def create_self_signed_cert(cn, comment) # cert generation ripped from WEBrick
     cert.issuer = name
     cert.not_before = Time.now
     cert.not_after = Time.now + (365*24*60*60)
-    cert.public_key = rsa.public_key
+    cert.public_key = key.public_key
 
     ef = OpenSSL::X509::ExtensionFactory.new(nil,cert)
     ef.issuer_certificate = cert
@@ -107,7 +105,7 @@ def create_self_signed_cert(cn, comment) # cert generation ripped from WEBrick
     ]
     aki = ef.create_extension("authorityKeyIdentifier", "keyid:always,issuer:always")
     cert.add_extension(aki)
-    cert.sign(rsa, OpenSSL::Digest::SHA1.new)
+    cert.sign(key, OpenSSL::Digest::SHA1.new)
 
     cert
   end

src/test/ruby/x509/test_x509ext.rb

@@ -268,6 +268,23 @@ def test_authority_key_identifier
     assert_equal keyid = "keyid:91:0D:0C:A9:43:73:DF:8C:A9:E3:C2:0A:05:E3:CF:BE:A7:38:8D:DD\n", ext.value
     assert !ext.critical?
     assert_equal [ "authorityKeyIdentifier", keyid, false ], ext.to_a
+
+    issuer = "DirName:/CN=localhost\n" + "serial:01\n"
+    ext = ef.create_extension("authorityKeyIdentifier", "keyid,issuer")
+    assert_equal keyid, ext.value
+    assert_equal [ "authorityKeyIdentifier", keyid, false ], ext.to_a
+
+    ext = ef.create_extension("authorityKeyIdentifier", "issuer")
+    assert_equal [ "authorityKeyIdentifier", issuer, false ], ext.to_a
+
+    ext = ef.create_extension("authorityKeyIdentifier", "keyid:always,issuer:always")
+    assert_equal keyid + issuer, ext.value
+    assert_equal [ "authorityKeyIdentifier", keyid + issuer, false ], ext.to_a
+
+    ext = ef.create_extension("authorityKeyIdentifier", "keyid:always,issuer")
+    assert_equal keyid, ext.value
+    assert_equal [ "authorityKeyIdentifier", keyid, false ], ext.to_a
+
     # cert.sign(key, OpenSSL::Digest::SHA1.new)
   end