feature(*): Add missing implementation to support Client Certificate Authorization

[MathiasKoch]

This pr does two major changes:

1. Introduce a new CryptoProvider trait

The new trait collects all the user-provided stuff for PKI, such as a source of random number, a verifier if server certificates are to be verified, a signer if client certificate authentication is used, as well as information around which CipherSuite is to be used, and which curve is to be used by the signer.

The trait looks as:

pub trait CryptoProvider {
    type CipherSuite: TlsCipherSuite;
    type SecureRandom: CryptoRngCore;
    type SignatureCurve: CurveArithmetic + DigestPrimitive;

    fn rng(&mut self) -> &mut Self::SecureRandom;

    fn verifier(&mut self) -> Result<&mut impl TlsVerifier<Self::CipherSuite>, crate::TlsError> {
        Err::<&mut NoVerify, _>(crate::TlsError::Unimplemented)
    }

    /// Decode and validate a private signing key from `key_der`.
    fn signer(
        &mut self,
        _key_der: &[u8],
    ) -> Result<Signer<Self::SignatureCurve, Self::SecureRandom>, crate::TlsError> {
        Err::<Signer<Self::SignatureCurve, Self::SecureRandom>, _>(crate::TlsError::Unimplemented)
    }
}

This trait replaces the previous generics of CipherSuite, RNG and Verifier, plus the new Signer that would have been otherwise added and collects them in a single generic, and the tradeoff of forcing users to implement the trait on a struct they provide to TlsContext. To make this a bit less painful, a SimpleProvider is provided by this PR as an easy-to-use way of providing only RNG and a CipherSuite.

2. Implement the remaining logic for Client Certificate Authorization

It implements the missing state ClientCertVerify, as well a providing the means for adding the required signature to prove ownership of the client certificate using the corresponding private key.

It also adds a test_client_certificate_auth integration test, that tests client certificate auth against a rustls server.

ClientCertVerify is described in TLS1.3 RFC8446 section 4.4.3 (https://datatracker.ietf.org/doc/html/rfc8446#section-4.4.3)

Fixes #68 Fixes #14

[MathiasKoch]

@lulf I think I have managed to come up with something I believe in :smile:

Only thing missing is the actual signer (https://github.com/MathiasKoch/embedded-tls/blob/b95e3e7b9d11438ee078c05acbfbdd84f7d9977f/src/connection.rs#L569-L571) Most likely something that implements https://docs.rs/signature/latest/signature/trait.Signer.html

Not quite sure how I should introduce that, with the least annoyance to everyone else? I don't think it fits into a trait called TlsVerifier?

[lulf]

@MathiasKoch Looking good, a separate signer trait makes more sense I guess.

[MathiasKoch]

The client certificate authentication test is currently failing with Err(InvalidCertificate(BadSignature)). I don't quite understand why. I have been trying to compare 1:1 with the implementation in rustls and https://github.com/RustCrypto/rustls-rustcrypto/blob/master/src/sign.rs

I have also set up an equal test using rustls client, with the same client certificates just to verify that the certificates and keys are good, and it passes.

[MathiasKoch]

@lulf Do you have any knowledge about this one? https://github.com/drogue-iot/embedded-tls/pull/135/commits/c8d634dd9b190ca68f6fa00b7c3a6deb1fa7344f

It seems to break my signature if I take end - 1, on the other hand all tests pass with just end? Any recollection on why it was minus 1 before?

[MathiasKoch]

Hmm.. Darn, trying to fix the examples not compiling reveals that there are some lifetime issues around the verifier function in CryptoProvider, that I need to solve for the webpki verifier. The lifetime of the hostname makes it hard..

[MathiasKoch]

@lulf Think I managed to fix the remaining points, and also got rid of the ugly trait bounds! I am really happy with the result of this! It should work for any signatures schemes now.

All I have left on my to-do, is to try to implement this on top of my ATECC crypto chip, but I don't think that will be a blocker for this PR? I can always open another PR with improvements for hardware crypto.

[MathiasKoch]

Hmm.. Damn,

This is still an issue, and the last one remaining.

Hmm.. Darn, trying to fix the examples not compiling reveals that there are some lifetime issues around the verifier function in CryptoProvider, that I need to solve for the webpki verifier. The lifetime of the hostname makes it hard..

[lulf]

@lulf Think I managed to fix the remaining points, and also got rid of the ugly trait bounds! I am really happy with the result of this! It should work for any signatures schemes now.

It looks great, well done! And great that you added tests 👍

All I have left on my to-do, is to try to implement this on top of my ATECC crypto chip, but I don't think that will be a blocker for this PR? I can always open another PR with improvements for hardware crypto.

Not at all, I think this PR looks pretty good as is.

Hmm.. Damn,

This is still an issue, and the last one remaining.

Hmm.. Darn, trying to fix the examples not compiling reveals that there are some lifetime issues around the verifier function in CryptoProvider, that I need to solve for the webpki verifier. The lifetime of the hostname makes it hard..

I'm not particularly keen on keeping the webpki verifier if that helps, it doesn't really work on cortex-m so it's kinda useless in IMHO, and I don't know of any wasm users of embedded-tls.

[MathiasKoch]

Just slightly changed the CryptoProvider trait to drop the associated SecureRandom in favor of a return type bound of impl CryptoRngCore. This makes it much easier to implement in practice.

I'm not particularly keen on keeping the webpki verifier if that helps, it doesn't really work on cortex-m so it's kinda useless in IMHO, and I don't know of any wasm users of embedded-tls.

I think I would like to see the TlsVerifier replaced by something similar to the new signer, where it is made up of RustCrypto traits instead, making it much more composable with stuff in the wild.

I can try to come up with something when I find some time, if this is something you agree with?

[lulf]

Just slightly changed the CryptoProvider trait to drop the associated SecureRandom in favor of a return type bound of impl CryptoRngCore. This makes it much easier to implement in practice.

I'm not particularly keen on keeping the webpki verifier if that helps, it doesn't really work on cortex-m so it's kinda useless in IMHO, and I don't know of any wasm users of embedded-tls.

I think I would like to see the TlsVerifier replaced by something similar to the new signer, where it is made up of RustCrypto traits instead, making it much more composable with stuff in the wild.

I can try to come up with something when I find some time, if this is something you agree with?

Sounds good to me! :rocket:

[MathiasKoch]

@lulf I think this is ready to go now.

I ended up removing the lifetime from TlsVerifier, and in exchange changed the hostname of webpki to an owned heapless String. Think this is a reasonable tradeoff for now, while I look into refactoring the TlsVerifier in a new PR.