From 5e469fda3ef7f9d1e6a36f87119744ac57028281 Mon Sep 17 00:00:00 2001 From: Emilio Wuerges Date: Tue, 11 Oct 2022 19:12:14 -0300 Subject: [PATCH 1/5] Improve CAA rdata display --- crates/proto/src/rr/rdata/caa.rs | 70 ++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/crates/proto/src/rr/rdata/caa.rs b/crates/proto/src/rr/rdata/caa.rs index 684ee579c1..cc5475bae2 100644 --- a/crates/proto/src/rr/rdata/caa.rs +++ b/crates/proto/src/rr/rdata/caa.rs @@ -843,20 +843,22 @@ impl fmt::Display for Value { match self { Value::Issuer(name, values) => { - match name { - Some(name) => write!(f, "{}", name)?, - None => write!(f, ";")?, - } - - if let Some(value) = values.first() { - write!(f, " {}", value)?; - for value in &values[1..] { + if name.is_none() && values.is_empty() { + write!(f, ";")?; + } else { + if let Some(name) = name { + write!(f, "{}", name)?; + } + for value in values.iter() { write!(f, "; {}", value)?; } } } Value::Url(url) => write!(f, "{}", url)?, - Value::Unknown(v) => write!(f, "{:?}", v)?, + Value::Unknown(v) => match str::from_utf8(v) { + Ok(text) => write!(f, "{}", text)?, + Err(_) => write!(f, "{:?}", v)?, + }, } f.write_str("\"") @@ -877,7 +879,7 @@ impl fmt::Display for KeyValue { // FIXME: this needs to be verified to be correct, add tests... impl fmt::Display for CAA { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { - let critical = if self.issuer_critical { "1" } else { "0" }; + let critical = if self.issuer_critical { "128" } else { "0" }; write!( f, @@ -1133,7 +1135,7 @@ mod tests { Some(Name::parse("example.com", None).unwrap()), vec![KeyValue::new("one", "1")], ); - assert_eq!(one_option.to_string(), "0 issue \"example.com one=1\""); + assert_eq!(one_option.to_string(), "0 issue \"example.com; one=1\""); let two_options = CAA::new_issue( false, @@ -1142,8 +1144,52 @@ mod tests { ); assert_eq!( two_options.to_string(), - "0 issue \"example.com one=1; two=2\"" + "0 issue \"example.com; one=1; two=2\"" + ); + + let flag_set = CAA::new_issue( + true, + Some(Name::parse("example.com", None).unwrap()), + vec![KeyValue::new("one", "1"), KeyValue::new("two", "2")], + ); + assert_eq!( + flag_set.to_string(), + "128 issue \"example.com; one=1; two=2\"" + ); + + // Examples from RFC 6844, with added quotes + assert_eq!( + CAA::new_issue( + false, + Some(Name::parse("ca.example.net", None).unwrap()), + vec![KeyValue::new("account", "230123")] + ) + .to_string(), + "0 issue \"ca.example.net; account=230123\"" ); + assert_eq!( + CAA::new_issue( + false, + Some(Name::parse("ca.example.net", None).unwrap()), + vec![KeyValue::new("policy", "ev")] + ) + .to_string(), + "0 issue \"ca.example.net; policy=ev\"" + ); + assert_eq!( + CAA::new_iodef(false, Url::parse("mailto:security@example.com").unwrap()).to_string(), + "0 iodef \"mailto:security@example.com\"" + ); + assert_eq!( + CAA::new_iodef(false, Url::parse("http://iodef.example.com/").unwrap()).to_string(), + "0 iodef \"http://iodef.example.com/\"" + ); + let unknown = CAA { + issuer_critical: true, + tag: Property::from("tbs".to_string()), + value: Value::Unknown("Unknown".as_bytes().to_vec()), + }; + assert_eq!(unknown.to_string(), "128 tbs \"Unknown\""); } #[test] From 71ab69fd51e9595d56504a41f000676bcb5d2754 Mon Sep 17 00:00:00 2001 From: Emilio Wuerges Date: Thu, 13 Oct 2022 16:10:19 -0300 Subject: [PATCH 2/5] add test with empty domain --- crates/proto/src/rr/rdata/caa.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/proto/src/rr/rdata/caa.rs b/crates/proto/src/rr/rdata/caa.rs index cc5475bae2..7ce8501483 100644 --- a/crates/proto/src/rr/rdata/caa.rs +++ b/crates/proto/src/rr/rdata/caa.rs @@ -1119,7 +1119,7 @@ mod tests { } #[test] - fn test_tostring() { + fn test_to_string() { let deny = CAA::new_issue(false, None, vec![]); assert_eq!(deny.to_string(), "0 issue \";\""); @@ -1157,6 +1157,13 @@ mod tests { "128 issue \"example.com; one=1; two=2\"" ); + let empty_domain = CAA::new_issue( + false, + None, + vec![KeyValue::new("one", "1"), KeyValue::new("two", "2")], + ); + assert_eq!(empty_domain.to_string(), "0 issue \"; one=1; two=2\""); + // Examples from RFC 6844, with added quotes assert_eq!( CAA::new_issue( From 1e8ec7cd59937bab53c53fd84274bbf5f8798baa Mon Sep 17 00:00:00 2001 From: Emilio Wuerges Date: Mon, 17 Oct 2022 14:06:18 -0300 Subject: [PATCH 3/5] moved tests to parser --- .../src/serialize/txt/rdata_parsers/caa.rs | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/crates/client/src/serialize/txt/rdata_parsers/caa.rs b/crates/client/src/serialize/txt/rdata_parsers/caa.rs index e616ae0d30..169d0f2447 100644 --- a/crates/client/src/serialize/txt/rdata_parsers/caa.rs +++ b/crates/client/src/serialize/txt/rdata_parsers/caa.rs @@ -102,13 +102,88 @@ pub(crate) fn parse<'i, I: Iterator>(mut tokens: I) -> ParseResu #[cfg(test)] mod tests { + use crate::serialize::txt::parse_rdata::RDataParser; + use trust_dns_proto::rr::{rdata::caa::KeyValue, Name, RData, RecordType}; + use super::*; + fn test_to_string_parse_is_reversible(expected_rdata: CAA, input_string: &str) { + let expected_rdata_string = expected_rdata.to_string(); + assert_eq!( + input_string, expected_rdata_string, + "input string does not match expected_rdata.to_string()" + ); + + match RData::try_from_str(RecordType::CAA, input_string).expect("CAA rdata parse failed") { + RData::CAA(parsed_rdata) => assert_eq!( + expected_rdata, + parsed_rdata, + "CAA rdata was not parsed as expected. input={:?} expected_rdata={:?} parsed_rdata={:?}", + input_string, + expected_rdata, + parsed_rdata, + ), + parsed_rdata => panic!("Parsed RData is not CAA: {:?}", parsed_rdata), + } + } #[test] fn test_parsing() { //nocerts CAA 0 issue \";\" assert!(parse(vec!["0", "issue", ";"].into_iter()).is_ok()); + // certs CAA 0 issuewild \"example.net\" assert!(parse(vec!["0", "issue", "example.net"].into_iter()).is_ok()); + + // issuer critical = true + test_to_string_parse_is_reversible(CAA::new_issue(true, None, vec![]), "128 issue \";\""); + + // deny + test_to_string_parse_is_reversible(CAA::new_issue(false, None, vec![]), "0 issue \";\""); + + // only hostname + test_to_string_parse_is_reversible( + CAA::new_issue( + false, + Some(Name::parse("example.com", None).unwrap()), + vec![], + ), + "0 issue \"example.com\"", + ); + + // hostname and one parameter + test_to_string_parse_is_reversible( + CAA::new_issue( + false, + Some(Name::parse("example.com", None).unwrap()), + vec![KeyValue::new("one", "1")], + ), + "0 issue \"example.com; one=1\"", + ); + + // hostname and two parameters + test_to_string_parse_is_reversible( + CAA::new_issue( + false, + Some(Name::parse("example.com", None).unwrap()), + vec![KeyValue::new("one", "1"), KeyValue::new("two", "2")], + ), + "0 issue \"example.com; one=1; two=2\"", + ); + + // no hostname and one parameter + test_to_string_parse_is_reversible( + CAA::new_issue(false, None, vec![KeyValue::new("one", "1")]), + "0 issue \"; one=1\"", + ); + + // no hostname and two parameters + test_to_string_parse_is_reversible( + CAA::new_issue( + false, + None, + vec![KeyValue::new("one", "1"), KeyValue::new("two", "2")], + ), + "0 issue \"; one=1; two=2\"", + ); } } From 1b684790146f500563f00e659d5f45ce725eb4ae Mon Sep 17 00:00:00 2001 From: Emilio Wuerges Date: Wed, 26 Oct 2022 12:51:36 -0300 Subject: [PATCH 4/5] Output `` instead of `;` for empty issue property. --- .../src/serialize/txt/rdata_parsers/caa.rs | 4 ++-- crates/proto/src/rr/rdata/caa.rs | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/crates/client/src/serialize/txt/rdata_parsers/caa.rs b/crates/client/src/serialize/txt/rdata_parsers/caa.rs index 169d0f2447..0dae5e6656 100644 --- a/crates/client/src/serialize/txt/rdata_parsers/caa.rs +++ b/crates/client/src/serialize/txt/rdata_parsers/caa.rs @@ -135,10 +135,10 @@ mod tests { assert!(parse(vec!["0", "issue", "example.net"].into_iter()).is_ok()); // issuer critical = true - test_to_string_parse_is_reversible(CAA::new_issue(true, None, vec![]), "128 issue \";\""); + test_to_string_parse_is_reversible(CAA::new_issue(true, None, vec![]), "128 issue \"\""); // deny - test_to_string_parse_is_reversible(CAA::new_issue(false, None, vec![]), "0 issue \";\""); + test_to_string_parse_is_reversible(CAA::new_issue(false, None, vec![]), "0 issue \"\""); // only hostname test_to_string_parse_is_reversible( diff --git a/crates/proto/src/rr/rdata/caa.rs b/crates/proto/src/rr/rdata/caa.rs index 7ce8501483..6712b7deca 100644 --- a/crates/proto/src/rr/rdata/caa.rs +++ b/crates/proto/src/rr/rdata/caa.rs @@ -843,15 +843,11 @@ impl fmt::Display for Value { match self { Value::Issuer(name, values) => { - if name.is_none() && values.is_empty() { - write!(f, ";")?; - } else { - if let Some(name) = name { - write!(f, "{}", name)?; - } - for value in values.iter() { - write!(f, "; {}", value)?; - } + if let Some(name) = name { + write!(f, "{}", name)?; + } + for value in values.iter() { + write!(f, "; {}", value)?; } } Value::Url(url) => write!(f, "{}", url)?, @@ -1121,7 +1117,7 @@ mod tests { #[test] fn test_to_string() { let deny = CAA::new_issue(false, None, vec![]); - assert_eq!(deny.to_string(), "0 issue \";\""); + assert_eq!(deny.to_string(), "0 issue \"\""); let empty_options = CAA::new_issue( false, From 1299d285e6f64a82d1068c7ea7ff4e26f46991ef Mon Sep 17 00:00:00 2001 From: Emilio Wuerges Date: Wed, 26 Oct 2022 15:37:40 -0300 Subject: [PATCH 5/5] return error for non utf8 unknown --- crates/proto/src/rr/rdata/caa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/proto/src/rr/rdata/caa.rs b/crates/proto/src/rr/rdata/caa.rs index 6712b7deca..e0ebd23d64 100644 --- a/crates/proto/src/rr/rdata/caa.rs +++ b/crates/proto/src/rr/rdata/caa.rs @@ -853,7 +853,7 @@ impl fmt::Display for Value { Value::Url(url) => write!(f, "{}", url)?, Value::Unknown(v) => match str::from_utf8(v) { Ok(text) => write!(f, "{}", text)?, - Err(_) => write!(f, "{:?}", v)?, + Err(_) => return Err(fmt::Error), }, }