Rework crypto API

[djc]

Here's my initial work towards fixing #1.

I feel like the DKIM signing API is a bit strange? It feels mistyped that the input for Signature::sign() is a Signature and the output is, too. Notably in this case Signature has an a field for algorithm (I would probably opt to make the field names expanded idiomatic snake_case and add comments to clarify the on-the-wire short name) which gets a default value and then there's some potentially surprising/weird logic to resolve that with the type of the key passed in. In my current version I've turned a mismatch into a run-time error, but this seems like the kind of thing that could be prevented by using different types for input and output.

I'm also confused about the strategy for the public API. All the internal modules seem public, which means that by moving PrivateKey into common::crypto it becomes a bit harder to reach for, but so far there are no pub use declarations in sight. (I also find the style of keeping struct definitions in different modules than impl blocks relatively hard to navigate.)

Curious to hear your feedback!

[mdecimus]

Thank you for the PR! Overall it looks great, my comments are in the review.

I feel like the DKIM signing API is a bit strange? It feels mistyped that the input for Signature::sign() is a Signature and the output is, too.

The sign function can only succeed if a domain, selector and headers were provided. I am returning self just so sign could be chained with one of the functions that generate the headers, for instance:

let h = Signature::new()
                .headers([
                    "From",
                    "To",
                    "Subject",
                    "X-Duplicate-Header",
                    "X-Does-Not-Exist",
                ])
                .domain("example.com")
                .selector("default")
                .header_canonicalization(Canonicalization::Simple)
                .body_canonicalization(Canonicalization::Simple)
                .sign(message_multiheader.as_bytes(), &pk_rsa)
                .unwrap()
                .to_header();

However, if you think this is going to confuse API users I am fine with the function changing to fn sign(&mut self, ...) -> crate::Result<()>.

I'm also confused about the strategy for the public API. All the internal modules seem public, which means that by moving PrivateKey into common::crypto it becomes a bit harder to reach for, but so far there are no pub use declarations in sight. (I also find the style of keeping struct definitions in different modules than impl blocks relatively hard to navigate.)

Originally all internal modules and structs were private (pub (crate)). However, while writing the fuzz tests I had to make some modules public in order to be able to access the functions I wanted to fuzz. At the time I did not have time to investigate how to fuzz the private functions of a library using cargo fuzz so for the time being I left them as public. If you have suggestions please let me know!

Thanks.

[djc]

The sign function can only succeed if a domain, selector and headers were provided.

This is a nice and clear invariant but it's not expressed in the type system as such. It might make sense to instead have a Signature::builder() with gives you a type that only exposes a headers() method, which then gives you a type that only exposes a domain() method, then one with selector() which exposes the sign() method and any optional setters. This requires a little boilerplate in the implementation but makes the public API less fallible at runtime. IMO ideally you'd only get a Signature type after calling sign().

If you have suggestions please let me know!

I would recommend moving all the API that's only public for the purposes of fuzzing into a module; rustls calls it internals and serde exposes it as __private, with #[doc(hidden)] to make the intention clear. I usually use cargo doc to review whether I think the public API is nicely minimal and clearly structured.

[mdecimus]

Thanks, I just merged the commit. I'll work on the SignatureBuilder and moving the fuzz functions into a module as soon as I have some time.