-
Notifications
You must be signed in to change notification settings - Fork 139
Description
To be fair I think there is 0 blame here on rcgen author,
the usage of feature flags in x509-parser is very confusing and took me a while to figure out what was going on.
Seems that in master branch tip it is fixed, but for the current 0.18 release you need both verify and verify-aws featurs for aws-lc-rs users. Otherwise you'll get errors such as:
csr.verify_signature()
| ^^^^^^^^^^^^^^^^ method not found in `X509CertificationRequest<'_>
If you look at the code you'll see that it is defined in 0.18 as:
/// Verify the cryptographic signature of this certification request
///
/// Uses the public key contained in the CSR, which must be the one of the entity
/// requesting the certification for this verification to succeed.
#[cfg(feature = "verify")]
pub fn verify_signature(&self) -> Result<(), X509Error> {
let spki = &self.certification_request_info.subject_pki;
verify_signature(
spki,
&self.signature_algorithm,
&self.signature_value,
self.certification_request_info.raw,
)
}
What I do wonder now... how does this passes your own E2E tests though, as I would hope you have something like hack setup where you do test this?
Seems you do not (https://github.com/rustls/rcgen/blob/main/.github/workflows/ci.yml)... so perhaps that's why this wasn't seen yet?
Happy to make a PR for you if you are overloaded as-is, for both the feature flag fix as well as hardening your CI / tests.
Up to you all.
For now users can easily fix it by adding the feature flag themselves by adding the x509-parser dep to their own direct dep tree as feature flags always work addative, so you can just add feature flags if not defined yet. Removing is sadly for now still impossible, but that's not required here, so no blocker. Should you need more time for a proper fix (e.g. waiting for a x509-parser release).