colin/hickory-dns
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

make dnssec methods more type safe

Browse Source
This commit is contained in:
Benjamin Fry 2023-10-26 17:00:46 -07:00
parent bc044e89f3
commit c0c2b2fa89
6 changed files with 162 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
crates/proto/src/rr/dnssec/signer.rs
View File

@ -675,12 +675,10 @@ mod tests {
let signer = SigSigner::sig0(sig0key, key, Name::root());
let origin: Name = Name::parse("example.com.", None).unwrap();
let rrsig = Record::new()
.set_name(origin.clone())
.set_ttl(86400)
.set_rr_type(RecordType::RRSIG)
.set_dns_class(DNSClass::IN)
.set_data(Some(RRSIG::new(
let rrsig = Record::from_rdata(
origin.clone(),
86400,
RRSIG::new(
RecordType::NS,
Algorithm::RSASHA256,
origin.num_labels(),
@ -690,8 +688,9 @@ mod tests {
signer.calculate_key_tag().unwrap(),
origin.clone(),
vec![],
)))
.clone();
),
);
let rrset = vec![
Record::new()
.set_name(origin.clone())
@ -814,12 +813,10 @@ MC0CAQACBQC+L6pNAgMBAAECBQCYj0ZNAgMA9CsCAwDHZwICeEUCAnE/AgMA3u0=
let signer = SigSigner::sig0(sig0key, key, Name::root());
let origin: Name = Name::parse("example.com.", None).unwrap();
let rrsig = Record::new()
.set_name(origin.clone())
.set_ttl(86400)
.set_rr_type(RecordType::RRSIG)
.set_dns_class(DNSClass::IN)
.set_data(Some(RRSIG::new(
let rrsig = Record::from_rdata(
origin.clone(),
86400,
RRSIG::new(
RecordType::NS,
Algorithm::RSASHA256,
origin.num_labels(),
@ -829,8 +826,8 @@ MC0CAQACBQC+L6pNAgMBAAECBQCYj0ZNAgMA9CsCAwDHZwICeEUCAnE/AgMA3u0=
signer.calculate_key_tag().unwrap(),
origin.clone(),
vec![],
)))
.clone();
),
);
let rrset = vec![
Record::new()
.set_name(origin.clone())

2
crates/proto/src/rr/dnssec/verifier.rs
View File

@ -14,7 +14,7 @@ use crate::{
rdata::{DNSKEY, KEY, RRSIG, SIG},
tbs, Algorithm, PublicKey, PublicKeyEnum,
},
DNSClass, Name, Record,
DNSClass, Name, RData, Record,
},
serialize::binary::BinEncodable,
};

4
crates/proto/src/rr/mod.rs
View File

@ -22,7 +22,7 @@ mod rr_key;
mod rr_set;
pub mod type_bit_map;
use std::fmt;
use std::fmt::{Debug, Display};
use crate::{
error::ProtoResult,
@ -44,7 +44,7 @@ pub use rr_key::RrKey;
/// RecordData that is stored in a DNS Record.
///
/// This trait allows for generic usage of `RecordData` types inside the `Record` type. Specific RecordData types can be used to enforce compile time constraints on a Record.
pub trait RecordData: Clone + Sized + PartialEq + Eq + fmt::Display + BinEncodable {
pub trait RecordData: Clone + Sized + PartialEq + Eq + Display + Debug + BinEncodable {
/// Attempts to convert to this RecordData from the RData type, if it is not the correct type the original is returned
#[allow(clippy::result_large_err)]
fn try_from_rdata(data: RData) -> Result<Self, RData>;

67
crates/proto/src/rr/resource.rs
View File

@ -80,7 +80,7 @@ pub struct Record<R: RecordData = RData> {
mdns_cache_flush: bool,
}
impl<R: RecordData> Default for Record<R> {
impl Default for Record<RData> {
fn default() -> Self {
Self {
// TODO: these really should all be Optionals, I was lazy.
@ -95,7 +95,7 @@ impl<R: RecordData> Default for Record<R> {
}
}
impl<R: RecordData> Record<R> {
impl Record<RData> {
/// Creates a default record, use the setters to build a more useful object.
///
/// There are no optional elements in this object, defaults are an empty name, type A, class IN,
@ -111,6 +111,7 @@ impl<R: RecordData> Record<R> {
/// * `name` - name of the resource records
/// * `rr_type` - the record type
/// * `ttl` - time-to-live is the amount of time this record should be cached before refreshing
#[deprecated = "consider using the typed variant `from_rdata`"]
pub fn with(name: Name, rr_type: RecordType, ttl: u32) -> Self {
Self {
name_labels: name,
@ -123,6 +124,33 @@ impl<R: RecordData> Record<R> {
}
}
/// ```text
/// TYPE two octets containing one of the RR type codes. This
/// field specifies the meaning of the data in the RDATA
/// field.
/// ```
#[deprecated(note = "use `set_record_type`")]
pub fn set_rr_type(&mut self, rr_type: RecordType) -> &mut Self {
self.rr_type = rr_type;
self
}
/// Generally Speaking, this is redundant to the RecordType stored in the associated RData and not recommended
/// to set this separately. Exceptions to this are for Update Messages, where the RecordType is used distinctly
/// as a means to express certain Update instructions. For queries and responses, it will always match the RData
///
/// ```text
/// TYPE two octets containing one of the RR type codes. This
/// field specifies the meaning of the data in the RDATA
/// field.
/// ```
pub fn set_record_type(&mut self, rr_type: RecordType) -> &mut Self {
self.rr_type = rr_type;
self
}
}
impl<R: RecordData> Record<R> {
/// Create a record with the specified initial values.
///
/// # Arguments
@ -156,7 +184,7 @@ impl<R: RecordData> Record<R> {
} = record;
match rdata.map(R::try_from_rdata) {
None => Ok(Self {
None => Err(Record {
name_labels,
rr_type,
dns_class,
@ -167,7 +195,7 @@ impl<R: RecordData> Record<R> {
}),
Some(Ok(rdata)) => Ok(Self {
name_labels,
rr_type,
rr_type: rdata.record_type(),
dns_class,
ttl,
rdata: Some(rdata),
@ -219,27 +247,6 @@ impl<R: RecordData> Record<R> {
self
}
/// ```text
/// TYPE two octets containing one of the RR type codes. This
/// field specifies the meaning of the data in the RDATA
/// field.
/// ```
// #[deprecated(note = "use `Record::set_record_type`")]
pub fn set_rr_type(&mut self, rr_type: RecordType) -> &mut Self {
self.rr_type = rr_type;
self
}
/// ```text
/// TYPE two octets containing one of the RR type codes. This
/// field specifies the meaning of the data in the RDATA
/// field.
/// ```
pub fn set_record_type(&mut self, rr_type: RecordType) -> &mut Self {
self.rr_type = rr_type;
self
}
/// ```text
/// CLASS two octets which specify the class of the data in the
/// RDATA field.
@ -272,12 +279,12 @@ impl<R: RecordData> Record<R> {
pub fn set_data(&mut self, rdata: Option<R>) -> &mut Self {
debug_assert!(
if let Some(rdata) = &rdata {
rdata.record_type() == self.record_type() || rdata.record_type() == RecordType::NULL
rdata.record_type() == self.rr_type || rdata.record_type() == RecordType::NULL
} else {
true
},
"record types do not match, {} <> {:?}",
self.record_type(),
self.rr_type,
rdata.map(|r| r.record_type())
);
@ -876,7 +883,7 @@ mod tests {
let mut record = Record::new();
record
.set_name(Name::from_str("www.example.com").unwrap())
.set_rr_type(RecordType::A)
.set_record_type(RecordType::A)
.set_dns_class(DNSClass::IN)
.set_ttl(5)
.set_data(Some(RData::A(A::new(192, 168, 0, 1))));
@ -884,8 +891,8 @@ mod tests {
let mut greater_name = record.clone();
greater_name.set_name(Name::from_str("zzz.example.com").unwrap());
let mut greater_type = record.clone();
greater_type.set_rr_type(RecordType::AAAA);
let mut greater_type = record.clone().into_record_of_rdata();
greater_type.set_record_type(RecordType::AAAA);
let mut greater_class = record.clone();
greater_class.set_dns_class(DNSClass::NONE);

154
crates/proto/src/xfer/dnssec_dns_handle.rs
View File

@ -20,10 +20,11 @@ use crate::{
op::{Edns, OpCode, Query},
rr::{
dnssec::{
rdata::{DNSSECRData, DNSKEY, RRSIG},
rdata::{DNSSECRData, DNSKEY, DS, RRSIG},
Algorithm, Proof, SupportedAlgorithms, TrustAnchor,
},
rdata::opt::EdnsOption,
resource::RecordRef,
DNSClass, Name, RData, Record, RecordData, RecordType,
},
xfer::{dns_handle::DnsHandle, DnsRequest, DnsRequestOptions, DnsResponse, FirstAnswer},
@ -33,11 +34,74 @@ use crate::{
use crate::rr::dnssec::Verifier;
#[derive(Debug)]
struct Rrset {
struct Rrset<R: RecordData = RData> {
pub(crate) name: Name,
pub(crate) record_type: RecordType,
pub(crate) record_class: DNSClass,
pub(crate) records: Vec<Record>,
pub(crate) records: Vec<Record<R>>,
}
impl Rrset<RData> {
/// Attempts to convert into an Rrset of the specific RecordData type
fn try_into_record<R: RecordData>(self) -> Result<Rrset<R>, Self> {
// To avoid allocating we scan first to make sure all the types are valid
if !self
.records
.iter()
.filter_map(|r| r.data())
.all(|data| R::try_borrow(data).is_some())
{
return Err(self);
}
let Self {
name,
record_type,
record_class,
records,
} = self;
let original_len = records.len();
// This allocation is unfortunate,
let ok = records
.into_iter()
.map_while(|r| Record::<R>::try_from(r.clone()).ok())
.collect::<Vec<Record<R>>>();
debug_assert_eq!(ok.len(), original_len);
Ok(Rrset {
name,
record_type,
record_class,
records: ok,
})
}
}
impl<R: RecordData> Rrset<R> {
/// Infallible conversion from a specific record type generic RData
fn into_rdata(self) -> Rrset<RData> {
let Self {
name,
record_type,
record_class,
records,
} = self;
let records = records
.into_iter()
.map(|r| r.into_record_of_rdata())
.collect::<Vec<Record<RData>>>();
Rrset {
name,
record_type,
record_class,
records,
}
}
}
/// Performs DNSSEC validation of all DNS responses from the wrapped DnsHandle
@ -292,16 +356,8 @@ where
.iter()
.chain(message_result.name_servers())
.chain(message_result.additionals())
.filter(|rr| is_dnssec(rr, RecordType::RRSIG))
.filter(|rr| {
if let Some(RData::DNSSEC(DNSSECRData::RRSIG(ref rrsig))) = rr.data() {
rrsig.type_covered() == record_type
} else {
false
}
})
.cloned()
.map(|rr| Record::<RRSIG>::try_from(rr).expect("the record type was checked above"))
.filter_map(|rr| RecordRef::<RRSIG>::try_from(rr).ok())
.map(|rr| rr.to_owned())
.collect();
// if there is already an active validation going on, assume the other validation will
@ -443,15 +499,26 @@ async fn verify_rrset<H>(
where
H: DnsHandle + Sync + Unpin,
{
// wrapper for some of the type conversion for typed DNSKEY fn calls.
let typed_verify_dnskey = move |handle, rrset: Rrset| {
async move {
// TODO: validate that this DNSKEY is stronger than the one lower in the chain,
// also, set the min algorithm to this algorithm to prevent downgrade attacks.
let rrset = rrset.try_into_record::<DNSKEY>().map_err(|_| {
ProtoError::from(ProtoErrorKind::Message("Could not validate all DNSKEYs"))
})?;
let result = verify_dnskey_rrset(handle, rrset, options).await?;
Ok(result.into_rdata())
}
};
// Special case for unsigned DNSKEYs, it's valid for a DNSKEY to be bare in the zone if
// it's a trust_anchor, though some DNS servers choose to self-sign in this case,
// for self-signed KEYS they will drop through to the standard validation logic.
if let RecordType::DNSKEY = rrset.record_type {
if rrsigs.is_empty() {
debug!("unsigned key: {}, {:?}", rrset.name, rrset.record_type);
// TODO: validate that this DNSKEY is stronger than the one lower in the chain,
// also, set the min algorithm to this algorithm to prevent downgrade attacks.
return verify_dnskey_rrset(handle.clone_with_context(), rrset, options).await;
return typed_verify_dnskey(handle.clone_with_context(), rrset).await;
}
}
@ -460,7 +527,7 @@ where
// validation of DNSKEY records
match rrset.record_type {
RecordType::DNSKEY => verify_dnskey_rrset(handle, rrset, options).await,
RecordType::DNSKEY => typed_verify_dnskey(handle.clone_with_context(), rrset).await,
_ => Ok(rrset),
}
}
@ -472,9 +539,9 @@ where
/// against the DS record.
async fn verify_dnskey_rrset<H>(
handle: DnssecDnsHandle<H>,
rrset: Rrset,
rrset: Rrset<DNSKEY>,
options: DnsRequestOptions,
) -> Result<Rrset, ProtoError>
) -> Result<Rrset<DNSKEY>, ProtoError>
where
H: DnsHandle + Sync + Unpin,
{
@ -490,9 +557,7 @@ where
.records
.iter()
.enumerate()
.filter(|&(_, rr)| is_dnssec(rr, RecordType::DNSKEY))
.filter_map(|(i, rr)| rr.data().map(|rr| (i, rr)))
.filter_map(|(i, rr)| DNSKEY::try_borrow(rr).map(|rr| (i, rr)))
.filter_map(|(i, rr)| rr.data().map(|d| (i, d)))
.filter_map(|(i, rdata)| {
if handle
.trust_anchor
@ -526,32 +591,19 @@ where
.records
.iter()
.enumerate()
.filter(|&(_, rr)| is_dnssec(rr, RecordType::DNSKEY))
.filter_map(|(i, rr)| {
if let Some(RData::DNSSEC(DNSSECRData::DNSKEY(ref rdata))) = rr.data() {
Some((i, rdata))
} else {
None
}
})
.filter_map(|(i, rr)| rr.data().map(|d| (i, d)))
.filter(|&(_, key_rdata)| {
ds_message
.answers()
.iter()
.filter(|ds| is_dnssec(ds, RecordType::DS))
.filter_map(|ds| {
if let Some(RData::DNSSEC(DNSSECRData::DS(ref ds_rdata))) = ds.data() {
Some((ds.name(), ds_rdata))
} else {
None
}
})
.filter_map(|r| r.data().map(|d| (d, r.name())))
.filter_map(|(ds, n)| DS::try_borrow(ds).map(|ds| (ds, n)))
// must be covered by at least one DS record
.any(|(ds_name, ds_rdata)| {
.any(|(ds_rdata, ds_name)| {
if ds_rdata.covers(&rrset.name, key_rdata).unwrap_or(false) {
debug!(
"validated dnskey ({}, {}) with {} {}",
rrset.name, key_rdata, ds_name, ds_rdata
"validated dnskey ({}, {key_rdata}) with {ds_name} {ds_rdata}",
rrset.name
);
true
@ -661,7 +713,6 @@ where
// Special case for self-signed DNSKEYS, validate with itself...
if rrsigs
.iter()
.filter(|rrsig| is_dnssec(rrsig, RecordType::RRSIG))
.filter_map(|rrsig| rrsig.data())
.any(|rrsig| RecordType::DNSKEY == rrset.record_type && rrsig.signer_name() == &rrset.name)
{
@ -672,20 +723,19 @@ where
return future::ready(
rrsigs
.into_iter()
// this filter is technically unnecessary, can probably remove it...
.filter(|rrsig| is_dnssec(rrsig, RecordType::RRSIG))
.filter_map(|rrsig| rrsig.into_data())
.filter_map(|sig| {
let rrset = Arc::clone(&rrset);
if rrset.records.iter().any(|r| {
if let Some(RData::DNSSEC(DNSSECRData::DNSKEY(ref dnskey))) = r.data() {
let dnskey_name = r.name();
if rrset
.records
.iter()
.filter_map(|r| r.data().map(|d| (d, r.name())))
.filter_map(|(d, n)| DNSKEY::try_borrow(d).map(|d| (d, n)))
.any(|(dnskey, dnskey_name)| {
verify_rrset_with_dnskey(dnskey_name, dnskey, &sig, &rrset).is_ok()
} else {
panic!("expected a DNSKEY here: {:?}", r.data());
}
}) {
})
{
Some(())
} else {
None
@ -710,8 +760,6 @@ where
// dns over TLS will mitigate this.
// TODO: strip RRSIGS to accepted algorithms and make algorithms configurable.
let verifications = rrsigs.into_iter()
// this filter is technically unnecessary, can probably remove it...
.filter(|rrsig| is_dnssec(rrsig, RecordType::RRSIG))
.filter_map(|rrsig|rrsig.into_data())
.map(|sig| {
let rrset = Arc::clone(&rrset);

12
crates/server/tests/authority_battery/dnssec.rs
View File

@ -220,7 +220,8 @@ pub fn test_nsec_nodata<A: Authority<Lookup = AuthLookup>>(authority: A, keys: &
&query,
&Name::from_str("example.com.").unwrap(),
&nsecs
));
)
.is_secure());
}
pub fn test_nsec_nxdomain_start<A: Authority<Lookup = AuthLookup>>(authority: A, keys: &[DNSKEY]) {
@ -252,7 +253,8 @@ pub fn test_nsec_nxdomain_start<A: Authority<Lookup = AuthLookup>>(authority: A,
&query,
&Name::from_str("example.com.").unwrap(),
&nsecs
));
)
.is_secure());
}
pub fn test_nsec_nxdomain_middle<A: Authority<Lookup = AuthLookup>>(authority: A, keys: &[DNSKEY]) {
@ -283,7 +285,8 @@ pub fn test_nsec_nxdomain_middle<A: Authority<Lookup = AuthLookup>>(authority: A
&query,
&Name::from_str("example.com.").unwrap(),
&nsecs
));
)
.is_secure());
}
pub fn test_nsec_nxdomain_wraps_end<A: Authority<Lookup = AuthLookup>>(
@ -317,7 +320,8 @@ pub fn test_nsec_nxdomain_wraps_end<A: Authority<Lookup = AuthLookup>>(
&query,
&Name::from_str("example.com.").unwrap(),
&nsecs
));
)
.is_secure());
}
pub fn test_rfc_6975_supported_algorithms<A: Authority<Lookup = AuthLookup>>(