-
Notifications
You must be signed in to change notification settings - Fork 106
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
Skip expired certs in partial chain hook #328
Skip expired certs in partial chain hook #328
Conversation
Demonstrating the fix, starting by reproducing the problem described in #327:
Here, Erlang/OTP selects the longest chain (with DST Root CA X3 as the root) and passes it to Mint's In the same container, starting a new IEx session with the branch from this PR:
Now Mint skips over the expired DST Root CA X3 in the chain, and starts looking for a trusted CA from the cross-signed ISRG Root X1 CA sent by the server. Since the public key of this certificate matches the ISRG Root X1 public key in the trust store, Mint returns it as the trusted CA. This time Erlang/OTP is happy with the (shortened) chain, since all certificates are valid. Verification still works once the DST Root CA X3 certificate is removed from the trust store:
|
This is great, thank you! I have done an initial review and it looks good. Let us know when it's ready for final review. 💜💜💜💜💜💜 |
I suppose it is. I will run a few more manual tests like the one above on other OTP versions, just to be sure. Should be able to do that later today... |
Ok, additional testing with the following OTP versions was successful:
Of these versions, only 24.0.3 requires the fix in this PR. The other versions do not require the fix; they were tested for regressions. |
@voltone did you do the testing manually? Do you think it would be beneficial to add those tests on those versions to CI? |
Yes, the end-to-end tests were done manually, using my blog as the server. Testing them in CI would require a stable endpoint that presents a suitable chain, and that will continue to do so indefinitely. Most servers with a Let's Encrypt certificate would probably work, at least for the time being, however:
How about I create a PR after Oct 1st that will check for regressions, without |
That sounds like the most practical solution. |
Co-authored-by: Eric Meadows-Jönsson <eric.meadows.jonsson@gmail.com>
Hey @ericmj @whatyouhide, will you be publishing a new release of Mint before the end of the month, in case people need it to resolve issues as a result of the DST Root CA expiry? |
Done! Thanks again for your PR and research into this issue! 💜 |
Fixes #327.
Opened as draft since I want to review the changes once more after clearing my head a bit, but it seems to work in testing...
BTW, note that on OTP 23.3.4.5 and 24.0.4 and later,
partial_chain
is no longer necessary at all. See https://blog.voltone.net/post/30. But I suppose it will be a good number of years before Mint can drop thepartial_chain
implementation.