verification: fill in policy API internals #9642
Conversation
Signed-off-by: William Woodruff <[email protected]> validation: remove Profile abstract from public APIs One step towards removing it entirely Signed-off-by: William Woodruff <[email protected]> policy: disambiguate references Signed-off-by: William Woodruff <[email protected]> policy: remove separate rfc5280 profile Signed-off-by: William Woodruff <[email protected]> policy: remove profile abstraction entirely Signed-off-by: William Woodruff <[email protected]> rust: permitted_algorithms filtering Signed-off-by: William Woodruff <[email protected]> verify: simplify policy API substantially No more manual monomorphization. Signed-off-by: William Woodruff <[email protected]> src, tests: remove verification code Signed-off-by: William Woodruff <[email protected]> validation: remove more validation code Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Not used, yet. Signed-off-by: William Woodruff <[email protected]>
This reverts commit a5154e1.
Will include these in a subsequent PR. Signed-off-by: William Woodruff <[email protected]>
Not needed yet. Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
No configurable profile, Web PKI only. Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
As part of the MVP. Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
Not part of these changes. Signed-off-by: William Woodruff <[email protected]>
Not needed yet; not part of MVP. Signed-off-by: William Woodruff <[email protected]>
| pub fn new(ops: B, subject: Option<Subject<'a>>, time: asn1::DateTime) -> Self { | ||
| /// Create a new policy with defaults for the certificate profile defined in | ||
| /// the CA/B Forum's Basic Requirements. | ||
| pub fn webpki(ops: B, subject: Option<Subject<'a>>, time: asn1::DateTime) -> Self { |
There was a problem hiding this comment.
Bad merge, will fix.
There was a problem hiding this comment.
Fixed in 90e502b -- I kept the doc change since the defaults are changing internally, but I can also revert that if you'd prefer.
| pub(crate) critical_ca_extensions: HashSet<ObjectIdentifier>, | ||
| pub(crate) critical_ee_extensions: HashSet<ObjectIdentifier>, |
There was a problem hiding this comment.
Do I understand right that the point of these to enforce that some extensions must be marked critical?
@reaperhulk let me know if you disagree, but I don't think we care about enforcing that: it's a lot of compatibility cost, for effectively no gain. I don't believe any other validators enforce "this extension must be marked critical", just the inverse (rejecting unknown critical extensions).
There was a problem hiding this comment.
Do I understand right that the point of these to enforce that some extensions must be marked critical?
Yep, that understanding is right.
I'll reconfirm against the WIP limbo suite, but I believe we found that at OpenSSL, at least, seems to care when some mandatory-critical extensions aren't marked as such.
There was a problem hiding this comment.
Just checked: OpenSSL and Go's crypto/x509 both reject certificates with AKI/SKI marked as critical, as RFC 5280 explicitly states that both MUST NOT be critical.
Both also check whether basicConstraints is critical, as is mandated under 5280.
Bad merge. Signed-off-by: William Woodruff <[email protected]>
Signed-off-by: William Woodruff <[email protected]>
There was a problem hiding this comment.
I'm going to apologize, because I think this is a recurring theme in my reviews:
This PR brings in the APIs around customizing the verification logic in various ways.
I think at this point it'd be better to get a minimal path verifier, and then layer these additional validations on top of that, rather than build in the extension points and then try to do a complete verifier.
Happy to hear arguments the other way, but I think it'll be much easier to review if we can do the functional pieces and then generalize.
| /// Creates a new policy with the given `CryptoOps`, an optional subject, | ||
| /// and a validation time. | ||
| /// Create a new policy with defaults for the certificate profile defined in | ||
| /// the CA/B Forum's Basic Requirements. |
There was a problem hiding this comment.
The only default from CAB here is the permitted algorithms and the EKU right?
There was a problem hiding this comment.
Yep, the actual policy logic itself isn't filled in yet.
Nope, you're absolutely right -- I'll make a concerted effort to pare this and the other stack of PRs down. |
Rust-only, unused, non-MVP. Signed-off-by: William Woodruff <[email protected]>
This follows #9405 by rounding out the internals of the Rust-side
PolicyAPI.Break-out from #8873.