Correct TXT sort order on unlikely edge cases

diff --git a/src/rr.rs b/src/rr.rs
index 14826e3d1e00c53c76b691e4c06ef9ed762dd791..41c0ee2df8d13ff8dcd6de0f667db01d3e9fe805 100644 (file)
--- a/src/rr.rs
+++ b/src/rr.rs
@@ -7,7 +7,7 @@ use alloc::vec::Vec;
 use alloc::string::String;
 use alloc::borrow::ToOwned;
 
-use core::cmp::Ordering;
+use core::cmp::{self, Ordering};
 
 use crate::ser::*;
 
@@ -171,8 +171,18 @@ pub struct Txt {
 impl PartialOrd for Txt {
        fn partial_cmp(&self, o: &Txt) -> Option<Ordering> {
                Some(self.name.cmp(&o.name)
-                       .then_with(|| self.data.len().cmp(&o.data.len()))
-                       .then_with(|| self.data.cmp(&o.data)))
+                       .then_with(|| {
+                               // Compare in wire encoding form, i.e. compare in 255-byte chunks
+                               for i in 1..(self.data.len() / 255) + 2 {
+                                       let start = (i - 1)*255;
+                                       let self_len = cmp::min(i * 255, self.data.len());
+                                       let o_len = cmp::min(i * 255, o.data.len());
+                                       let slice_cmp = self_len.cmp(&o_len)
+                                               .then_with(|| self.data[start..self_len].cmp(&o.data[start..o_len]));
+                                       if !slice_cmp.is_eq() { return slice_cmp; }
+                               }
+                               Ordering::Equal
+                       }))
        }
 }
 impl StaticRecord for Txt {

diff --git a/src/validation.rs b/src/validation.rs
index 30a541ac76231ddc0246969f01764ce17c86651a..491cbaa172d706f392eb5dd4c4039e5825cbba2e 100644 (file)
--- a/src/validation.rs
+++ b/src/validation.rs
@@ -550,6 +550,41 @@ mod tests {
                (cname_resp, cname_rrsig, txt_resp, txt_rrsig)
        }
 
+       fn matcorallo_txt_sort_edge_cases_records() -> (Vec<Txt>, RRSig) {
+               let txts = vec![Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab".to_owned().into_bytes(),
+               }, Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_owned().into_bytes(),
+               }, Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabaa".to_owned().into_bytes(),
+               }, Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaba".to_owned().into_bytes(),
+               }, Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_owned().into_bytes(),
+               }, Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaab".to_owned().into_bytes(),
+               }, Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_owned().into_bytes(),
+               }, Txt {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       data: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaba".to_owned().into_bytes(),
+               }];
+               let rrsig = RRSig {
+                       name: "txt_sort_order.matcorallo.com.".try_into().unwrap(),
+                       ty: Txt::TYPE, alg: 13, labels: 3, orig_ttl: 30, expiration: 1708449632,
+                       inception: 1707234632, key_tag: 34530, key_name: "matcorallo.com.".try_into().unwrap(),
+                       signature: base64::decode("elAhELwzkGpUMvzeiYZpg1+yRFPjmOeEd1ir1vYx2Dku9kzsXmAlejOYDPWdaJ6ekvHdMejCN/MtyI+iFAYqsw==").unwrap(),
+               };
+               (txts, rrsig)
+       }
+
        #[test]
        fn check_txt_record_a() {
                let dnskeys = mattcorallo_dnskey().0;
@@ -662,6 +697,27 @@ mod tests {
                } else { panic!(); }
        }
 
+       #[test]
+       fn check_txt_sort_order() {
+               let mut rr_stream = Vec::new();
+               for rr in root_dnskey().1 { write_rr(&rr, 1, &mut rr_stream); }
+               for rr in com_dnskey().1 { write_rr(&rr, 1, &mut rr_stream); }
+               for rr in matcorallo_dnskey().1 { write_rr(&rr, 1, &mut rr_stream); }
+               let (mut txts, rrsig) = matcorallo_txt_sort_edge_cases_records();
+               write_rr(&rrsig, 1, &mut rr_stream);
+               for txt in txts.iter() { write_rr(txt, 1, &mut rr_stream); }
+
+               let mut rrs = parse_rr_stream(&rr_stream).unwrap();
+               rrs.shuffle(&mut rand::rngs::OsRng);
+               let verified_rrs = verify_rr_stream(&rrs).unwrap();
+               let mut verified_txts = verified_rrs.verified_rrs
+                       .iter().map(|rr| if let RR::Txt(txt) = rr { txt.clone() } else { panic!(); })
+                       .collect::<Vec<_>>();
+               verified_txts.sort();
+               txts.sort();
+               assert_eq!(verified_txts, txts);
+       }
+
        #[test]
        fn rfc9102_parse_test() {
                // Note that this is the `AuthenticationChain` field only, and ignores the