Skip to content

The great rename plus cleanup. #488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 56 commits into from
Mar 31, 2025
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
3df971c
The great rename.
Philip-NLnetLabs Feb 5, 2025
0bf028f
Most of crypto requires ring.
Philip-NLnetLabs Feb 5, 2025
c09721b
Introduce unstable-sign and make sign conditional on ring.
Philip-NLnetLabs Feb 6, 2025
d04cc9f
Make cypto depend on unstable-sign for now.
Philip-NLnetLabs Feb 6, 2025
82b65ee
Get rid of the apex_owner parameter to sign_rrset_in. Take the name f…
Philip-NLnetLabs Feb 6, 2025
c2037fe
Fix tests.
Philip-NLnetLabs Feb 6, 2025
21cb3a8
Remove NsecToNsec3TransitionState, Nsec3ToNsecTransitionState,
Philip-NLnetLabs Feb 6, 2025
cdfbc54
Remove RrsigValidityPeriodStrategy.
Philip-NLnetLabs Feb 6, 2025
318ea4b
Fix tests.
Philip-NLnetLabs Feb 7, 2025
6cabdc4
No longer generate and sign the DNSKEY RRset.
Philip-NLnetLabs Feb 12, 2025
c991ea2
Fix documentation example.
Philip-NLnetLabs Feb 12, 2025
e6f225d
Introduce crypto::common::DigestContext, move nsec3_hash to dnssec::c…
Philip-NLnetLabs Feb 13, 2025
7b4885b
Add SHA-256 and use DigestContext in the signature cache.
Philip-NLnetLabs Feb 13, 2025
6e5227d
Move #[allow(unreachable_code)] to a different place.
Philip-NLnetLabs Feb 13, 2025
7ba5b4f
Move DnskeyExt from crypto::validate to dnssec::validator::base.
Philip-NLnetLabs Feb 14, 2025
5cfce9f
Add crypto::common::PublicKey. Move RrsigExt to dnssec::validator::base
Philip-NLnetLabs Feb 17, 2025
6be0156
Removed Key because it is mostly redundant.
Philip-NLnetLabs Feb 20, 2025
4e5b755
Remove PublicKeyBytes.
Philip-NLnetLabs Feb 26, 2025
1b40d27
Bump ring version to 0.17.2 to get AsRef<[u8]> for UnparsedPublicKey.
Philip-NLnetLabs Feb 26, 2025
e7b532a
Merge branch 'initial-nsec3-generation' into dnssec-restructure
Philip-NLnetLabs Feb 26, 2025
3b6786e
Clippy.
Philip-NLnetLabs Feb 26, 2025
3eeca20
Fmt
Philip-NLnetLabs Feb 26, 2025
eaf49fe
crypto is now independent of dnssec::sign.
Philip-NLnetLabs Feb 27, 2025
b7ecbcf
Fix some feature test issues.
Philip-NLnetLabs Feb 27, 2025
9f139b5
More feature issues.
Philip-NLnetLabs Feb 27, 2025
55cdb06
Remove hard dependency on ring for DNSSEC signing and validation.
Philip-NLnetLabs Mar 3, 2025
ce2ef08
Fix dependencies for the keyset example.
Philip-NLnetLabs Mar 3, 2025
8932a09
Introduce crypto::sign, get rid of crypto::misc.
Philip-NLnetLabs Mar 4, 2025
016ab54
Improve documentation for the crypto module.
Philip-NLnetLabs Mar 5, 2025
c27c048
Clippy found places where documentation was missing.
Philip-NLnetLabs Mar 5, 2025
b8dac08
Cleanup documentation for the dnssec module.
Philip-NLnetLabs Mar 6, 2025
486b4b9
Spell feature flags correctly.
Philip-NLnetLabs Mar 6, 2025
2e85e09
Improve feature tests.
Philip-NLnetLabs Mar 7, 2025
8609839
Remove Nsec3HashProvider.
Philip-NLnetLabs Mar 11, 2025
c465794
Fix example.
Philip-NLnetLabs Mar 11, 2025
7764b95
Rename Default to Rsa.
Philip-NLnetLabs Mar 25, 2025
feff956
Add support for ED448 to from_dnskey.
Philip-NLnetLabs Mar 25, 2025
097d1fe
Add support for MessageDigest::sha1 and MessageDigest::sha512 to dnskey.
Philip-NLnetLabs Mar 25, 2025
dc21eea
Use unreachable for some match cases that cannot happen.
Philip-NLnetLabs Mar 26, 2025
138d5f2
Some simplifications.
Philip-NLnetLabs Mar 26, 2025
da20ab1
Spelling correction.
ximon18 Mar 26, 2025
2348d9d
Typo correction.
ximon18 Mar 26, 2025
5762aeb
Update src/dnssec/sign/denial/config.rs
Philip-NLnetLabs Mar 26, 2025
d42be3f
Update Cargo.toml
Philip-NLnetLabs Mar 26, 2025
4f95050
Remove previous unstable-sign feature line.
Philip-NLnetLabs Mar 26, 2025
492db9d
Restore stelline.
Philip-NLnetLabs Mar 26, 2025
96dee31
Small doc fix.
Philip-NLnetLabs Mar 26, 2025
2fd6963
remove superfluous map_err.
Philip-NLnetLabs Mar 26, 2025
c53a21e
Rename DigestContext to DigestBuilder.
Philip-NLnetLabs Mar 27, 2025
f76adfc
Also rename in doc and test.
Philip-NLnetLabs Mar 27, 2025
d1dd7b0
Move doc string.
Philip-NLnetLabs Mar 28, 2025
dc82bcc
Use SecAlg instead of pointers.
Philip-NLnetLabs Mar 28, 2025
89a011d
Extract common RSA encoding code.
Philip-NLnetLabs Mar 28, 2025
25e43ab
Some comments.
Philip-NLnetLabs Mar 28, 2025
ef814a5
Move tests around.
Philip-NLnetLabs Mar 28, 2025
bbeb5f9
[crypto] Make minor changes
bal-e Mar 31, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
211 changes: 138 additions & 73 deletions Cargo.lock

