Skip to content

Commit

Permalink
x509: fix handling of multiple URIs in Certificate#crl_uris
Browse files Browse the repository at this point in the history
The implementation of OpenSSL::X509::Certificate#crl_uris makes the
assumption that each DistributionPoint in the CRL distribution points
extension contains a single general name of type URI. This is not
guaranteed by RFC 5280. A DistributionPoint may contain zero or more
than one URIs.

Let's include all URIs found in the extension. If only non-URI pointers
are found, return nil.

Fixes: ruby#775
  • Loading branch information
rhenium committed Aug 16, 2024
1 parent 461cfcb commit 71f4fef
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 5 deletions.
10 changes: 5 additions & 5 deletions lib/openssl/x509.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
33 changes: 33 additions & 0 deletions test/openssl/test_x509cert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,39 @@ def test_crl_uris
)
end

def test_crl_uris_multiple_general_names
# Single DistributionPoint contains multiple general names of type URI
ef = OpenSSL::X509::ExtensionFactory.new
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)
Expand Down

0 comments on commit 71f4fef

Please sign in to comment.