Description
verify_certificate_identity relies on a naive string-based approach to extract DNS: (and IP:) entries from the subjectAltName extension. However, the SAN is truly an ASN.1 structure, and when the library stringifies it, otherName entries and semicolons (or other delimiters) will appear in the output. This leads to the regex split call misreading the line, causing verify_certificate_identity to fail on certificates that include otherName (common in Active Directory environments, for example).
For instance, a SAN might be stringified like:
"subjectAltName = critical, otherName:[1.3.6.1.4.1.311.20.2.3, [CONTEXT 0]HOST1$@example.com];DNS:host1.example.com, DNS:example.com, DNS:MYDOMAIN"
(See
for where this happens?)When split(/,\s+/) is applied to this string (see https://github.com/jruby/jruby-openssl/blob/976a3f5152b36129ad478175473bd63345286450/lib/openssl/ssl.rb#L273C9-L273C32), the returned array is
[
"subjectAltName = critical",
"otherName:[1.3.6.1.4.1.311.20.2.3",
"[CONTEXT 0]HOST1$@example.com];DNS:host1.example.com",
"DNS:example.com", "DNS:MYDOMAIN"
]
The DNS:host1.example.com entry will not be found and extracted by the regex in if /\ADNS:(.*)/ (https://github.com/jruby/jruby-openssl/blob/976a3f5152b36129ad478175473bd63345286450/lib/openssl/ssl.rb#L277C11-L277C26)
Activity
alexjfisher commentedon Jan 3, 2025
@kares In f18c794 it looks like you might have tested the MRI openssl implementation and decided the custom approach was still needed???
There does seem to be an issue using the version from https://github.com/ruby/openssl as-is. Specifically,
san.value(https://github.com/ruby/openssl/blob/e5153dbbb4baa568b48bf2caf9af9b79dbfe2f1b/lib/openssl/ssl.rb#L323C54-L323C63) doesn't return a raw string injruby-opensslcompared toruby-openssl, but a single element Array ofOpenSSL::ASN1::OctetStringinstead??alexjfisher commentedon Jan 3, 2025
I've had a go at adding a test and fixing this issue. I'm not confident in my approach though. PR to follow shortly.
verify_certificate_identity#325kares commentedon Jan 6, 2025
yes, unfortunately the ASN.1 parsing isn't 100% complete and there are differences in how the
ext.valueis presented. there were changes (fixes) to ASN.1 since but still brittle IMO.changes (e.g. "Same here.
san.valueis changed tosan.value.last.value") in your PR is risky to be put in. those changes would require more tests.