Large diffs are not rendered by default.

13 changes: 8 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ parking_lot = { version = "0.12", optional = true }
moka = { version = "0.12.3", optional = true, features = ["future"] }
openssl = { version = "0.10.57", optional = true } # 0.10.57 upgrades to 'bitflags' 2.x
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 }
ring = { version = "0.17.2", optional = true }
rustversion = { version = "1", optional = true }
secrecy = { version = "0.10", optional = true }
serde = { version = "1.0.130", optional = true, features = ["derive"] }
Expand Down Expand Up @@ -72,11 +72,12 @@ zonefile = ["bytes", "serde", "std"]

# Unstable features
unstable-client-transport = ["moka", "net", "tracing"]
unstable-crypto = ["bytes"]
unstable-crypto-sign = ["dep:secrecy", "unstable-crypto"]
unstable-server-transport = ["arc-swap", "chrono/clock", "libc", "net", "siphasher", "tracing"]
unstable-sign = ["std", "dep:secrecy", "dep:smallvec", "dep:serde", "time/formatting", "tracing", "unstable-validate"]
unstable-sign = ["std", "dep:secrecy", "dep:smallvec", "dep:serde", "time/formatting", "tracing", "unstable-crypto-sign"]
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"]
unstable-validator = ["zonefile", "unstable-client-transport", "unstable-crypto"]
unstable-xfr = ["net"]
unstable-zonetree = ["futures-util", "parking_lot", "rustversion", "serde", "std", "tokio", "tracing", "unstable-xfr", "zonefile"]

