rustls / webpki

WebPKI X.509 Certificate Validation in Rust
https://docs.rs/rustls-webpki/latest/webpki/
Other
96 stars 51 forks source link

How to specify Key Usage and not Extended Key Usage when doing certificate verification? #236

Open clauverjat opened 8 months ago

clauverjat commented 8 months ago

Hello,

I'm currently looking into using rustls-webpki for one of my projects to validate a certificate chain. Upon reviewing the documentation, I identified the webpki::EndEntityCert::verify_for_usage method to do the job and I was able to check my certificate chain and I found the library nice to use.

However, I wanted to give a (hopefully helpful) feedback regarding the EndEntityCert::verify_for_usage method that I find a bit confusing. It caught my attention that the usage argument, which accepts a webpki::KeyUsage struct, is actually used by the method to check the Extended Key Usage ^2, rather than the Key Usage^1. Though this is clearly described in the documentation I found it a bit surprising. The x509v3 format allows specifying "key usages" through both the x509v3 Key Usage and x509v3 Extended Key Usage extensions. The verify_for_usage function can only check the latter but not the former. And I was unable to locate a function in the library to check for the key usage (against the " Key Usage" extension) on the resulting path -- if it existed one could simply provide it through a function passed via the verify_path argument. This leaves me unclear on how I am supposed to ensure that a key is used in compliance with the intended purposes stated in the Key Usage extension.

For a moment I thought that this might be because the Extended Key Usage extension when defined could override the Key Usage extension. However, RFC 5280 Section 4.2.1.12 on Extended Key Usage explicitly states:

If a certificate contains both a key usage extension and an extended key usage extension, then both extensions MUST be processed independently and the certificate MUST only be used for a purpose consistent with both extensions. If there is no purpose consistent with both extensions, then the certificate MUST NOT be used for any purpose.

So this is not the case and the key should only be used for purposes compatible with both extensions.

Given the above, I believe it would be better if rustls-webpki provided a way to check the Key Usage along with the check regarding the Extended Key Usage. But I might have missed something, so if there are specific reasons why webpki only checks the Extended Key Usage I'd be curious to know.

Best regards

ctz commented 8 months ago

It's a correct observation that we don't currently look at the keyUsage extension. But if that were to change I don't think it would appear in the public API, instead:

I don't foresee the need for any other keyUsage checks, because I think that covers the entirety of the needs of this crate's API. There is no need for things like keyAgreement, dataEncipherment and such, and indeed those are "NOT RECOMMENDED" by the Baseline Requirements.

We check EKU, though, because this is a relatively useful control that allows server- and client-auth only issuers, and accomplishing that means EndEntityCert.verify_for_usage() must know if the certificate is being presented in the context of client- or server-auth.

clauverjat commented 8 months ago

Thanks Joe for these informations. I still think that there'll be value in making the keyUsage verification part of the public API. To give a little more background on what I want to do, the certificate chain I want to validate is not used in a TLS connection but as way to verify a TPM's Attestation Identity Key. From your response and the documentation, it seems that webpki is very much focused on TLS certificate validation (I guess it's also the reason for the name webpki).

That being said, I think Rust would very much benefit from a library that does certificate validation for other use cases. You might answer that I can / should use OpenSSL bindings for that... I honestly don't like relying on a C library for parsing untrusted data and making security critical decisions... and OpenSSL security track record is not reassuring me (should I even mention Heartbleed?).

By the way I should also mention that I almost got stuck using verify_for_usage for my usecase. You see the "KeyUsage" that is exposed by the library can only be constructed through webpki::KeyUsage methods :

In my case I was able to find a required OID that was used for the extendedKeyUsage (and it's actually an improvement to check it compared to what I was doing previously with openssl which didn't not check any extension for usage). But in general users might want to check a cert chain without caring for its EKU. So having a "any()" method that does not do any check on EKU might be nice. But then, I guess the method verify_for_usage name would be a misnomer... which brings me to my point maybe there should a generic verify method that simply build and verify a certificate chain like OpenSSL's verify_cert function ?

Let me know what you think

ctz commented 8 months ago

Have you considered https://github.com/pyca/cryptography/tree/main/src/rust/cryptography-x509-verification ? As far as I understand, that has a more general goal than this one.

clauverjat commented 8 months ago

Thanks, I wasn't aware of this crate. It looks like what I was searching for. I heard that pyca/cryptography was rewriting some parts in Rust , but I didn't know they had written a crate that could be consumed in isolation from the rest of their project.

There is one caveat though, the crate is not published on crates.io (which also explains why it didn't come up during my initial research). Thus I suspect they don't really support the rust crate per se, the rust crate being an implementation detail for the Python library. Of course that does not mean that we cannot use the crate.

Diving a little more in their GH, I found @reaperhulk's PR https://github.com/pyca/cryptography/pull/8740 in which he states that the goal of his work was to make it possible to publish their implementation as a crate. So it seems the crate may get published in the future.

reaperhulk commented 8 months ago

@clauverjat we haven’t had an explicit ask from anyone on the rust side yet but I’d encourage you to open an issue against us so we can talk about what remaining work is needed to ship a separate crate.