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

certificate_complete_chain: handle duplicate intermediate subjects #403

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Fixes #399.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

certificate_complete_chain

Copy link
Collaborator

@Ajpantuso Ajpantuso left a comment

Choose a reason for hiding this comment

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

LGTM, but just wondering if there's a better key value that would trivialize this such as using the cert fingerprint instead of the subject?

@felixfontein
Copy link
Contributor Author

I guess one could also include a public key fingerprint into the key, but that's not always available in the intermediate / leaf certificate.

The module doesn't do proper chain building (which would be required for validating a certificate) anyway, and will not properly handle cases such as multiple paths; it only does an approximation which allows to pick up the correct parts of a chain in the common case. (My personal case is: given a full chain, pick the root from the system's root store.)

@Ajpantuso
Copy link
Collaborator

I guess one could also include a public key fingerprint into the key, but that's not always available in the intermediate / leaf certificate.

I was suggesting a fingerprint of the cert, but that's effectively the same thing as using the cert object as the key (at least with the current rust backend). But again, this should work fine as is.

@felixfontein felixfontein merged commit 11a1454 into ansible-collections:main Feb 14, 2022
@patchback
Copy link

patchback bot commented Feb 14, 2022

Backport to stable-1: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-1/11a14543c8be4d5efb5cabad23a9222aa6ee4d0d/pr-403

Backported as #405

🤖 @patchback
I'm built with octomachinery and
my source is open — https:/sanitizers/patchback-github-app.

@felixfontein felixfontein deleted the certificate_complete_chain-intermediate-subject branch February 14, 2022 12:29
patchback bot pushed a commit that referenced this pull request Feb 14, 2022
)

* Allow multiple intermediate CAs to have same subject.

* Add tests.

* Fix test name.

* Don't use CN for SAN.

* Make a bit more compatible.

* Include jinja2 compat for CentOS 6.

(cherry picked from commit 11a1454)
@felixfontein
Copy link
Contributor Author

@Ajpantuso thanks for reviewing this!
@shk3bq4d thanks testing this!

felixfontein added a commit that referenced this pull request Feb 14, 2022
) (#405)

* Allow multiple intermediate CAs to have same subject.

* Add tests.

* Fix test name.

* Don't use CN for SAN.

* Make a bit more compatible.

* Include jinja2 compat for CentOS 6.

(cherry picked from commit 11a1454)

Co-authored-by: Felix Fontein <[email protected]>
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.

certificate_complete_chain fails with multiple intermediates with same CN (and same root)
2 participants