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

v0.101.5 broke semver for CertificateRevocationList.verify_signature #185

Closed
cpu opened this issue Sep 20, 2023 · 6 comments
Closed

v0.101.5 broke semver for CertificateRevocationList.verify_signature #185

cpu opened this issue Sep 20, 2023 · 6 comments

Comments

@cpu
Copy link
Member

cpu commented Sep 20, 2023

ac2faa7 was backported to the v0.101.x release in v0.101.5. This work originated in #164, but shouldn't have been backported to the 0.101.x release stream, because it changes the CertificateRevocationList trait in a semver breaking way, adding a new argument to the verify_signature fn. This trait and its fns are part of the public API, e.g. in rcgen's webpki tests (reported by est31).

Let's fix this in a point release and leave this note in place until then so the issue is discoverable.

@djc
Copy link
Member

djc commented Sep 21, 2023

Sorry for missing this in review. 😞 Seems surprising that our semver check in CI didn't catch this?

@cpu
Copy link
Member Author

cpu commented Sep 21, 2023

Sorry for missing this in review. 😞

It happens :-( I missed it reviewing the orignal work, and in the backport too.

Seems surprising that our semver check in CI didn't catch this?

Agreed. I thought initially the rel-0.101 branch might not have that in its CI, but it does:

semver:
name: Check semver compatibility
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v3
with:
persist-credentials: false
- name: Check semver
uses: obi1kenobi/cargo-semver-checks-action@v2

@cpu
Copy link
Member Author

cpu commented Sep 21, 2023

v0.101.6 resolves this issue.

@cpu cpu closed this as completed Sep 21, 2023
@hawkw
Copy link
Contributor

hawkw commented Sep 21, 2023

Just to make sure I understand this change correctly, does v0.101.6 still contain a fix for GHSA-fh2r-99q2-6mmg/RUSTSEC-2023-0053, or does reverting the semver-breaking change once again make v0.101.6 vulnerable to the CPU exhaustion DoS? Thanks in advance.

@cpu
Copy link
Member Author

cpu commented Sep 21, 2023

Just to make sure I understand this change correctly, does v0.101.6 still contain a fix for https://github.com/advisories/GHSA-fh2r-99q2-6mmg/[RUSTSEC-2023-0053](https://rustsec.org/advisories/RUSTSEC-2023-0053.html)

That's correct: we're still limiting the number of signature validation operations that can occur during path building (including CRL signature validation operations) in v0.101.6. The only change is that the mechanism for doing that hasn't bled into the API.

@hawkw
Copy link
Contributor

hawkw commented Sep 21, 2023

Okay, cool, thanks for confirming!

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

No branches or pull requests

3 participants