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

fix(convert_pem_str_to_cert): always return the first cert #69

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

Conversation

jimethn
Copy link

@jimethn jimethn commented Oct 11, 2024

All usages of it expect it to return a single cert, so if the files contain multiple certs it crashes the program anyway.

All usages of it expect it to return a single cert, so if the files
contain multiple certs it crashes the program anyway.
@Kishi85
Copy link
Contributor

Kishi85 commented Oct 24, 2024

The reason why convert_pem_str_to_cert can return a list is to allow for intermediate CAs to be provided (not a use-case when using Let's Encrypt) when parsing the CA part of the ACME response and this PR would break that behaviour by just returning the first CA of the chain.

Could you please open an issue for this providing a backtrace of the crash so we can fix the broken part that does not work with a list return value (at first glance the issue might be tool.is_cert_valid). Also likely depends on which ACME provider you are using as this does not happen with Let's Encrypt otherwise it would have occurred during testing.

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.

2 participants