Skip to content

Add logging to NSEC3 and RRSIG generation. #523

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.

Sign up for GitHub

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

Open
ximon18 wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add logging to NSEC3 and RRSIG generation. #523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
192ee48
Remove incorrect logic for determining the apex from signing function…
ximon18 May 8, 2025
fd2ba79
Add logging to NSEC3 and RRSIG generation as is done for NSEC generat…
ximon18 May 8, 2025
5686da0
Fix compilation failures caused by git hunk staging and unstaging.
ximon18 May 8, 2025
2b0453b
Fix outdated doc test.
ximon18 May 8, 2025
c60bf01
Cargo fmt.
ximon18 May 8, 2025
b386697
Clippy.
ximon18 May 8, 2025
e29edf3
Merge branch 'fix-signing-with-early-glue' into add-logging-to-nsec3-…
ximon18 May 8, 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
115 changes: 75 additions & 40 deletions src/dnssec/sign/denial/nsec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use core::cmp::min;
use core::fmt::Debug;
use core::fmt::{Debug, Display};

use std::vec::Vec;

use octseq::builder::{EmptyBuilder, FromBuilder, OctetsBuilder, Truncate};
use tracing::{debug, trace};

use crate::base::iana::Rtype;
use crate::base::name::ToName;
Expand Down Expand Up @@ -67,39 +68,56 @@ impl Default for GenerateNsecConfig {
// TODO: Add (mutable?) iterator based variant.
#[allow(clippy::type_complexity)]
pub fn generate_nsecs<N, Octs>(
records: RecordsIter<'_, N, ZoneRecordData<Octs, N>>,
apex_owner: &N,
mut records: RecordsIter<'_, N, ZoneRecordData<Octs, N>>,
config: &GenerateNsecConfig,
) -> Result<Vec<Record<N, Nsec<Octs, N>>>, SigningError>
where
N: ToName + Clone + PartialEq,
N: ToName + Clone + Display + PartialEq,
Octs: FromBuilder,
Octs::Builder: EmptyBuilder + Truncate + AsRef<[u8]> + AsMut<[u8]>,
<Octs::Builder as OctetsBuilder>::AppendError: Debug,
{
let mut res = Vec::new();
// The generated collection of NSEC RRs that will be returned to the
// caller.
let mut nsecs = Vec::new();

// The CLASS to use for NSEC records. This will be determined per the rules
// in RFC 9077 once the apex SOA RR is found.
let mut zone_class = None;

// The TTL to use for NSEC records. This will be determined per the rules
// in RFC 9077 once the apex SOA RR is found.
let mut nsec_ttl = None;

// The owner name of a zone cut if we currently are at or below one.
let mut cut: Option<N> = None;

// Because of the next name thing, we need to keep the last NSEC around.
let mut prev: Option<(N, RtypeBitmap<Octs>)> = None;

// We also need the apex for the last NSEC.
let first_rr = records.first();
let apex_owner = first_rr.owner().clone();
let zone_class = first_rr.class();
let mut ttl = None;
// Skip any glue or other out-of-zone records that sort earlier than
// the zone apex.
records.skip_before(apex_owner);

for owner_rrs in records {
// If the owner is out of zone, we have moved out of our zone and are
// done.
if !owner_rrs.is_in_zone(&apex_owner) {
if !owner_rrs.is_in_zone(apex_owner) {
debug!(
"Stopping at owner {} as it is out of zone and assumed to trail the zone",
owner_rrs.owner()
);
break;
}

// If the owner is below a zone cut, we must ignore it.
if let Some(ref cut) = cut {
if owner_rrs.owner().ends_with(cut) {
debug!(
"Excluding owner {} as it is below a zone cut",
owner_rrs.owner()
);
continue;
}
}
Expand All @@ -110,18 +128,20 @@ where
// If this owner is the parent side of a zone cut, we keep the owner
// name for later. This also means below that if `cut.is_some()` we
// are at the parent side of a zone.
cut = if owner_rrs.is_zone_cut(&apex_owner) {
cut = if owner_rrs.is_zone_cut(apex_owner) {
trace!("Zone cut detected at owner {}", owner_rrs.owner());
Some(name.clone())
} else {
None
};

if let Some((prev_name, bitmap)) = prev.take() {
// SAFETY: ttl will be set below before prev is set to Some.
res.push(Record::new(
// SAFETY: nsec_ttl and zone_class will be set below before prev
// is set to Some.
nsecs.push(Record::new(
prev_name.clone(),
zone_class,
ttl.unwrap(),
zone_class.unwrap(),
nsec_ttl.unwrap(),
Nsec::new(name.clone(), bitmap),
));
}
Expand All @@ -135,7 +155,7 @@ where
bitmap.add(Rtype::RRSIG).unwrap();

if config.assume_dnskeys_will_be_added
&& owner_rrs.owner() == &apex_owner
&& owner_rrs.owner() == apex_owner
{
// Assume there's gonna be a DNSKEY.
bitmap.add(Rtype::DNSKEY).unwrap();
Expand All @@ -161,68 +181,81 @@ where
// that we require already excludes "pseudo" record types,
// those are only included as member variants of the
// AllRecordData type.
trace!("Adding {} to the bitmap", rrset.rtype());
bitmap.add(rrset.rtype()).unwrap()
}

if rrset.rtype() == Rtype::SOA {
if rrset.len() > 1 {
debug!("Aborting at owner {} as SOA RRSET containers more than one ({}) RR which is unexpected.", owner_rrs.owner(), rrset.len());
return Err(SigningError::SoaRecordCouldNotBeDetermined);
}

let soa_rr = rrset.first();

// Check that the RDATA for the SOA record can be parsed.
let ZoneRecordData::Soa(ref soa_data) = soa_rr.data() else {
debug!(
"Aborting at owner {} as SOA RR could not be parsed",
owner_rrs.owner()
);
return Err(SigningError::SoaRecordCouldNotBeDetermined);
};

// RFC 9077 updated RFC 4034 (NSEC) and RFC 5155 (NSEC3) to
// say that the "TTL of the NSEC(3) RR that is returned MUST
// be the lesser of the MINIMUM field of the SOA record and
// the TTL of the SOA itself".
ttl = Some(min(soa_data.minimum(), soa_rr.ttl()));
nsec_ttl = Some(min(soa_data.minimum(), soa_rr.ttl()));

zone_class = Some(rrset.class());
}
}

if ttl.is_none() {
if nsec_ttl.is_none() {
debug!("Aborting as SOA TTL could not be determined");
return Err(SigningError::SoaRecordCouldNotBeDetermined);
}

prev = Some((name, bitmap.finalize()));
}

if let Some((prev_name, bitmap)) = prev {
res.push(Record::new(
// SAFETY: nsec_ttl and zone_class will be set above before prev
// is set to Some.
nsecs.push(Record::new(
prev_name.clone(),
zone_class,
ttl.unwrap(),
zone_class.unwrap(),
nsec_ttl.unwrap(),
Nsec::new(apex_owner.clone(), bitmap),
));
}

Ok(res)
Ok(nsecs)
}

#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;

use crate::base::Ttl;
use crate::base::{Name, Ttl};
use crate::dnssec::sign::records::SortedRecords;
use crate::dnssec::sign::test_util::*;
use crate::zonetree::types::StoredRecordData;
use crate::zonetree::StoredName;

use super::*;
use core::str::FromStr;

type StoredSortedRecords = SortedRecords<StoredName, StoredRecordData>;

#[test]
fn soa_is_required() {
let cfg = GenerateNsecConfig::default()
.without_assuming_dnskeys_will_be_added();
let apex = Name::from_str("a.").unwrap();
let records = StoredSortedRecords::from_iter([mk_a_rr("some_a.a.")]);
let res = generate_nsecs(records.owner_rrs(), &cfg);
let res = generate_nsecs(&apex, records.owner_rrs(), &cfg);
assert!(matches!(
res,
Err(SigningError::SoaRecordCouldNotBeDetermined)
Expand All @@ -233,11 +266,12 @@ mod tests {
fn multiple_soa_rrs_in_the_same_rrset_are_not_permitted() {
let cfg = GenerateNsecConfig::default()
.without_assuming_dnskeys_will_be_added();
let apex = Name::from_str("a.").unwrap();
let records = StoredSortedRecords::from_iter([
mk_soa_rr("a.", "b.", "c."),
mk_soa_rr("a.", "d.", "e."),
]);
let res = generate_nsecs(records.owner_rrs(), &cfg);
let res = generate_nsecs(&apex, records.owner_rrs(), &cfg);
assert!(matches!(
res,
Err(SigningError::SoaRecordCouldNotBeDetermined)
Expand All @@ -248,19 +282,18 @@ mod tests {
fn records_outside_zone_are_ignored() {
let cfg = GenerateNsecConfig::default()
.without_assuming_dnskeys_will_be_added();
let a_apex = Name::from_str("a.").unwrap();
let b_apex = Name::from_str("b.").unwrap();
let records = StoredSortedRecords::from_iter([
mk_soa_rr("b.", "d.", "e."),
mk_a_rr("some_a.b."),
mk_soa_rr("a.", "b.", "c."),
mk_a_rr("some_a.a."),
]);

// First generate NSECs for the total record collection. As the
// collection is sorted in canonical order the a zone preceeds the b
// zone and NSECs should only be generated for the first zone in the
// collection.
let a_and_b_records = records.owner_rrs();
let nsecs = generate_nsecs(a_and_b_records, &cfg).unwrap();
// Generate NSEs for the a. zone.
let nsecs =
generate_nsecs(&a_apex, records.owner_rrs(), &cfg).unwrap();

assert_eq!(
nsecs,
Expand All @@ -270,11 +303,9 @@ mod tests {
]
);

// Now skip the a zone in the collection and generate NSECs for the
// remaining records which should only generate NSECs for the b zone.
let mut b_records_only = records.owner_rrs();
b_records_only.skip_before(&mk_name("b."));
let nsecs = generate_nsecs(b_records_only, &cfg).unwrap();
// Generate NSECs for the b. zone.
let nsecs =
generate_nsecs(&b_apex, records.owner_rrs(), &cfg).unwrap();

assert_eq!(
nsecs,
Expand All @@ -289,13 +320,14 @@ mod tests {
fn occluded_records_are_ignored() {
let cfg = GenerateNsecConfig::default()
.without_assuming_dnskeys_will_be_added();
let apex = Name::from_str("a.").unwrap();
let records = StoredSortedRecords::from_iter([
mk_soa_rr("a.", "b.", "c."),
mk_ns_rr("some_ns.a.", "some_a.other.b."),
mk_a_rr("some_a.some_ns.a."),
]);

let nsecs = generate_nsecs(records.owner_rrs(), &cfg).unwrap();
let nsecs = generate_nsecs(&apex, records.owner_rrs(), &cfg).unwrap();

// Implicit negative test.
assert_eq!(
Expand All @@ -314,12 +346,13 @@ mod tests {
fn expect_dnskeys_at_the_apex() {
let cfg = GenerateNsecConfig::default();

let apex = Name::from_str("a.").unwrap();
let records = StoredSortedRecords::from_iter([
mk_soa_rr("a.", "b.", "c."),
mk_a_rr("some_a.a."),
]);

let nsecs = generate_nsecs(records.owner_rrs(), &cfg).unwrap();
let nsecs = generate_nsecs(&apex, records.owner_rrs(), &cfg).unwrap();

assert_eq!(
nsecs,
Expand All @@ -340,8 +373,9 @@ mod tests {
"../../../../test-data/zonefiles/rfc4035-appendix-A.zone"
);

let apex = Name::from_str("example.").unwrap();
let records = bytes_to_records(&zonefile[..]);
let nsecs = generate_nsecs(records.owner_rrs(), &cfg).unwrap();
let nsecs = generate_nsecs(&apex, records.owner_rrs(), &cfg).unwrap();

assert_eq!(nsecs.len(), 10);

Expand Down Expand Up @@ -453,14 +487,15 @@ mod tests {
fn existing_nsec_records_are_ignored() {
let cfg = GenerateNsecConfig::default();

let apex = Name::from_str("a.").unwrap();
let records = StoredSortedRecords::from_iter([
mk_soa_rr("a.", "b.", "c."),
mk_a_rr("some_a.a."),
mk_nsec_rr("a.", "some_a.a.", "SOA NSEC"),
mk_nsec_rr("some_a.a.", "a.", "A RRSIG NSEC"),
]);

let nsecs = generate_nsecs(records.owner_rrs(), &cfg).unwrap();
let nsecs = generate_nsecs(&apex, records.owner_rrs(), &cfg).unwrap();

assert_eq!(
nsecs,
Expand Down
Loading
Loading