rust: handle the case where the leaf cert is a CA

[tetsuo-cpp]

No description provided.

[woodruffw]

I might be misunderstanding, but isn't this checked under permits_ca?

https://github.com/trail-of-forks/cryptography/blob/c3074edf7ffc09f466c6ff5e2671d0d3bd533825/src/rust/cryptography-x509-validation/src/policy/mod.rs#L508-L521

https://github.com/trail-of-forks/cryptography/blob/c3074edf7ffc09f466c6ff5e2671d0d3bd533825/src/rust/cryptography-x509-validation/src/policy/mod.rs#L523-L537

[woodruffw]

Whoops, looks like I did indeed misunderstand a bit: I think what we need here is not to add this in permits_basic, but instead to add a new permits_leaf that calls either permits_ca or permits_ee depending on the state of these extensions.

xref trail-of-forks#1 (comment)

[tetsuo-cpp]

22cdb19

[tetsuo-cpp]

The permits_ca function already checks the name constraints extension so I don't think we need to dispatch based on that.

We unfortunately need to leave the key usage check in side permits_ca since it gets used directly in chain building when finding issuers.

[tetsuo-cpp]

@woodruffw Do you mind if I revert this change back to how it used to be?

        let extensions = leaf.extensions()?;
        if let Some(key_usage) = extensions.get_extension(&KEY_USAGE_OID) {
            let key_usage: KeyUsage = key_usage.value()?;
            if key_usage.key_cert_sign() {
                return self.permits_ca(leaf);
            }
        }
        return self.permits_ee(leaf);

There are two things here:

• In the case of a CA certificate, we end up doing a bunch of irrelevant permits_ee first. As you point out, it's not the common case so it's not that big a deal, but it's still something.
• When an EE cert fails validation for whatever reason, it calls permits_ca and then errors out (since it's not a CA). The error message that gets propagated out of the API ends up being a check from permits_ca and can be pretty confusing (it got me while I was debugging).

[woodruffw]

Yeah, that's fine by me 🙂 -- the combinator form is arguably more idiomatic, but your point about the error being masked is a good one.

If you revert this, would you mind adding a short comment explaining? That way I don't try and get clever and refactor it again.