Configurable cryptography

[thomcc]

By default it has the use_ring feature enabled, and uses ring. Someone who wants to use openssl can turn on the use_openssl feature, and someone who wants neither (e.g. mozilla/application-services) can turn off both and provide a custom impl with set_cryptographer.

Since there's nothing sane we can do if both features are on, we detect it and give a explicit error message in a build.rs.

Annoyingly, this means testing / linting both requires

cargo test
cargo test --no-default-features --features 'use_openssl'

Cargo doesn't really support mutually exclusive features well.

Note: this needs the latest stable version of Rust to compile (e.g. the one that came out yesterday). I don't know if anything needs to change in the tc config for that to work.

Supports mozilla/application-services#959, assuming @eoger thinks we can shoehorn what we need into the trait I've defined there.

There's probably a way this could have less overhead in the ring/openssl cases, but it would be harder to test, so I didn't go with that. As it is, the dynamic machinery is used every time.

[thomcc]

Note: let's not land this before @eoger gets a chance to review.

[djmitche]

Sure thing. I'd like to get the CI updated before we land, too.

[thomcc]

Given the account that seems prudent :P.

I'll probably need a pointer on how to do that though, unless slapping in a rustup update is acceptable.

[djmitche]

Let's add a section to the README about this, perhaps "Upgrading", that notes the changes here (seems mostly just things now returning Result, but also Key::new no longer taking a reference).

I guess the "dynamic machinery" you indicate is dyn HmacKey etc., meaning that crypto operations involve a vtable call or two. That's sort of sad since it's in the hot path for this library, but it's a hot path that involves a bunch of crypto math so an indirect jump or two probably isn't a big deal.

Otherwise, I'm quite happy with this. I'll fix up the CI in another push, but don't let that slow you down.

[djmitche]

👍

[eoger]

Crypto parts looks good to me, thanks a lot for taking on that work Thom

[djmitche]

I'll get this merged, but I'd like #27 to get merged too before making a release, since otherwise that PR would constitute a further breaking change.

[rfk]

Hi @djmitche, anything else we can do to help with a new release to include this change?

[djmitche]

I think we can do that. I have another PR in the works to support signing http.request.Builder values, that is another breaking change, but there are after all a countably infinite set of possible values for the X in X.Y.Z :)

[djmitche]

v3.0.0 should be on crates.io shortly!