Expand Down Expand Up @@ -155,7 +156,9 @@ required-features = ["net", "unstable-client-transport", "unstable-server-transp

[[example]]
name = "keyset"
required-features = ["serde", "unstable-sign"]
# Note that we have to pick a crypto backend eventhough the example doesn't
# need one.
required-features = ["serde", "unstable-sign", "ring"]

# This example is commented out because it is difficult, if not impossible,
# when including the sqlx dependency, to make the dependency tree compatible
Expand Down
16 changes: 9 additions & 7 deletions examples/client-transports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,15 @@ where
{
// Create a validating transport
let anchor_file = std::fs::File::open("examples/root.key").unwrap();
let ta =
domain::validator::anchor::TrustAnchors::from_reader(anchor_file)
.unwrap();
let vc = Arc::new(domain::validator::context::ValidationContext::new(
ta,
conn.clone(),
));
let ta = domain::dnssec::validator::anchor::TrustAnchors::from_reader(
anchor_file,
)
.unwrap();
let vc =
Arc::new(domain::dnssec::validator::context::ValidationContext::new(
ta,
conn.clone(),
));
let val_conn = domain::net::client::validator::Connection::new(conn, vc);

// Send a query message.
Expand Down
2 changes: 1 addition & 1 deletion examples/keyset.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Demonstrate the use of key sets.
use domain::base::Name;
use domain::sign::keys::keyset::{
use domain::dnssec::sign::keys::keyset::{
Action, Error, KeySet, KeyType, RollType, UnixTime,
};
use itertools::{Either, Itertools};
Expand Down
256 changes: 256 additions & 0 deletions src/crypto/common.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
//! DNSSEC message digests and signature verification 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.

#![cfg(any(feature = "ring", feature = "openssl"))]
#![cfg_attr(docsrs, doc(cfg(any(feature = "ring", feature = "openssl"))))]

use core::fmt;
use std::error;
use std::vec::Vec;

use crate::rdata::Dnskey;

#[cfg(feature = "openssl")]
use super::openssl;

#[cfg(feature = "ring")]
use super::ring;

//----------- DigestType -----------------------------------------------------

/// Type of message digest to compute.
pub enum DigestType {
/// [FIPS Secure Hash Standard] Section 6.1.
///
/// [FIPS Secure Hash Standard]: http://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf
Sha1,

/// [FIPS Secure Hash Standard] Section 6.2.
///
/// [FIPS Secure Hash Standard]: http://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf
Sha256,

/// [FIPS Secure Hash Standard] Section 6.5.
///
/// [FIPS Secure Hash Standard]: http://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.180-4.pdf
Sha384,
}

//----------- DigestContext --------------------------------------------------

/// Context for computing a message digest.
pub enum DigestContext {
#[cfg(feature = "ring")]
/// Use ring to compute the message digest.
Ring(ring::DigestContext),
#[cfg(feature = "openssl")]
/// Use openssl to compute the message digest.
Openssl(openssl::DigestContext),
}

impl DigestContext {
#[allow(unreachable_code)]
/// Create a new context for a specified digest type.
pub fn new(digest_type: DigestType) -> Self {
#[cfg(feature = "ring")]
return Self::Ring(ring::DigestContext::new(digest_type));

#[cfg(feature = "openssl")]
return Self::Openssl(openssl::DigestContext::new(digest_type));

#[cfg(not(any(feature = "ring", feature = "openssl")))]
compile_error!("Either feature \"ring\" or \"openssl\" must be enabled for this crate.");
}

/// Add input to the digest computation.
pub fn update(&mut self, data: &[u8]) {
match self {
#[cfg(feature = "ring")]
DigestContext::Ring(digest_context) => {
digest_context.update(data)
}
#[cfg(feature = "openssl")]
DigestContext::Openssl(digest_context) => {
digest_context.update(data)
}
}
}

/// Finish computing the digest.
pub fn finish(self) -> Digest {
match self {
#[cfg(feature = "ring")]
DigestContext::Ring(digest_context) => {
Digest::Ring(digest_context.finish())
}
#[cfg(feature = "openssl")]
DigestContext::Openssl(digest_context) => {
Digest::Openssl(digest_context.finish())
}
}
}
}

//----------- Digest ---------------------------------------------------------

/// A message digest.
pub enum Digest {
#[cfg(feature = "ring")]
/// A message digest computed using ring.
Ring(ring::Digest),
#[cfg(feature = "openssl")]
/// A message digest computed using openssl.
Openssl(openssl::Digest),
}

impl AsRef<[u8]> for Digest {
fn as_ref(&self) -> &[u8] {
match self {
#[cfg(feature = "ring")]
Digest::Ring(digest) => digest.as_ref(),
#[cfg(feature = "openssl")]
Digest::Openssl(digest) => digest.as_ref(),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that Digest needs to be an enum over all the cryptographic backends. At the end of the day, it's just a (DigestType, Vec<u8>), right? Perhaps we should define it that way for simplicity.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that that would require an extra Vec. No it just gets AsRef from the underlying ring or openssl type.


//----------- PublicKey ------------------------------------------------------

/// A public key for verifying a signature.
pub enum PublicKey {
#[cfg(feature = "ring")]
/// A public key implemented using ring.
Ring(ring::PublicKey),

#[cfg(feature = "openssl")]
/// A public key implemented using openssl.
Openssl(openssl::PublicKey),
}

impl PublicKey {
#[allow(unreachable_code)]
/// Create a public key from a [`Dnskey`].
pub fn from_dnskey(
dnskey: &Dnskey<impl AsRef<[u8]>>,
) -> Result<Self, AlgorithmError> {
#[cfg(feature = "ring")]
return Ok(Self::Ring(ring::PublicKey::from_dnskey(dnskey)?));

#[cfg(feature = "openssl")]
return Ok(Self::Openssl(openssl::PublicKey::from_dnskey(dnskey)?));

#[cfg(not(any(feature = "ring", feature = "openssl")))]
compile_error!("Either feature \"ring\" or \"openssl\" must be enabled for this crate.");
}

/// Verify a signature.
pub fn verify(
&self,
signed_data: &[u8],
signature: &[u8],
) -> Result<(), AlgorithmError> {
match self {
#[cfg(feature = "ring")]
PublicKey::Ring(public_key) => {
public_key.verify(signed_data, signature)
}
#[cfg(feature = "openssl")]
PublicKey::Openssl(public_key) => {
public_key.verify(signed_data, signature)
}
}
}
}

/// Return the RSA exponent and modulus components from DNSKEY record data.
pub fn rsa_exponent_modulus(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is specific to DNSKEY records, but this module seems to handle cryptography outside the context of DNS. Would the dnssec module be more appropriate to handle it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to keep crypto self contained and not depend on dnssec. So crypto uses the types from base and dnssec uses crypto.

dnskey: &Dnskey<impl AsRef<[u8]>>,
min_len: usize,
) -> Result<(Vec<u8>, Vec<u8>), AlgorithmError> {
let public_key = dnskey.public_key().as_ref();
if public_key.len() <= 3 {
return Err(AlgorithmError::InvalidData);
}

let (pos, exp_len) = match public_key[0] {
0 => (
3,
(usize::from(public_key[1]) << 8) | usize::from(public_key[2]),
),
len => (1, usize::from(len)),
};

// Check if there's enough space for exponent and modulus.
if public_key[pos..].len() < pos + exp_len {
return Err(AlgorithmError::InvalidData);
};

// Check for minimum supported key size
if public_key[pos..].len() < min_len {
return Err(AlgorithmError::Unsupported);
}

let (e, n) = public_key[pos..].split_at(exp_len);
Ok((e.to_vec(), n.to_vec()))
}

//------------ AlgorithmError ------------------------------------------------

/// An algorithm error during verification.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum AlgorithmError {
/// Unsupported algorithm.
Unsupported,

/// Bad signature.
BadSig,

/// Invalid data.
InvalidData,
}

//--- Display, Error

impl fmt::Display for AlgorithmError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(match self {
AlgorithmError::Unsupported => "unsupported algorithm",
AlgorithmError::BadSig => "bad signature",
AlgorithmError::InvalidData => "invalid data",
})
}
}

impl error::Error for AlgorithmError {}

//----------- FromDnskeyError ------------------------------------------------

/// An error in reading a DNSKEY record.
#[derive(Clone, Debug)]
pub enum FromDnskeyError {
/// The key's algorithm is not supported.
UnsupportedAlgorithm,

/// The key's protocol is not supported.
UnsupportedProtocol,

/// The key is not valid.
InvalidKey,
}

//--- Display, Error

impl fmt::Display for FromDnskeyError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(match self {
Self::UnsupportedAlgorithm => "unsupported algorithm",
Self::UnsupportedProtocol => "unsupported protocol",
Self::InvalidKey => "malformed key",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this has only been moved so is not new, but looking at it I wonder are "invalid" and "malformed" really the same thing? Something can be well formed but still invalid, right? So maybe it would be better to make these consistent, in which case what is it: invalid or malformed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is helpful to start a separate discussion about code that just got moved. The code that got moved has many different authors. Do we want have a discussion about all of that in this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As, in my opinion, excluding minor somewhat unrelated changes wouldn't simplify the review process, and this is a general "cleanup" PR, I'm in favour of taking any additional minor improvements at the same time in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fine as long as there is no expectation that I will make the changes.

})
}
}

impl error::Error for FromDnskeyError {}
Loading