rust: Use extension policy mechanism to check for unaccounted critical extensions #6
Conversation
| @@ -369,6 +356,21 @@ impl<'a, B: CryptoOps> Policy<'a, B> { | |||
| } | |||
| } | |||
|
|
|||
| // Check that all critical extensions in this certificate are accounted for. | |||
| for extension in extensions.iter() { | |||
There was a problem hiding this comment.
Wasn't exactly sure how best to unit test this in Rust. Perhaps via the chain validation API (so in cryptography-x509-validation/src/lib.rs)?
There was a problem hiding this comment.
I think we should probably do it with a limbo test (if there isn't one already?) -- a leaf/intermediate with an unknown critical extension should fail validation.
(It's unclear to me whether a root with an unknown critical extension should fail validation -- roots are defined as a representation of a trust authority, and a TA is defined just in terms of a public key and subject.)
| let policies_handle_extension = |policies: &Vec<ExtensionPolicy<B>>| { | ||
| policies.iter().any(|x| x.oid == extension.extn_id) | ||
| }; | ||
| if !policies_handle_extension(&self.common_extension_policies) | ||
| && !policies_handle_extension(&self.ca_extension_policies) | ||
| && !policies_handle_extension(&self.ee_extension_policies) | ||
| { | ||
| return Err("certificate contains unaccounted critical extension".into()); | ||
| } |
There was a problem hiding this comment.
FWIW, you can probably reduce this indirection by doing Iterator::chain + Iteratory::any instead, or set subtraction between the two. I'll play around with it and push something up 🙂
Signed-off-by: William Woodruff <[email protected]>
woodruffw
force-pushed
the
alex/critical-extension-check
branch
from
e823b33 to
9916348
Compare
| // Check that all critical extensions in this certificate are accounted for. | ||
| let critical_extensions = extensions | ||
| .iter() | ||
| .filter(|e| e.critical) | ||
| .map(|e| e.extn_id) | ||
| .collect::<HashSet<_>>(); | ||
| let checked_extensions = self | ||
| .common_extension_policies | ||
| .iter() | ||
| .chain(self.ca_extension_policies.iter()) | ||
| .chain(self.ee_extension_policies.iter()) | ||
| .map(|p| p.oid.clone()) | ||
| .collect::<HashSet<_>>(); | ||
| let unchecked_extensions = critical_extensions | ||
| .difference(&checked_extensions) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| if !unchecked_extensions.is_empty() { | ||
| // TODO: Render the OIDs here. | ||
| return Err("certificate contains unaccounted-for critical extensions".into()); | ||
| } |
There was a problem hiding this comment.
This is slightly more verbose, but should be more efficient than the previous critical extension sweep.
No description provided.