custom matching rules in fuzzer for special case record data

Cargo.lock

@@ -46,6 +46,12 @@ dependencies = [
  "winapi",
 ]
 
+[[package]]
+name = "arbitrary"
+version = "1.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "510c76ecefdceada737ea728f4f9a84bd2e1ef29f1ba555e560940fe279954de"
+
 [[package]]
 name = "async-attributes"
 version = "1.1.2"
@@ -800,6 +806,17 @@ version = "0.2.99"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a7f823d141fe0a24df1e23b4af4e3c7ba9e5966ec514ea068c93024aa7deb765"
 
+[[package]]
+name = "libfuzzer-sys"
+version = "0.4.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "36a9a84a6e8b55dfefb04235e55edb2b9a2a18488fcae777a6bdaa6f06f1deb3"
+dependencies = [
+ "arbitrary",
+ "cc",
+ "once_cell",
+]
+
 [[package]]
 name = "libsqlite3-sys"
 version = "0.23.1"
@@ -1787,6 +1804,14 @@ dependencies = [
  "webpki-roots 0.22.1",
 ]
 
+[[package]]
+name = "trust-dns-proto-fuzz"
+version = "0.0.0"
+dependencies = [
+ "libfuzzer-sys",
+ "trust-dns-proto",
+]
+
 [[package]]
 name = "trust-dns-resolver"
 version = "0.21.0-alpha.4"

Cargo.toml

@@ -11,9 +11,10 @@ members = ["crates/proto",
            "crates/https",
            "crates/native-tls",
            "crates/openssl",
-           "crates/rustls"]
+           "crates/rustls",
+           "fuzz"]
 
-exclude = ["fuzz"]
+#exclude = ["fuzz"]
 
 [patch.crates-io]
 # tokio = { path = "../tokio/tokio" }

crates/proto/src/rr/rdata/opt.rs

@@ -539,31 +539,31 @@ mod tests {
         let read_rdata = read(&mut decoder, restrict).expect("Decoding error");
         assert_eq!(rdata, read_rdata);
     }
-}
-
-#[test]
-pub fn test_read_empty_option_at_end_of_opt() {
-    let bytes: Vec<u8> = vec![
-        0x00, 0x0a, 0x00, 0x08, 0x0b, 0x64, 0xb4, 0xdc, 0xd7, 0xb0, 0xcc, 0x8f, 0x00, 0x08, 0x00,
-        0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x00,
-    ];
-
-    let mut decoder: BinDecoder<'_> = BinDecoder::new(&*bytes);
-    let read_rdata = read(&mut decoder, Restrict::new(bytes.len() as u16));
-    assert!(
-        read_rdata.is_ok(),
-        "error decoding: {:?}",
-        read_rdata.unwrap_err()
-    );
-
-    let opt = read_rdata.unwrap();
-    let mut options = HashMap::default();
-    options.insert(EdnsCode::Subnet, EdnsOption::Unknown(8, vec![0, 1, 0, 0]));
-    options.insert(
-        EdnsCode::Cookie,
-        EdnsOption::Unknown(10, vec![0x0b, 0x64, 0xb4, 0xdc, 0xd7, 0xb0, 0xcc, 0x8f]),
-    );
-    options.insert(EdnsCode::Keepalive, EdnsOption::Unknown(11, vec![]));
-    let options = OPT::new(options);
-    assert_eq!(opt, options);
+
+    #[test]
+    fn test_read_empty_option_at_end_of_opt() {
+        let bytes: Vec<u8> = vec![
+            0x00, 0x0a, 0x00, 0x08, 0x0b, 0x64, 0xb4, 0xdc, 0xd7, 0xb0, 0xcc, 0x8f, 0x00, 0x08,
+            0x00, 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x00,
+        ];
+
+        let mut decoder: BinDecoder<'_> = BinDecoder::new(&*bytes);
+        let read_rdata = read(&mut decoder, Restrict::new(bytes.len() as u16));
+        assert!(
+            read_rdata.is_ok(),
+            "error decoding: {:?}",
+            read_rdata.unwrap_err()
+        );
+
+        let opt = read_rdata.unwrap();
+        let mut options = HashMap::default();
+        options.insert(EdnsCode::Subnet, EdnsOption::Unknown(8, vec![0, 1, 0, 0]));
+        options.insert(
+            EdnsCode::Cookie,
+            EdnsOption::Unknown(10, vec![0x0b, 0x64, 0xb4, 0xdc, 0xd7, 0xb0, 0xcc, 0x8f]),
+        );
+        options.insert(EdnsCode::Keepalive, EdnsOption::Unknown(11, vec![]));
+        let options = OPT::new(options);
+        assert_eq!(opt, options);
+    }
 }

fuzz/fuzz_targets/message.rs

@@ -1,21 +1,18 @@
 #![no_main]
 use libfuzzer_sys::fuzz_target;
-use trust_dns_proto::op::Message;
-use trust_dns_proto::serialize::binary::{BinDecodable, BinEncodable};
+
+use trust_dns_proto::{
+    op::Message,
+    rr::Record,
+    serialize::binary::{BinDecodable, BinEncodable},
+};
 
 fuzz_target!(|data: &[u8]| {
     if let Ok(msg) = Message::from_bytes(data) {
-        // Temporary hack to pass over messages with CAA records, because there's an empty string
-        // -> None round-trip failure inside CAA that we're not looking for right now.
-        if let Some(add) = msg.additionals().get(0) {
-            if add.rr_type() == trust_dns_proto::rr::record_type::RecordType::CAA {
-                return;
-            }
-        }
         let new_data = msg.to_bytes().unwrap();
         match Message::from_bytes(&new_data) {
             Ok(reparsed) => {
-                if msg != reparsed {
+                if !messages_equal(&msg, &reparsed) {
                     for (m, r) in format!("{:#?}", msg)
                         .lines()
                         .zip(format!("{:#?}", reparsed).lines())
@@ -34,3 +31,76 @@ fuzz_target!(|data: &[u8]| {
         }
     }
 });
+
+fn messages_equal(msg1: &Message, msg2: &Message) -> bool {
+    if msg1 == msg2 {
+        return true;
+    }
+
+    // see if there are some of the records that don't round trip properly...
+    // compare headers
+    if msg1.header() != msg2.header() {
+        return false;
+    }
+
+    // compare queries
+    if msg1.queries() != msg2.queries() {
+        return false;
+    }
+
+    // now compare answers
+    if !records_equal(msg1.answers(), msg2.answers()) {
+        return false;
+    }
+    if !records_equal(msg1.name_servers(), msg2.name_servers()) {
+        return false;
+    }
+    if !records_equal(msg1.additionals(), msg2.additionals()) {
+        return false;
+    }
+
+    // everything is effectively equal
+    true
+}
+
+fn records_equal(records1: &[Record], records2: &[Record]) -> bool {
+    for (record1, record2) in records1.iter().zip(records2.iter()) {
+        if !record_equal(record1, record2) {
+            return false;
+        }
+    }
+
+    true
+}
+
+/// Some RDATAs don't roundtrip elegantly, so we have custom matching rules here.
+fn record_equal(record1: &Record, record2: &Record) -> bool {
+    use trust_dns_proto::rr::RData;
+
+    if record1.record_type() != record2.record_type() {
+        return false;
+    }
+
+    // if the record data matches, we're fine
+    if record1.data() == record2.data() {
+        return true;
+    }
+
+    // custom rules to match..
+    match (record1.data(), record2.data()) {
+        (Some(RData::CAA(_)), _) | (_, Some(RData::CAA(_))) => {
+            // FIXME: evaluate why these don't work
+            // Temporary hack to pass over messages with CAA records, because there's an empty string
+            // -> None round-trip failure inside CAA that we're not looking for right now;
+            return true;
+        }
+        (None, Some(RData::OPT(opt))) | (Some(RData::OPT(opt)), None) => {
+            if opt.as_ref().is_empty() {
+                return true;
+            }
+        }
+        _ => return false,
+    }
+
+    false
+}