revert: remove ttl information from datagram (mozilla#1940)

mozilla#1568 introduced TTL information to
datagrams. On the input path it would take the ttl information, but not act on
it. On the output path it would set only "the default TTL on many OSes".

https://github.com/mozilla/neqo/blob/66504908e5fa070a8a5fa67d8b5a201d2c9a5cc5/neqo-transport/src/path.rs#L576

This commit removes the ttl information from `Datagram`, thus reverting a subset
of mozilla#1568.

This is partially motivated by mozilla#1920,
introducing `quinn-udp` as the IO library of choice, which does not support
reading and writing TTL.

See also discussion in
mozilla#1920 (comment).

fuzz/fuzz_targets/client_initial.rs

@@ -57,13 +57,7 @@ fuzz_target!(|data: &[u8]| {
         &mut ciphertext,
         (header_enc.len() - 1)..header_enc.len(),
     );
-    let fuzzed_ci = Datagram::new(
-        ci.source(),
-        ci.destination(),
-        ci.tos(),
-        ci.ttl(),
-        ciphertext,
-    );
+    let fuzzed_ci = Datagram::new(ci.source(), ci.destination(), ci.tos(), ciphertext);
 
     let mut server = default_server();
     let _response = server.process(Some(&fuzzed_ci), now());

fuzz/fuzz_targets/server_initial.rs

@@ -63,13 +63,7 @@ fuzz_target!(|data: &[u8]| {
         &mut ciphertext,
         (header_enc.len() - 1)..header_enc.len(),
     );
-    let fuzzed_si = Datagram::new(
-        si.source(),
-        si.destination(),
-        si.tos(),
-        si.ttl(),
-        ciphertext,
-    );
+    let fuzzed_si = Datagram::new(si.source(), si.destination(), si.tos(), ciphertext);
     let _response = client.process(Some(&fuzzed_si), now());
 });
 

neqo-bin/src/udp.rs

@@ -117,7 +117,6 @@ impl Socket {
                     meta.addr,
                     *local_address,
                     meta.ecn.map(|n| IpTos::from(n as u8)).unwrap_or_default(),
-                    None, // TODO: get the real TTL https://github.com/quinn-rs/quinn/issues/1749
                     d,
                 )
             })
@@ -141,7 +140,6 @@ mod tests {
             sender.local_addr()?,
             receiver.local_addr()?,
             IpTos::from((IpTosDscp::Le, IpTosEcn::Ect1)),
-            None,
             "Hello, world!".as_bytes().to_vec(),
         );
 

neqo-common/src/datagram.rs

@@ -13,23 +13,15 @@ pub struct Datagram {
     src: SocketAddr,
     dst: SocketAddr,
     tos: IpTos,
-    ttl: Option<u8>,
     d: Vec<u8>,
 }
 
 impl Datagram {
-    pub fn new<V: Into<Vec<u8>>>(
-        src: SocketAddr,
-        dst: SocketAddr,
-        tos: IpTos,
-        ttl: Option<u8>,
-        d: V,
-    ) -> Self {
+    pub fn new<V: Into<Vec<u8>>>(src: SocketAddr, dst: SocketAddr, tos: IpTos, d: V) -> Self {
         Self {
             src,
             dst,
             tos,
-            ttl,
             d: d.into(),
         }
     }
@@ -49,11 +41,6 @@ impl Datagram {
         self.tos
     }
 
-    #[must_use]
-    pub fn ttl(&self) -> Option<u8> {
-        self.ttl
-    }
-
     pub fn set_tos(&mut self, tos: IpTos) {
         self.tos = tos;
     }
@@ -71,9 +58,8 @@ impl std::fmt::Debug for Datagram {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         write!(
             f,
-            "Datagram {:?} TTL {:?} {:?}->{:?}: {}",
+            "Datagram {:?} {:?}->{:?}: {}",
             self.tos,
-            self.ttl,
             self.src,
             self.dst,
             hex_with_len(&self.d)
@@ -89,6 +75,6 @@ fn fmt_datagram() {
     let d = datagram([0; 1].to_vec());
     assert_eq!(
         &format!("{d:?}"),
-        "Datagram IpTos(Cs0, Ect0) TTL Some(128) [fe80::1]:443->[fe80::1]:443: [1]: 00"
+        "Datagram IpTos(Cs0, Ect0) [fe80::1]:443->[fe80::1]:443: [1]: 00"
     );
 }

neqo-transport/src/connection/mod.rs

@@ -1231,7 +1231,6 @@ impl Connection {
                 d.source(),
                 d.destination(),
                 d.tos(),
-                d.ttl(),
                 &d[d.len() - remaining..],
             )
         } else {

neqo-transport/src/connection/tests/handshake.rs

@@ -618,7 +618,7 @@ fn corrupted_initial() {
         .find(|(_, &v)| v != 0)
         .unwrap();
     corrupted[idx] ^= 0x76;
-    let dgram = Datagram::new(d.source(), d.destination(), d.tos(), d.ttl(), corrupted);
+    let dgram = Datagram::new(d.source(), d.destination(), d.tos(), corrupted);
     server.process_input(&dgram, now());
     // The server should have received two packets,
     // the first should be dropped, the second saved.
@@ -714,7 +714,7 @@ fn extra_initial_invalid_cid() {
     let mut copy = hs.to_vec();
     assert_ne!(copy[5], 0); // The DCID should be non-zero length.
     copy[6] ^= 0xc4;
-    let dgram_copy = Datagram::new(hs.destination(), hs.source(), hs.tos(), hs.ttl(), copy);
+    let dgram_copy = Datagram::new(hs.destination(), hs.source(), hs.tos(), copy);
     let nothing = client.process(Some(&dgram_copy), now).dgram();
     assert!(nothing.is_none());
 }
@@ -836,7 +836,6 @@ fn drop_initial_packet_from_wrong_address() {
         SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2)), 443),
         p.destination(),
         p.tos(),
-        p.ttl(),
         &p[..],
     );
 
@@ -864,7 +863,6 @@ fn drop_handshake_packet_from_wrong_address() {
         SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 2)), 443),
         p.destination(),
         p.tos(),
-        p.ttl(),
         &p[..],
     );
 

neqo-transport/src/connection/tests/migration.rs

@@ -53,7 +53,7 @@ fn loopback() -> SocketAddr {
 }
 
 fn change_path(d: &Datagram, a: SocketAddr) -> Datagram {
-    Datagram::new(a, a, d.tos(), d.ttl(), &d[..])
+    Datagram::new(a, a, d.tos(), &d[..])
 }
 
 fn new_port(a: SocketAddr) -> SocketAddr {
@@ -62,13 +62,7 @@ fn new_port(a: SocketAddr) -> SocketAddr {
 }
 
 fn change_source_port(d: &Datagram) -> Datagram {
-    Datagram::new(
-        new_port(d.source()),
-        d.destination(),
-        d.tos(),
-        d.ttl(),
-        &d[..],
-    )
+    Datagram::new(new_port(d.source()), d.destination(), d.tos(), &d[..])
 }
 
 /// As these tests use a new path, that path often has a non-zero RTT.

neqo-transport/src/path.rs

@@ -534,8 +534,6 @@ pub struct Path {
     rtt: RttEstimate,
     /// A packet sender for the path, which includes congestion control and a pacer.
     sender: PacketSender,
-    /// The IP TTL to use for outgoing packets on this path.
-    ttl: u8,
 
     /// The number of bytes received on this path.
     /// Note that this value might saturate on a long-lived connection,
@@ -573,7 +571,6 @@ impl Path {
             challenge: None,
             rtt: RttEstimate::default(),
             sender,
-            ttl: 64, // This is the default TTL on many OSes.
             received_bytes: 0,
             sent_bytes: 0,
             ecn_info: EcnInfo::default(),
@@ -711,7 +708,7 @@ impl Path {
         // with the current value.
         let tos = self.tos();
         self.ecn_info.on_packet_sent();
-        Datagram::new(self.local, self.remote, tos, Some(self.ttl), payload)
+        Datagram::new(self.local, self.remote, tos, payload)
     }
 
     /// Get local address as `SocketAddr`

neqo-transport/src/server.rs

@@ -348,13 +348,8 @@ impl Server {
                         &initial.dst_cid,
                     );
                     if let Ok(p) = packet {
-                        let retry = Datagram::new(
-                            dgram.destination(),
-                            dgram.source(),
-                            dgram.tos(),
-                            dgram.ttl(),
-                            p,
-                        );
+                        let retry =
+                            Datagram::new(dgram.destination(), dgram.source(), dgram.tos(), p);
                         Some(retry)
                     } else {
                         qerror!([self], "unable to encode retry, dropping packet");
@@ -603,7 +598,6 @@ impl Server {
                 dgram.destination(),
                 dgram.source(),
                 dgram.tos(),
-                dgram.ttl(),
                 vn,
             ));
         }

neqo-transport/tests/connection.rs

@@ -42,7 +42,6 @@ fn truncate_long_packet() {
         dupe.source(),
         dupe.destination(),
         dupe.tos(),
-        dupe.ttl(),
         &dupe[..(dupe.len() - tail)],
     );
     let hs_probe = client.process(Some(&truncated), now()).dgram();
@@ -114,7 +113,6 @@ fn reorder_server_initial() {
         server_initial.source(),
         server_initial.destination(),
         server_initial.tos(),
-        server_initial.ttl(),
         packet,
     );
 
@@ -160,7 +158,6 @@ fn set_payload(server_packet: &Option<Datagram>, client_dcid: &[u8], payload: &[
         server_initial.source(),
         server_initial.destination(),
         server_initial.tos(),
-        server_initial.ttl(),
         packet,
     )
 }
@@ -262,7 +259,6 @@ fn overflow_crypto() {
             server_initial.source(),
             server_initial.destination(),
             server_initial.tos(),
-            server_initial.ttl(),
             packet,
         );
         client.process_input(&dgram, now());

neqo-transport/tests/retry.rs

@@ -156,13 +156,7 @@ fn retry_different_ip() {
     let dgram = dgram.unwrap();
     let other_v4 = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 2));
     let other_addr = SocketAddr::new(other_v4, 443);
-    let from_other = Datagram::new(
-        other_addr,
-        dgram.destination(),
-        dgram.tos(),
-        dgram.ttl(),
-        &dgram[..],
-    );
+    let from_other = Datagram::new(other_addr, dgram.destination(), dgram.tos(), &dgram[..]);
     let dgram = server.process(Some(&from_other), now()).dgram();
     assert!(dgram.is_none());
 }
@@ -183,13 +177,7 @@ fn new_token_different_ip() {
     // Now rewrite the source address.
     let d = dgram.unwrap();
     let src = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 2)), d.source().port());
-    let dgram = Some(Datagram::new(
-        src,
-        d.destination(),
-        d.tos(),
-        d.ttl(),
-        &d[..],
-    ));
+    let dgram = Some(Datagram::new(src, d.destination(), d.tos(), &d[..]));
     let dgram = server.process(dgram.as_ref(), now()).dgram(); // Retry
     assert!(dgram.is_some());
     assertions::assert_retry(dgram.as_ref().unwrap());
@@ -214,13 +202,7 @@ fn new_token_expired() {
     let the_future = now() + Duration::from_secs(60 * 60 * 24 * 30);
     let d = dgram.unwrap();
     let src = SocketAddr::new(d.source().ip(), d.source().port() + 1);
-    let dgram = Some(Datagram::new(
-        src,
-        d.destination(),
-        d.tos(),
-        d.ttl(),
-        &d[..],
-    ));
+    let dgram = Some(Datagram::new(src, d.destination(), d.tos(), &d[..]));
     let dgram = server.process(dgram.as_ref(), the_future).dgram(); // Retry
     assert!(dgram.is_some());
     assertions::assert_retry(dgram.as_ref().unwrap());
@@ -281,13 +263,7 @@ fn retry_bad_integrity() {
 
     let mut tweaked = retry.to_vec();
     tweaked[retry.len() - 1] ^= 0x45; // damage the auth tag
-    let tweaked_packet = Datagram::new(
-        retry.source(),
-        retry.destination(),
-        retry.tos(),
-        retry.ttl(),
-        tweaked,
-    );
+    let tweaked_packet = Datagram::new(retry.source(), retry.destination(), retry.tos(), tweaked);
 
     // The client should ignore this packet.
     let dgram = client.process(Some(&tweaked_packet), now()).dgram();
@@ -454,7 +430,6 @@ fn mitm_retry() {
         client_initial2.source(),
         client_initial2.destination(),
         client_initial2.tos(),
-        client_initial2.ttl(),
         notoken_packet,
     );
     qdebug!("passing modified Initial to the main server");

neqo-transport/tests/server.rs

@@ -158,7 +158,6 @@ fn duplicate_initial_new_path() {
         SocketAddr::new(initial.source().ip(), initial.source().port() ^ 23),
         initial.destination(),
         initial.tos(),
-        initial.ttl(),
         &initial[..],
     );
 
@@ -373,13 +372,7 @@ fn new_token_different_port() {
     // Now rewrite the source port, which should not change that the token is OK.
     let d = dgram.unwrap();
     let src = SocketAddr::new(d.source().ip(), d.source().port() + 1);
-    let dgram = Some(Datagram::new(
-        src,
-        d.destination(),
-        d.tos(),
-        d.ttl(),
-        &d[..],
-    ));
+    let dgram = Some(Datagram::new(src, d.destination(), d.tos(), &d[..]));
     let dgram = server.process(dgram.as_ref(), now()).dgram(); // Retry
     assert!(dgram.is_some());
     assertions::assert_initial(dgram.as_ref().unwrap(), false);
@@ -434,13 +427,7 @@ fn bad_client_initial() {
         &mut ciphertext,
         (header_enc.len() - 1)..header_enc.len(),
     );
-    let bad_dgram = Datagram::new(
-        dgram.source(),
-        dgram.destination(),
-        dgram.tos(),
-        dgram.ttl(),
-        ciphertext,
-    );
+    let bad_dgram = Datagram::new(dgram.source(), dgram.destination(), dgram.tos(), ciphertext);
 
     // The server should reject this.
     let response = server.process(Some(&bad_dgram), now());
@@ -522,13 +509,7 @@ fn bad_client_initial_connection_close() {
         &mut ciphertext,
         (header_enc.len() - 1)..header_enc.len(),
     );
-    let bad_dgram = Datagram::new(
-        dgram.source(),
-        dgram.destination(),
-        dgram.tos(),
-        dgram.ttl(),
-        ciphertext,
-    );
+    let bad_dgram = Datagram::new(dgram.source(), dgram.destination(), dgram.tos(), ciphertext);
 
     // The server should ignore this and go to Draining.
     let mut now = now();
@@ -551,7 +532,6 @@ fn version_negotiation_ignored() {
         dgram.source(),
         dgram.destination(),
         dgram.tos(),
-        dgram.ttl(),
         input.clone(),
     );
     let vn = server.process(Some(&damaged), now()).dgram();