Merge pull request #776 from rhenium/ky/x509cert-crl_uris-fix-nil

x509: fix handling of multiple URIs in Certificate#crl_uris

Author: rhenium

lib/openssl/x509.rb

@@ -122,8 +122,8 @@ module CRLDistributionPoints
         include Helpers
 
         # Get the distributionPoint fullName URI from the certificate's CRL
-        # distribution points extension, as described in RFC5280 Section
-        # 4.2.1.13
+        # distribution points extension, as described in RFC 5280 Section
+        # 4.2.1.13.
         #
         # Returns an array of strings or nil or raises ASN1::ASN1Error.
         def crl_uris
@@ -135,19 +135,19 @@ def crl_uris
             raise ASN1::ASN1Error, "invalid extension"
           end
 
-          crl_uris = cdp_asn1.map do |crl_distribution_point|
+          crl_uris = cdp_asn1.flat_map do |crl_distribution_point|
             distribution_point = crl_distribution_point.value.find do |v|
               v.tag_class == :CONTEXT_SPECIFIC && v.tag == 0
             end
             full_name = distribution_point&.value&.find do |v|
               v.tag_class == :CONTEXT_SPECIFIC && v.tag == 0
             end
-            full_name&.value&.find do |v|
+            full_name&.value&.select do |v|
               v.tag_class == :CONTEXT_SPECIFIC && v.tag == 6 # uniformResourceIdentifier
             end
           end
 
-          crl_uris&.map(&:value)
+          crl_uris.empty? ? nil : crl_uris.map(&:value)
         end
       end
 

test/openssl/test_x509cert.rb

@@ -68,17 +68,14 @@ def test_validity
     assert_equal(now.getutc, cert.not_after)
   end
 
-  def test_extension
+  def test_extension_factory
     ca_exts = [
       ["basicConstraints","CA:TRUE",true],
       ["keyUsage","keyCertSign, cRLSign",true],
       ["subjectKeyIdentifier","hash",false],
       ["authorityKeyIdentifier","issuer:always,keyid:always",false],
     ]
     ca_cert = issue_cert(@ca, @rsa2048, 1, ca_exts, nil, nil)
-    keyid = get_subject_key_id(ca_cert.to_der, hex: false)
-    assert_equal keyid, ca_cert.authority_key_identifier
-    assert_equal keyid, ca_cert.subject_key_identifier
     ca_cert.extensions.each_with_index{|ext, i|
       assert_equal(ca_exts[i].first, ext.oid)
       assert_equal(ca_exts[i].last, ext.critical?)
@@ -90,33 +87,112 @@ def test_extension
       ["authorityKeyIdentifier","issuer:always,keyid:always",false],
       ["extendedKeyUsage","clientAuth, emailProtection, codeSigning",false],
       ["subjectAltName","email:[email protected]",false],
-      ["authorityInfoAccess","caIssuers;URI:http://www.example.com/caIssuers,OCSP;URI:http://www.example.com/ocsp",false],
     ]
     ee1_cert = issue_cert(@ee1, @rsa1024, 2, ee1_exts, ca_cert, @rsa2048)
     assert_equal(ca_cert.subject.to_der, ee1_cert.issuer.to_der)
     ee1_cert.extensions.each_with_index{|ext, i|
       assert_equal(ee1_exts[i].first, ext.oid)
       assert_equal(ee1_exts[i].last, ext.critical?)
     }
-    assert_nil(ee1_cert.crl_uris)
+  end
+
+  def test_akiski
+    ca_cert = generate_cert(@ca, @rsa2048, 4, nil)
+    ef = OpenSSL::X509::ExtensionFactory.new(ca_cert, ca_cert)
+    ca_cert.add_extension(
+      ef.create_extension("subjectKeyIdentifier", "hash", false))
+    ca_cert.add_extension(
+      ef.create_extension("authorityKeyIdentifier", "issuer:always,keyid:always", false))
+    ca_cert.sign(@rsa2048, "sha256")
+
+    ca_keyid = get_subject_key_id(ca_cert.to_der, hex: false)
+    assert_equal ca_keyid, ca_cert.authority_key_identifier
+    assert_equal ca_keyid, ca_cert.subject_key_identifier
+
+    ee_cert = generate_cert(@ee1, Fixtures.pkey("p256"), 5, ca_cert)
+    ef = OpenSSL::X509::ExtensionFactory.new(ca_cert, ee_cert)
+    ee_cert.add_extension(
+      ef.create_extension("subjectKeyIdentifier", "hash", false))
+    ee_cert.add_extension(
+      ef.create_extension("authorityKeyIdentifier", "issuer:always,keyid:always", false))
+    ee_cert.sign(@rsa2048, "sha256")
+
+    ee_keyid = get_subject_key_id(ee_cert.to_der, hex: false)
+    assert_equal ca_keyid, ee_cert.authority_key_identifier
+    assert_equal ee_keyid, ee_cert.subject_key_identifier
+  end
+
+  def test_akiski_missing
+    cert = issue_cert(@ee1, @rsa2048, 1, [], nil, nil)
+    assert_nil(cert.authority_key_identifier)
+    assert_nil(cert.subject_key_identifier)
+  end
+
+  def test_crl_uris_no_crl_distribution_points
+    cert = issue_cert(@ee1, @rsa2048, 1, [], nil, nil)
+    assert_nil(cert.crl_uris)
+  end
 
