Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: 32_x509_get_cert_info allow single colon. #487

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sebastianas
Copy link

Starting with 3.4.0 the double colon in emailAddress has been removed. Adapt the test to allow a single colon in 3.4.0 and later.

The commit in question is openssl/openssl@de8861a7e3100 ("Remove duplicate colon in otherName display")

Starting with 3.4.0 the double colon in emailAddress has been removed.
Adapt the test to allow a single colon in 3.4.0 and later.

Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
@h-vn
Copy link
Contributor

h-vn commented Oct 29, 2024

Thanks for the patch and looking up the respective OpenSSL commit. It helps to verify that there's no functional change, other than print output, behind this.

@axon-obriend
Copy link

This is preventing Finance::Quote from building. It would be great to get this merged. Thanks!

@blackberryoctopus
Copy link

This is preventing the install of Net::SSLeay on macOS. It would be super helpful to get this PR merged.
I noticed you are a contributor to the project, @h-vn. How can this PR be prioritized for review?

@Jacob-Mellichamp
Copy link

Preventing x86 MSWin32 from building as well. I was able to get the Unit test to work with this change. Thank you @sebastianas

@@ -188,6 +188,10 @@ for my $f (keys (%$dump)) {
) {
$ext_data =~ s{(othername:) [^, ]+}{$1<unsupported>}g;
}
# Starting with 3.4.0 the double colon in emailAddress has been removed.
Copy link

@dolmen dolmen Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful to give here a reference to the commit of the fix in the OpenSSL repo:
openssl/openssl@de8861a7e3100

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's actually hilarious that a unit test change happened in the openssl repo and had this ripple effect. I wonder how many other languages had a similar problem transpire

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Starting with 3.4.0 the double colon in emailAddress has been removed.
# Starting with 3.4.0 the double colon in emailAddress has been removed.
# See https://github.com/openssl/openssl/commit/de8861a7e3100

@dolmen
Copy link

dolmen commented Jan 20, 2025

@kiwiroy
Copy link

kiwiroy commented Jan 26, 2025

Using perl ./examples/x509_cert_details.pl -dump -pem t/data/extended-cert.cert.pem > t/data/extended-cert.cert.dump against openssl version 3.4.0 indicates the change in output for nid 85 and 86.

diff --git a/t/data/extended-cert.cert.dump b/t/data/extended-cert.cert.dump
index 84ccf63..0ec0dd3 100644
--- a/t/data/extended-cert.cert.dump
+++ b/t/data/extended-cert.cert.dump
@@ -64,7 +64,7 @@
                                      },
                                      {
                                        critical => 0,
-                                       data => "email:intermediate-ca\@net-ssleay.example, URI:http://intermediate-ca.net-ssleay.example, DNS:intermediate-ca.net-ssleay.example, Registered ID:1.2.0.0, IP Address:192.168.0.1, IP Address:FD25:F814:AFB5:9873:0:0:0:1, othername: emailAddress::ica\@net-ssleay.example",
+                                       data => "email:intermediate-ca\@net-ssleay.example, URI:http://intermediate-ca.net-ssleay.example, DNS:intermediate-ca.net-ssleay.example, Registered ID:1.2.0.0, IP Address:192.168.0.1, IP Address:FD25:F814:AFB5:9873:0:0:0:1, othername: emailAddress:ica\@net-ssleay.example",
                                        ln => "X509v3 Issuer Alternative Name",
                                        nid => 86,
                                        oid => "2.5.29.18",
@@ -80,7 +80,7 @@
                                      },
                                      {
                                        critical => 0,
-                                       data => "email:john.doe\@net-ssleay.example, URI:http://johndoe.net-ssleay.example, DNS:johndoe.net-ssleay.example, Registered ID:1.2.3.4, IP Address:192.168.0.2, IP Address:FD25:F814:AFB5:9873:0:0:0:2, othername: emailAddress::jd\@net-ssleay.example",
+                                       data => "email:john.doe\@net-ssleay.example, URI:http://johndoe.net-ssleay.example, DNS:johndoe.net-ssleay.example, Registered ID:1.2.3.4, IP Address:192.168.0.2, IP Address:FD25:F814:AFB5:9873:0:0:0:2, othername: emailAddress:jd\@net-ssleay.example",
                                        ln => "X509v3 Subject Alternative Name",
                                        nid => 85,
                                        oid => "2.5.29.17",
@@ -205,7 +205,6 @@
   ns_cert_type     => [],
   pubkey_alg       => "rsaEncryption",
   pubkey_bits      => 2048,
-  pubkey_security_bits => 112,
   pubkey_id        => 6,
   pubkey_size      => 256,
   serial           => { dec => 2, hex => "02", long => 2 },

Will fix #493, #494, and #511

@abraxxa
Copy link

abraxxa commented Feb 7, 2025

Can a maintainer please review and merge this and get a new version released to CPAN?
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants