Skip to content
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

Jump to bottom

Improve CAA rdata display #1794

Merged
merged 5 commits into from
Oct 29, 2022
Merged

Improve CAA rdata display #1794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5e469fd
Improve CAA rdata display
wuerges Oct 11, 2022
71ab69f
add test with empty domain
wuerges Oct 13, 2022
1e8ec7c
moved tests to parser
wuerges Oct 17, 2022
1b68479
Output `` instead of `;` for empty issue property.
wuerges Oct 26, 2022
1299d28
return error for non utf8 unknown
wuerges Oct 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
75 changes: 75 additions & 0 deletions crates/client/src/serialize/txt/rdata_parsers/caa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,88 @@ pub(crate) fn parse<'i, I: Iterator<Item = &'i str>>(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\"",
);
}
}
79 changes: 64 additions & 15 deletions crates/proto/src/rr/rdata/caa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,20 +843,18 @@ impl fmt::Display for Value {

match self {
Value::Issuer(name, values) => {
match name {
Some(name) => write!(f, "{}", name)?,
None => write!(f, ";")?,
if let Some(name) = name {
write!(f, "{}", name)?;
}

if let Some(value) = values.first() {
write!(f, " {}", value)?;
for value in &values[1..] {
write!(f, "; {}", value)?;
}
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(_) => return Err(fmt::Error),
},
wuerges marked this conversation as resolved.
Show resolved Hide resolved
}

f.write_str("\"")
Expand All @@ -877,7 +875,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,
Expand Down Expand Up @@ -1117,9 +1115,9 @@ 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 \";\"");
assert_eq!(deny.to_string(), "0 issue \"\"");

let empty_options = CAA::new_issue(
false,
Expand All @@ -1133,7 +1131,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,
Expand All @@ -1142,8 +1140,59 @@ 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\""
);

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(
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]
Expand Down