+  def test_crl_uris
+    # Multiple DistributionPoint contains a single general name each
     ef = OpenSSL::X509::ExtensionFactory.new
     ef.config = OpenSSL::Config.parse(<<~_cnf_)
       [crlDistPts]
       URI.1 = http://www.example.com/crl
       URI.2 = ldap://ldap.example.com/cn=ca?certificateRevocationList;binary
     _cnf_
-    cdp_cert = generate_cert(@ee1, @rsa1024, 3, ca_cert)
+    cdp_cert = generate_cert(@ee1, @rsa2048, 3, nil)
     ef.subject_certificate = cdp_cert
     cdp_cert.add_extension(ef.create_extension("crlDistributionPoints", "@crlDistPts"))
     cdp_cert.sign(@rsa2048, "sha256")
     assert_equal(
       ["http://www.example.com/crl", "ldap://ldap.example.com/cn=ca?certificateRevocationList;binary"],
       cdp_cert.crl_uris
     )
+  end
 
+  def test_crl_uris_multiple_general_names
+    # Single DistributionPoint contains multiple general names of type URI
     ef = OpenSSL::X509::ExtensionFactory.new
-    aia_cert = generate_cert(@ee1, @rsa1024, 4, ca_cert)
+    ef.config = OpenSSL::Config.parse(<<~_cnf_)
+      [crlDistPts_section]
+      fullname = URI:http://www.example.com/crl, URI:ldap://ldap.example.com/cn=ca?certificateRevocationList;binary
+    _cnf_
+    cdp_cert = generate_cert(@ee1, @rsa2048, 3, nil)
+    ef.subject_certificate = cdp_cert
+    cdp_cert.add_extension(ef.create_extension("crlDistributionPoints", "crlDistPts_section"))
+    cdp_cert.sign(@rsa2048, "sha256")
+    assert_equal(
+      ["http://www.example.com/crl", "ldap://ldap.example.com/cn=ca?certificateRevocationList;binary"],
+      cdp_cert.crl_uris
+    )
+  end
+
+  def test_crl_uris_no_uris
+    # The only DistributionPointName is a directoryName
+    ef = OpenSSL::X509::ExtensionFactory.new
+    ef.config = OpenSSL::Config.parse(<<~_cnf_)
+      [crlDistPts_section]
+      fullname = dirName:dirname_section
+      [dirname_section]
+      CN = dirname
+    _cnf_
+    cdp_cert = generate_cert(@ee1, @rsa2048, 3, nil)
+    ef.subject_certificate = cdp_cert
+    cdp_cert.add_extension(ef.create_extension("crlDistributionPoints", "crlDistPts_section"))
+    cdp_cert.sign(@rsa2048, "sha256")
+    assert_nil(cdp_cert.crl_uris)
+  end
+
+  def test_aia_missing
+    cert = issue_cert(@ee1, @rsa2048, 1, [], nil, nil)
+    assert_nil(cert.ca_issuer_uris)
+    assert_nil(cert.ocsp_uris)
+  end
+
+  def test_aia
+    ef = OpenSSL::X509::ExtensionFactory.new
+    aia_cert = generate_cert(@ee1, @rsa2048, 4, nil)
     ef.subject_certificate = aia_cert
     aia_cert.add_extension(
       ef.create_extension(
@@ -137,13 +213,6 @@ def test_extension
       ["http://www.example.com/ocsp", "ldap://ldap.example.com/cn=ca?authorityInfoAccessOcsp;binary"],
       aia_cert.ocsp_uris
     )
-
-    no_exts_cert = issue_cert(@ca, @rsa2048, 5, [], nil, nil)
-    assert_equal nil, no_exts_cert.authority_key_identifier
-    assert_equal nil, no_exts_cert.subject_key_identifier
-    assert_equal nil, no_exts_cert.crl_uris
-    assert_equal nil, no_exts_cert.ca_issuer_uris
-    assert_equal nil, no_exts_cert.ocsp_uris
   end
 
   def test_invalid_extension