From 21cb192395adf4912f1a247f416f0197972b8f27 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 21 Feb 2024 02:21:26 +0000 Subject: [PATCH] Use `bitcoin_hashes` rather than `ring` for hashing While `ring` is great, it struggles with platform support and has a fairly involved dependency tree due to its reliance on C backends. Further, while the `RustCrypto` org tries to stick to Rust, in doing so it takes on more (unnecessary) dependencies and has a particularly unusable MSRV policy. Finally, its contributor base has historically not been particularly friendly. Thus, for the best platform support, we'd like to avoid both. Here we take the first of several steps towards that goal, using `bitcoin_hashes` for our SHA-1/SHA-2 operations instead. --- Cargo.toml | 3 ++- src/crypto/hash.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++ src/crypto/mod.rs | 22 +++++++++++++++++++ src/http.rs | 2 ++ src/lib.rs | 2 ++ src/ser.rs | 2 +- src/validation.rs | 10 ++++----- 7 files changed, 89 insertions(+), 7 deletions(-) create mode 100644 src/crypto/hash.rs create mode 100644 src/crypto/mod.rs diff --git a/Cargo.toml b/Cargo.toml index e94264d..71db9ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,12 +16,13 @@ features = ["std", "validation", "tokio"] [features] default = ["validation"] std = [] -validation = ["ring", "hex_lit"] +validation = ["bitcoin_hashes", "ring", "hex_lit"] tokio = ["tokio_crate/net", "tokio_crate/io-util", "std"] build_server = ["tokio", "tokio_crate/rt-multi-thread", "tokio_crate/macros"] [dependencies] ring = { version = "0.17", default-features = false, features = ["alloc"], optional = true } +bitcoin_hashes = { version = "0.13", default-features = false, optional = true } hex_lit = { version = "0.1", default-features = false, features = ["rust_v_1_46"], optional = true } tokio_crate = { package = "tokio", version = "1.0", default-features = false, optional = true } diff --git a/src/crypto/hash.rs b/src/crypto/hash.rs new file mode 100644 index 0000000..8b913de --- /dev/null +++ b/src/crypto/hash.rs @@ -0,0 +1,55 @@ +//! Simple wrapper around various hash options to provide a single enum which can calculate +//! different hashes. + +use bitcoin_hashes::Hash; +use bitcoin_hashes::HashEngine as _; +use bitcoin_hashes::sha1::Hash as Sha1; +use bitcoin_hashes::sha256::Hash as Sha256; +use bitcoin_hashes::sha512::Hash as Sha512; + +pub(crate) enum Hasher { + Sha1(::Engine), + Sha256(::Engine), + #[allow(unused)] + Sha512(::Engine), +} + +pub(crate) enum HashResult { + Sha1(Sha1), + Sha256(Sha256), + Sha512(Sha512), +} + +impl AsRef<[u8]> for HashResult { + fn as_ref(&self) -> &[u8] { + match self { + HashResult::Sha1(hash) => hash.as_ref(), + HashResult::Sha256(hash) => hash.as_ref(), + HashResult::Sha512(hash) => hash.as_ref(), + } + } +} + +impl Hasher { + pub(crate) fn sha1() -> Hasher { Hasher::Sha1(Sha1::engine()) } + pub(crate) fn sha256() -> Hasher { Hasher::Sha256(Sha256::engine()) } + #[allow(unused)] + pub(crate) fn sha512() -> Hasher { Hasher::Sha512(Sha512::engine()) } + + pub(crate) fn update(&mut self, buf: &[u8]) { + match self { + Hasher::Sha1(hasher) => hasher.input(buf), + Hasher::Sha256(hasher) => hasher.input(buf), + Hasher::Sha512(hasher) => hasher.input(buf), + } + } + + pub(crate) fn finish(self) -> HashResult { + match self { + Hasher::Sha1(hasher) => HashResult::Sha1(Sha1::from_engine(hasher)), + Hasher::Sha256(hasher) => HashResult::Sha256(Sha256::from_engine(hasher)), + Hasher::Sha512(hasher) => HashResult::Sha512(Sha512::from_engine(hasher)), + } + } +} + diff --git a/src/crypto/mod.rs b/src/crypto/mod.rs new file mode 100644 index 0000000..4c501c8 --- /dev/null +++ b/src/crypto/mod.rs @@ -0,0 +1,22 @@ +//! Implementations of cryptographic verification +//! +//! Sadly, the choices for cryptographic verification in Rust are somewhat limited. For us (RSA and +//! secp256r1/secp384r1) there's really only `ring` and `RustCrypto`. +//! +//! While `ring` is great, it struggles with platform support and has a fairly involved dependency +//! tree due to its reliance on C backends. +//! +//! `RustCrypto`, on the other hand, tries to stick to Rust, which is great, but in doing so takes +//! on more (unnecessary) dependencies and has a particularly unusable MSRV policy. Thus, its +//! somewhat difficult to take on as a dependency. +//! +//! Instead, we go our own way here, and luckily actually implementing the required algorithms +//! isn't all that difficult, at least if we're okay with performance being marginally sub-par. +//! Because we don't ever do any signing, we don't need to worry about constant-time-ness, further +//! reducing complexity. +//! +//! While we could similarly go our own way on hashing, too, rust-bitcoin's `bitcoin_hashes` crate +//! does what we need without any unnecessary dependencies and with a very conservative MSRV +//! policy. Thus we go ahead and use that for our hashing needs. + +pub mod hash; diff --git a/src/http.rs b/src/http.rs index 073ba66..9ed34e7 100644 --- a/src/http.rs +++ b/src/http.rs @@ -8,6 +8,8 @@ pub mod rr; pub mod ser; pub mod query; +#[cfg(feature = "validation")] +mod crypto; #[cfg(feature = "validation")] pub mod validation; diff --git a/src/lib.rs b/src/lib.rs index 902f6a3..c0d5f1f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,5 +36,7 @@ pub mod rr; pub mod ser; pub mod query; +#[cfg(feature = "validation")] +mod crypto; #[cfg(feature = "validation")] pub mod validation; diff --git a/src/ser.rs b/src/ser.rs index 6063ba9..dfd8993 100644 --- a/src/ser.rs +++ b/src/ser.rs @@ -109,7 +109,7 @@ pub(crate) trait Writer { fn write(&mut self, buf: &[u8]); } impl Writer for Vec { fn write(&mut self, buf: &[u8]) { self.extend_from_slice(buf); } } impl Writer for QueryBuf { fn write(&mut self, buf: &[u8]) { self.extend_from_slice(buf); } } #[cfg(feature = "validation")] -impl Writer for ring::digest::Context { fn write(&mut self, buf: &[u8]) { self.update(buf); } } +impl Writer for crate::crypto::hash::Hasher { fn write(&mut self, buf: &[u8]) { self.update(buf); } } pub(crate) fn write_name(out: &mut W, name: &str) { let canonical_name = name.to_ascii_lowercase(); if canonical_name == "." { diff --git a/src/validation.rs b/src/validation.rs index cfcb66f..5d75726 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -7,6 +7,7 @@ use core::cmp; use ring::signature; +use crate::crypto; use crate::rr::*; use crate::ser::write_name; @@ -185,13 +186,12 @@ where T: IntoIterator, I: Iterator + Clone { for ds in dses.clone() { if ds.alg != dnskey.alg { continue; } if dnskey.key_tag() == ds.key_tag { - let alg = match ds.digest_type { - 1 if trust_sha1 => &ring::digest::SHA1_FOR_LEGACY_USE_ONLY, - 2 => &ring::digest::SHA256, - 4 => &ring::digest::SHA384, + let mut ctx = match ds.digest_type { + 1 if trust_sha1 => crypto::hash::Hasher::sha1(), + 2 => crypto::hash::Hasher::sha256(), + // TODO: 4 => crypto::hash::Hasher::sha384(), _ => continue, }; - let mut ctx = ring::digest::Context::new(alg); write_name(&mut ctx, &dnskey.name); ctx.update(&dnskey.flags.to_be_bytes()); ctx.update(&dnskey.protocol.to_be_bytes()); -- 2.39.5