-
Notifications
You must be signed in to change notification settings - Fork 68
Introduce new XRequiredIfYPresent EKU policy with tests #372
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #372 +/- ##
==========================================
+ Coverage 97.52% 97.56% +0.04%
==========================================
Files 20 20
Lines 4081 4156 +75
==========================================
+ Hits 3980 4055 +75
Misses 101 101 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
249f37c
to
5abeabd
Compare
It's not obvious to me that we'd want to support this kind of policy in the library. Couldn't you just handle this in an extra validation step? |
Is there an equivalent API exposed by another popular certificate chain verifier library as a point of reference? At face value it seems off to encode a policy this fine-grained into the API. |
This introduces new ExtendedKeyUsage policy, which requires EKU OID X to be present only if EKU OID Y is present. This is to support SPDM EKU OID checks for SPDM-RS, which leverages rustls-webpki. E.g. Requester's cert EKU OID { id-DMTF-spdm 4 } is required when OID { id-DMTF-spdm 3 } is present. Responder's cert EKU OID { id-DMTF-spdm 3 } is required when EKU OID { id-DMTF-spdm 4 } is present. Signed-off-by: Stanislaw Grams <[email protected]>
…Present This introduces three test cases for XRequiredIfYPresent ExtendedKeyUsage policy. Signed-off-by: Stanislaw Grams <[email protected]>
5abeabd
to
086a348
Compare
I wonder if it is instead worth doing something like: impl Cert {
pub fn satisfies_extended_key_usage(&self, eku: ExtendedKeyUsage, _verified_path: &VerifiedPath<'_>)
-> Result<(), Error>;
} That would allow the particular logic to be constructed as needed, and then applied either after path verification, or during it using the existing (The unused |
I don't think there is any similar API proposed as a point of reference, as libspdm (the standard SPDM library) seems to implement this policy on its own and does not reuse any 3rd party verifier. I proposed the PR mainly because I wanted to reuse webpki library but couldn't find a correct way to skip EKU verification within verify_for_usage as eku is not an optional argument and for spdm-rs use case the "required" or "required_if_present" policy will always fail. If you disagree to push this PR, please close it, I'll try to find another solution |
With a casual look this seems closer to what @ctz is proposing. Hopefully (🤞 ) some higher level of libspdm is doing the end to end path building and signature validation or you have bigger issues. I think we would want to manage this similarly and have webpki do it's usual path building & verification, but offering your code a place to interrogate the EKUs for a specific end entity cert after that process has completed successfully. |
If we take this route we should probably check |
The newly defined API is resolving my issue, but it seems it lacks pub re-exports of KeyPurposeId and KeyPurposeIdIter. See #380 |
The spdm-rs library, which utilizes rustls-webpki for certificate validation, encountered an issue with custom EKU OID validation as described in spdm-rs issue #220.
To maintain backward compatibility with SPDM 1.0, spdm-rs must support the following EKU OID verification scenarios:
The current implementation of rustls/webpki does not support this specific EKU policy.
This PR introduces a new policy to enable the required EKU OID validation logic, ensuring proper interoperability with SPDM 1.0 devices.