From 7c9ee4c668e88facf0a1829601b4ace0a73ce690 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Mon, 4 Nov 2024 09:46:08 +0100 Subject: [PATCH 01/11] [lib] Rewrite feature flag documentation --- Cargo.toml | 2 +- src/lib.rs | 94 +++++++++++++++++++++++++++++++++++------------------- 2 files changed, 62 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 78d0f2eda..8eff4e592 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,9 +61,9 @@ ring = ["dep:ring"] openssl = ["dep:openssl"] # Crate features +net = ["bytes", "futures-util", "rand", "std", "tokio"] resolv = ["net", "smallvec", "unstable-client-transport"] resolv-sync = ["resolv", "tokio/rt"] -net = ["bytes", "futures-util", "rand", "std", "tokio"] tsig = ["bytes", "ring", "smallvec"] zonefile = ["bytes", "serde", "std"] diff --git a/src/lib.rs b/src/lib.rs index 119adc66f..0d0a4a2ba 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -61,61 +61,79 @@ //! //! # Reference of feature flags //! -//! The following is the complete list of the feature flags with the -//! exception of unstable features which are described below. +//! Several feature flags simply enable support for other crates, e.g. by +//! adding `impl`s for their types. They are optional and do not introduce +//! new functionality into this crate. //! //! * `bytes`: Enables using the types `Bytes` and `BytesMut` from the //! [bytes](https://github.com/tokio-rs/bytes) crate as octet sequences. -//! * `chrono`: Adds the [chrono](https://github.com/chronotope/chrono) -//! crate as a dependency. This adds support for generating serial numbers -//! from time stamps. +//! //! * `heapless`: enables the use of the `Vec` type from the //! [heapless](https://github.com/japaric/heapless) crate as octet //! sequences. -//! * `interop`: Activate interoperability tests that rely on other software -//! to be installed in the system (currently NSD and dig) and will fail if -//! it isn’t. This feature is not meaningful for users of the crate. +//! +//! * `smallvec`: enables the use of the `Smallvec` type from the +//! [smallvec](https://github.com/servo/rust-smallvec) crate as octet +//! sequences. +//! +//! Some flags enable support for specific kinds of operations that are not +//! otherwise possible. They are gated as they may not always be necessary +//! and they may introduce new dependencies. +//! +//! * `chrono`: Adds the [chrono](https://github.com/chronotope/chrono) +//! crate as a dependency. This adds support for generating serial numbers +//! from time stamps. +//! //! * `rand`: Enables a number of methods that rely on a random number //! generator being available in the system. -//! * `resolv`: Enables the asynchronous stub resolver via the -#![cfg_attr(feature = "resolv", doc = " [resolv]")] -#![cfg_attr(not(feature = "resolv"), doc = " resolv")] -//! module. -//! * `resolv-sync`: Enables the synchronous version of the stub resolver. -//! * `ring`: Enables crypto functionality via the -//! [ring](https://github.com/briansmith/ring) crate. +//! //! * `serde`: Enables serde serialization for a number of basic types. -//! * `sign`: basic DNSSEC signing support. This will enable the -#![cfg_attr(feature = "unstable-sign", doc = " [sign]")] -#![cfg_attr(not(feature = "unstable-sign"), doc = " sign")] -//! module and requires the `std` feature. Note that this will not directly -//! enable actual signing. For that you will also need to pick a crypto -//! module via an additional feature. Currently we only support the `ring` -//! module, but support for OpenSSL is coming soon. +//! //! * `siphasher`: enables the dependency on the //! [siphasher](https://github.com/jedisct1/rust-siphash) crate which allows //! generating and checking hashes in [standard server //! cookies][crate::base::opt::cookie::StandardServerCookie]. -//! * `smallvec`: enables the use of the `Smallvec` type from the -//! [smallvec](https://github.com/servo/rust-smallvec) crate as octet -//! sequences. +//! //! * `std`: support for the Rust std library. This feature is enabled by //! default. +//! +//! A special case here is cryptographic backends. Certain modules (e.g. for +//! DNSSEC signing and validation) require a backend to provide cryptography. +//! At least one such module should be enabled. +//! +//! * `openssl`: Enables crypto functionality via OpenSSL through the +//! [rust-openssl](https://github.com/sfackler/rust-openssl) crate. +//! +//! * `ring`: Enables crypto functionality via the +//! [ring](https://github.com/briansmith/ring) crate. +//! +//! Some flags represent entire categories of functionality within this crate. +//! Each flag is associated with a particular module. Note that some of these +//! modules are under heavy development, and so have unstable feature flags +//! which are categorized separately. +//! +//! * `net`: Enables sending and receiving DNS messages via the +#![cfg_attr(feature = "net", doc = " [net]")] +#![cfg_attr(not(feature = "net"), doc = " net")] +//! module. +//! +//! * `resolv`: Enables the asynchronous stub resolver via the +#![cfg_attr(feature = "resolv", doc = " [resolv]")] +#![cfg_attr(not(feature = "resolv"), doc = " resolv")] +//! module. +//! +//! * `resolv-sync`: Enables the synchronous version of the stub resolver. +//! //! * `tsig`: support for signing and validating message exchanges via TSIG //! signatures. This enables the #![cfg_attr(feature = "tsig", doc = " [tsig]")] #![cfg_attr(not(feature = "tsig"), doc = " tsig")] -//! module and currently pulls in the -//! `bytes`, `ring`, and `smallvec` features. -//! * `validate`: basic DNSSEC validation support. This feature enables the -#![cfg_attr(feature = "unstable-validate", doc = " [validate]")] -#![cfg_attr(not(feature = "unstable-validate"), doc = " validate")] -//! module and currently also enables the `std` and `ring` -//! features. +//! module and currently enables `bytes`, `ring`, and `smallvec`. +//! //! * `zonefile`: reading and writing of zonefiles. This feature enables the #![cfg_attr(feature = "zonefile", doc = " [zonefile]")] #![cfg_attr(not(feature = "zonefile"), doc = " zonefile")] -//! module and currently also enables the `bytes` and `std` features. +//! module and currently also enables `bytes`, `serde`, and `std`. //! //! # Unstable features //! @@ -137,6 +155,16 @@ //! a client perspective; primarily the `net::client` module. //! * `unstable-server-transport`: receiving and sending DNS messages from //! a server perspective; primarily the `net::server` module. +//! * `unstable-sign`: basic DNSSEC signing support. This will enable the +#![cfg_attr(feature = "unstable-sign", doc = " [sign]")] +#![cfg_attr(not(feature = "unstable-sign"), doc = " sign")] +//! module and requires the `std` feature. In order to actually perform any +//! signing, also enable one or more cryptographic backend modules (`ring` +//! and `openssl`). +//! * `unstable-validate`: basic DNSSEC validation support. This enables the +#![cfg_attr(feature = "unstable-validate", doc = " [validate]")] +#![cfg_attr(not(feature = "unstable-validate"), doc = " validate")] +//! module and currently also enables the `std` and `ring` features. //! * `unstable-validator`: a DNSSEC validator, primarily the `validator` //! and the `net::client::validator` modules. //! * `unstable-xfr`: zone transfer related functionality.. From cea9ae390860f3b098015336a9c449dac6664e27 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Mon, 4 Nov 2024 09:48:49 +0100 Subject: [PATCH 02/11] [workflows/ci] Use 'apt-get' instead of 'apt' --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cbad43917..02a0af673 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: with: rust-version: ${{ matrix.rust }} - if: matrix.os == 'ubuntu-latest' - run: sudo apt install libssl-dev + run: sudo apt-get install -y libssl-dev - if: matrix.os == 'windows-latest' id: vcpkg uses: johnwason/vcpkg-action@v6 @@ -53,7 +53,7 @@ jobs: with: rust-version: "1.68.2" - name: Install OpenSSL - run: sudo apt install libssl-dev + run: sudo apt-get install -y libssl-dev - name: Install nightly Rust run: rustup install nightly - name: Check with minimal-versions From 354bf0a9a678c1eef5e972005447d4015817ea80 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Mon, 4 Nov 2024 10:29:28 +0100 Subject: [PATCH 03/11] [sign] Clarify documentation as per @ximon18 --- src/sign/bytes.rs | 14 ++++++++------ src/sign/common.rs | 8 ++++---- src/sign/mod.rs | 34 ++++++++++++++++++++-------------- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/sign/bytes.rs b/src/sign/bytes.rs index 1187a6dbf..3326ee086 100644 --- a/src/sign/bytes.rs +++ b/src/sign/bytes.rs @@ -184,7 +184,7 @@ impl SecretKeyBytes { mut data: &str, ) -> Result, BindFormatError> { // Look for the 'PrivateKey' field. - while let Some((key, val, rest)) = parse_dns_pair(data)? { + while let Some((key, val, rest)) = parse_bind_entry(data)? { data = rest; if key != "PrivateKey" { @@ -203,7 +203,7 @@ impl SecretKeyBytes { } // The first line should specify the key format. - let (_, _, data) = parse_dns_pair(data)? + let (_, _, data) = parse_bind_entry(data)? .filter(|&(k, v, _)| { k == "Private-key-format" && v.strip_prefix("v1.") @@ -213,7 +213,7 @@ impl SecretKeyBytes { .ok_or(BindFormatError::UnsupportedFormat)?; // The second line should specify the algorithm. - let (_, val, data) = parse_dns_pair(data)? + let (_, val, data) = parse_bind_entry(data)? .filter(|&(k, _, _)| k == "Algorithm") .ok_or(BindFormatError::Misformatted)?; @@ -248,6 +248,7 @@ impl SecretKeyBytes { //--- Drop impl Drop for SecretKeyBytes { + /// Securely clear the secret key bytes from memory. fn drop(&mut self) { // Zero the bytes for each field. match self { @@ -351,7 +352,7 @@ impl RsaSecretKeyBytes { let mut d_q = None; let mut q_i = None; - while let Some((key, val, rest)) = parse_dns_pair(data)? { + while let Some((key, val, rest)) = parse_bind_entry(data)? { let field = match key { "Modulus" => &mut n, "PublicExponent" => &mut e, @@ -413,6 +414,7 @@ impl<'a> From<&'a RsaSecretKeyBytes> for RsaPublicKeyBytes { //--- Drop impl Drop for RsaSecretKeyBytes { + /// Securely clear the secret key bytes from memory. fn drop(&mut self) { // Zero the bytes for each field. self.n.fill(0u8); @@ -428,8 +430,8 @@ impl Drop for RsaSecretKeyBytes { //----------- Helpers for parsing the BIND format ---------------------------- -/// Extract the next key-value pair in a DNS private key file. -fn parse_dns_pair( +/// Extract the next key-value pair in a BIND-format private key file. +fn parse_bind_entry( data: &str, ) -> Result, BindFormatError> { // TODO: Use 'trim_ascii_start()' etc. once they pass the MSRV. diff --git a/src/sign/common.rs b/src/sign/common.rs index fc10803e3..fe0fd1113 100644 --- a/src/sign/common.rs +++ b/src/sign/common.rs @@ -26,10 +26,10 @@ use super::ring; /// A key pair based on a built-in backend. /// -/// This supports any built-in backend (currently, that is OpenSSL and Ring). -/// Wherever possible, the Ring backend is preferred over OpenSSL -- but for -/// more uncommon or insecure algorithms, that Ring does not support, OpenSSL -/// must be used. +/// This supports any built-in backend (currently, that is OpenSSL and Ring, +/// if their respective feature flags are enabled). Wherever possible, it +/// will prefer the Ring backend over OpenSSL -- but for more uncommon or +/// insecure algorithms, that Ring does not support, OpenSSL must be used. pub enum KeyPair { /// A key backed by Ring. #[cfg(feature = "ring")] diff --git a/src/sign/mod.rs b/src/sign/mod.rs index 99bd1f11f..b65384945 100644 --- a/src/sign/mod.rs +++ b/src/sign/mod.rs @@ -8,11 +8,10 @@ //! "offline" (outside of a name server). Once generated, signatures can be //! serialized as DNS records and stored alongside the authenticated records. //! -//! A DNSSEC key actually has two components: a cryptographic key, which can -//! be used to make and verify signatures, and key metadata, which defines how -//! the key should be used. These components are brought together by the -//! [`SigningKey`] type. It must be instantiated with a cryptographic key -//! type, such as [`common::KeyPair`], in order to be used. +//! Signatures can be generated using a [`SigningKey`], which combines +//! cryptographic key material with additional information that defines how +//! the key should be used. [`SigningKey`] relies on a cryptographic backend +//! to provide the underlying signing operation (e.g. [`common::KeyPair`]). //! //! # Example Usage //! @@ -47,12 +46,13 @@ //! This crate supports OpenSSL and Ring for performing cryptography. These //! cryptographic backends are gated on the `openssl` and `ring` features, //! respectively. They offer mostly equivalent functionality, but OpenSSL -//! supports a larger set of signing algorithms. A [`common`] backend is -//! provided for users that wish to use either or both backends at runtime. +//! supports a larger set of signing algorithms (and, for RSA keys, supports +//! weaker key sizes). A [`common`] backend is provided for users that wish +//! to use either or both backends at runtime. //! -//! Each backend module exposes a `KeyPair` type, representing a cryptographic -//! key that can be used for signing, and a `generate()` function for creating -//! new keys. +//! Each backend module (`openssl`, `ring`, and `common`) exposes a `KeyPair` +//! type, representing a cryptographic key that can be used for signing, and a +//! `generate()` function for creating new keys. //! //! Users can choose to bring their own cryptography by providing their own //! `KeyPair` type that implements [`SignRaw`]. Note that `async` signing @@ -237,10 +237,11 @@ impl SigningKey { /// information (for zone signing keys, DNS records; for key signing keys, /// subsidiary public keys). /// -/// Before a key can be used for signing, it should be validated. If the -/// implementing type allows [`sign_raw()`] to be called on unvalidated keys, -/// it will have to check the validity of the key for every signature; this is -/// unnecessary overhead when many signatures have to be generated. +/// Implementing types should validate keys during construction, so that +/// signing does not fail due to invalid keys. If the implementing type +/// allows [`sign_raw()`] to be called on unvalidated keys, it will have to +/// check the validity of the key for every signature; this is unnecessary +/// overhead when many signatures have to be generated. /// /// [`sign_raw()`]: SignRaw::sign_raw() pub trait SignRaw { @@ -281,6 +282,11 @@ pub enum GenerateParams { /// A ~3000-bit key corresponds to a 128-bit security level. However, /// RSA is mostly used with 2048-bit keys. Some backends (like Ring) /// do not support smaller key sizes than that. + /// + /// For more information about security levels, see [NIST SP 800-57 + /// part 1 revision 5], page 54, table 2. + /// + /// [NIST SP 800-57 part 1 revision 5]: https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-57pt1r5.pdf bits: u32, }, From ca10361847a154f77f26e0824139b1ea89f2a862 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Mon, 4 Nov 2024 11:03:29 +0100 Subject: [PATCH 04/11] [sign] Use 'secrecy' to protect private keys --- Cargo.lock | 10 +++++ Cargo.toml | 3 +- src/sign/bytes.rs | 104 +++++++++++++++++--------------------------- src/sign/openssl.rs | 37 +++++++++------- src/sign/ring.rs | 31 ++++++------- 5 files changed, 90 insertions(+), 95 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eaf9191fb..ca7fb4b69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -240,6 +240,7 @@ dependencies = [ "rstest", "rustls-pemfile", "rustversion", + "secrecy", "serde", "serde_json", "serde_test", @@ -1027,6 +1028,15 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "secrecy" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e891af845473308773346dc847b2c23ee78fe442e0472ac50e22a18a93d3ae5a" +dependencies = [ + "zeroize", +] + [[package]] name = "semver" version = "1.0.23" diff --git a/Cargo.toml b/Cargo.toml index 8eff4e592..4c60ad9d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ openssl = { version = "0.10.57", optional = true } # 0.10.57 upgrades to proc-macro2 = { version = "1.0.69", optional = true } # Force proc-macro2 to at least 1.0.69 for minimal-version build ring = { version = "0.17", optional = true } rustversion = { version = "1", optional = true } +secrecy = { version = "0.10", optional = true } serde = { version = "1.0.130", optional = true, features = ["derive"] } siphasher = { version = "1", optional = true } smallvec = { version = "1.3", optional = true } @@ -70,7 +71,7 @@ zonefile = ["bytes", "serde", "std"] # Unstable features unstable-client-transport = ["moka", "net", "tracing"] unstable-server-transport = ["arc-swap", "chrono/clock", "libc", "net", "siphasher", "tracing"] -unstable-sign = ["std", "unstable-validate"] +unstable-sign = ["std", "dep:secrecy", "unstable-validate"] unstable-stelline = ["tokio/test-util", "tracing", "tracing-subscriber", "tsig", "unstable-client-transport", "unstable-server-transport", "zonefile"] unstable-validate = ["bytes", "std", "ring"] unstable-validator = ["unstable-validate", "zonefile", "unstable-client-transport"] diff --git a/src/sign/bytes.rs b/src/sign/bytes.rs index 3326ee086..6393a0aca 100644 --- a/src/sign/bytes.rs +++ b/src/sign/bytes.rs @@ -2,6 +2,7 @@ use core::{fmt, str}; +use secrecy::{ExposeSecret, SecretBox}; use std::boxed::Box; use std::vec::Vec; @@ -89,22 +90,22 @@ pub enum SecretKeyBytes { /// An ECDSA P-256/SHA-256 keypair. /// /// The private key is a single 32-byte big-endian integer. - EcdsaP256Sha256(Box<[u8; 32]>), + EcdsaP256Sha256(SecretBox<[u8; 32]>), /// An ECDSA P-384/SHA-384 keypair. /// /// The private key is a single 48-byte big-endian integer. - EcdsaP384Sha384(Box<[u8; 48]>), + EcdsaP384Sha384(SecretBox<[u8; 48]>), /// An Ed25519 keypair. /// /// The private key is a single 32-byte string. - Ed25519(Box<[u8; 32]>), + Ed25519(SecretBox<[u8; 32]>), /// An Ed448 keypair. /// /// The private key is a single 57-byte string. - Ed448(Box<[u8; 57]>), + Ed448(SecretBox<[u8; 57]>), } //--- Inspection @@ -139,23 +140,27 @@ impl SecretKeyBytes { } Self::EcdsaP256Sha256(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 13 (ECDSAP256SHA256)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } Self::EcdsaP384Sha384(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 14 (ECDSAP384SHA384)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } Self::Ed25519(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 15 (ED25519)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } Self::Ed448(s) => { + let s = s.expose_secret(); writeln!(w, "Algorithm: 16 (ED448)")?; - writeln!(w, "PrivateKey: {}", base64::encode_display(&**s)) + writeln!(w, "PrivateKey: {}", base64::encode_display(s)) } } } @@ -182,7 +187,7 @@ impl SecretKeyBytes { /// Parse private keys for most algorithms (except RSA). fn parse_pkey( mut data: &str, - ) -> Result, BindFormatError> { + ) -> Result, BindFormatError> { // Look for the 'PrivateKey' field. while let Some((key, val, rest)) = parse_bind_entry(data)? { data = rest; @@ -191,11 +196,15 @@ impl SecretKeyBytes { continue; } - return base64::decode::>(val) - .map_err(|_| BindFormatError::Misformatted)? - .into_boxed_slice() + // TODO: Evaluate security of 'base64::decode()'. + let val: Vec = base64::decode(val) + .map_err(|_| BindFormatError::Misformatted)?; + let val: Box<[u8]> = val.into_boxed_slice(); + let val: Box<[u8; N]> = val .try_into() - .map_err(|_| BindFormatError::Misformatted); + .map_err(|_| BindFormatError::Misformatted)?; + + return Ok(val.into()); } // The 'PrivateKey' field was not found. @@ -245,22 +254,6 @@ impl SecretKeyBytes { } } -//--- Drop - -impl Drop for SecretKeyBytes { - /// Securely clear the secret key bytes from memory. - fn drop(&mut self) { - // Zero the bytes for each field. - match self { - Self::RsaSha256(_) => {} - Self::EcdsaP256Sha256(s) => s.fill(0), - Self::EcdsaP384Sha384(s) => s.fill(0), - Self::Ed25519(s) => s.fill(0), - Self::Ed448(s) => s.fill(0), - } - } -} - //----------- RsaSecretKeyBytes --------------------------------------------------- /// An RSA secret key expressed as raw bytes. @@ -276,22 +269,22 @@ pub struct RsaSecretKeyBytes { pub e: Box<[u8]>, /// The private exponent. - pub d: Box<[u8]>, + pub d: SecretBox<[u8]>, /// The first prime factor of `d`. - pub p: Box<[u8]>, + pub p: SecretBox<[u8]>, /// The second prime factor of `d`. - pub q: Box<[u8]>, + pub q: SecretBox<[u8]>, /// The exponent corresponding to the first prime factor of `d`. - pub d_p: Box<[u8]>, + pub d_p: SecretBox<[u8]>, /// The exponent corresponding to the second prime factor of `d`. - pub d_q: Box<[u8]>, + pub d_q: SecretBox<[u8]>, /// The inverse of the second prime factor modulo the first. - pub q_i: Box<[u8]>, + pub q_i: SecretBox<[u8]>, } //--- Conversion to and from the BIND format @@ -309,17 +302,17 @@ impl RsaSecretKeyBytes { w.write_str("PublicExponent: ")?; writeln!(w, "{}", base64::encode_display(&self.e))?; w.write_str("PrivateExponent: ")?; - writeln!(w, "{}", base64::encode_display(&self.d))?; + writeln!(w, "{}", base64::encode_display(&self.d.expose_secret()))?; w.write_str("Prime1: ")?; - writeln!(w, "{}", base64::encode_display(&self.p))?; + writeln!(w, "{}", base64::encode_display(&self.p.expose_secret()))?; w.write_str("Prime2: ")?; - writeln!(w, "{}", base64::encode_display(&self.q))?; + writeln!(w, "{}", base64::encode_display(&self.q.expose_secret()))?; w.write_str("Exponent1: ")?; - writeln!(w, "{}", base64::encode_display(&self.d_p))?; + writeln!(w, "{}", base64::encode_display(&self.d_p.expose_secret()))?; w.write_str("Exponent2: ")?; - writeln!(w, "{}", base64::encode_display(&self.d_q))?; + writeln!(w, "{}", base64::encode_display(&self.d_q.expose_secret()))?; w.write_str("Coefficient: ")?; - writeln!(w, "{}", base64::encode_display(&self.q_i))?; + writeln!(w, "{}", base64::encode_display(&self.q_i.expose_secret()))?; Ok(()) } @@ -390,12 +383,12 @@ impl RsaSecretKeyBytes { Ok(Self { n: n.unwrap(), e: e.unwrap(), - d: d.unwrap(), - p: p.unwrap(), - q: q.unwrap(), - d_p: d_p.unwrap(), - d_q: d_q.unwrap(), - q_i: q_i.unwrap(), + d: d.unwrap().into(), + p: p.unwrap().into(), + q: q.unwrap().into(), + d_p: d_p.unwrap().into(), + d_q: d_q.unwrap().into(), + q_i: q_i.unwrap().into(), }) } } @@ -411,23 +404,6 @@ impl<'a> From<&'a RsaSecretKeyBytes> for RsaPublicKeyBytes { } } -//--- Drop - -impl Drop for RsaSecretKeyBytes { - /// Securely clear the secret key bytes from memory. - fn drop(&mut self) { - // Zero the bytes for each field. - self.n.fill(0u8); - self.e.fill(0u8); - self.d.fill(0u8); - self.p.fill(0u8); - self.q.fill(0u8); - self.d_p.fill(0u8); - self.d_q.fill(0u8); - self.q_i.fill(0u8); - } -} - //----------- Helpers for parsing the BIND format ---------------------------- /// Extract the next key-value pair in a BIND-format private key file. diff --git a/src/sign/openssl.rs b/src/sign/openssl.rs index 85257137a..a7250081a 100644 --- a/src/sign/openssl.rs +++ b/src/sign/openssl.rs @@ -12,7 +12,7 @@ #![cfg_attr(docsrs, doc(cfg(feature = "openssl")))] use core::fmt; -use std::vec::Vec; +use std::{boxed::Box, vec::Vec}; use openssl::{ bn::BigNum, @@ -20,6 +20,7 @@ use openssl::{ error::ErrorStack, pkey::{self, PKey, Private}, }; +use secrecy::ExposeSecret; use crate::{ base::iana::SecAlg, @@ -70,12 +71,12 @@ impl KeyPair { let n = num(&s.n)?; let e = num(&s.e)?; - let d = secure_num(&s.d)?; - let p = secure_num(&s.p)?; - let q = secure_num(&s.q)?; - let d_p = secure_num(&s.d_p)?; - let d_q = secure_num(&s.d_q)?; - let q_i = secure_num(&s.q_i)?; + let d = secure_num(s.d.expose_secret())?; + let p = secure_num(s.p.expose_secret())?; + let q = secure_num(s.q.expose_secret())?; + let d_p = secure_num(s.d_p.expose_secret())?; + let d_q = secure_num(s.d_q.expose_secret())?; + let q_i = secure_num(s.q_i.expose_secret())?; // NOTE: The 'openssl' crate doesn't seem to expose // 'EVP_PKEY_fromdata', which could be used to replace the @@ -101,7 +102,7 @@ impl KeyPair { let mut ctx = bn::BigNumContext::new_secure()?; let group = nid::Nid::X9_62_PRIME256V1; let group = ec::EcGroup::from_curve_name(group)?; - let n = secure_num(s.as_slice())?; + let n = secure_num(s.expose_secret().as_slice())?; let p = ec::EcPoint::from_bytes(&group, &**p, &mut ctx)?; let k = ec::EcKey::from_private_components(&group, &n, &p)?; k.check_key().map_err(|_| FromBytesError::InvalidKey)?; @@ -117,7 +118,7 @@ impl KeyPair { let mut ctx = bn::BigNumContext::new_secure()?; let group = nid::Nid::SECP384R1; let group = ec::EcGroup::from_curve_name(group)?; - let n = secure_num(s.as_slice())?; + let n = secure_num(s.expose_secret().as_slice())?; let p = ec::EcPoint::from_bytes(&group, &**p, &mut ctx)?; let k = ec::EcKey::from_private_components(&group, &n, &p)?; k.check_key().map_err(|_| FromBytesError::InvalidKey)?; @@ -128,7 +129,8 @@ impl KeyPair { use openssl::memcmp; let id = pkey::Id::ED25519; - let k = PKey::private_key_from_raw_bytes(&**s, id)?; + let s = s.expose_secret(); + let k = PKey::private_key_from_raw_bytes(s, id)?; if memcmp::eq(&k.raw_public_key().unwrap(), &**p) { k } else { @@ -140,7 +142,8 @@ impl KeyPair { use openssl::memcmp; let id = pkey::Id::ED448; - let k = PKey::private_key_from_raw_bytes(&**s, id)?; + let s = s.expose_secret(); + let k = PKey::private_key_from_raw_bytes(s, id)?; if memcmp::eq(&k.raw_public_key().unwrap(), &**p) { k } else { @@ -182,20 +185,24 @@ impl KeyPair { SecAlg::ECDSAP256SHA256 => { let key = self.pkey.ec_key().unwrap(); let key = key.private_key().to_vec_padded(32).unwrap(); - SecretKeyBytes::EcdsaP256Sha256(key.try_into().unwrap()) + let key: Box<[u8; 32]> = key.try_into().unwrap(); + SecretKeyBytes::EcdsaP256Sha256(key.into()) } SecAlg::ECDSAP384SHA384 => { let key = self.pkey.ec_key().unwrap(); let key = key.private_key().to_vec_padded(48).unwrap(); - SecretKeyBytes::EcdsaP384Sha384(key.try_into().unwrap()) + let key: Box<[u8; 48]> = key.try_into().unwrap(); + SecretKeyBytes::EcdsaP384Sha384(key.into()) } SecAlg::ED25519 => { let key = self.pkey.raw_private_key().unwrap(); - SecretKeyBytes::Ed25519(key.try_into().unwrap()) + let key: Box<[u8; 32]> = key.try_into().unwrap(); + SecretKeyBytes::Ed25519(key.into()) } SecAlg::ED448 => { let key = self.pkey.raw_private_key().unwrap(); - SecretKeyBytes::Ed448(key.try_into().unwrap()) + let key: Box<[u8; 57]> = key.try_into().unwrap(); + SecretKeyBytes::Ed448(key.into()) } _ => unreachable!(), } diff --git a/src/sign/ring.rs b/src/sign/ring.rs index 3b97cf006..d1e29c395 100644 --- a/src/sign/ring.rs +++ b/src/sign/ring.rs @@ -16,6 +16,7 @@ use std::{boxed::Box, sync::Arc, vec::Vec}; use ring::signature::{ EcdsaKeyPair, Ed25519KeyPair, KeyPair as _, RsaKeyPair, }; +use secrecy::ExposeSecret; use crate::{ base::iana::SecAlg, @@ -76,12 +77,12 @@ impl KeyPair { n: s.n.as_ref(), e: s.e.as_ref(), }, - d: s.d.as_ref(), - p: s.p.as_ref(), - q: s.q.as_ref(), - dP: s.d_p.as_ref(), - dQ: s.d_q.as_ref(), - qInv: s.q_i.as_ref(), + d: s.d.expose_secret(), + p: s.p.expose_secret(), + q: s.q.expose_secret(), + dP: s.d_p.expose_secret(), + dQ: s.d_q.expose_secret(), + qInv: s.q_i.expose_secret(), }; ring::signature::RsaKeyPair::from_components(&components) .map_err(|_| FromBytesError::InvalidKey) @@ -95,7 +96,7 @@ impl KeyPair { let alg = &ring::signature::ECDSA_P256_SHA256_FIXED_SIGNING; EcdsaKeyPair::from_private_key_and_public_key( alg, - s.as_slice(), + s.expose_secret(), p.as_slice(), &*rng, ) @@ -110,7 +111,7 @@ impl KeyPair { let alg = &ring::signature::ECDSA_P384_SHA384_FIXED_SIGNING; EcdsaKeyPair::from_private_key_and_public_key( alg, - s.as_slice(), + s.expose_secret(), p.as_slice(), &*rng, ) @@ -120,7 +121,7 @@ impl KeyPair { (SecretKeyBytes::Ed25519(s), PublicKeyBytes::Ed25519(p)) => { Ed25519KeyPair::from_seed_and_public_key( - s.as_slice(), + s.expose_secret(), p.as_slice(), ) .map_err(|_| FromBytesError::InvalidKey) @@ -240,8 +241,8 @@ pub fn generate( // Manually parse the PKCS#8 document for the private key. let sk: Box<[u8]> = Box::from(&doc.as_ref()[36..68]); - let sk = sk.try_into().unwrap(); - let sk = SecretKeyBytes::EcdsaP256Sha256(sk); + let sk: Box<[u8; 32]> = sk.try_into().unwrap(); + let sk = SecretKeyBytes::EcdsaP256Sha256(sk.into()); // Manually parse the PKCS#8 document for the public key. let pk: Box<[u8]> = Box::from(&doc.as_ref()[73..138]); @@ -258,8 +259,8 @@ pub fn generate( // Manually parse the PKCS#8 document for the private key. let sk: Box<[u8]> = Box::from(&doc.as_ref()[35..83]); - let sk = sk.try_into().unwrap(); - let sk = SecretKeyBytes::EcdsaP384Sha384(sk); + let sk: Box<[u8; 48]> = sk.try_into().unwrap(); + let sk = SecretKeyBytes::EcdsaP384Sha384(sk.into()); // Manually parse the PKCS#8 document for the public key. let pk: Box<[u8]> = Box::from(&doc.as_ref()[88..185]); @@ -275,8 +276,8 @@ pub fn generate( // Manually parse the PKCS#8 document for the private key. let sk: Box<[u8]> = Box::from(&doc.as_ref()[16..48]); - let sk = sk.try_into().unwrap(); - let sk = SecretKeyBytes::Ed25519(sk); + let sk: Box<[u8; 32]> = sk.try_into().unwrap(); + let sk = SecretKeyBytes::Ed25519(sk.into()); // Manually parse the PKCS#8 document for the public key. let pk: Box<[u8]> = Box::from(&doc.as_ref()[51..83]); From 9268dd3b69c39f81535d46f058db1a9f95cea43c Mon Sep 17 00:00:00 2001 From: Ximon Eighteen <3304436+ximon18@users.noreply.github.com> Date: Mon, 4 Nov 2024 22:33:46 +0100 Subject: [PATCH 05/11] Display NSEC3 without trailing space if the bitmap is empty. --- src/rdata/dnssec.rs | 5 +++++ src/rdata/nsec3.rs | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/rdata/dnssec.rs b/src/rdata/dnssec.rs index fdb79dd52..eb0259411 100644 --- a/src/rdata/dnssec.rs +++ b/src/rdata/dnssec.rs @@ -2169,6 +2169,11 @@ impl> RtypeBitmap { ) -> Result<(), Target::AppendError> { target.append_slice(self.0.as_ref()) } + + #[must_use] + pub fn is_empty(&self) -> bool { + self.iter().next().is_none() + } } //--- AsRef diff --git a/src/rdata/nsec3.rs b/src/rdata/nsec3.rs index e2a19468d..a09e4c309 100644 --- a/src/rdata/nsec3.rs +++ b/src/rdata/nsec3.rs @@ -358,7 +358,10 @@ impl> fmt::Display for Nsec3 { self.hash_algorithm, self.flags, self.iterations, self.salt )?; base32::display_hex(&self.next_owner, f)?; - write!(f, " {}", self.types) + if !self.types.is_empty() { + write!(f, " {}", self.types)?; + } + Ok(()) } } From fb7e9efebd8e77d5470dc81e69b00de205d8c8f5 Mon Sep 17 00:00:00 2001 From: Ximon Eighteen <3304436+ximon18@users.noreply.github.com> Date: Mon, 4 Nov 2024 23:27:08 +0100 Subject: [PATCH 06/11] Backport NSEC3 improvements and upstream dnssec-key branch compatibility fixes from the downstream multiple-signing-key branch. --- src/sign/mod.rs | 1 + src/sign/records.rs | 319 ++++++++++++++++++++++++++++++++------------ src/validate.rs | 3 + 3 files changed, 237 insertions(+), 86 deletions(-) diff --git a/src/sign/mod.rs b/src/sign/mod.rs index b365a78f5..e5f94a843 100644 --- a/src/sign/mod.rs +++ b/src/sign/mod.rs @@ -23,6 +23,7 @@ pub use self::bytes::{RsaSecretKeyBytes, SecretKeyBytes}; pub mod common; pub mod openssl; +pub mod records; pub mod ring; //----------- SigningKey ----------------------------------------------------- diff --git a/src/sign/records.rs b/src/sign/records.rs index 61697f0fe..44380347c 100644 --- a/src/sign/records.rs +++ b/src/sign/records.rs @@ -2,13 +2,15 @@ use core::convert::From; use core::fmt::Display; +use std::collections::HashMap; use std::fmt::Debug; +use std::hash::Hash; use std::string::String; use std::vec::Vec; use std::{fmt, io, slice}; use octseq::builder::{EmptyBuilder, FromBuilder, OctetsBuilder, Truncate}; -use octseq::{FreezeBuilder, OctetsFrom}; +use octseq::{FreezeBuilder, OctetsFrom, OctetsInto}; use crate::base::cmp::CanonicalOrd; use crate::base::iana::{Class, Nsec3HashAlg, Rtype}; @@ -20,11 +22,11 @@ use crate::rdata::dnssec::{ ProtoRrsig, RtypeBitmap, RtypeBitmapBuilder, Timestamp, }; use crate::rdata::nsec3::{Nsec3Salt, OwnerHash}; -use crate::rdata::{Dnskey, Ds, Nsec, Nsec3, Nsec3param, Rrsig}; +use crate::rdata::{Nsec, Nsec3, Nsec3param, Rrsig}; use crate::utils::base32; +use crate::validate::{nsec3_hash, Nsec3HashError}; -use super::key::SigningKey; -use super::ring::{nsec3_hash, Nsec3HashError}; +use super::{SignRaw, SigningKey}; //------------ SortedRecords ------------------------------------------------- @@ -75,19 +77,18 @@ impl SortedRecords { } #[allow(clippy::type_complexity)] - pub fn sign( + pub fn sign( &self, - apex: &FamilyName, + apex: &FamilyName, expiration: Timestamp, inception: Timestamp, - key: Key, - ) -> Result>>, Key::Error> + key: SigningKey, + ) -> Result>>, ErrorTypeToBeDetermined> where N: ToName + Clone, D: RecordData + ComposeRecordData, - Key: SigningKey, - Octets: From + AsRef<[u8]>, - ApexName: ToName + Clone, + ConcreteSecretKey: SignRaw, + Octets: AsRef<[u8]> + OctetsFrom>, { let mut res = Vec::new(); let mut buf = Vec::new(); @@ -148,12 +149,12 @@ impl SortedRecords { buf.clear(); let rrsig = ProtoRrsig::new( rrset.rtype(), - key.algorithm()?, + key.algorithm(), name.owner().rrsig_label_count(), rrset.ttl(), expiration, inception, - key.key_tag()?, + key.public_key().key_tag(), apex.owner().clone(), ); rrsig.compose_canonical(&mut buf).unwrap(); @@ -162,31 +163,34 @@ impl SortedRecords { } // Create and push the RRSIG record. + let signature = key.raw_secret_key().sign_raw(&buf).unwrap(); + let signature = signature.as_ref().to_vec(); + let Ok(signature) = signature.try_octets_into() else { + return Err(ErrorTypeToBeDetermined); + }; + res.push(Record::new( name.owner().clone(), name.class(), rrset.ttl(), - rrsig - .into_rrsig(key.sign(&buf)?.into()) - .expect("long signature"), + rrsig.into_rrsig(signature).expect("long signature"), )); } } Ok(res) } - pub fn nsecs( + pub fn nsecs( &self, - apex: &FamilyName, + apex: &FamilyName, ttl: Ttl, ) -> Vec>> where - N: ToName + Clone, + N: ToName + Clone + PartialEq, D: RecordData, Octets: FromBuilder, Octets::Builder: EmptyBuilder + Truncate + AsRef<[u8]> + AsMut<[u8]>, - ::AppendError: fmt::Debug, - ApexName: ToName, + ::AppendError: Debug, { let mut res = Vec::new(); @@ -240,8 +244,13 @@ impl SortedRecords { } let mut bitmap = RtypeBitmap::::builder(); - // Assume there’s gonna be an RRSIG. + // Assume there's gonna be an RRSIG. bitmap.add(Rtype::RRSIG).unwrap(); + if family.owner() == &apex_owner { + // Assume there's gonna be a DNSKEY. + bitmap.add(Rtype::DNSKEY).unwrap(); + } + bitmap.add(Rtype::NSEC).unwrap(); for rrset in family.rrsets() { bitmap.add(rrset.rtype()).unwrap() } @@ -275,10 +284,11 @@ impl SortedRecords { apex: &FamilyName, ttl: Ttl, params: Nsec3param, - opt_out: bool, + opt_out: Nsec3OptOut, + capture_hash_to_owner_mappings: bool, ) -> Result, Nsec3HashError> where - N: ToName + Clone + From> + Display, + N: ToName + Clone + From> + Display + Ord + Hash, N: From::Octets>>, D: RecordData, Octets: FromBuilder + OctetsFrom> + Clone + Default, @@ -289,6 +299,7 @@ impl SortedRecords { + AsMut<[u8]> + EmptyBuilder + FreezeBuilder, + ::Octets: AsRef<[u8]>, { // TODO: // - Handle name collisions? (see RFC 5155 7.1 Zone Signing) @@ -296,10 +307,23 @@ impl SortedRecords { // Reject old algorithms? if not, map 3 to 6 and 5 to 7, or reject // use of 3 and 5? + // RFC 5155 7.1 step 2: + // "If Opt-Out is being used, set the Opt-Out bit to one." + let mut nsec3_flags = params.flags(); + if matches!( + opt_out, + Nsec3OptOut::OptOut | Nsec3OptOut::OptOutFlagsOnly + ) { + // Set the Opt-Out flag. + nsec3_flags |= 0b0000_0001; + } + // RFC 5155 7.1 step 5: _"Sort the set of NSEC3 RRs into hash order." // We store the NSEC3s as we create them in a self-sorting vec. let mut nsec3s = SortedRecords::new(); + let mut ents = Vec::::new(); + // The owner name of a zone cut if we currently are at or below one. let mut cut: Option> = None; @@ -313,6 +337,13 @@ impl SortedRecords { let apex_owner = families.first_owner().clone(); let apex_label_count = apex_owner.iter_labels().count(); + let mut last_nent_stack: Vec = vec![]; + let mut nsec3_hash_map = if capture_hash_to_owner_mappings { + Some(HashMap::::new()) + } else { + None + }; + for family in families { // If the owner is out of zone, we have moved out of our zone and // are done. @@ -343,7 +374,7 @@ impl SortedRecords { // "If Opt-Out is being used, owner names of unsigned // delegations MAY be excluded." let has_ds = family.records().any(|rec| rec.rtype() == Rtype::DS); - if cut.is_some() && !has_ds && opt_out { + if cut.is_some() && !has_ds && opt_out == Nsec3OptOut::OptOut { continue; } @@ -352,9 +383,20 @@ impl SortedRecords { // the original owner name is greater than 1, additional NSEC3 // RRs need to be added for every empty non-terminal between // the apex and the original owner name." + let mut last_nent_distance_to_apex = 0; + let mut last_nent = None; + while let Some(this_last_nent) = last_nent_stack.pop() { + if name.owner().ends_with(&this_last_nent) { + last_nent_distance_to_apex = + this_last_nent.iter_labels().count() + - apex_label_count; + last_nent = Some(this_last_nent); + break; + } + } let distance_to_root = name.owner().iter_labels().count(); let distance_to_apex = distance_to_root - apex_label_count; - if distance_to_apex > 1 { + if distance_to_apex > last_nent_distance_to_apex { // Are there any empty nodes between this node and the apex? // The zone file records are already sorted so if all of the // parent labels had records at them, i.e. they were non-empty @@ -375,7 +417,8 @@ impl SortedRecords { // It will NOT construct the last name as that will be dealt // with in the next outer loop iteration. // - a.b.c.mail.example.com - for n in (1..distance_to_apex - 1).rev() { + let distance = distance_to_apex - last_nent_distance_to_apex; + for n in (1..=distance - 1).rev() { let rev_label_it = name.owner().iter_labels().skip(n); // Create next longest ENT name. @@ -386,22 +429,9 @@ impl SortedRecords { let name = builder.append_origin(&apex_owner).unwrap().into(); - // Create the type bitmap, empty for an ENT NSEC3. - let bitmap = RtypeBitmap::::builder(); - - let rec = Self::mk_nsec3( - &name, - params.hash_algorithm(), - params.flags(), - params.iterations(), - params.salt(), - &apex_owner, - bitmap, - ttl, - )?; - - // Store the record by order of its owner name. - let _ = nsec3s.insert(rec); + if let Err(pos) = ents.binary_search(&name) { + ents.insert(pos, name); + } } } @@ -423,18 +453,42 @@ impl SortedRecords { if distance_to_apex == 0 { bitmap.add(Rtype::NSEC3PARAM).unwrap(); + bitmap.add(Rtype::DNSKEY).unwrap(); } - // RFC 5155 7.1 step 2: - // "If Opt-Out is being used, set the Opt-Out bit to one." - let mut nsec3_flags = params.flags(); - if opt_out { - // Set the Opt-Out flag. - nsec3_flags |= 0b0000_0001; + let rec = Self::mk_nsec3( + name.owner(), + params.hash_algorithm(), + nsec3_flags, + params.iterations(), + params.salt(), + &apex_owner, + bitmap, + ttl, + )?; + + if let Some(nsec3_hash_map) = &mut nsec3_hash_map { + nsec3_hash_map + .insert(rec.owner().clone(), name.owner().clone()); + } + + // Store the record by order of its owner name. + if nsec3s.insert(rec).is_err() { + return Err(Nsec3HashError::CollisionDetected); } + if let Some(last_nent) = last_nent { + last_nent_stack.push(last_nent); + } + last_nent_stack.push(name.owner().clone()); + } + + for name in ents { + // Create the type bitmap, empty for an ENT NSEC3. + let bitmap = RtypeBitmap::::builder(); + let rec = Self::mk_nsec3( - name.owner(), + &name, params.hash_algorithm(), nsec3_flags, params.iterations(), @@ -444,7 +498,14 @@ impl SortedRecords { ttl, )?; - let _ = nsec3s.insert(rec); + if let Some(nsec3_hash_map) = &mut nsec3_hash_map { + nsec3_hash_map.insert(rec.owner().clone(), name); + } + + // Store the record by order of its owner name. + if nsec3s.insert(rec).is_err() { + return Err(Nsec3HashError::CollisionDetected); + } } // RFC 5155 7.1 step 7: @@ -484,9 +545,15 @@ impl SortedRecords { // "If a hash collision is detected, then a new salt has to be // chosen, and the signing process restarted." // - // TODO + // Handled above. - Ok(Nsec3Records::new(nsec3s.records, nsec3param)) + let res = Nsec3Records::new(nsec3s.records, nsec3param); + + if let Some(nsec3_hash_map) = nsec3_hash_map { + Ok(res.with_hashes(nsec3_hash_map)) + } else { + Ok(res) + } } pub fn write(&self, target: &mut W) -> Result<(), io::Error> @@ -495,9 +562,49 @@ impl SortedRecords { D: RecordData + fmt::Display, W: io::Write, { - for record in &self.records { - writeln!(target, "{}", record)?; + for record in self.records.iter().filter(|r| r.rtype() == Rtype::SOA) + { + writeln!(target, "{record}")?; } + + for record in self.records.iter().filter(|r| r.rtype() != Rtype::SOA) + { + writeln!(target, "{record}")?; + } + + Ok(()) + } + + pub fn write_with_comments( + &self, + target: &mut W, + comment_cb: F, + ) -> Result<(), io::Error> + where + N: fmt::Display, + D: RecordData + fmt::Display, + W: io::Write, + C: fmt::Display, + F: Fn(&Record) -> Option, + { + for record in self.records.iter().filter(|r| r.rtype() == Rtype::SOA) + { + if let Some(comment) = comment_cb(record) { + writeln!(target, "{record} ;{}", comment)?; + } else { + writeln!(target, "{record}")?; + } + } + + for record in self.records.iter().filter(|r| r.rtype() != Rtype::SOA) + { + if let Some(comment) = comment_cb(record) { + writeln!(target, "{record} ;{}", comment)?; + } else { + writeln!(target, "{record}")?; + } + } + Ok(()) } } @@ -578,7 +685,7 @@ impl SortedRecords { } } -impl Default for SortedRecords { +impl Default for SortedRecords { fn default() -> Self { Self::new() } @@ -623,21 +730,34 @@ where //------------ Nsec3Records --------------------------------------------------- -/// The set of records created by [`SortedRecords::nsec3s()`]. pub struct Nsec3Records { /// The NSEC3 records. - pub nsec3s: Vec>>, + pub recs: Vec>>, /// The NSEC3PARAM record. - pub nsec3param: Record>, + pub param: Record>, + + /// A map of hashes to owner names. + /// + /// For diagnostic purposes. None if not generated. + pub hashes: Option>, } impl Nsec3Records { pub fn new( - nsec3s: Vec>>, - nsec3param: Record>, + recs: Vec>>, + param: Record>, ) -> Self { - Self { nsec3s, nsec3param } + Self { + recs, + param, + hashes: None, + } + } + + pub fn with_hashes(mut self, hashes: HashMap) -> Self { + self.hashes = Some(hashes); + self } } @@ -719,30 +839,6 @@ impl FamilyName { { Record::new(self.owner.clone(), self.class, ttl, data) } - - pub fn dnskey>( - &self, - ttl: Ttl, - key: K, - ) -> Result>, K::Error> - where - N: Clone, - { - key.dnskey() - .map(|dnskey| self.clone().into_record(ttl, dnskey.convert())) - } - - pub fn ds( - &self, - ttl: Ttl, - key: K, - ) -> Result>, K::Error> - where - N: ToName + Clone, - { - key.ds(&self.owner) - .map(|ds| self.clone().into_record(ttl, ds)) - } } impl<'a, N: Clone> FamilyName<&'a N> { @@ -947,3 +1043,54 @@ where Some(Rrset::new(res)) } } + +//------------ ErrorTypeToBeDetermined --------------------------------------- + +#[derive(Debug)] +pub struct ErrorTypeToBeDetermined; + +//------------ Nsec3OptOut --------------------------------------------------- + +/// The different types of NSEC3 opt-out. +#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)] +pub enum Nsec3OptOut { + /// No opt-out. The opt-out flag of NSEC3 RRs will NOT be set and insecure + /// delegations will be included in the NSEC3 chain. + #[default] + NoOptOut, + + /// Opt-out. The opt-out flag of NSEC3 RRs will be set and insecure + /// delegations will NOT be included in the NSEC3 chain. + OptOut, + + /// Opt-out (flags only). The opt-out flag of NSEC3 RRs will be set and + /// insecure delegations will be included in the NSEC3 chain. + OptOutFlagsOnly, +} + +// TODO: Add tests for nsec3s() that validate the following from RFC 5155: +// +// https://www.rfc-editor.org/rfc/rfc5155.html#section-7.1 +// 7.1. Zone Signing +// "Zones using NSEC3 must satisfy the following properties: +// +// o Each owner name within the zone that owns authoritative RRSets +// MUST have a corresponding NSEC3 RR. Owner names that correspond +// to unsigned delegations MAY have a corresponding NSEC3 RR. +// However, if there is not a corresponding NSEC3 RR, there MUST be +// an Opt-Out NSEC3 RR that covers the "next closer" name to the +// delegation. Other non-authoritative RRs are not represented by +// NSEC3 RRs. +// +// o Each empty non-terminal MUST have a corresponding NSEC3 RR, unless +// the empty non-terminal is only derived from an insecure delegation +// covered by an Opt-Out NSEC3 RR. +// +// o The TTL value for any NSEC3 RR SHOULD be the same as the minimum +// TTL value field in the zone SOA RR. +// +// o The Type Bit Maps field of every NSEC3 RR in a signed zone MUST +// indicate the presence of all types present at the original owner +// name, except for the types solely contributed by an NSEC3 RR +// itself. Note that this means that the NSEC3 type itself will +// never be present in the Type Bit Maps." diff --git a/src/validate.rs b/src/validate.rs index c806a48f9..612493237 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -1703,6 +1703,9 @@ pub enum Nsec3HashError { /// /// See: [OwnerHashError](crate::rdata::nsec3::OwnerHashError) OwnerHashError, + + /// The hashing process produced a hash that already exists. + CollisionDetected, } /// Compute an [RFC 5155] NSEC3 hash using default settings. From 414ea6c6b6f0ec1aecb1e1d66e48fcf4405b020e Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Wed, 30 Oct 2024 10:24:58 +0100 Subject: [PATCH 07/11] [sign,validate] Add 'display_as_bind()' to key bytes types --- src/sign/bytes.rs | 35 ++++++++++++++++++++++++++++++----- src/sign/openssl.rs | 10 +++++----- src/validate.rs | 30 +++++++++++++++++------------- 3 files changed, 52 insertions(+), 23 deletions(-) diff --git a/src/sign/bytes.rs b/src/sign/bytes.rs index 5b49f3328..1187a6dbf 100644 --- a/src/sign/bytes.rs +++ b/src/sign/bytes.rs @@ -130,7 +130,7 @@ impl SecretKeyBytes { /// The key is formatted in the private key v1.2 format and written to the /// given formatter. See the type-level documentation for a description /// of this format. - pub fn format_as_bind(&self, w: &mut impl fmt::Write) -> fmt::Result { + pub fn format_as_bind(&self, mut w: impl fmt::Write) -> fmt::Result { writeln!(w, "Private-key-format: v1.2")?; match self { Self::RsaSha256(k) => { @@ -160,6 +160,19 @@ impl SecretKeyBytes { } } + /// Display this secret key in the conventional format used by BIND. + /// + /// This is a simple wrapper around [`Self::format_as_bind()`]. + pub fn display_as_bind(&self) -> impl fmt::Display + '_ { + struct Display<'a>(&'a SecretKeyBytes); + impl fmt::Display for Display<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.format_as_bind(f) + } + } + Display(self) + } + /// Parse a secret key from the conventional format used by BIND. /// /// This parser supports the private key v1.2 format, but it should be @@ -289,7 +302,7 @@ impl RsaSecretKeyBytes { /// given formatter. Note that the header and algorithm lines are not /// written. See the type-level documentation of [`SecretKeyBytes`] for a /// description of this format. - pub fn format_as_bind(&self, w: &mut impl fmt::Write) -> fmt::Result { + pub fn format_as_bind(&self, mut w: impl fmt::Write) -> fmt::Result { w.write_str("Modulus: ")?; writeln!(w, "{}", base64::encode_display(&self.n))?; w.write_str("PublicExponent: ")?; @@ -309,6 +322,19 @@ impl RsaSecretKeyBytes { Ok(()) } + /// Display this secret key in the conventional format used by BIND. + /// + /// This is a simple wrapper around [`Self::format_as_bind()`]. + pub fn display_as_bind(&self) -> impl fmt::Display + '_ { + struct Display<'a>(&'a RsaSecretKeyBytes); + impl fmt::Display for Display<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.format_as_bind(f) + } + } + Display(self) + } + /// Parse a secret key from the conventional format used by BIND. /// /// This parser supports the private key v1.2 format, but it should be @@ -464,7 +490,7 @@ impl std::error::Error for BindFormatError {} #[cfg(test)] mod tests { - use std::{string::String, vec::Vec}; + use std::{string::ToString, vec::Vec}; use crate::base::iana::SecAlg; @@ -496,8 +522,7 @@ mod tests { let path = format!("test-data/dnssec-keys/K{}.private", name); let data = std::fs::read_to_string(path).unwrap(); let key = super::SecretKeyBytes::parse_from_bind(&data).unwrap(); - let mut same = String::new(); - key.format_as_bind(&mut same).unwrap(); + let same = key.display_as_bind().to_string(); let data = data.lines().collect::>(); let same = same.lines().collect::>(); assert_eq!(data, same); diff --git a/src/sign/openssl.rs b/src/sign/openssl.rs index c5620c22e..e1922ffdb 100644 --- a/src/sign/openssl.rs +++ b/src/sign/openssl.rs @@ -433,7 +433,10 @@ impl std::error::Error for GenerateError {} #[cfg(test)] mod tests { - use std::{string::String, vec::Vec}; + use std::{ + string::{String, ToString}, + vec::Vec, + }; use crate::{ base::iana::SecAlg, @@ -503,10 +506,7 @@ mod tests { let gen_key = SecretKeyBytes::parse_from_bind(&data).unwrap(); let key = KeyPair::from_bytes(&gen_key, pub_key).unwrap(); - - let equiv = key.to_bytes(); - let mut same = String::new(); - equiv.format_as_bind(&mut same).unwrap(); + let same = key.to_bytes().display_as_bind().to_string(); let data = data.lines().collect::>(); let same = same.lines().collect::>(); diff --git a/src/validate.rs b/src/validate.rs index 612493237..37826d796 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -311,23 +311,28 @@ impl> Key { /// Serialize this key in the conventional format used by BIND. /// - /// A user-specified DNS class can be used in the record; however, this - /// will almost always just be `IN`. - /// /// See the type-level documentation for a description of this format. - pub fn format_as_bind( - &self, - class: Class, - w: &mut impl fmt::Write, - ) -> fmt::Result { + pub fn format_as_bind(&self, mut w: impl fmt::Write) -> fmt::Result { writeln!( w, - "{} {} DNSKEY {}", + "{} IN DNSKEY {}", self.owner().fmt_with_dot(), - class, self.to_dnskey().display_zonefile(false), ) } + + /// Display this key in the conventional format used by BIND. + /// + /// See the type-level documentation for a description of this format. + pub fn display_as_bind(&self) -> impl fmt::Display + '_ { + struct Display<'a, Octs>(&'a Key); + impl<'a, Octs: AsRef<[u8]>> fmt::Display for Display<'a, Octs> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.format_as_bind(f) + } + } + Display(self) + } } //--- Comparison @@ -1244,7 +1249,7 @@ mod test { use crate::utils::base64; use bytes::Bytes; use std::str::FromStr; - use std::string::String; + use std::string::{String, ToString}; type Name = crate::base::name::Name>; type Ds = crate::rdata::Ds>; @@ -1378,8 +1383,7 @@ mod test { let path = format!("test-data/dnssec-keys/K{}.key", name); let data = std::fs::read_to_string(path).unwrap(); let key = Key::>::parse_from_bind(&data).unwrap(); - let mut bind_fmt_key = String::new(); - key.format_as_bind(Class::IN, &mut bind_fmt_key).unwrap(); + let bind_fmt_key = key.display_as_bind().to_string(); let same = Key::parse_from_bind(&bind_fmt_key).unwrap(); assert_eq!(key, same); } From 2bde7aab351fbd9643ac1ffb5c34dc5ab6da474a Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Wed, 30 Oct 2024 11:02:57 +0100 Subject: [PATCH 08/11] [sign,validate] remove unused imports --- src/sign/openssl.rs | 5 +---- src/validate.rs | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/sign/openssl.rs b/src/sign/openssl.rs index e1922ffdb..814a55da2 100644 --- a/src/sign/openssl.rs +++ b/src/sign/openssl.rs @@ -433,10 +433,7 @@ impl std::error::Error for GenerateError {} #[cfg(test)] mod tests { - use std::{ - string::{String, ToString}, - vec::Vec, - }; + use std::{string::ToString, vec::Vec}; use crate::{ base::iana::SecAlg, diff --git a/src/validate.rs b/src/validate.rs index 37826d796..3293df0f0 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -1249,7 +1249,7 @@ mod test { use crate::utils::base64; use bytes::Bytes; use std::str::FromStr; - use std::string::{String, ToString}; + use std::string::ToString; type Name = crate::base::name::Name>; type Ds = crate::rdata::Ds>; From 98db88b5812aedf4860bee84009b54bc122d6f06 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Thu, 31 Oct 2024 11:28:37 +0100 Subject: [PATCH 09/11] [sign] Document everything --- src/sign/common.rs | 4 ++ src/sign/mod.rs | 90 ++++++++++++++++++++++++++++++++++++++++++++- src/sign/openssl.rs | 8 ++++ src/sign/ring.rs | 7 ++++ 4 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/sign/common.rs b/src/sign/common.rs index d5aaf5b67..fc10803e3 100644 --- a/src/sign/common.rs +++ b/src/sign/common.rs @@ -1,4 +1,8 @@ //! DNSSEC signing using built-in backends. +//! +//! This backend supports all the algorithms supported by Ring and OpenSSL, +//! depending on whether the respective crate features are enabled. See the +//! documentation for each backend for more information. use core::fmt; use std::sync::Arc; diff --git a/src/sign/mod.rs b/src/sign/mod.rs index e5f94a843..586bedada 100644 --- a/src/sign/mod.rs +++ b/src/sign/mod.rs @@ -7,6 +7,87 @@ //! made "online" (in an authoritative name server while it is running) or //! "offline" (outside of a name server). Once generated, signatures can be //! serialized as DNS records and stored alongside the authenticated records. +//! +//! A DNSSEC key actually has two components: a cryptographic key, which can +//! be used to make and verify signatures, and key metadata, which defines how +//! the key should be used. These components are brought together by the +//! [`SigningKey`] type. It must be instantiated with a cryptographic key +//! type, such as [`common::KeyPair`], in order to be used. +//! +//! # Example Usage +//! +//! At the moment, only "low-level" signing is supported. +//! +//! ``` +//! # use domain::sign::*; +//! # use domain::base::Name; +//! // Generate a new ED25519 key. +//! let params = GenerateParams::Ed25519; +//! let (sec_bytes, pub_bytes) = common::generate(params).unwrap(); +//! +//! // Parse the key into Ring or OpenSSL. +//! let key_pair = common::KeyPair::from_bytes(&sec_bytes, &pub_bytes).unwrap(); +//! +//! // Associate the key with important metadata. +//! let owner: Name> = "www.example.org.".parse().unwrap(); +//! let flags = 257; // key signing key +//! let key = SigningKey::new(owner, flags, key_pair); +//! +//! // Access the public key (with metadata). +//! let pub_key = key.public_key(); +//! println!("{:?}", pub_key); +//! +//! // Sign arbitrary byte sequences with the key. +//! let sig = key.raw_secret_key().sign_raw(b"Hello, World!").unwrap(); +//! println!("{:?}", sig); +//! ``` +//! +//! # Cryptography +//! +//! This crate supports OpenSSL and Ring for performing cryptography. These +//! cryptographic backends are gated on the `openssl` and `ring` features, +//! respectively. They offer mostly equivalent functionality, but OpenSSL +//! supports a larger set of signing algorithms. A [`common`] backend is +//! provided for users that wish to use either or both backends at runtime. +//! +//! Each backend module exposes a `KeyPair` type, representing a cryptographic +//! key that can be used for signing, and a `generate()` function for creating +//! new keys. +//! +//! Users can choose to bring their own cryptography by providing their own +//! `KeyPair` type that implements [`SignRaw`]. Note that `async` signing +//! (useful for interacting with cryptographic hardware like HSMs) is not +//! currently supported. +//! +//! While each cryptographic backend can support a limited number of signature +//! algorithms, even the types independent of a cryptographic backend (e.g. +//! [`SecretKeyBytes`] and [`GenerateParams`]) support a limited number of +//! algorithms. They are: +//! +//! - RSA/SHA-256 +//! - ECDSA P-256/SHA-256 +//! - ECDSA P-384/SHA-384 +//! - Ed25519 +//! - Ed448 +//! +//! # Importing and Exporting +//! +//! The [`SecretKeyBytes`] type is a generic representation of a secret key as +//! a byte slice. While it does not offer any cryptographic functionality, it +//! is useful to transfer secret keys stored in memory, independent of any +//! cryptographic backend. +//! +//! The `KeyPair` types of the cryptographic backends in this module each +//! support a `from_bytes()` function that parses the generic representation +//! into a functional cryptographic key. Importantly, these functions require +//! both the public and private keys to be provided -- the pair are verified +//! for consistency. In some cases, it may also be possible to serialize an +//! existing cryptographic key back to the generic bytes representation. +//! +//! [`SecretKeyBytes`] also supports importing and exporting keys from and to +//! the conventional private-key format popularized by BIND. This format is +//! used by a variety of tools for storing DNSSEC keys on disk. See the +//! type-level documentation for a specification of the format. #![cfg(feature = "unstable-sign")] #![cfg_attr(docsrs, doc(cfg(feature = "unstable-sign")))] @@ -195,7 +276,14 @@ pub trait SignRaw { #[derive(Clone, Debug, PartialEq, Eq)] pub enum GenerateParams { /// Generate an RSA/SHA-256 keypair. - RsaSha256 { bits: u32 }, + RsaSha256 { + /// The number of bits in the public modulus. + /// + /// A ~3000-bit key corresponds to a 128-bit security level. However, + /// RSA is mostly used with 2048-bit keys. Some backends (like Ring) + /// do not support smaller key sizes than that. + bits: u32, + }, /// Generate an ECDSA P-256/SHA-256 keypair. EcdsaP256Sha256, diff --git a/src/sign/openssl.rs b/src/sign/openssl.rs index 814a55da2..85257137a 100644 --- a/src/sign/openssl.rs +++ b/src/sign/openssl.rs @@ -1,4 +1,12 @@ //! DNSSEC signing using OpenSSL. +//! +//! This backend supports the following algorithms: +//! +//! - RSA/SHA-256 (512-bit keys or larger) +//! - ECDSA P-256/SHA-256 +//! - ECDSA P-384/SHA-384 +//! - Ed25519 +//! - Ed448 #![cfg(feature = "openssl")] #![cfg_attr(docsrs, doc(cfg(feature = "openssl")))] diff --git a/src/sign/ring.rs b/src/sign/ring.rs index 1b747642f..09435188c 100644 --- a/src/sign/ring.rs +++ b/src/sign/ring.rs @@ -1,4 +1,11 @@ //! DNSSEC signing using `ring`. +//! +//! This backend supports the following algorithms: +//! +//! - RSA/SHA-256 (2048-bit keys or larger) +//! - ECDSA P-256/SHA-256 +//! - ECDSA P-384/SHA-384 +//! - Ed25519 #![cfg(feature = "ring")] #![cfg_attr(docsrs, doc(cfg(feature = "ring")))] From 8877c2286a7c6f1f752d6af1905ce86f03ea5450 Mon Sep 17 00:00:00 2001 From: Ximon Eighteen <3304436+ximon18@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:03:07 +0100 Subject: [PATCH 10/11] Update to work with changes in the upstream dnssec-key branch using a partial backport of changes from the downstream multiple-signing-key branch. --- src/sign/records.rs | 50 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/src/sign/records.rs b/src/sign/records.rs index 44380347c..facb6ac94 100644 --- a/src/sign/records.rs +++ b/src/sign/records.rs @@ -157,18 +157,19 @@ impl SortedRecords { key.public_key().key_tag(), apex.owner().clone(), ); + + buf.clear(); rrsig.compose_canonical(&mut buf).unwrap(); for record in rrset.iter() { record.compose_canonical(&mut buf).unwrap(); } - - // Create and push the RRSIG record. let signature = key.raw_secret_key().sign_raw(&buf).unwrap(); let signature = signature.as_ref().to_vec(); let Ok(signature) = signature.try_octets_into() else { return Err(ErrorTypeToBeDetermined); }; + // Create and push the RRSIG record. res.push(Record::new( name.owner().clone(), name.class(), @@ -1094,3 +1095,48 @@ pub enum Nsec3OptOut { // name, except for the types solely contributed by an NSEC3 RR // itself. Note that this means that the NSEC3 type itself will // never be present in the Type Bit Maps." +// #[cfg(test)] +// mod tests { +// use core::str::FromStr; + +// use crate::rdata::A; + +// use super::*; + +// #[test] +// fn nsec3s() { +// fn mk_test_record(name: &str) -> Record>, A> { +// Record::new( +// Name::>::from_str(name).unwrap(), +// Class::IN, +// Ttl::from_days(1), +// A::new("127.0.0.1".parse().unwrap()), +// ) +// } + +// let mut recs = SortedRecords::new(); +// recs.insert(mk_test_record("mail.example.com")).unwrap(); +// recs.insert(mk_test_record("x.y.mail.example.com")).unwrap(); +// recs.insert(mk_test_record("a.b.c.mail.example.com")) +// .unwrap(); +// recs.insert(mk_test_record("a.other.c.mail.example.com")) +// .unwrap(); + +// for rec in recs.families() { +// println!("{}", rec.family_name().owner()); +// } + +// let mut recs = SortedRecords::new(); +// recs.insert(mk_test_record("y.mail.example.com")).unwrap(); +// recs.insert(mk_test_record("c.mail.example.com")).unwrap(); +// recs.insert(mk_test_record("b.c.mail.example.com")).unwrap(); +// recs.insert(mk_test_record("c.mail.example.com")); +// recs.insert(mk_test_record("other.c.mail.example.com")) +// .unwrap(); + +// println!(); +// for rec in recs.families() { +// println!("{}", rec.family_name().owner()); +// } +// } +// } From 40d65ac129ec1e0dbfc1bcc90446975bb00f5014 Mon Sep 17 00:00:00 2001 From: Ximon Eighteen <3304436+ximon18@users.noreply.github.com> Date: Mon, 4 Nov 2024 23:45:17 +0100 Subject: [PATCH 11/11] Minor tweaks. --- src/sign/records.rs | 50 ++------------------------------------------- 1 file changed, 2 insertions(+), 48 deletions(-) diff --git a/src/sign/records.rs b/src/sign/records.rs index facb6ac94..44380347c 100644 --- a/src/sign/records.rs +++ b/src/sign/records.rs @@ -157,19 +157,18 @@ impl SortedRecords { key.public_key().key_tag(), apex.owner().clone(), ); - - buf.clear(); rrsig.compose_canonical(&mut buf).unwrap(); for record in rrset.iter() { record.compose_canonical(&mut buf).unwrap(); } + + // Create and push the RRSIG record. let signature = key.raw_secret_key().sign_raw(&buf).unwrap(); let signature = signature.as_ref().to_vec(); let Ok(signature) = signature.try_octets_into() else { return Err(ErrorTypeToBeDetermined); }; - // Create and push the RRSIG record. res.push(Record::new( name.owner().clone(), name.class(), @@ -1095,48 +1094,3 @@ pub enum Nsec3OptOut { // name, except for the types solely contributed by an NSEC3 RR // itself. Note that this means that the NSEC3 type itself will // never be present in the Type Bit Maps." -// #[cfg(test)] -// mod tests { -// use core::str::FromStr; - -// use crate::rdata::A; - -// use super::*; - -// #[test] -// fn nsec3s() { -// fn mk_test_record(name: &str) -> Record>, A> { -// Record::new( -// Name::>::from_str(name).unwrap(), -// Class::IN, -// Ttl::from_days(1), -// A::new("127.0.0.1".parse().unwrap()), -// ) -// } - -// let mut recs = SortedRecords::new(); -// recs.insert(mk_test_record("mail.example.com")).unwrap(); -// recs.insert(mk_test_record("x.y.mail.example.com")).unwrap(); -// recs.insert(mk_test_record("a.b.c.mail.example.com")) -// .unwrap(); -// recs.insert(mk_test_record("a.other.c.mail.example.com")) -// .unwrap(); - -// for rec in recs.families() { -// println!("{}", rec.family_name().owner()); -// } - -// let mut recs = SortedRecords::new(); -// recs.insert(mk_test_record("y.mail.example.com")).unwrap(); -// recs.insert(mk_test_record("c.mail.example.com")).unwrap(); -// recs.insert(mk_test_record("b.c.mail.example.com")).unwrap(); -// recs.insert(mk_test_record("c.mail.example.com")); -// recs.insert(mk_test_record("other.c.mail.example.com")) -// .unwrap(); - -// println!(); -// for rec in recs.families() { -// println!("{}", rec.family_name().owner()); -// } -// } -// }