Skip to content

Allow multiple Certificates with the same SubjectDN in the store #198

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

Merged

Conversation

jpsikorra
Copy link
Contributor

Description

The current implementation of OpenSSL::X509::Store in jruby-openssl does not allow multiple X509Certificates with the same SubjectDN (see Certificate.java#L57). This differs from the C implementation. This may be tested by adding two certificates with the same SubjectDN to a OpenSSL::X509::Store and executing verify on certificate signed by one of them. It will work in CRuby and will not work in JRuby depending on the order the certificates are added.

Changes

The matches method of Certificate was updated to compare on hashCode().
Because getFirstIssuer() expects a grouping based on SubjectX500Principal of the certificate objects in the store, this was implemented, too.

How this was tested

I tested if this change will allow the usage of multiple certificates with the same subjectdn on windows and linux. More tests are probably necessary.

@headius headius merged commit 4d9fd5e into jruby:master May 15, 2020
@headius headius added this to the 0.10.5 milestone May 15, 2020
Copy link
Member

@kares kares left a comment

Choose a reason for hiding this comment

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

unfortunately this is already merged, we missed a test-case for the behaviour.
believe the copy-ing attempt to insert after the same name cert isn't working.
could we at least understand why you decided to do that, MRI compatibility?

@jpsikorra
Copy link
Contributor Author

The insert after the same-named cert is done because of the way getFirstIssuer works. It finds the first certificate with a matching subject. Then it starts from that index to look for a real match. As soon as the subject does not match, it returns. That implies that certificates in the store must be ordered by subject if multiple certificates with the same subject exist.

kares added a commit that referenced this pull request Oct 25, 2021
an attempt to have a test for the changes from #198
@kares
Copy link
Member

kares commented Oct 25, 2021

the store updates presented in the PR are reverted in 0.11.0 - there's 2 reasons for that:

  1. we've seen a regression Certificate verify failed: cert chain from bundle with two roots with the same subject name #232 blaming this change
    (unfortunately we do not have a reproducer and I am failing to come up with one)
  2. the change actually blocked the OpenSSL 1.1.1 verification port: OpenSSL 1.1.1 cert verification port #239
    (and took a while to figure out - since the cert changing logic isn't found in the C code)

the cert check part is staying and as noted in the desc - there's similar code in OpenSSL itself.

also tried to add a test for the same-subject in store behavior: 2950531 presented here, however it would have been better to have an explicit test in the PR

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.

3 participants