Strengthen validation of end-entity key usage when present#15018
Strengthen validation of end-entity key usage when present#15018sjudson wants to merge 3 commits into
Conversation
|
|
||
| if !key_usage.digital_signature() { | ||
| return Err(ValidationError::new(ValidationErrorKind::Other( | ||
| "EE keyUsage must assert digitalSignature when present".to_string(), |
There was a problem hiding this comment.
At this point we have a keyUsage, so "when present" feels redundant 🙂
| "EE keyUsage must assert digitalSignature when present".to_string(), | |
| "EE keyUsage must assert digitalSignature".to_string(), |
| ))); | ||
| } | ||
|
|
||
| if !key_usage.digital_signature() { |
There was a problem hiding this comment.
Note: CABF 7.1.2.7.11 requires digitalSignature for subscriber certs (i.e. EEs) that convey ECC keys, but does not require it for subscriber certs that convey RSA keys:
This is an incredible mess in practice: the BR says that an RSA-conveying subscriber cert MUST have at least one keyUsage, but no specific one is a MUST in practice unless the validator knows the TLS version being used.
Given that, I think we have two options here:
- Continue to just enforce
digitalSignature. This...probably works fine in practice, even though it's technically stricter than what the BRs say? - Differentiate this check based on the subscriber's key type: for ECC keys it should be the current check, while for RSA keys it should at be at least one of
digitalSignature | keyEncipherment | dataEncipherment.
Curious what @alex and @reaperhulk think about this as well 🙂
There was a problem hiding this comment.
I guess I'm still unsure if we want to enforce this at all, or if we want to allow configuring it on the policy builder? (Or just require users to provide their own extension validator for KU)
There was a problem hiding this comment.
I think it makes sense to enforce in our default policy, since we're otherwise relatively strict about CABF (and the limbo results suggest that other validators check this). But it is indeed somewhat annoying to enforce correctly.
There was a problem hiding this comment.
Do we know if the other validators are key-type aware?
There was a problem hiding this comment.
Do we know if the other validators are key-type aware?
Didn't test that yet, will do so tonight 🙂
There was a problem hiding this comment.
Yes the word "modern" was doing perhaps a bit too much heavy lifting in my initial description.
I've now added distinct handling for RSA that considers keyEncipherment sufficient (and allows both, despite being NOT RECOMMENDED) to be more permissive for the default validation.
3 participants
This PR aligns the default behavior of the server and client certificate verifiers with modern CABF requirements. Along with the existing negative check that
keyCertSignis not set, we add the requirement thatdigitalSignatureis.As before, this default validation can be overridden by way of the
ExtensionPolicyinterface.