commit 2867498353792c57689fcdcb9b2908f45e456627 parent f08a35174e5e8c7a2f1adf05b7ad4a33a230fa87 Author: Max Leonard Inden <mail@max-inden.de> Date: Thu, 18 Dec 2025 13:10:49 +0000 Bug 2006102 - Update neqo to v0.21.0 r=necko-reviewers,supply-chain-reviewers,kershaw Differential Revision: https://phabricator.services.mozilla.com/D276469 Diffstat:
96 files changed, 2163 insertions(+), 1022 deletions(-)
diff --git a/.cargo/config.toml.in b/.cargo/config.toml.in @@ -110,9 +110,9 @@ git = "https://github.com/mozilla/mp4parse-rust" rev = "f955be5d2a04a631c0f1777d6f35370ea1a99e2d" replace-with = "vendored-sources" -[source."git+https://github.com/mozilla/neqo?tag=v0.20.0"] +[source."git+https://github.com/mozilla/neqo?tag=v0.21.0"] git = "https://github.com/mozilla/neqo" -tag = "v0.20.0" +tag = "v0.21.0" replace-with = "vendored-sources" [source."git+https://github.com/rust-lang/rust-bindgen?rev=9366e0af8da529c958b4cd4fcbe492d951c86f5c"] diff --git a/Cargo.lock b/Cargo.lock @@ -4783,7 +4783,7 @@ dependencies = [ [[package]] name = "mtu" version = "0.2.9" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "bindgen 0.72.0", "cfg_aliases", @@ -4827,8 +4827,8 @@ dependencies = [ [[package]] name = "neqo-bin" -version = "0.20.0" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +version = "0.21.0" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "clap", "clap-verbosity-flag", @@ -4852,8 +4852,8 @@ dependencies = [ [[package]] name = "neqo-common" -version = "0.20.0" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +version = "0.21.0" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "enum-map", "env_logger", @@ -4866,8 +4866,8 @@ dependencies = [ [[package]] name = "neqo-crypto" -version = "0.20.0" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +version = "0.21.0" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "bindgen 0.72.0", "enum-map", @@ -4879,14 +4879,14 @@ dependencies = [ "serde_derive", "strum", "thiserror 2.0.12", - "toml 0.5.999", + "toml 0.9.8", "windows", ] [[package]] name = "neqo-http3" -version = "0.20.0" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +version = "0.21.0" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "enumset", "log", @@ -4904,8 +4904,8 @@ dependencies = [ [[package]] name = "neqo-qpack" -version = "0.20.0" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +version = "0.21.0" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "log", "neqo-common", @@ -4918,8 +4918,8 @@ dependencies = [ [[package]] name = "neqo-transport" -version = "0.20.0" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +version = "0.21.0" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "enum-map", "enumset", @@ -4938,8 +4938,8 @@ dependencies = [ [[package]] name = "neqo-udp" -version = "0.20.0" -source = "git+https://github.com/mozilla/neqo?tag=v0.20.0#126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +version = "0.21.0" +source = "git+https://github.com/mozilla/neqo?tag=v0.21.0#b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" dependencies = [ "cfg_aliases", "libc", diff --git a/netwerk/socket/neqo_glue/Cargo.toml b/netwerk/socket/neqo_glue/Cargo.toml @@ -10,11 +10,11 @@ name = "neqo_glue" [dependencies] firefox-on-glean = { path = "../../../toolkit/components/glean/api" } -neqo-udp = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } -neqo-http3 = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } -neqo-transport = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo", features = ["gecko"] } -neqo-common = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } -neqo-qpack = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } +neqo-udp = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } +neqo-http3 = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } +neqo-transport = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo", features = ["gecko"] } +neqo-common = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } +neqo-qpack = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } nserror = { path = "../../../xpcom/rust/nserror" } nsstring = { path = "../../../xpcom/rust/nsstring" } xpcom = { path = "../../../xpcom/rust/xpcom" } @@ -31,7 +31,7 @@ zlib-rs = "0.4.2" winapi = {version = "0.3", features = ["ws2def"] } [dependencies.neqo-crypto] -tag = "v0.20.0" +tag = "v0.21.0" git = "https://github.com/mozilla/neqo" default-features = false features = ["gecko"] diff --git a/netwerk/socket/neqo_glue/src/lib.rs b/netwerk/socket/neqo_glue/src/lib.rs @@ -560,7 +560,7 @@ impl NeqoHttp3Conn { fn record_stats_in_glean(&self) { use firefox_on_glean::metrics::networking as glean; use neqo_common::Ecn; - use neqo_transport::ecn; + use neqo_transport::{ecn, CongestionEvent}; // Metric values must be recorded as integers. Glean does not support // floating point distributions. In order to represent values <1, they @@ -694,10 +694,10 @@ impl NeqoHttp3Conn { } // Ignore connections that never had loss induced congestion events (and prevent dividing by zero). - if stats.cc.congestion_events_loss != 0 { + if stats.cc.congestion_events[CongestionEvent::Loss] != 0 { if let Ok(spurious) = i64::try_from( - (stats.cc.congestion_events_spurious * PRECISION_FACTOR_USIZE) - / stats.cc.congestion_events_loss, + (stats.cc.congestion_events[CongestionEvent::Spurious] * PRECISION_FACTOR_USIZE) + / stats.cc.congestion_events[CongestionEvent::Loss], ) { glean::http_3_spurious_congestion_event_ratio .accumulate_single_sample_signed(spurious); diff --git a/netwerk/test/http3server/Cargo.toml b/netwerk/test/http3server/Cargo.toml @@ -6,11 +6,11 @@ edition = "2021" license = "MPL-2.0" [dependencies] -neqo-bin = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } -neqo-transport = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo", features = ["gecko"] } -neqo-common = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } -neqo-http3 = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } -neqo-qpack = { tag = "v0.20.0", git = "https://github.com/mozilla/neqo" } +neqo-bin = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } +neqo-transport = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo", features = ["gecko"] } +neqo-common = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } +neqo-http3 = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } +neqo-qpack = { tag = "v0.21.0", git = "https://github.com/mozilla/neqo" } log = "0.4.0" base64 = "0.22" cfg-if = "1.0" @@ -21,7 +21,7 @@ tokio = { version = "1", features = ["rt-multi-thread"] } mozilla-central-workspace-hack = { version = "0.1", features = ["http3server"], optional = true } [dependencies.neqo-crypto] -tag = "v0.20.0" +tag = "v0.21.0" git = "https://github.com/mozilla/neqo" default-features = false features = ["gecko"] diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml @@ -4263,11 +4263,10 @@ criteria = "safe-to-deploy" delta = "0.2.6 -> 0.2.9" [[audits.mtu]] -who = "Oskar Mansfeld <git@omansfeld.net>" +who = "Max Leonard Inden <mail@max-inden.de>" criteria = "safe-to-deploy" -delta = "0.2.9 -> 0.2.9@git:126b1df97c7f88e4b66ef16dbfe4708dc6f104d9" +delta = "0.2.9 -> 0.2.9@git:b3338f904e2b9cdd59fbb2ade6b36d1a2ced4eba" importable = false -notes = "mtu crate is now part of neqo and maintained by Mozilla employees" [[audits.naga]] who = "Dzmitry Malyshau <kvark@fastmail.com>" diff --git a/third_party/rust/mtu/.cargo-checksum.json b/third_party/rust/mtu/.cargo-checksum.json @@ -1 +1 @@ -{"files":{".clippy.toml":"6ab1a673bd5c7ba29bd77e62f42183db3ace327c23d446d5b4b0618f6c39d639","Cargo.toml":"44d90acbefc8884ffac0e3df85e3e63150d19f5ae991f9cd612a0074de6d016e","LICENSE-APACHE":"a60eea817514531668d7e00765731449fe14d059d3249e0bc93b36de45f759f2","LICENSE-MIT":"4ad721b5b6a3d39ca3e2202f403d897c4a1d42896486dd58963a81f8e64ef61d","README.md":"24df0e24154b7f0096570ad221aea02bd53a0f1124a2adafff5730af5443a65c","SECURITY.md":"75455814b6cf997e22a927eb979b4356d788583aa1eb96e90853aaab0f82ad1b","build.rs":"a4bcd0562c80914a8e909e8b10507605bfd6f0f268fad9ef4d79f4c48bdaed6c","src/bsd.rs":"f6d472effbdd95f6fd4285dfb39d37a99da66ed7283906862ad29a3c2233fb19","src/lib.rs":"69aa3d9508a8c23979a94a6ebebd13767acf4c4e2345640a9faacaedb7eecb14","src/linux.rs":"d4d2e42d8e0835d64ac154b4bddb5fe9e9228e5d8c9ccd25d6afa89cfb6b6523","src/routesocket.rs":"be837947e2c3f9301a174499217fe8920ff492918bf85ca5eb281eb7ad2240e1","src/windows.rs":"b3383619f53463608c10bdc30dcb3b9c05a5fd8328a64cbd14fabefc5b5d0a8a"},"package":null} -\ No newline at end of file +{"files":{".clippy.toml":"6ab1a673bd5c7ba29bd77e62f42183db3ace327c23d446d5b4b0618f6c39d639","Cargo.toml":"44d90acbefc8884ffac0e3df85e3e63150d19f5ae991f9cd612a0074de6d016e","LICENSE-APACHE":"a60eea817514531668d7e00765731449fe14d059d3249e0bc93b36de45f759f2","LICENSE-MIT":"4ad721b5b6a3d39ca3e2202f403d897c4a1d42896486dd58963a81f8e64ef61d","README.md":"24df0e24154b7f0096570ad221aea02bd53a0f1124a2adafff5730af5443a65c","SECURITY.md":"75455814b6cf997e22a927eb979b4356d788583aa1eb96e90853aaab0f82ad1b","build.rs":"a4bcd0562c80914a8e909e8b10507605bfd6f0f268fad9ef4d79f4c48bdaed6c","src/bsd.rs":"8f9a16fe7741853e15f55f0755ac53fe0374c664f24e4fadb460dd77d734ea62","src/lib.rs":"f1a1e6253777ae9a067ec96bc18799a8bad62fd423c218ba5330439d5893dedb","src/linux.rs":"f4bd05e05791fe1728c02a73d309ae1565111d5632cf0d402c20698d5ee4839b","src/routesocket.rs":"b4e27289bfc91ef9ea1ec29b2c5ff407bad49ff01391115e852f2133667de7aa","src/windows.rs":"b3383619f53463608c10bdc30dcb3b9c05a5fd8328a64cbd14fabefc5b5d0a8a"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/mtu/src/bsd.rs b/third_party/rust/mtu/src/bsd.rs @@ -383,3 +383,16 @@ pub fn interface_and_mtu_impl(remote: IpAddr) -> Result<(String, usize)> { let (if_name, mtu2) = if_name_mtu(if_index.into())?; Ok((if_name, mtu1.or(mtu2).ok_or_else(default_err)?)) } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod test { + use super::*; + + #[test] + fn sockaddr_len_valid_and_invalid() { + assert!(sockaddr_len(AF_INET).is_ok()); + assert!(sockaddr_len(AF_INET6).is_ok()); + assert!(sockaddr_len(AddressFamily::MAX).is_err()); + } +} diff --git a/third_party/rust/mtu/src/lib.rs b/third_party/rust/mtu/src/lib.rs @@ -200,6 +200,21 @@ mod test { } #[test] + #[cfg(not(target_os = "windows"))] + fn aligned_by() { + for (size, align, expected) in [ + (0, 8, 8), + (1, 8, 8), + (7, 8, 8), + (8, 8, 8), + (9, 8, 16), + (17, 8, 24), + ] { + assert_eq!(crate::aligned_by(size, align), expected); + } + } + + #[test] fn inet_v6() { let res = interface_and_mtu(IpAddr::V6(Ipv6Addr::new( 0x2606, 0x4700, 0, 0, 0, 0, 0x6810, 0x84e5, // cloudflare.com diff --git a/third_party/rust/mtu/src/linux.rs b/third_party/rust/mtu/src/linux.rs @@ -161,6 +161,9 @@ impl TryFrom<&[u8]> for nlmsghdr { } fn parse_c_int(buf: &[u8]) -> Result<c_int> { + if buf.len() < size_of::<c_int>() { + return Err(default_err()); + } let bytes = <&[u8] as TryInto<[u8; size_of::<c_int>()]>>::try_into(&buf[..size_of::<c_int>()]) .map_err(|_| default_err())?; Ok(c_int::from_ne_bytes(bytes)) @@ -349,3 +352,74 @@ pub fn interface_and_mtu_impl(remote: IpAddr) -> Result<(String, usize)> { let if_index = if_index(remote, &mut fd)?; if_name_mtu(if_index, &mut fd) } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod test { + use std::net::{Ipv4Addr, Ipv6Addr}; + + use super::*; + + #[test] + fn nlmsghdr_try_from() { + assert!(nlmsghdr::try_from([0u8; 4].as_slice()).is_err()); + let mut buf = [0u8; 32]; + buf[0..4].copy_from_slice(&20u32.to_ne_bytes()); + assert_eq!(nlmsghdr::try_from(buf.as_slice()).unwrap().nlmsg_len, 20); + } + + #[test] + fn rtattr_try_from() { + assert!(rtattr::try_from([0u8; 2].as_slice()).is_err()); + let mut buf = [0u8; 8]; + buf[0..2].copy_from_slice(&8u16.to_ne_bytes()); + buf[2..4].copy_from_slice(&3u16.to_ne_bytes()); + let attr = rtattr::try_from(buf.as_slice()).unwrap(); + assert_eq!((attr.rta_len, attr.rta_type), (8, 3)); + } + + #[test] + fn rtattrs_iteration() { + assert_eq!(RtAttrs(&[]).count(), 0); + assert_eq!(RtAttrs(&[0u8; 2]).count(), 0); + + let mut buf = [0u8; 16]; + for (offset, rta_type) in [(0, 1u16), (8, 2u16)] { + buf[offset..offset + 2].copy_from_slice(&8u16.to_ne_bytes()); + buf[offset + 2..offset + 4].copy_from_slice(&rta_type.to_ne_bytes()); + } + let types: Vec<_> = RtAttrs(&buf).map(|a| a.hdr.rta_type).collect(); + assert_eq!(types, [1, 2]); + } + + #[test] + fn addr_bytes() { + assert_eq!(AddrBytes::new(IpAddr::V4(Ipv4Addr::LOCALHOST)).len(), 4); + assert_eq!(AddrBytes::new(IpAddr::V6(Ipv6Addr::LOCALHOST)).len(), 16); + + let v4: [u8; 16] = AddrBytes::new(IpAddr::V4(Ipv4Addr::new(1, 2, 3, 4))).into(); + assert_eq!(v4, [1, 2, 3, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]); + } + + #[test] + fn parse_c_int_valid_and_invalid() { + assert!(parse_c_int(&[0u8; 2]).is_err()); + assert_eq!(parse_c_int(&42i32.to_ne_bytes()).unwrap(), 42); + } + + #[test] + fn if_index_msg_len_and_slice() { + let msg = IfIndexMsg::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 1); + assert!(msg.len() > 0); + let slice: &[u8] = (&msg).into(); + assert_eq!(slice.len(), msg.len()); + } + + #[test] + fn if_info_msg_len_and_slice() { + let msg = IfInfoMsg::new(1, 1); + assert!(msg.len() > 0); + let slice: &[u8] = (&msg).into(); + assert_eq!(slice.len(), msg.len()); + } +} diff --git a/third_party/rust/mtu/src/routesocket.rs b/third_party/rust/mtu/src/routesocket.rs @@ -79,3 +79,17 @@ impl Read for RouteSocket { check_result(res) } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod test { + #![expect(clippy::unwrap_used, reason = "OK in tests.")] + use super::*; + + #[test] + fn check_result_error_and_success() { + assert!(check_result(-1).is_err()); + assert_eq!(check_result(0).unwrap(), 0); + assert_eq!(check_result(42).unwrap(), 42); + } +} diff --git a/third_party/rust/neqo-bin/.cargo-checksum.json b/third_party/rust/neqo-bin/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"f96203aee024db3626553b7df4f8e14c1a0d07fc75fc1364781791606b142d8d","benches/main.rs":"39c35b9b22958d1deaeb68a3974022296522c97cd47e86085370d8f246a07ac1","src/bin/client.rs":"9df4af3b25159adccfca36d6001443cf295993351fec51f833827d91ebb67fd4","src/bin/server.rs":"f55f26c8f0a34de415ede8c4865b845b3d755c3f5fe4f5574b5ee7f3a3601598","src/client/http09.rs":"7ee588c1a8317f70f8a45cdaf0fdfc49419340dd4b41700de1219e8f5ab6c097","src/client/http3.rs":"0084e2671e761cc46ce18e856f193ebe875386d6bbc4f2dc88f2bd563ca865c9","src/client/mod.rs":"6cb39cdcbcceb4e6bfb311ab3aa80787306b48ab310d8fdba6ad47c8a101a0aa","src/lib.rs":"ef20c29297d978a192011371e6af12be26d57063b616d3e21fb3d2750987ce88","src/send_data.rs":"ef8ad949e8b787f77f091a4705672b9801dc79c863d9d54a5296e0839789802e","src/server/http09.rs":"6f8f9bec9c2b8d524f2c331fc0db81c17f71c8c8ac00d50e4b6670c3e226b2b2","src/server/http3.rs":"e38b375132b2455ff1aad816871fd2f279ca79550e821846c2952d9e1c3a8ec5","src/server/mod.rs":"21de7f6126d160dd33d14769be0f673a1a781c42a7ac0f7caf84cf164ee686ab","src/udp.rs":"2a97c56e14ff271bb1aff6d62d9594c2314ce2398d1c0f6300e9623cbb7c2676"},"package":null} -\ No newline at end of file +{"files":{"Cargo.toml":"5471fa4724e85d9d2ded5dd4bfbe17ea85ab00d968b6f725d25b918d1b82c496","benches/main.rs":"39c35b9b22958d1deaeb68a3974022296522c97cd47e86085370d8f246a07ac1","src/bin/client.rs":"9df4af3b25159adccfca36d6001443cf295993351fec51f833827d91ebb67fd4","src/bin/server.rs":"f55f26c8f0a34de415ede8c4865b845b3d755c3f5fe4f5574b5ee7f3a3601598","src/client/http09.rs":"7ee588c1a8317f70f8a45cdaf0fdfc49419340dd4b41700de1219e8f5ab6c097","src/client/http3.rs":"0084e2671e761cc46ce18e856f193ebe875386d6bbc4f2dc88f2bd563ca865c9","src/client/mod.rs":"f9903f4b0859003840cb16b619751793217c3db6429d9391eb34f34637daa361","src/lib.rs":"ef20c29297d978a192011371e6af12be26d57063b616d3e21fb3d2750987ce88","src/send_data.rs":"ef8ad949e8b787f77f091a4705672b9801dc79c863d9d54a5296e0839789802e","src/server/http09.rs":"6f8f9bec9c2b8d524f2c331fc0db81c17f71c8c8ac00d50e4b6670c3e226b2b2","src/server/http3.rs":"e38b375132b2455ff1aad816871fd2f279ca79550e821846c2952d9e1c3a8ec5","src/server/mod.rs":"d3ee1e42c88091efd0c21df1b07d6363aaefa3c77680c123d3b10ff436623dfe","src/udp.rs":"2a97c56e14ff271bb1aff6d62d9594c2314ce2398d1c0f6300e9623cbb7c2676"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/neqo-bin/Cargo.toml b/third_party/rust/neqo-bin/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.81.0" name = "neqo-bin" -version = "0.20.0" +version = "0.21.0" authors = ["The Neqo Authors <necko@mozilla.com>"] build = false autolib = false diff --git a/third_party/rust/neqo-bin/src/client/mod.rs b/third_party/rust/neqo-bin/src/client/mod.rs @@ -49,47 +49,17 @@ pub enum Error { #[error("argument error: {0}")] Argument(&'static str), #[error(transparent)] - Http3(neqo_http3::Error), + Http3(#[from] neqo_http3::Error), #[error(transparent)] - Io(io::Error), + Io(#[from] io::Error), #[error(transparent)] - Qlog(qlog::Error), + Qlog(#[from] qlog::Error), #[error(transparent)] - Transport(neqo_transport::Error), + Transport(#[from] neqo_transport::Error), #[error("application error: {0}")] Application(AppError), #[error(transparent)] - Crypto(neqo_crypto::Error), -} - -impl From<neqo_crypto::Error> for Error { - fn from(err: neqo_crypto::Error) -> Self { - Self::Crypto(err) - } -} - -impl From<io::Error> for Error { - fn from(err: io::Error) -> Self { - Self::Io(err) - } -} - -impl From<neqo_http3::Error> for Error { - fn from(err: neqo_http3::Error) -> Self { - Self::Http3(err) - } -} - -impl From<qlog::Error> for Error { - fn from(err: qlog::Error) -> Self { - Self::Qlog(err) - } -} - -impl From<neqo_transport::Error> for Error { - fn from(err: neqo_transport::Error) -> Self { - Self::Transport(err) - } + Crypto(#[from] neqo_crypto::Error), } impl From<CloseReason> for Error { diff --git a/third_party/rust/neqo-bin/src/server/mod.rs b/third_party/rust/neqo-bin/src/server/mod.rs @@ -55,45 +55,15 @@ pub enum Error { #[error("invalid argument: {0}")] Argument(&'static str), #[error(transparent)] - Http3(neqo_http3::Error), + Http3(#[from] neqo_http3::Error), #[error(transparent)] - Io(io::Error), - #[error("qlog error")] - Qlog, + Io(#[from] io::Error), #[error(transparent)] - Transport(neqo_transport::Error), + Qlog(#[from] qlog::Error), #[error(transparent)] - Crypto(neqo_crypto::Error), -} - -impl From<neqo_crypto::Error> for Error { - fn from(err: neqo_crypto::Error) -> Self { - Self::Crypto(err) - } -} - -impl From<io::Error> for Error { - fn from(err: io::Error) -> Self { - Self::Io(err) - } -} - -impl From<neqo_http3::Error> for Error { - fn from(err: neqo_http3::Error) -> Self { - Self::Http3(err) - } -} - -impl From<qlog::Error> for Error { - fn from(_err: qlog::Error) -> Self { - Self::Qlog - } -} - -impl From<neqo_transport::Error> for Error { - fn from(err: neqo_transport::Error) -> Self { - Self::Transport(err) - } + Transport(#[from] neqo_transport::Error), + #[error(transparent)] + Crypto(#[from] neqo_crypto::Error), } pub type Res<T> = Result<T, Error>; diff --git a/third_party/rust/neqo-common/.cargo-checksum.json b/third_party/rust/neqo-common/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"11507d35393dba233e10ec7f56c5f76df462288b4df2dc72230dc3f62cdc9a32","benches/decoder.rs":"c59e667951e2ada510ca20850bf2710bc22d91e5caa7d4987541284b0650391a","build.rs":"d9accad1f92a1d82aff73a588269342db882918173e8d9b2b914c514e42e2839","src/bytes.rs":"b9ce44977af8d0731b51798fa9bd752fa4be437603a08446eb55889c2348281c","src/codec.rs":"4861ca281690afb525a848e6ec3281cc18e35c2ee4fcea115a7a22097f3b47b4","src/datagram.rs":"2ad1a6e1f8a157a0361b7b4e7c161d62c7bf742c4247190507b9d050d113a923","src/event.rs":"289cf8e265c33e7cded58820ac81e5b575e3f84dd52fa18b0761f4094fb361c0","src/fuzz.rs":"b50a43089c959c759bae21da5daadbaefc81cf10a6b8dca787c85619be31854f","src/header.rs":"3d68c721614f86a9de018b53b3fdd918551b0247439c2ff6308f2f7bd35795c6","src/hrtime.rs":"fd1fbf9ddd38c77e92abe25d7ab9e62872c1cd62ffae8743835bf94f76b6ddc8","src/incrdecoder.rs":"62f61d2600dafb1eec7d6cc85b3c7b07aba0ccd1149892b1dfa1a441f30927a3","src/lib.rs":"2bb6289a73dc07edfd2bc5bccda9542d403066656f41042116ed31f4fc4725ca","src/log.rs":"61a9b24bf6bf1493da67082bcf7fef8fe55f0a23d7f2a9ad13748982c54c85e2","src/qlog.rs":"2c072bb9ad31aad99c1f41421f162fbc48fbd4a17f4e554187b41267afef144b","src/tos.rs":"e09a69a20d54178a4c74b63596c607dbe8ace4ae0758a65f9878ea63d40e3c80","tests/log.rs":"c73187e390ee1a7c4a72266cb7ce5c326e862803dbcf86c2b9a892462fa22356"},"package":null} -\ No newline at end of file +{"files":{"Cargo.toml":"80af06b33624818053aef63db5217824aa2b4b44bf4f9c3cbab7322dd79c4227","benches/decoder.rs":"3b1675ea67b414df9729cbbd2af1788f569a3bf417f33c9b166f3afd0d5cd1a6","build.rs":"d9accad1f92a1d82aff73a588269342db882918173e8d9b2b914c514e42e2839","src/bytes.rs":"2cb550506696feb376cad2ca9923acfbcdc534bd43345bd14e2ffc90ea1677b1","src/codec.rs":"d4f278e56666b2d9069f68baa7294435b39963a1fe3526b5fc8c144af8d362d4","src/datagram.rs":"41b3555d1c45fd8f7c017e5403f1821a536e1de795ce8377b6802996f2fb4079","src/event.rs":"289cf8e265c33e7cded58820ac81e5b575e3f84dd52fa18b0761f4094fb361c0","src/fuzz.rs":"b50a43089c959c759bae21da5daadbaefc81cf10a6b8dca787c85619be31854f","src/header.rs":"5fba60b93bdd51a5a5a3257896dc35117743e4a5aa6acede0cf1194a72cd3a4d","src/hrtime.rs":"92ad3ad9a69a2d8f2529f50c35a8fb7a6c02f296ffff31592bbccc114c38cd7a","src/incrdecoder.rs":"d311441518cca437504544564016e0beeed18f59afb5d57bcc1b840478bdfec1","src/lib.rs":"366b62b4fc07f537f9a7cf7a0080067ebeb1ed2ad589942abe4ef889154eb457","src/log.rs":"61a9b24bf6bf1493da67082bcf7fef8fe55f0a23d7f2a9ad13748982c54c85e2","src/qlog.rs":"4bb9b04ed804da14c26229894187867e78c8d51fb3ec92e459a4a645c8293ed3","src/tos.rs":"678c4cce6f1cdc0b38f9000a7a12db89edd999e0fa1cea3a22ea28c75a419dc6","tests/log.rs":"c73187e390ee1a7c4a72266cb7ce5c326e862803dbcf86c2b9a892462fa22356"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/neqo-common/Cargo.toml b/third_party/rust/neqo-common/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.81.0" name = "neqo-common" -version = "0.20.0" +version = "0.21.0" authors = ["The Neqo Authors <necko@mozilla.com>"] build = "build.rs" autolib = false @@ -41,7 +41,6 @@ repository = "https://github.com/mozilla/neqo/" [features] bench = [ - "neqo-crypto/bench", "test-fixture/bench", "log/release_max_level_info", ] @@ -107,9 +106,6 @@ version = "4" default-features = false package = "codspeed-criterion-compat" -[dev-dependencies.neqo-crypto] -path = "../neqo-crypto" - [dev-dependencies.regex] version = "1.9" default-features = false diff --git a/third_party/rust/neqo-common/benches/decoder.rs b/third_party/rust/neqo-common/benches/decoder.rs @@ -4,7 +4,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![expect(clippy::unwrap_used, reason = "OK in a bench.")] #![expect( clippy::significant_drop_tightening, reason = "Inherent in codspeed criterion_group! macro." @@ -14,20 +13,13 @@ use std::hint::black_box; use criterion::{criterion_group, criterion_main, Criterion}; use neqo_common::Decoder; -use neqo_crypto::{init, randomize}; -fn randomize_buffer(n: usize, mask: u8) -> Vec<u8> { +/// Fill the buffer with sequentially increasing values, wrapping at 255. +fn fill_buffer(n: usize, mask: u8) -> Vec<u8> { let mut buf = vec![0; n]; - // NSS doesn't like randomizing larger buffers, so chunk them up. - // https://searchfox.org/nss/rev/968939484921b0ceecca189cd1b66e97950c39da/lib/freebl/drbg.c#29 - for chunk in buf.chunks_mut(0x10000) { - randomize(chunk); - } - // Masking the top bits off causes the resulting values to be interpreted as - // smaller varints, which stresses the decoder differently. - // This is worth testing because most varints contain small values. - for x in &mut buf[..] { - *x &= mask; + #[expect(clippy::cast_possible_truncation, reason = "% makes this safe")] + for (i, x) in buf.iter_mut().enumerate() { + *x = (i % 256) as u8 & mask; } buf } @@ -35,7 +27,7 @@ fn randomize_buffer(n: usize, mask: u8) -> Vec<u8> { fn decoder(c: &mut Criterion, count: usize, mask: u8) { c.bench_function(&format!("decode {count} bytes, mask {mask:x}"), |b| { b.iter_batched_ref( - || randomize_buffer(count, mask), + || fill_buffer(count, mask), |buf| { let mut dec = Decoder::new(&buf[..]); while black_box(dec.decode_varint()).is_some() { @@ -48,7 +40,6 @@ fn decoder(c: &mut Criterion, count: usize, mask: u8) { } fn benchmark_decoder(c: &mut Criterion) { - init().unwrap(); for mask in [0xff, 0x7f, 0x3f] { for exponent in [12, 20] { decoder(c, 1 << exponent, mask); diff --git a/third_party/rust/neqo-common/src/bytes.rs b/third_party/rust/neqo-common/src/bytes.rs @@ -113,8 +113,8 @@ mod tests { #[test] fn is_empty() { - let b = Bytes::new(vec![1, 2, 3, 4], 4); - assert!(b.is_empty()); + assert!(Bytes::new(vec![1, 2, 3, 4], 4).is_empty()); + assert!(!Bytes::new(vec![1, 2, 3, 4], 3).is_empty()); } #[test] @@ -134,5 +134,22 @@ mod tests { let a = Bytes::new(vec![1, 2, 3, 4], 1); let b = Bytes::from(vec![2, 3, 4]); assert_eq!(a, b); + assert_ne!(a, Bytes::from(vec![9, 9, 9])); + } + + #[test] + fn as_mut() { + let mut b = Bytes::new(vec![1, 2, 3], 1); + b.as_mut()[0] = 9; + assert_eq!(b.as_ref(), &[9, 3]); + } + + #[test] + fn partial_eq_array_and_slice() { + let b = Bytes::from(vec![1, 2, 3]); + assert_eq!(b, [1, 2, 3]); + assert_ne!(b, [9, 9, 9]); + assert_eq!(b, [1, 2, 3][..]); + assert_ne!(b, [9, 9, 9][..]); } } diff --git a/third_party/rust/neqo-common/src/codec.rs b/third_party/rust/neqo-common/src/codec.rs @@ -211,7 +211,6 @@ impl<'b> PartialEq<Decoder<'b>> for Decoder<'_> { } /// Encoder is good for building data structures. -#[derive(Clone, PartialEq, Eq)] pub struct Encoder<B = Vec<u8>> { buf: B, /// Tracks the starting position of the buffer when the [`Encoder`] is created. @@ -220,6 +219,23 @@ pub struct Encoder<B = Vec<u8>> { start: usize, } +impl Clone for Encoder { + fn clone(&self) -> Self { + Self { + buf: self.as_ref().to_vec(), + start: 0, + } + } +} + +impl<B: Buffer> PartialEq for Encoder<B> { + fn eq(&self, other: &Self) -> bool { + self.as_ref() == other.as_ref() + } +} + +impl<B: Buffer> Eq for Encoder<B> {} + impl<B: Buffer> Encoder<B> { /// Get the length of the [`Encoder`]. /// @@ -409,6 +425,17 @@ impl Encoder<Vec<u8>> { Self::default() } + /// Skip the first `n` bytes from the encoder buffer without copying. + /// This advances the internal offset, making those bytes inaccessible. + /// + /// # Panics + /// + /// Panics if `n` is greater than the current length of the encoder. + pub fn skip(&mut self, n: usize) { + assert!(n <= self.len(), "Cannot skip beyond buffer length"); + self.start += n; + } + /// Static helper function for previewing the results of encoding without doing it. /// /// # Panics @@ -504,8 +531,11 @@ impl From<&[u8]> for Encoder { } impl From<Encoder> for Vec<u8> { - fn from(buf: Encoder) -> Self { - buf.buf + fn from(mut enc: Encoder) -> Self { + if enc.start > 0 { + enc.buf.drain(..enc.start); + } + enc.buf } } @@ -953,6 +983,7 @@ mod tests { let enc = Encoder::from_hex("010203"); let buf = &[1, 2, 3]; assert_eq!(enc.as_decoder(), Decoder::new(buf)); + assert_ne!(enc.as_decoder(), Decoder::new(&[9, 9, 9])); } struct UintTestCase { @@ -1104,13 +1135,27 @@ mod tests { } #[test] + fn truncate() { + let mut enc = Encoder::from_hex("0102030405"); + enc.truncate(3); + assert_eq!(enc, Encoder::from_hex("010203")); + } + + #[test] + fn with_capacity() { + let mut enc = Encoder::with_capacity(10); + enc.encode_byte(1); + assert_eq!(enc.as_ref(), &[1]); + } + + #[test] fn buffer_write_zeroes() { fn check_write_zeroes<B: Buffer>(mut buf: B) { const NUM_BYTES: usize = 5; assert!(buf.is_empty()); - buf.pad_to(NUM_BYTES, 0); + assert!(!buf.is_empty()); assert_eq!(buf.position(), NUM_BYTES); let written = &buf.as_slice()[..NUM_BYTES]; @@ -1127,6 +1172,25 @@ mod tests { } #[test] + fn buffer_truncate() { + fn check_truncate<B: Buffer>(mut buf: B) { + buf.write_all(&[1, 2, 3, 4, 5]).unwrap(); + assert_eq!(buf.position(), 5); + buf.truncate(3); + assert_eq!(buf.position(), 3); + assert_eq!(buf.as_slice(), &[1, 2, 3]); + } + + check_truncate(Vec::<u8>::new()); + + let mut buf = Vec::<u8>::new(); + check_truncate(&mut buf); + + let mut buf = [0; 16]; + check_truncate(Cursor::new(&mut buf[..])); + } + + #[test] fn buffer_rotate_right() { fn check_rotate_right<B: Buffer>(mut buf: B) { const DATA: [u8; 5] = [1, 2, 3, 4, 5]; @@ -1177,6 +1241,7 @@ mod tests { let mut enc = Encoder::new_borrowed_vec(&mut non_empty_vec); assert!(enc.is_empty()); enc.encode_byte(5); + assert!(!enc.is_empty()); assert_eq!(enc.len(), 1); assert_eq!(non_empty_vec.len(), 5); @@ -1195,6 +1260,26 @@ mod tests { assert_eq!(Buffer::position(&buf), 0); } + #[test] + fn encoder_skip() { + let mut enc = Encoder::from_hex("010203040506"); + + enc.skip(2); + assert_eq!(enc.len(), 4); + assert_eq!(enc.as_ref(), &[0x03, 0x04, 0x05, 0x06]); + + enc.skip(4); + assert_eq!(enc.len(), 0); + assert!(enc.is_empty()); + } + + #[test] + #[should_panic(expected = "Cannot skip beyond buffer length")] + fn encoder_skip_too_much() { + let mut enc = Encoder::from_hex("0102"); + enc.skip(3); + } + /// [`Encoder::as_decoder`] should only expose the bytes actively encoded through this /// [`Encoder`], not all bytes of the underlying [`Buffer`]. #[test] @@ -1208,4 +1293,42 @@ mod tests { assert_eq!(decoder.as_ref(), &[5, 6, 7]); assert_eq!(buffer, &[1, 2, 3, 4, 5, 6, 7]); } + + /// Converting an [`Encoder`] to [`Vec<u8>`] should respect the `start` offset. + #[test] + fn into_vec_respects_skip() { + let mut enc = Encoder::from_hex("010203040506"); + enc.skip(2); + let v: Vec<u8> = enc.into(); + assert_eq!(v, vec![0x03, 0x04, 0x05, 0x06]); + } + + /// Converting an [`Encoder`] without skip should return the full buffer. + #[test] + fn into_vec_without_skip() { + let enc = Encoder::from_hex("010203"); + let v: Vec<u8> = enc.into(); + assert_eq!(v, vec![0x01, 0x02, 0x03]); + } + + #[test] + fn partial_eq_respects_skip() { + let mut enc1 = Encoder::from_hex("010203040506"); + enc1.skip(2); + let enc2 = Encoder::from_hex("03040506"); + assert_eq!(enc1, enc2); + assert_ne!(enc1, Encoder::from_hex("ffffff")); + } + + /// [`Clone`] should not clone skipped bytes. + #[test] + fn clone_respects_skip() { + let mut enc = Encoder::from_hex("010203040506"); + enc.skip(2); + let cloned = enc.clone(); + assert_eq!(cloned.as_ref(), &[0x03, 0x04, 0x05, 0x06]); + assert_eq!(cloned.len(), 4); + let v: Vec<u8> = cloned.into(); + assert_eq!(v, vec![0x03, 0x04, 0x05, 0x06]); + } } diff --git a/third_party/rust/neqo-common/src/datagram.rs b/third_party/rust/neqo-common/src/datagram.rs @@ -399,4 +399,37 @@ mod tests { assert_eq!(datagrams[1].d, &[41, 51, 61]); assert_eq!(datagrams[2].d, &[71]); } + + #[test] + fn datagram_len_and_accessors() { + let mut d = Datagram::new(DEFAULT_ADDR, DEFAULT_ADDR, Tos::default(), vec![1, 2, 3]); + assert_eq!(d.len(), 3); + assert!(!d.is_empty()); + assert_eq!(d.as_ref(), &[1, 2, 3]); + d.as_mut()[0] = 9; + assert_eq!(d.as_ref(), &[9, 2, 3]); + d.set_tos(Ecn::Ce.into()); + assert_eq!(d.tos(), Ecn::Ce.into()); + } + + #[test] + fn batch_data_and_try_from() { + let d = Datagram::new(DEFAULT_ADDR, DEFAULT_ADDR, Tos::default(), vec![1, 2, 3]); + let batch = DatagramBatch::from(d); + assert_eq!(batch.data(), &[1, 2, 3]); + let d2: Datagram = batch.try_into().unwrap(); + assert_eq!(d2.as_ref(), &[1, 2, 3]); + } + + #[test] + fn batch_try_from_multiple_fails() { + let batch = DatagramBatch::new( + DEFAULT_ADDR, + DEFAULT_ADDR, + Tos::default(), + NonZeroUsize::new(2).unwrap(), + vec![1, 2, 3, 4], + ); + assert!(Datagram::try_from(batch).is_err()); + } } diff --git a/third_party/rust/neqo-common/src/header.rs b/third_party/rust/neqo-common/src/header.rs @@ -218,12 +218,32 @@ mod tests { #[test] fn header_comparison_with_bytes() { let header = Header::new("test", b"value"); - - // Test PartialEq with byte slice assert_eq!(header, ("test", b"value".as_ref())); + assert_ne!(header, ("test", b"other".as_ref())); + assert_ne!(header, ("other", b"value".as_ref())); + } - // Test with string (converted to bytes) - let header2 = Header::new("test2", "string_value"); - assert_eq!(header2, ("test2", b"string_value".as_ref())); + #[test] + fn is_allowed_for_response() { + assert!(Header::new("content-type", "text/html").is_allowed_for_response()); + for name in ["connection", "host", "keep-alive", "transfer-encoding"] { + assert!(!Header::new(name, "x").is_allowed_for_response()); + } + } + + #[test] + fn headers_ext() { + let headers = [ + Header::new("content-type", "text/html"), + Header::new("x-custom", "value"), + ]; + assert!(headers.iter().contains_header("content-type", "text/html")); + assert!(!headers.iter().contains_header("content-type", "other")); + assert!(!headers.iter().contains_header("missing", "value")); + assert_eq!( + headers.iter().find_header("x-custom").unwrap().name(), + "x-custom" + ); + assert!(headers.iter().find_header("missing").is_none()); } } diff --git a/third_party/rust/neqo-common/src/hrtime.rs b/third_party/rust/neqo-common/src/hrtime.rs @@ -16,7 +16,7 @@ use windows::Win32::Media::{timeBeginPeriod, timeEndPeriod}; /// A quantized `Duration`. This currently just produces 16 discrete values /// corresponding to whole milliseconds. Future implementations might choose /// a different allocation, such as a logarithmic scale. -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] struct Period(u8); impl Period { @@ -380,6 +380,73 @@ impl Drop for Time { } } +// Unit tests for Period and PeriodSet data structures (platform-independent) +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod unit_tests { + use std::time::Duration; + + use super::{Period, PeriodSet}; + + #[test] + fn period_from_duration() { + assert_eq!(Period::from(Duration::from_millis(0)), Period::MIN); + assert_eq!(Period::from(Duration::from_millis(1)), Period::MIN); + assert_eq!(Period::from(Duration::from_millis(5)), Period(5)); + assert_eq!(Period::from(Duration::from_millis(16)), Period::MAX); + assert_eq!(Period::from(Duration::from_millis(100)), Period::MAX); + } + + #[test] + fn period_set_add_remove_min() { + let mut ps = PeriodSet::default(); + assert!(ps.min().is_none()); + + ps.add(Period(5)); + assert_eq!(ps.min(), Some(Period(5))); + + ps.add(Period(3)); + assert_eq!(ps.min(), Some(Period(3))); + + ps.add(Period(5)); // Add another 5 + ps.remove(Period(3)); + assert_eq!(ps.min(), Some(Period(5))); + + ps.remove(Period(5)); + assert_eq!(ps.min(), Some(Period(5))); // Still one 5 left + + ps.remove(Period(5)); + assert!(ps.min().is_none()); + } + + #[test] + fn period_set_max_ignored() { + let mut ps = PeriodSet::default(); + ps.add(Period::MAX); + assert!(ps.min().is_none()); // MAX not tracked + + ps.add(Period(5)); + assert_eq!(ps.min(), Some(Period(5))); + + ps.remove(Period::MAX); // Should not panic + assert_eq!(ps.min(), Some(Period(5))); + } + + #[cfg(target_os = "macos")] + #[test] + fn period_scaled() { + assert_eq!(Period(5).scaled(2.0).to_bits(), 10.0_f64.to_bits()); + assert_eq!(Period(4).scaled(0.5).to_bits(), 2.0_f64.to_bits()); + } + + #[cfg(windows)] + #[test] + fn period_as_u32() { + assert_eq!(Period(5).as_u32(), 5); + assert_eq!(Period::MIN.as_u32(), 1); + } +} + // Only run these tests in CI on Linux, where the timer accuracies are OK enough to pass the tests, // but only when not running sanitizers. #[cfg(all(target_os = "linux", not(neqo_sanitize)))] diff --git a/third_party/rust/neqo-common/src/incrdecoder.rs b/third_party/rust/neqo-common/src/incrdecoder.rs @@ -263,4 +263,19 @@ mod tests { assert!(res); } } + + #[test] + fn decoding_in_progress() { + let mut dec = IncrementalDecoderUint::default(); + assert!(!dec.decoding_in_progress()); + let mut dv = Decoder::new(&[0x40]); // Start of 2-byte varint + assert!(dec.consume(&mut dv).is_none()); + assert!(dec.decoding_in_progress()); + } + + #[test] + fn buffer_min_remaining() { + let dec = IncrementalDecoderBuffer::new(5); + assert_eq!(dec.min_remaining(), 5); + } } diff --git a/third_party/rust/neqo-common/src/lib.rs b/third_party/rust/neqo-common/src/lib.rs @@ -106,3 +106,32 @@ pub enum MessageType { Request, Response, } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use super::*; + + #[test] + fn hex_output() { + assert_eq!(hex([]), ""); + assert_eq!(hex([0xab, 0xcd]), "abcd"); + } + + #[test] + fn const_minmax() { + for (a, b, min, max) in [(2, 5, 2, 5), (5, 2, 2, 5), (3, 3, 3, 3)] { + assert_eq!(const_min(a, b), min); + assert_eq!(const_max(a, b), max); + } + } + + #[test] + fn hex_snip_middle_boundary() { + let short: Vec<u8> = (0..16).collect(); + assert!(hex_snip_middle(&short).ends_with("0e0f")); + let long: Vec<u8> = (0..20).collect(); + let s = hex_snip_middle(&long); + assert!(s.starts_with("[20]: 00") && s.contains("..") && s.ends_with("1213")); + } +} diff --git a/third_party/rust/neqo-common/src/qlog.rs b/third_party/rust/neqo-common/src/qlog.rs @@ -22,7 +22,12 @@ use crate::Role; #[derive(Debug, Clone, Default)] pub struct Qlog { - inner: Rc<RefCell<Option<SharedStreamer>>>, + /// Both the inner and the outer `Option` are set to `None` + /// on failure. The inner `None` will disable qlog for all other + /// references (correctness). The outer `None` will prevent + /// the local instance from de-referencing the `Rc` again + /// (performance). + inner: Option<Rc<RefCell<Option<SharedStreamer>>>>, } pub struct SharedStreamer { @@ -51,8 +56,7 @@ impl Qlog { // As a server, the original DCID is chosen by the client. Using // create_new() prevents attackers from overwriting existing logs. .create_new(true) - .open(&qlog_path) - .map_err(qlog::Error::IoError)?; + .open(&qlog_path)?; let streamer = QlogStreamer::new( qlog::QLOG_VERSION.to_string(), @@ -69,6 +73,9 @@ impl Qlog { /// Create an enabled `Qlog` configuration. /// + /// This needs to be called before the connection is used, because otherwise `Qlog`-logging will + /// remain disabled (for performance reasons). + /// /// # Errors /// /// Will return `qlog::Error` if it cannot write to the new log. @@ -76,10 +83,10 @@ impl Qlog { streamer.start_log()?; Ok(Self { - inner: Rc::new(RefCell::new(Some(SharedStreamer { + inner: Some(Rc::new(RefCell::new(Some(SharedStreamer { qlog_path, streamer, - }))), + })))), }) } @@ -90,20 +97,7 @@ impl Qlog { } /// If logging enabled, closure may generate an event to be logged. - pub fn add_event_with_instant<F>(&self, f: F, now: Instant) - where - F: FnOnce() -> Option<qlog::events::Event>, - { - self.add_event_with_stream(|s| { - if let Some(evt) = f() { - s.add_event_with_instant(evt, now)?; - } - Ok(()) - }); - } - - /// If logging enabled, closure may generate an event to be logged. - pub fn add_event_data_with_instant<F>(&self, f: F, now: Instant) + pub fn add_event_at<F>(&mut self, f: F, now: Instant) where F: FnOnce() -> Option<qlog::events::EventData>, { @@ -117,15 +111,31 @@ impl Qlog { /// If logging enabled, closure is given the Qlog stream to write events and /// frames to. - pub fn add_event_with_stream<F>(&self, f: F) + pub fn add_event_with_stream<F>(&mut self, f: F) where F: FnOnce(&mut QlogStreamer) -> Result<(), qlog::Error>, { - if let Some(inner) = self.inner.borrow_mut().as_mut() { - if let Err(e) = f(&mut inner.streamer) { - log::error!("Qlog event generation failed with error {e}; closing qlog."); - *self.inner.borrow_mut() = None; - } + let Some(inner) = self.inner.as_mut() else { + return; + }; + + let mut borrow = inner.borrow_mut(); + + let Some(shared_streamer) = borrow.as_mut() else { + drop(borrow); + // Set the outer Option to None to prevent future dereferences. + self.inner = None; + return; + }; + + if let Err(e) = f(&mut shared_streamer.streamer) { + log::error!("Qlog event generation failed with error {e}; closing qlog."); + // Set the inner Option to None to disable future logging for other references. + *borrow = None; + // Explicitly drop the RefCell borrow to release the mutable borrow. + drop(borrow); + // Set the outer Option to None to prevent future dereferences. + self.inner = None; } } } @@ -176,7 +186,6 @@ pub fn new_trace(role: Role) -> TraceSeq { #[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] mod test { - use qlog::events::Event; use regex::Regex; use test_fixture::EXPECTED_LOG_HEADER; @@ -198,9 +207,9 @@ mod test { } #[test] - fn add_event_with_instant() { - let (log, contents) = test_fixture::new_neqo_qlog(); - log.add_event_with_instant(|| Some(Event::with_time(0.0, EV_DATA)), test_fixture::now()); + fn add_event_at() { + let (mut log, contents) = test_fixture::new_neqo_qlog(); + log.add_event_at(|| Some(EV_DATA), test_fixture::now()); assert_eq!( Regex::new("\"time\":[0-9]+.[0-9]+,") .unwrap() diff --git a/third_party/rust/neqo-common/src/tos.rs b/third_party/rust/neqo-common/src/tos.rs @@ -369,8 +369,8 @@ mod tests { #[test] fn tos_is_ecn_marked() { - let tos: Tos = (Dscp::Af41, Ecn::Ce).into(); - assert!(tos.is_ecn_marked()); + assert!(Tos::from((Dscp::Af41, Ecn::Ce)).is_ecn_marked()); + assert!(!Tos::from((Dscp::Af41, Ecn::NotEct)).is_ecn_marked()); } #[test] @@ -378,4 +378,18 @@ mod tests { assert!(Ecn::Ce.is_marked()); assert!(!Ecn::NotEct.is_marked()); } + + #[test] + fn ecn_is_ect() { + assert!(Ecn::Ect0.is_ect()); + assert!(Ecn::Ect1.is_ect()); + assert!(!Ecn::Ce.is_ect()); + assert!(!Ecn::NotEct.is_ect()); + } + + #[test] + fn dscp_into_tos_non_default() { + let tos: Tos = Dscp::Af41.into(); + assert_eq!(u8::from(tos), u8::from(Dscp::Af41)); + } } diff --git a/third_party/rust/neqo-crypto/.cargo-checksum.json b/third_party/rust/neqo-crypto/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"ca9b945ff25ac1f4a539fc94ae2854c01c5350c2bdc84071e648873f06956931","bindings/bindings.toml":"edffd81bae5081805f92fd527fd1fb474abf07a96c7b1536629ed0b2a328b638","bindings/nspr_err.h":"2d5205d017b536c2d838bcf9bc4ec79f96dd50e7bb9b73892328781f1ee6629d","bindings/nspr_error.h":"e41c03c77b8c22046f8618832c9569fbcc7b26d8b9bbc35eea7168f35e346889","bindings/nspr_io.h":"085b289849ef0e77f88512a27b4d9bdc28252bd4d39c6a17303204e46ef45f72","bindings/nspr_time.h":"2e637fd338a5cf0fd3fb0070a47f474a34c2a7f4447f31b6875f5a9928d0a261","bindings/nss_ciphers.h":"95ec6344a607558b3c5ba8510f463b6295f3a2fb3f538a01410531045a5f62d1","bindings/nss_init.h":"ef49045063782fb612aff459172cc6a89340f15005808608ade5320ca9974310","bindings/nss_p11.h":"0b81e64fe6db49b2ecff94edd850be111ef99ec11220e88ceb1c67be90143a78","bindings/nss_secerr.h":"713e8368bdae5159af7893cfa517dabfe5103cede051dee9c9557c850a2defc6","bindings/nss_ssl.h":"af222fb957b989e392e762fa2125c82608a0053aff4fb97e556691646c88c335","bindings/nss_sslerr.h":"24b97f092183d8486f774cdaef5030d0249221c78343570d83a4ee5b594210ae","bindings/nss_sslopt.h":"b7807eb7abdad14db6ad7bc51048a46b065a0ea65a4508c95a12ce90e59d1eea","build.rs":"37e36650c067e04fecddca49b6e32a2b9dae14118e066a2baa676ad1ae6d33d3","min_version.txt":"0f9ddf9ddaeb5137a5ab3d238d06286822f9579b1f46ba76312a8c6d76176500","src/aead.rs":"7f627f7dcb08444891b4898a8ab8c0bc4984c035212572547323046ec46e4bb1","src/aead_null.rs":"e8946edbff657763885dd52ccc5516726f316286b6e0c84671458d02a3c7e44a","src/agent.rs":"69e2d99c47c12bf24d4659e723fb85096d6286d134ced65054c40631e88c7c0c","src/agentio.rs":"eb13376f2aed4b0b822784d43d341709b3a33f6ba52560ff48ca3e339d1e86da","src/auth.rs":"bbba836237b0c5d079f1348a96bc46b5bb6fb3cd34ca568581c9f7f8800444d1","src/cert.rs":"afecc277b918e9123d6099fc2b7f5a4ef58c9c3c1b3ca9d4790bda0a46665fe3","src/constants.rs":"83606aeb646b2833a8094f9d980c266ecc3e8cb40c93a4820da221988319dd1a","src/ech.rs":"cf6670ce7ceaaa67c8b0f93b5063cf4a0b92a0b176bbbb664b0a58f1b922b710","src/err.rs":"40658d015ac45cdd29b3bc34540c93b80a20baf5d470e0c59754fc45ce6af204","src/exp.rs":"70549c53ce8df99d62d3343697abd2a177d67ff56703a3d26048bdcdc8b87a0d","src/ext.rs":"7082cd7b44ba97275a8aefe0c31c2419d750f9621486c9c017864c82a7580423","src/hkdf.rs":"76c5abc8b2d6ee12d8a86cd730af2cf47a59b2fbfd3b8a635a1826636156794d","src/hp.rs":"04a461676c02d308f1f851b975846f83daa50ee08de9e573b4136ce4d54b4473","src/lib.rs":"42bdd28c9cd22178e2a0ab1736a0ea49cb240c78cc924d26296086d469a1f2fe","src/min_version.rs":"c6e1f98b9f56db0622ac38c1be131c55acf4a0f09ed0d6283f4d6308e2d1301a","src/p11.rs":"d46cb6c19f5c7b6fd91ce9488538475c817f62cce03f3275acca5bf6f1ec7e61","src/prio.rs":"198475faf39ffa3fe3857dff8a75a6ab0d3d54a6be7e496f008868b30653b924","src/replay.rs":"7bf84ce1964658e69d81a810f3b8d71d36d5a7fc336d83c04fb585a6a98e6d33","src/result.rs":"27067d9aba61e8162fb92bca03f9a462cf4fe2f0a259d52696b63e1f6a959a5c","src/secrets.rs":"b021c91b9c1b63373474c39e817a7d9083681be13b5466c4d2b776db9a65b9f8","src/selfencrypt.rs":"7eb5e815b2efcec42f6b4cab432a33a0679a3b1657b342971b0d0383bff16d1a","src/ssl.rs":"0a63ed08047a370f64efb314ee5686c3c47e29b3307c0f552d6cd8346ae06c03","src/time.rs":"c4c9987bfe273f19a2f5ef09920ccfe384ab1c1eaf2b2281eb4b02aa8d3b9970","tests/aead.rs":"2e99fba2f155aa8442709c4847f171f0cdfc179b2a7cd2afd853b550d02f7792","tests/agent.rs":"81266b780a40f1d8d31edbe1f43a37fd641f2cb44f75365c67b068c0d3442bb3","tests/ext.rs":"40e3bb0e5ea00fe411cfaf1a006fd4b11a22503f66d3738423361a8b7f80fe13","tests/handshake.rs":"7c6dbdf1b2ae74d15f0a3242d9969abf04ea9839eddcf1aae73379142f33a433","tests/hkdf.rs":"1d2098dc8398395864baf13e4886cfd1da6d36118727c3b264f457ee3da6b048","tests/hp.rs":"dab2631fb5a4f47227e05f508eaca4b4aa225bafced60e703e6fd1c329ac6ab1","tests/init.rs":"3cfe8411ca31ad7dfb23822bb1570e1a5b2b334857173bdd7df086b65b81d95a","tests/selfencrypt.rs":"2e0b548fc84f388b0b2367fb8d9e3e0bd25c4814a1e997b13b7849a54a529703"},"package":null} -\ No newline at end of file +{"files":{"Cargo.toml":"3e9c4183e23ef10dd0888dfd1c6122d77bd498bacb9b39804fde82e50b307f71","bindings/bindings.toml":"edffd81bae5081805f92fd527fd1fb474abf07a96c7b1536629ed0b2a328b638","bindings/nspr_err.h":"2d5205d017b536c2d838bcf9bc4ec79f96dd50e7bb9b73892328781f1ee6629d","bindings/nspr_error.h":"e41c03c77b8c22046f8618832c9569fbcc7b26d8b9bbc35eea7168f35e346889","bindings/nspr_io.h":"085b289849ef0e77f88512a27b4d9bdc28252bd4d39c6a17303204e46ef45f72","bindings/nspr_time.h":"2e637fd338a5cf0fd3fb0070a47f474a34c2a7f4447f31b6875f5a9928d0a261","bindings/nss_ciphers.h":"95ec6344a607558b3c5ba8510f463b6295f3a2fb3f538a01410531045a5f62d1","bindings/nss_init.h":"ef49045063782fb612aff459172cc6a89340f15005808608ade5320ca9974310","bindings/nss_p11.h":"0b81e64fe6db49b2ecff94edd850be111ef99ec11220e88ceb1c67be90143a78","bindings/nss_secerr.h":"713e8368bdae5159af7893cfa517dabfe5103cede051dee9c9557c850a2defc6","bindings/nss_ssl.h":"af222fb957b989e392e762fa2125c82608a0053aff4fb97e556691646c88c335","bindings/nss_sslerr.h":"24b97f092183d8486f774cdaef5030d0249221c78343570d83a4ee5b594210ae","bindings/nss_sslopt.h":"b7807eb7abdad14db6ad7bc51048a46b065a0ea65a4508c95a12ce90e59d1eea","build.rs":"37e36650c067e04fecddca49b6e32a2b9dae14118e066a2baa676ad1ae6d33d3","min_version.txt":"0f9ddf9ddaeb5137a5ab3d238d06286822f9579b1f46ba76312a8c6d76176500","src/aead.rs":"32c6920e2d8d54e689ed2b7055d890f58ea064137234f6610255840e8c2c3dec","src/aead_null.rs":"b1c7d899ef77009929a6d8793290d674ed697f782b90ea32cedeba69288e4b29","src/agent.rs":"69e2d99c47c12bf24d4659e723fb85096d6286d134ced65054c40631e88c7c0c","src/agentio.rs":"eb13376f2aed4b0b822784d43d341709b3a33f6ba52560ff48ca3e339d1e86da","src/auth.rs":"bbba836237b0c5d079f1348a96bc46b5bb6fb3cd34ca568581c9f7f8800444d1","src/cert.rs":"056cb38a5307a940c32e79fe5f490dab42d84c9d04ed0c9c3899055407f4797f","src/constants.rs":"83606aeb646b2833a8094f9d980c266ecc3e8cb40c93a4820da221988319dd1a","src/ech.rs":"cf6670ce7ceaaa67c8b0f93b5063cf4a0b92a0b176bbbb664b0a58f1b922b710","src/err.rs":"40658d015ac45cdd29b3bc34540c93b80a20baf5d470e0c59754fc45ce6af204","src/exp.rs":"70549c53ce8df99d62d3343697abd2a177d67ff56703a3d26048bdcdc8b87a0d","src/ext.rs":"7082cd7b44ba97275a8aefe0c31c2419d750f9621486c9c017864c82a7580423","src/hkdf.rs":"76c5abc8b2d6ee12d8a86cd730af2cf47a59b2fbfd3b8a635a1826636156794d","src/hp.rs":"04a461676c02d308f1f851b975846f83daa50ee08de9e573b4136ce4d54b4473","src/lib.rs":"42bdd28c9cd22178e2a0ab1736a0ea49cb240c78cc924d26296086d469a1f2fe","src/min_version.rs":"c6e1f98b9f56db0622ac38c1be131c55acf4a0f09ed0d6283f4d6308e2d1301a","src/p11.rs":"d46cb6c19f5c7b6fd91ce9488538475c817f62cce03f3275acca5bf6f1ec7e61","src/prio.rs":"198475faf39ffa3fe3857dff8a75a6ab0d3d54a6be7e496f008868b30653b924","src/replay.rs":"7bf84ce1964658e69d81a810f3b8d71d36d5a7fc336d83c04fb585a6a98e6d33","src/result.rs":"27067d9aba61e8162fb92bca03f9a462cf4fe2f0a259d52696b63e1f6a959a5c","src/secrets.rs":"b021c91b9c1b63373474c39e817a7d9083681be13b5466c4d2b776db9a65b9f8","src/selfencrypt.rs":"99bb2e8e07cc75a5fc1993f02942b3f0c3ef5d949278e847bc54fed2166bb3ac","src/ssl.rs":"0a63ed08047a370f64efb314ee5686c3c47e29b3307c0f552d6cd8346ae06c03","src/time.rs":"c4c9987bfe273f19a2f5ef09920ccfe384ab1c1eaf2b2281eb4b02aa8d3b9970","tests/aead.rs":"f73c1fc4bc891282aad84fb1a09fa30388acd398be9ab81769ac3b2e3df4f7ba","tests/agent.rs":"81266b780a40f1d8d31edbe1f43a37fd641f2cb44f75365c67b068c0d3442bb3","tests/ext.rs":"40e3bb0e5ea00fe411cfaf1a006fd4b11a22503f66d3738423361a8b7f80fe13","tests/handshake.rs":"7c6dbdf1b2ae74d15f0a3242d9969abf04ea9839eddcf1aae73379142f33a433","tests/hkdf.rs":"1d2098dc8398395864baf13e4886cfd1da6d36118727c3b264f457ee3da6b048","tests/hp.rs":"dab2631fb5a4f47227e05f508eaca4b4aa225bafced60e703e6fd1c329ac6ab1","tests/init.rs":"3cfe8411ca31ad7dfb23822bb1570e1a5b2b334857173bdd7df086b65b81d95a","tests/selfencrypt.rs":"655a7ea4748bda3a0ce7f093972edcd7c763d0cc076024f4a8a981c05e5d8fa4"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/neqo-crypto/Cargo.toml b/third_party/rust/neqo-crypto/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.81.0" name = "neqo-crypto" -version = "0.20.0" +version = "0.21.0" authors = ["The Neqo Authors <necko@mozilla.com>"] build = "build.rs" autolib = false @@ -131,6 +131,7 @@ default-features = false [build-dependencies.serde] version = "1.0" +features = ["std"] default-features = false [build-dependencies.serde_derive] @@ -138,7 +139,11 @@ version = "1.0" default-features = false [build-dependencies.toml] -version = "0.5" +version = "0.9" +features = [ + "parse", + "serde", +] default-features = false [target."cfg(windows)".dependencies.windows] diff --git a/third_party/rust/neqo-crypto/src/aead.rs b/third_party/rust/neqo-crypto/src/aead.rs @@ -55,8 +55,7 @@ pub trait Aead { /// # Errors /// /// Returns `Error` when encryption fails. - fn encrypt_in_place<'a>(&self, count: u64, aad: &[u8], data: &'a mut [u8]) - -> Res<&'a mut [u8]>; + fn encrypt_in_place(&self, count: u64, aad: &[u8], data: &mut [u8]) -> Res<usize>; /// Decrypt ciphertext with associated data. /// @@ -76,8 +75,7 @@ pub trait Aead { /// # Errors /// /// Returns `Error` when decryption or authentication fails. - fn decrypt_in_place<'a>(&self, count: u64, aad: &[u8], data: &'a mut [u8]) - -> Res<&'a mut [u8]>; + fn decrypt_in_place(&self, count: u64, aad: &[u8], data: &mut [u8]) -> Res<usize>; } experimental_api!(SSL_MakeAead( @@ -174,12 +172,7 @@ impl Aead for RealAead { Ok(&output[..l.try_into()?]) } - fn encrypt_in_place<'a>( - &self, - count: u64, - aad: &[u8], - data: &'a mut [u8], - ) -> Res<&'a mut [u8]> { + fn encrypt_in_place(&self, count: u64, aad: &[u8], data: &mut [u8]) -> Res<usize> { if data.len() < self.expansion() { return Err(Error::from(SEC_ERROR_BAD_DATA)); } @@ -199,7 +192,7 @@ impl Aead for RealAead { ) }?; debug_assert_eq!(usize::try_from(l)?, data.len()); - Ok(data) + Ok(data.len()) } fn decrypt<'a>( @@ -229,12 +222,7 @@ impl Aead for RealAead { Ok(&output[..l.try_into()?]) } - fn decrypt_in_place<'a>( - &self, - count: u64, - aad: &[u8], - data: &'a mut [u8], - ) -> Res<&'a mut [u8]> { + fn decrypt_in_place(&self, count: u64, aad: &[u8], data: &mut [u8]) -> Res<usize> { let mut l: c_uint = 0; unsafe { // Note that NSS insists upon having extra space available for decryption, so @@ -253,7 +241,7 @@ impl Aead for RealAead { ) }?; debug_assert_eq!(usize::try_from(l)?, data.len() - self.expansion()); - Ok(&mut data[..l.try_into()?]) + Ok(l.try_into()?) } } diff --git a/third_party/rust/neqo-crypto/src/aead_null.rs b/third_party/rust/neqo-crypto/src/aead_null.rs @@ -63,15 +63,10 @@ impl Aead for AeadNull { Ok(&output[..l + self.expansion()]) } - fn encrypt_in_place<'a>( - &self, - _count: u64, - _aad: &[u8], - data: &'a mut [u8], - ) -> Res<&'a mut [u8]> { + fn encrypt_in_place(&self, _count: u64, _aad: &[u8], data: &mut [u8]) -> Res<usize> { let pos = data.len() - self.expansion(); data[pos..].copy_from_slice(AEAD_NULL_TAG); - Ok(data) + Ok(data.len()) } fn decrypt<'a>( @@ -87,14 +82,8 @@ impl Aead for AeadNull { }) } - fn decrypt_in_place<'a>( - &self, - count: u64, - aad: &[u8], - data: &'a mut [u8], - ) -> Res<&'a mut [u8]> { + fn decrypt_in_place(&self, count: u64, aad: &[u8], data: &mut [u8]) -> Res<usize> { self.decrypt_check(count, aad, data) - .map(move |len| &mut data[..len]) } } diff --git a/third_party/rust/neqo-crypto/src/cert.rs b/third_party/rust/neqo-crypto/src/cert.rs @@ -42,22 +42,19 @@ fn peer_certificate_chain(fd: *mut PRFileDesc) -> Option<ItemArray> { // 2^24 items. Casting its length is therefore safe even on 32 bits targets. fn stapled_ocsp_responses(fd: *mut PRFileDesc) -> Option<Vec<Vec<u8>>> { let ocsp_nss = unsafe { SSL_PeerStapledOCSPResponses(fd) }; - match NonNull::new(ocsp_nss as *mut SECItemArray) { - Some(ocsp_ptr) => { - let mut ocsp_helper: Vec<Vec<u8>> = Vec::new(); - let Ok(len) = isize::try_from(unsafe { ocsp_ptr.as_ref().len }) else { - qerror!("[{fd:p}] Received illegal OSCP length"); - return None; - }; - for idx in 0..len { - let itemp: *const SECItem = unsafe { ocsp_ptr.as_ref().items.offset(idx).cast() }; - let item = unsafe { null_safe_slice((*itemp).data, (*itemp).len) }; - ocsp_helper.push(item.to_owned()); - } - Some(ocsp_helper) - } - None => None, - } + let ocsp_ptr = NonNull::new(ocsp_nss as *mut SECItemArray)?; + let Ok(len) = usize::try_from(unsafe { ocsp_ptr.as_ref().len }) else { + qerror!("[{fd:p}] Received illegal OCSP length"); + return None; + }; + Some( + (0..len) + .map(|idx| { + let itemp: *const SECItem = unsafe { ocsp_ptr.as_ref().items.add(idx).cast() }; + unsafe { null_safe_slice((*itemp).data, (*itemp).len) }.to_owned() + }) + .collect(), + ) } fn signed_cert_timestamp(fd: *mut PRFileDesc) -> Option<Vec<u8>> { diff --git a/third_party/rust/neqo-crypto/src/selfencrypt.rs b/third_party/rust/neqo-crypto/src/selfencrypt.rs @@ -129,24 +129,25 @@ impl SelfEncrypt { /// when the keys have been rotated; or when NSS fails. #[expect(clippy::similar_names, reason = "aad is similar to aead.")] pub fn open(&self, aad: &[u8], ciphertext: &[u8]) -> Res<Vec<u8>> { + const OFFSET: usize = 2 + SelfEncrypt::SALT_LENGTH; if *ciphertext.first().ok_or(Error::SelfEncrypt)? != Self::VERSION { return Err(Error::SelfEncrypt); } let Some(key) = self.select_key(*ciphertext.get(1).ok_or(Error::SelfEncrypt)?) else { return Err(Error::SelfEncrypt); }; - let offset = 2 + Self::SALT_LENGTH; + let salt = ciphertext.get(2..OFFSET).ok_or(Error::SelfEncrypt)?; - let mut extended_aad = Encoder::with_capacity(offset + aad.len()); - extended_aad.encode(&ciphertext[0..offset]); + let mut extended_aad = Encoder::with_capacity(OFFSET + aad.len()); + extended_aad.encode(&ciphertext[..OFFSET]); extended_aad.encode(aad); - let aead = self.make_aead(key, &ciphertext[2..offset])?; + let aead = self.make_aead(key, salt)?; // NSS insists on having extra space available for decryption. - let padded_len = ciphertext.len() - offset; + let padded_len = ciphertext.len() - OFFSET; let mut output = vec![0; padded_len]; let decrypted = - aead.decrypt(0, extended_aad.as_ref(), &ciphertext[offset..], &mut output)?; + aead.decrypt(0, extended_aad.as_ref(), &ciphertext[OFFSET..], &mut output)?; let final_len = decrypted.len(); output.truncate(final_len); qtrace!( diff --git a/third_party/rust/neqo-crypto/tests/aead.rs b/third_party/rust/neqo-crypto/tests/aead.rs @@ -131,3 +131,20 @@ fn aead_encrypt_in_place_too_small_buffer() { let result = aead.encrypt_in_place(1, AAD, &mut small_buffer); assert!(result.is_err()); } + +#[test] +fn encrypt_decrypt_in_place() { + let aead = make_aead(TLS_AES_128_GCM_SHA256); + + let plaintext = b"hello world"; + let mut buffer = Vec::from(plaintext); + buffer.resize(plaintext.len() + aead.expansion(), 0); + + let encrypted_len = aead.encrypt_in_place(0, b"aad", &mut buffer).unwrap(); + assert_eq!(encrypted_len, plaintext.len() + aead.expansion()); + assert_eq!(encrypted_len, buffer.len()); + + let decrypted_len = aead.decrypt_in_place(0, b"aad", &mut buffer).unwrap(); + assert_eq!(decrypted_len, plaintext.len()); + assert_eq!(&buffer[..decrypted_len], plaintext); +} diff --git a/third_party/rust/neqo-crypto/tests/selfencrypt.rs b/third_party/rust/neqo-crypto/tests/selfencrypt.rs @@ -99,3 +99,11 @@ fn truncate() { let res = se.open(AAD, &sealed[0..(sealed.len() - 1)]); assert_bad_data(res); } + +#[test] +fn truncate_header() { + let (se, _) = sealed(); + // Ciphertext too short to contain the salt (needs 2 byte header + 16 byte salt). + let res = se.open(AAD, &[1, 0, 0, 0, 0]); + assert_eq!(res.unwrap_err(), Error::SelfEncrypt); +} diff --git a/third_party/rust/neqo-http3/.cargo-checksum.json b/third_party/rust/neqo-http3/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"16e2fa39beb601ec03c20f87d931243030ec9354d9be0c6a3964e2cb5a99bc7c","benches/streams.rs":"2ec9282e63d9e5974367aab13b3c774e54e4d41ec4473d2ff0ec08c9596bc95b","src/buffered_send_stream.rs":"3eb0520b3a207597d6e7e232df8a7fc2c7bce65997af5bf92dbac2f6350d06ca","src/client_events.rs":"6e4b5a3e3a7038ca8a1cae33bf6e050f402c1e415cd53670058d6d99ae9e1b26","src/conn_params.rs":"3994bc99dc2ef77c01cc37e59b7ca91c463b228603958612497313251021a9fa","src/connection.rs":"3a61818868f518e66044b8840c6d7a0bb71236e757f36f8a6e5b1bc3af85e52d","src/connection_client.rs":"087214b7666f832524b8455f64fff9607163a7d6385aa002f6b2c5a64de23ee1","src/connection_server.rs":"7c49dd96095770258e281a680e1c34af3e1eb46510d0a0b15013de626076bd8b","src/control_stream_local.rs":"df0f7b272897f30dd2fcec9d13895cb8e8b1b453a96e8983fef05a8c878e7bc1","src/control_stream_remote.rs":"652e2bfcc3e020f7a9f4e3a107557e291757a7fc2e30cf9fe95c966d2be8c122","src/features/extended_connect/connect_udp_session.rs":"4a72424b06972f0ef265f33ad93cb008af16746a700c56bca4d95099a8dab26c","src/features/extended_connect/mod.rs":"edb2e04806052a899adb316b06596f1d23a40c8fa847dd2d931bc40332b505b2","src/features/extended_connect/session.rs":"d265dc67106a74cc5542b41f809836ae52b2d2223287198a94ca211b4a2b69b0","src/features/extended_connect/tests/mod.rs":"fd6aee37243713e80fc526552f21f0222338cec9890409b6575a2a637b17ec1f","src/features/extended_connect/tests/webtransport/datagrams.rs":"16a69b41aaada5339b85153b7194d2c1e9151ce9f25b29e02b0f24bb9500b331","src/features/extended_connect/tests/webtransport/mod.rs":"235101fed8d5c3fddd3e797f724c3013752e02462733f12298d7c9a82f666e3b","src/features/extended_connect/tests/webtransport/negotiation.rs":"b0083f8737bdea9bc0de1940c627d497fee8b79ebc218bbcea0a562ae530527f","src/features/extended_connect/tests/webtransport/sessions.rs":"7bd9fdf099cbe794ed438dc3c85f254c975e319ed2d984214c6a0c29829136d5","src/features/extended_connect/tests/webtransport/streams.rs":"e84374071268ecec302bc1c3e825bc2b7219dc11a7b88043061565f256c48542","src/features/extended_connect/webtransport_session.rs":"de19ee1daa77e83ad7ac3b858602967dcad01acca55cf6de59cc650664fa2423","src/features/extended_connect/webtransport_streams.rs":"4704ab559df3c0dad0602cd887d4cb13a17d812bf2005202ed57bfd4a8f96f8b","src/features/mod.rs":"7424e5f1939324953ed6acce76c5774f2bdfae3d1dfbdd938a4eded8a94d5c9e","src/frames/connect_udp_frame.rs":"112a8b1f848b7f0b1fc0d54aaf3e35560cd54e1ffdc1f1bc01028d798fbd45df","src/frames/hframe.rs":"6f0162e9bb8bacbff14f5b0f47d53b85b7c584bdb8a9151df60e834ae21fcbb1","src/frames/mod.rs":"3fb83b0f836de5d1cb00377d5d8ba874a70422fa1f02c28728945848a7ec61c4","src/frames/reader.rs":"468a2f3199b22feda9f8ae937d17c99c89123beb0f7e48b9bb1950e8a61e34b6","src/frames/tests/hframe.rs":"43a7735fc859692633e7f3c031710a9fb635611756cb4b9f387bac0a38c0fa09","src/frames/tests/mod.rs":"3ee262c649cd0ea0120da78943dfcab5b9a08064f433076d70ac399ccf489325","src/frames/tests/reader.rs":"b75cd92553238db3accae5256557e35fcba4d5d1204b4833b934236fae5c2c5d","src/frames/tests/wtframe.rs":"c6598d24f5e12972f02de6e1394362671633982db637a07e1c0bb9b56d93ea2a","src/frames/wtframe.rs":"19120bc6d42aa2738c9650d4bafaf20b760ef9266173f348bcc62e42c1925111","src/headers_checks.rs":"4cf29e5d12a1d427346f35a225b629867f336dbef0211045af9040a9ad258a0c","src/lib.rs":"814e61abffe9d32c88ed5d63941970bcb6802b02a7b64742aa6d0fe4a7523ae9","src/priority.rs":"5fa28fe1a235c4ffb0ab9a4506e979d7bd1a7d0136f2d525ca083bb81733db96","src/push_controller.rs":"cd05db6143b39b8913ca871bbcd00bb43271b9c9dd8ef53610313b929bbae80a","src/push_id.rs":"bf931466d0490cbe8977cd7a732d1d4970e16220f331899f5e7dab8873ece5de","src/qlog.rs":"9ae732d611f74b99fee124faed5d883ec130b1bd991c4af38608bc5bff274cc6","src/qpack_decoder_receiver.rs":"6f6ce0cf5946688f9811bc09ea69a6c02d7b173ba3a64cac31b4daa970f3004b","src/qpack_encoder_receiver.rs":"db30ea43d4cdb5f0fde2dc49e7d8b8ba12e38acbcb8b4417fe68d2551cefa2ea","src/recv_message.rs":"d008459fc7e75b39f23ef63c5c88bd194c784fbc8902e6dd66bb5439f77fcfe4","src/request_target.rs":"01f05026ea7ad47085ffe462df08401ccd162737e3b7a995e8dece31dd46ada6","src/send_message.rs":"916d93bcf4b38f68ea5fb5dfaea7555aa43a82f946124c85423caf67f74ee3b5","src/server.rs":"3448df84f6af734356e81f5896d2501a399195c85246884499ef3452fc23f68d","src/server_connection_events.rs":"4fa828e071ec595d6be554190a8d2115c3110bd0e7a69d767b0a6795daa8c9fe","src/server_events.rs":"8814a8ea3cb68218d05903eb64da7dea1fa5a7f4932ef887daae956b45a9d041","src/settings.rs":"88616d45069d942e08fe0a8ea0e52d5eff7f91998df67aa12d9484bb6f50ec5d","src/stream_type_reader.rs":"bf60d820146e20657ee76ec9fc06fc51f53c9cbe0fd83f7fbeacb9089f65d4e9","tests/classic_connect.rs":"9c9efe4929a236231ef825858c0fb6133bc2f47ed48f424859ade096d511f792","tests/connect_udp.rs":"9c45bd7dc96201b42b4fe2bfd6c394d10a599f2162104898d639d7920be2004d","tests/httpconn.rs":"bbf72898d2b06ded382fd5f48e1d48a2f6724a44d752929980910a6ce844b0b6","tests/non_ascii_headers.rs":"f86162d95da6f68871a2163ebeb7704c63ddcdf7eb1b8912ebe4dd4b672a73e3","tests/priority.rs":"b641c791e95e21713697c511a0f15d69ee141f017e1ffc7d9b46caa5ed47737c","tests/send_message.rs":"eabb424f1d068e84c5d53a8e0c04a019720e4ac6e969c175de7d8aba3dbd6ae5","tests/webtransport.rs":"b503cce443ec5212c79ed273b31eee7bf01bfd81ab41c9ac99f3846ad54bcfec"},"package":null} -\ No newline at end of file +{"files":{"Cargo.toml":"2cc25378c492fdf967cd3a2238527a65699ef03e8d254ee2ecde16277b0ab9ba","benches/streams.rs":"2ec9282e63d9e5974367aab13b3c774e54e4d41ec4473d2ff0ec08c9596bc95b","src/buffered_send_stream.rs":"5d4e7014b70f09c7aa8242735997ab5afe49e23caf591eae5d6eb0338315d1ea","src/client_events.rs":"1720882b8ef82474bcb3313cc7f8c53364ae12f6d704218b622396433c95bba1","src/conn_params.rs":"b345dce614ec5b51bded39b3b32a0b301cc822a4130cc9e83ef32110ef90a8da","src/connection.rs":"3a61818868f518e66044b8840c6d7a0bb71236e757f36f8a6e5b1bc3af85e52d","src/connection_client.rs":"087214b7666f832524b8455f64fff9607163a7d6385aa002f6b2c5a64de23ee1","src/connection_server.rs":"7c49dd96095770258e281a680e1c34af3e1eb46510d0a0b15013de626076bd8b","src/control_stream_local.rs":"a06f671bbbbd32cafcbe83e4b8815b4d2709c757df5febd58a33031ef8da8f24","src/control_stream_remote.rs":"652e2bfcc3e020f7a9f4e3a107557e291757a7fc2e30cf9fe95c966d2be8c122","src/features/extended_connect/connect_udp_session.rs":"4a72424b06972f0ef265f33ad93cb008af16746a700c56bca4d95099a8dab26c","src/features/extended_connect/mod.rs":"edb2e04806052a899adb316b06596f1d23a40c8fa847dd2d931bc40332b505b2","src/features/extended_connect/session.rs":"d265dc67106a74cc5542b41f809836ae52b2d2223287198a94ca211b4a2b69b0","src/features/extended_connect/tests/mod.rs":"fd6aee37243713e80fc526552f21f0222338cec9890409b6575a2a637b17ec1f","src/features/extended_connect/tests/webtransport/datagrams.rs":"16a69b41aaada5339b85153b7194d2c1e9151ce9f25b29e02b0f24bb9500b331","src/features/extended_connect/tests/webtransport/mod.rs":"235101fed8d5c3fddd3e797f724c3013752e02462733f12298d7c9a82f666e3b","src/features/extended_connect/tests/webtransport/negotiation.rs":"b0083f8737bdea9bc0de1940c627d497fee8b79ebc218bbcea0a562ae530527f","src/features/extended_connect/tests/webtransport/sessions.rs":"7bd9fdf099cbe794ed438dc3c85f254c975e319ed2d984214c6a0c29829136d5","src/features/extended_connect/tests/webtransport/streams.rs":"e84374071268ecec302bc1c3e825bc2b7219dc11a7b88043061565f256c48542","src/features/extended_connect/webtransport_session.rs":"de19ee1daa77e83ad7ac3b858602967dcad01acca55cf6de59cc650664fa2423","src/features/extended_connect/webtransport_streams.rs":"4704ab559df3c0dad0602cd887d4cb13a17d812bf2005202ed57bfd4a8f96f8b","src/features/mod.rs":"bf5155f5e2cb123355172fee832e87bfb6bbf33d039f8792465c37a2958ae102","src/frames/connect_udp_frame.rs":"112a8b1f848b7f0b1fc0d54aaf3e35560cd54e1ffdc1f1bc01028d798fbd45df","src/frames/hframe.rs":"b445375c109e82bd3c3f738fa2043346e6f56d6a85259978474bc51670c17dc4","src/frames/mod.rs":"3fb83b0f836de5d1cb00377d5d8ba874a70422fa1f02c28728945848a7ec61c4","src/frames/reader.rs":"535e839d7fd1bb311ce9af5a570ee0da4a11d901326d595389fce66dfdc23b26","src/frames/tests/hframe.rs":"43a7735fc859692633e7f3c031710a9fb635611756cb4b9f387bac0a38c0fa09","src/frames/tests/mod.rs":"3ee262c649cd0ea0120da78943dfcab5b9a08064f433076d70ac399ccf489325","src/frames/tests/reader.rs":"b75cd92553238db3accae5256557e35fcba4d5d1204b4833b934236fae5c2c5d","src/frames/tests/wtframe.rs":"c6598d24f5e12972f02de6e1394362671633982db637a07e1c0bb9b56d93ea2a","src/frames/wtframe.rs":"13ce3f2d3cdcdf5a79565531092974639d1fbf3c63dd568ed4e581d4556111a7","src/headers_checks.rs":"32ed7a25f526d90815d25ecc5ab9752a77499a70bfa9bce32bf3dd3da6be69a0","src/lib.rs":"5d4eb8a774a734f06e8d890d85aa1cbae594cb6dbcc869263ae4bd26b020be5d","src/priority.rs":"52225b007345314ab90d5726562778d3e754b3f982d1a4d07ce5c4aaa3c5678a","src/push_controller.rs":"6b474f6ce47875d799ec0be265effb6c8ef306bd9fee08193d0d58ee07439410","src/push_id.rs":"bf931466d0490cbe8977cd7a732d1d4970e16220f331899f5e7dab8873ece5de","src/qlog.rs":"04467f49e19e2b9c1a1d89aa3f74bad6dae3866a81b07051bda5170fa1e8f194","src/qpack_decoder_receiver.rs":"6f6ce0cf5946688f9811bc09ea69a6c02d7b173ba3a64cac31b4daa970f3004b","src/qpack_encoder_receiver.rs":"db30ea43d4cdb5f0fde2dc49e7d8b8ba12e38acbcb8b4417fe68d2551cefa2ea","src/recv_message.rs":"d008459fc7e75b39f23ef63c5c88bd194c784fbc8902e6dd66bb5439f77fcfe4","src/request_target.rs":"01f05026ea7ad47085ffe462df08401ccd162737e3b7a995e8dece31dd46ada6","src/send_message.rs":"4509c6914877bb6177d3e2503b83a1a7d71c07a8f69bf8d6fd50d59094cba0e2","src/server.rs":"3448df84f6af734356e81f5896d2501a399195c85246884499ef3452fc23f68d","src/server_connection_events.rs":"a4bca2f18ada884a73a0f526160c3e344ec1cf807ca92a0ac35dae32c1358c8e","src/server_events.rs":"8814a8ea3cb68218d05903eb64da7dea1fa5a7f4932ef887daae956b45a9d041","src/settings.rs":"cbfbebd6a1a7262394ef89906b3bce00d6066638cdf9a39311dc8c0018811b0c","src/stream_type_reader.rs":"bf60d820146e20657ee76ec9fc06fc51f53c9cbe0fd83f7fbeacb9089f65d4e9","tests/classic_connect.rs":"9c9efe4929a236231ef825858c0fb6133bc2f47ed48f424859ade096d511f792","tests/connect_udp.rs":"9c45bd7dc96201b42b4fe2bfd6c394d10a599f2162104898d639d7920be2004d","tests/httpconn.rs":"bbf72898d2b06ded382fd5f48e1d48a2f6724a44d752929980910a6ce844b0b6","tests/non_ascii_headers.rs":"f86162d95da6f68871a2163ebeb7704c63ddcdf7eb1b8912ebe4dd4b672a73e3","tests/priority.rs":"b641c791e95e21713697c511a0f15d69ee141f017e1ffc7d9b46caa5ed47737c","tests/send_message.rs":"eabb424f1d068e84c5d53a8e0c04a019720e4ac6e969c175de7d8aba3dbd6ae5","tests/webtransport.rs":"b503cce443ec5212c79ed273b31eee7bf01bfd81ab41c9ac99f3846ad54bcfec"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/neqo-http3/Cargo.toml b/third_party/rust/neqo-http3/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.81.0" name = "neqo-http3" -version = "0.20.0" +version = "0.21.0" authors = ["The Neqo Authors <necko@mozilla.com>"] build = false autolib = false @@ -56,6 +56,13 @@ bench = [ "neqo-transport/bench", "log/release_max_level_info", ] +build-fuzzing-corpus = [ + "neqo-common/build-fuzzing-corpus", + "neqo-transport/disable-encryption", + "neqo-crypto/disable-encryption", + "neqo-crypto/disable-random", + "test-fixture/disable-random", +] disable-encryption = [ "neqo-transport/disable-encryption", "neqo-crypto/disable-encryption", diff --git a/third_party/rust/neqo-http3/src/buffered_send_stream.rs b/third_party/rust/neqo-http3/src/buffered_send_stream.rs @@ -83,6 +83,22 @@ impl BufferedStream { Ok(sent) } + /// Flush the buffer and return the stream ID and buffer if ready to send atomically. + fn prepare_atomic_send( + &mut self, + conn: &mut Connection, + now: Instant, + ) -> Res<Option<(StreamId, &mut Vec<u8>)>> { + self.send_buffer(conn, now)?; + let Self::Initialized { stream_id, buf } = self else { + return Ok(None); + }; + if !buf.is_empty() { + return Ok(None); + } + Ok(Some((*stream_id, buf))) + } + /// # Errors /// /// Returns `neqo_transport` errors. @@ -92,19 +108,42 @@ impl BufferedStream { to_send: &[u8], now: Instant, ) -> Res<bool> { - // First try to send anything that is in the buffer. - self.send_buffer(conn, now)?; - let Self::Initialized { stream_id, buf } = self else { + let Some((stream_id, _)) = self.prepare_atomic_send(conn, now)? else { return Ok(false); }; - if !buf.is_empty() { - return Ok(false); + let sent = conn.stream_send_atomic(stream_id, to_send)?; + if sent { + qlog::h3_data_moved_down(conn.qlog_mut(), stream_id, to_send.len(), now); } - let res = conn.stream_send_atomic(*stream_id, to_send)?; - if res { - qlog::h3_data_moved_down(conn.qlog_mut(), *stream_id, to_send.len(), now); + Ok(sent) + } + + /// Encode data using the provided closure and send it atomically. + /// + /// This avoids allocating a temporary encoder at the call site by reusing + /// the stream's internal buffer as scratch space. + /// + /// # Errors + /// + /// Returns `neqo_transport` errors. + pub fn send_atomic_with<F: FnOnce(&mut Encoder<&mut Vec<u8>>)>( + &mut self, + conn: &mut Connection, + f: F, + now: Instant, + ) -> Res<bool> { + let Some((stream_id, buf)) = self.prepare_atomic_send(conn, now)? else { + return Ok(false); + }; + f(&mut Encoder::new_borrowed_vec(buf)); + let len = buf.len(); + let res = conn.stream_send_atomic(stream_id, buf); + buf.clear(); + let sent = res?; + if sent { + qlog::h3_data_moved_down(conn.qlog_mut(), stream_id, len, now); } - Ok(res) + Ok(sent) } #[must_use] diff --git a/third_party/rust/neqo-http3/src/client_events.rs b/third_party/rust/neqo-http3/src/client_events.rs @@ -450,3 +450,19 @@ impl EventProvider for Http3ClientEvents { self.events.borrow_mut().pop_front() } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use neqo_common::event::Provider as _; + + use super::{Http3ClientEvent, Http3ClientEvents}; + + #[test] + fn has_events() { + let events = Http3ClientEvents::default(); + assert!(!events.has_events()); + events.insert(Http3ClientEvent::GoawayReceived); + assert!(events.has_events()); + } +} diff --git a/third_party/rust/neqo-http3/src/conn_params.rs b/third_party/rust/neqo-http3/src/conn_params.rs @@ -170,4 +170,27 @@ mod tests { .http3_datagram(true); assert!(params.get_http3_datagram()); } + + #[test] + fn max_table_size_accepts_limit() { + // QPACK spec limits table size to (1 << 30) - 1. + let limit = (1 << 30) - 1; + let params = Http3Parameters::default() + .max_table_size_encoder(limit) + .max_table_size_decoder(limit); + assert_eq!(params.get_qpack_settings().max_table_size_encoder, limit); + assert_eq!(params.get_qpack_settings().max_table_size_decoder, limit); + } + + #[test] + #[should_panic(expected = "assertion")] + fn max_table_size_encoder_rejects_above_limit() { + _ = Http3Parameters::default().max_table_size_encoder(1 << 30); + } + + #[test] + #[should_panic(expected = "assertion")] + fn max_table_size_decoder_rejects_above_limit() { + _ = Http3Parameters::default().max_table_size_decoder(1 << 30); + } } diff --git a/third_party/rust/neqo-http3/src/control_stream_local.rs b/third_party/rust/neqo-http3/src/control_stream_local.rs @@ -10,7 +10,7 @@ use std::{ time::Instant, }; -use neqo_common::{qtrace, Encoder}; +use neqo_common::qtrace; use neqo_transport::{Connection, StreamId, StreamType}; use rustc_hash::FxHashMap as HashMap; @@ -82,9 +82,10 @@ impl ControlStreamLocal { // in case multiple priority_updates were issued, ignore now irrelevant if let Some(hframe) = stream.priority_update_frame() { - let mut enc = Encoder::new(); - hframe.encode(&mut enc); - if self.stream.send_atomic(conn, enc.as_ref(), now)? { + if self + .stream + .send_atomic_with(conn, |e| hframe.encode(e), now)? + { stream.priority_update_sent()?; } else { self.outstanding_priority_update.push_front(update_id); diff --git a/third_party/rust/neqo-http3/src/features/mod.rs b/third_party/rust/neqo-http3/src/features/mod.rs @@ -100,3 +100,21 @@ pub(crate) enum ConnectType { /// Extended CONNECT see <https://www.rfc-editor.org/rfc/rfc9220>. Extended(ExtendedConnectType), } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use crate::{features::NegotiationState, settings::HSettingType}; + + #[test] + fn negotiation_state_locally_enabled() { + let disabled = NegotiationState::new(false, HSettingType::EnableWebTransport); + assert!(!disabled.locally_enabled()); + + let negotiating = NegotiationState::new(true, HSettingType::EnableWebTransport); + assert!(negotiating.locally_enabled()); + + assert!(NegotiationState::Negotiated.locally_enabled()); + assert!(NegotiationState::Failed.locally_enabled()); + } +} diff --git a/third_party/rust/neqo-http3/src/frames/hframe.rs b/third_party/rust/neqo-http3/src/frames/hframe.rs @@ -154,6 +154,9 @@ impl HFrame { } impl FrameDecoder<Self> for HFrame { + #[cfg(feature = "build-fuzzing-corpus")] + const FUZZING_CORPUS: Option<&'static str> = Some("hframe"); + fn frame_type_allowed(frame_type: HFrameType) -> Res<()> { if HFrameType::RESERVED.contains(&frame_type) { return Err(Error::HttpFrameUnexpected); diff --git a/third_party/rust/neqo-http3/src/frames/reader.rs b/third_party/rust/neqo-http3/src/frames/reader.rs @@ -18,6 +18,11 @@ use crate::{Error, RecvStream, Res}; const MAX_READ_SIZE: usize = 2048; // Given a practical MTU of 1500 bytes, this seems reasonable. pub trait FrameDecoder<T> { + /// Fuzzing corpus name for this frame type. If `Some`, decoded frames will be + /// written to the fuzzing corpus with this name. + #[cfg(feature = "build-fuzzing-corpus")] + const FUZZING_CORPUS: Option<&'static str> = None; + fn is_known_type(frame_type: HFrameType) -> bool; /// # Errors @@ -70,6 +75,7 @@ pub struct StreamReaderRecvStreamWrapper<'a> { } impl<'a> StreamReaderRecvStreamWrapper<'a> { + #[cfg_attr(fuzzing, expect(private_interfaces, reason = "OK for fuzzing."))] pub fn new(conn: &'a mut Connection, recv_stream: &'a mut Box<dyn RecvStream>) -> Self { Self { recv_stream, conn } } @@ -261,6 +267,12 @@ impl FrameReader { self.frame_len, if len > 0 { None } else { Some(&[]) }, )? { + #[cfg(feature = "build-fuzzing-corpus")] + if let Some(corpus) = T::FUZZING_CORPUS { + // Write zero-length frames to the fuzzing corpus to test parsing of frames with + // only type and length fields. + self.write_item_to_fuzzing_corpus(corpus, None); + } self.reset(); return Ok(Some(f)); } else if T::is_known_type(self.frame_type) { @@ -282,8 +294,30 @@ impl FrameReader { } fn frame_data_decoded<T: FrameDecoder<T>>(&mut self, data: &[u8]) -> Res<Option<T>> { + #[cfg(feature = "build-fuzzing-corpus")] + if let Some(corpus) = T::FUZZING_CORPUS { + self.write_item_to_fuzzing_corpus(corpus, Some(data)); + } + let res = T::decode(self.frame_type, self.frame_len, Some(data))?; self.reset(); Ok(res) } + + #[cfg(feature = "build-fuzzing-corpus")] + /// Write `HFrame` data to indicated fuzzing corpus. + /// + /// The output consists of the varint-encoded frame type and length, followed by the optional + /// payload data. + fn write_item_to_fuzzing_corpus(&self, corpus: &str, data: Option<&[u8]>) { + // We need to include the frame type and length varints before the data + // to create a complete frame that the fuzzer can process. + let mut encoder = neqo_common::Encoder::default(); + encoder.encode_varint(self.frame_type.0); + encoder.encode_varint(self.frame_len); + if let Some(d) = data { + encoder.encode(d); + } + neqo_common::write_item_to_fuzzing_corpus(corpus, encoder.as_ref()); + } } diff --git a/third_party/rust/neqo-http3/src/frames/wtframe.rs b/third_party/rust/neqo-http3/src/frames/wtframe.rs @@ -28,15 +28,24 @@ impl WebTransportFrame { const CLOSE_MAX_MESSAGE_SIZE: u64 = 1024; pub fn encode(&self, enc: &mut Encoder) { + #[cfg(feature = "build-fuzzing-corpus")] + let start = enc.len(); + enc.encode_varint(Self::CLOSE_SESSION); let Self::CloseSession { error, message } = &self; enc.encode_varint(4 + message.len() as u64); enc.encode_uint(4, *error); enc.encode(message.as_bytes()); + + #[cfg(feature = "build-fuzzing-corpus")] + neqo_common::write_item_to_fuzzing_corpus("wtframe", &enc.as_ref()[start..]); } } impl FrameDecoder<Self> for WebTransportFrame { + #[cfg(feature = "build-fuzzing-corpus")] + const FUZZING_CORPUS: Option<&'static str> = Some("wtframe"); + fn decode(frame_type: HFrameType, frame_len: u64, data: Option<&[u8]>) -> Res<Option<Self>> { if let Some(payload) = data { let mut dec = Decoder::from(payload); @@ -61,3 +70,55 @@ impl FrameDecoder<Self> for WebTransportFrame { frame_type == HFrameType(Self::CLOSE_SESSION) } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use super::{HFrameType, WebTransportFrame}; + use crate::frames::reader::FrameDecoder as _; + + #[test] + fn is_known_type_close_session() { + assert!(WebTransportFrame::is_known_type(HFrameType( + WebTransportFrame::CLOSE_SESSION + ))); + } + + #[test] + fn is_known_type_unknown() { + assert!(!WebTransportFrame::is_known_type(HFrameType(0x1234))); + assert!(!WebTransportFrame::is_known_type(HFrameType(0))); + } + + #[test] + fn decode_close_session_too_large() { + // Message size exceeds CLOSE_MAX_MESSAGE_SIZE (1024) + 4 bytes for error code. + let large_message = vec![0u8; 1025]; + let mut payload = vec![0, 0, 0, 0]; // 4-byte error code + payload.extend(&large_message); + let frame_len = payload.len() as u64; + + let result = WebTransportFrame::decode( + HFrameType(WebTransportFrame::CLOSE_SESSION), + frame_len, + Some(&payload), + ); + assert!(result.is_err()); + } + + #[test] + fn decode_close_session_at_limit() { + // Message size exactly at CLOSE_MAX_MESSAGE_SIZE (1024). + let message = vec![b'a'; 1024]; + let mut payload = vec![0, 0, 0, 0]; // 4-byte error code + payload.extend(&message); + let frame_len = payload.len() as u64; + + let result = WebTransportFrame::decode( + HFrameType(WebTransportFrame::CLOSE_SESSION), + frame_len, + Some(&payload), + ); + assert!(result.is_ok()); + } +} diff --git a/third_party/rust/neqo-http3/src/headers_checks.rs b/third_party/rust/neqo-http3/src/headers_checks.rs @@ -209,17 +209,9 @@ mod tests { #[test] fn invalid_scheme_webtransport_connect() { - assert!(headers_valid( - &[ - Header::new(":method", "CONNECT"), - Header::new(":protocol", "webtransport"), - Header::new(":authority", "something.com"), - Header::new(":scheme", "http"), - Header::new(":path", "/here"), - ], - MessageType::Request - ) - .is_err()); + let mut headers = create_connect_headers(); + headers[2] = Header::new(":scheme", "http"); + assert!(headers_valid(&headers, MessageType::Request).is_err()); } #[test] @@ -255,4 +247,71 @@ mod tests { let headers = vec![Header::new(":status", "not-a-number")]; assert!(is_interim(&headers).is_err()); } + + #[test] + fn protocol_requires_connect_method() { + // :protocol is only valid with CONNECT method. + let mut headers = create_connect_headers(); + headers[0] = Header::new(":method", "GET"); + assert!(headers_valid(&headers, MessageType::Request).is_err()); + } + + #[test] + fn classic_connect_valid() { + // Classic CONNECT only requires :method and :authority. + let headers = vec![ + Header::new(":method", "CONNECT"), + Header::new(":authority", "proxy.example.com:443"), + ]; + assert!(headers_valid(&headers, MessageType::Request).is_ok()); + } + + #[test] + fn response_requires_status() { + let headers = vec![Header::new(":status", "200")]; + assert!(headers_valid(&headers, MessageType::Response).is_ok()); + } + + #[test] + fn response_missing_status() { + let headers: Vec<Header> = vec![]; + assert!(headers_valid(&headers, MessageType::Response).is_err()); + } + + #[test] + fn regular_request_valid() { + let headers = vec![ + Header::new(":method", "GET"), + Header::new(":scheme", "https"), + Header::new(":path", "/index.html"), + ]; + assert!(headers_valid(&headers, MessageType::Request).is_ok()); + } + + #[test] + fn regular_request_missing_method() { + let headers = vec![ + Header::new(":scheme", "https"), + Header::new(":path", "/index.html"), + ]; + assert!(headers_valid(&headers, MessageType::Request).is_err()); + } + + #[test] + fn regular_request_missing_scheme() { + let headers = vec![ + Header::new(":method", "GET"), + Header::new(":path", "/index.html"), + ]; + assert!(headers_valid(&headers, MessageType::Request).is_err()); + } + + #[test] + fn regular_request_missing_path() { + let headers = vec![ + Header::new(":method", "GET"), + Header::new(":scheme", "https"), + ]; + assert!(headers_valid(&headers, MessageType::Request).is_err()); + } } diff --git a/third_party/rust/neqo-http3/src/lib.rs b/third_party/rust/neqo-http3/src/lib.rs @@ -142,6 +142,9 @@ mod connection_server; mod control_stream_local; mod control_stream_remote; pub mod features; +#[cfg(fuzzing)] +pub mod frames; +#[cfg(not(fuzzing))] mod frames; mod headers_checks; mod priority; @@ -179,6 +182,8 @@ pub use server_events::{ ConnectUdpRequest, ConnectUdpServerEvent, Http3OrWebTransportStream, Http3ServerEvent, WebTransportRequest, WebTransportServerEvent, }; +#[cfg(fuzzing)] +pub use settings::HSettings; use stream_type_reader::NewStreamType; use thiserror::Error; @@ -649,3 +654,87 @@ impl CloseType { matches!(self, Self::ResetApp(_)) } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use neqo_transport::StreamId; + + use super::{Error, Http3StreamInfo, Http3StreamType}; + + #[test] + fn stream_info_is_http() { + let http = Http3StreamInfo::new(StreamId::new(0), Http3StreamType::Http); + assert!(http.is_http()); + + let control = Http3StreamInfo::new(StreamId::new(2), Http3StreamType::Control); + assert!(!control.is_http()); + + let wt = Http3StreamInfo::new( + StreamId::new(4), + Http3StreamType::WebTransport(StreamId::new(0)), + ); + assert!(!wt.is_http()); + } + + #[test] + fn error_codes() { + for (error, expected) in [ + (Error::HttpNone, 0x100), + (Error::HttpGeneralProtocol, 0x101), + (Error::HttpGeneralProtocolStream, 0x101), + (Error::InvalidHeader, 0x101), + (Error::HttpInternal(0), 0x102), + (Error::HttpStreamCreation, 0x103), + (Error::HttpClosedCriticalStream, 0x104), + (Error::HttpFrameUnexpected, 0x105), + (Error::HttpFrame, 0x106), + (Error::HttpExcessiveLoad, 0x107), + (Error::HttpId, 0x108), + (Error::HttpSettings, 0x109), + (Error::HttpMissingSettings, 0x10a), + (Error::HttpRequestRejected, 0x10b), + (Error::HttpRequestCancelled, 0x10c), + (Error::HttpRequestIncomplete, 0x10d), + (Error::HttpMessage, 0x10e), + (Error::HttpConnect, 0x10f), + (Error::HttpVersionFallback, 0x110), + ] { + assert_eq!(error.code(), expected); + } + } + + #[test] + fn error_mapping() { + use neqo_transport::Error as Te; + use Error::{ + InvalidInput, StreamLimit, Transport, TransportStreamDoesNotExist, Unavailable, + }; + + assert!(matches!( + Error::map_stream_send_errors(&Transport(Te::InvalidStreamId)), + TransportStreamDoesNotExist + )); + assert!(matches!( + Error::map_stream_send_errors(&Transport(Te::FinalSize)), + TransportStreamDoesNotExist + )); + assert!(matches!( + Error::map_stream_send_errors(&Transport(Te::InvalidInput)), + InvalidInput + )); + assert!(matches!( + Error::map_stream_create_errors(&Te::ConnectionState), + Unavailable + )); + assert!(matches!( + Error::map_stream_create_errors(&Te::StreamLimit), + StreamLimit + )); + // Note: map_stream_recv_errors with NoMoreData has debug_assert, skip in debug builds. + assert!(matches!( + Error::map_stream_recv_errors(&Transport(Te::InvalidStreamId)), + TransportStreamDoesNotExist + )); + } +} diff --git a/third_party/rust/neqo-http3/src/priority.rs b/third_party/rust/neqo-http3/src/priority.rs @@ -33,10 +33,13 @@ impl Priority { #[must_use] pub fn new(urgency: u8, incremental: bool) -> Self { assert!(urgency < 8); - Self { + let priority = Self { urgency, incremental, - } + }; + #[cfg(feature = "build-fuzzing-corpus")] + neqo_common::write_item_to_fuzzing_corpus("priority", priority.to_string().as_bytes()); + priority } /// Constructs a priority from raw bytes (either a field value of frame content). @@ -45,6 +48,9 @@ impl Priority { /// /// When the contained syntax is invalid. pub fn from_bytes(bytes: &[u8]) -> Res<Self> { + #[cfg(feature = "build-fuzzing-corpus")] + neqo_common::write_item_to_fuzzing_corpus("priority", bytes); + let dict: Dictionary = Parser::new(bytes).parse().map_err(|_| Error::HttpFrame)?; let urgency = match dict.get("u") { Some(ListEntry::Item(Item { @@ -201,4 +207,24 @@ mod test { }; assert_eq!(p.maybe_encode_frame(StreamId::new(4)), Some(expected)); } + + #[test] + fn priority_update_sent_clears_pending() { + let mut p = PriorityHandler::new(false, Priority::new(5, false)); + assert!(p.maybe_update_priority(Priority::new(6, false))); + assert!(p.maybe_encode_frame(StreamId::new(4)).is_some()); + p.priority_update_sent(); + // After sending, no more pending update. + assert!(p.maybe_encode_frame(StreamId::new(4)).is_none()); + } + + #[test] + fn from_bytes_invalid_urgency_defaults() { + // Urgency outside 0-7 should default to 3. + let p = Priority::from_bytes(b"u=8").unwrap(); + assert_eq!(p, Priority::default()); + + let p = Priority::from_bytes(b"u=-1").unwrap(); + assert_eq!(p, Priority::default()); + } } diff --git a/third_party/rust/neqo-http3/src/push_controller.rs b/third_party/rust/neqo-http3/src/push_controller.rs @@ -503,3 +503,19 @@ impl HttpRecvStreamEvents for RecvPushEvents { ); } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use super::{Http3ClientEvents, PushController}; + + #[test] + fn can_receive_push() { + let events = Http3ClientEvents::default(); + let disabled = PushController::new(0, events.clone()); + assert!(!disabled.can_receive_push()); + + let enabled = PushController::new(1, events); + assert!(enabled.can_receive_push()); + } +} diff --git a/third_party/rust/neqo-http3/src/qlog.rs b/third_party/rust/neqo-http3/src/qlog.rs @@ -12,8 +12,8 @@ use neqo_common::qlog::Qlog; use neqo_transport::StreamId; use qlog::events::{DataRecipient, EventData}; -pub fn h3_data_moved_up(qlog: &Qlog, stream_id: StreamId, amount: usize, now: Instant) { - qlog.add_event_data_with_instant( +pub fn h3_data_moved_up(qlog: &mut Qlog, stream_id: StreamId, amount: usize, now: Instant) { + qlog.add_event_at( || { let ev_data = EventData::DataMoved(qlog::events::quic::DataMoved { stream_id: Some(stream_id.as_u64()), @@ -30,8 +30,8 @@ pub fn h3_data_moved_up(qlog: &Qlog, stream_id: StreamId, amount: usize, now: In ); } -pub fn h3_data_moved_down(qlog: &Qlog, stream_id: StreamId, amount: usize, now: Instant) { - qlog.add_event_data_with_instant( +pub fn h3_data_moved_down(qlog: &mut Qlog, stream_id: StreamId, amount: usize, now: Instant) { + qlog.add_event_at( || { let ev_data = EventData::DataMoved(qlog::events::quic::DataMoved { stream_id: Some(stream_id.as_u64()), diff --git a/third_party/rust/neqo-http3/src/send_message.rs b/third_party/rust/neqo-http3/src/send_message.rs @@ -209,11 +209,9 @@ impl SendStream for SendMessage { let data_frame = HFrame::Data { len: to_send as u64, }; - let mut enc = Encoder::default(); - data_frame.encode(&mut enc); let sent_fh = self .stream - .send_atomic(conn, enc.as_ref(), now) + .send_atomic_with(conn, |e| data_frame.encode(e), now) .map_err(|e| Error::map_stream_send_errors(&e))?; debug_assert!(sent_fh); diff --git a/third_party/rust/neqo-http3/src/server_connection_events.rs b/third_party/rust/neqo-http3/src/server_connection_events.rs @@ -279,3 +279,18 @@ impl Http3ServerConnEvents { }); } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod tests { + use super::Http3ServerConnEvents; + use crate::connection::Http3State; + + #[test] + fn has_events() { + let events = Http3ServerConnEvents::default(); + assert!(!events.has_events()); + events.connection_state_change(Http3State::Connected); + assert!(events.has_events()); + } +} diff --git a/third_party/rust/neqo-http3/src/settings.rs b/third_party/rust/neqo-http3/src/settings.rs @@ -95,6 +95,9 @@ impl HSettings { pub fn encode_frame_contents<B: Buffer>(&self, enc: &mut Encoder<B>) { enc.encode_vvec_with(|enc_inner| { + #[cfg(feature = "build-fuzzing-corpus")] + let start = enc_inner.len(); + for iter in &self.settings { match iter.setting_type { HSettingType::MaxHeaderListSize => { @@ -129,6 +132,9 @@ impl HSettings { } } } + + #[cfg(feature = "build-fuzzing-corpus")] + neqo_common::write_item_to_fuzzing_corpus("hsettings", &enc_inner.as_ref()[start..]); }); } @@ -136,6 +142,9 @@ impl HSettings { /// /// Returns an error if settings types are reserved of settings value are not permitted. pub fn decode_frame_contents(&mut self, dec: &mut Decoder) -> Res<()> { + #[cfg(feature = "build-fuzzing-corpus")] + neqo_common::write_item_to_fuzzing_corpus("hsettings", dec.as_ref()); + while dec.remaining() > 0 { let t = dec.decode_varint(); let v = dec.decode_varint(); @@ -384,4 +393,88 @@ mod tests { Err(Error::NotEnoughData) ); } + + #[test] + fn datagram_settings() { + for setting in [SETTINGS_H3_DATAGRAM, SETTINGS_H3_DATAGRAM_DRAFT04] { + // Valid value accepted. + let mut enc = Encoder::new(); + enc.encode_varint(setting).encode_varint(1u64); + let mut s = HSettings::new(&[]); + s.decode_frame_contents(&mut enc.as_decoder()).unwrap(); + assert_eq!(s.get(HSettingType::EnableH3Datagram), 1); + + // Invalid value rejected. + enc = Encoder::new(); + enc.encode_varint(setting).encode_varint(2u64); + let mut s = HSettings::new(&[]); + assert_eq!( + s.decode_frame_contents(&mut enc.as_decoder()), + Err(Error::HttpSettings) + ); + } + + // Duplicate: first wins. + for (first, second, expected) in [ + (SETTINGS_H3_DATAGRAM, SETTINGS_H3_DATAGRAM_DRAFT04, 1), + (SETTINGS_H3_DATAGRAM_DRAFT04, SETTINGS_H3_DATAGRAM, 0), + ] { + let mut enc = Encoder::new(); + enc.encode_varint(first).encode_varint(expected); + enc.encode_varint(second).encode_varint(1 - expected); + let mut s = HSettings::new(&[]); + s.decode_frame_contents(&mut enc.as_decoder()).unwrap(); + assert_eq!(s.get(HSettingType::EnableH3Datagram), expected); + } + } + + fn make_0rtt_token(settings: &[(u64, u64)]) -> Vec<u8> { + let mut enc = Encoder::new(); + enc.encode_varint(SETTINGS_ZERO_RTT_VERSION); + for (k, v) in settings { + enc.encode_varint(*k).encode_varint(*v); + } + enc.into() + } + + #[test] + fn zero_rtt_checker() { + use neqo_crypto::{ZeroRttCheckResult, ZeroRttChecker as _}; + use neqo_transport::ConnectionParameters; + + use crate::Http3Parameters; + + // Server with datagram enabled, connect disabled. + let params = Http3Parameters::default() + .connection_parameters(ConnectionParameters::default().datagram_size(1200)); + let checker = HttpZeroRttChecker::new(params); + + // Token requests datagram=1: accepted (server has it). + let token = make_0rtt_token(&[(SETTINGS_H3_DATAGRAM, 1)]); + assert_eq!(checker.check(&token), ZeroRttCheckResult::Accept); + + // Token requests datagram=0: accepted (not requesting feature). + let token = make_0rtt_token(&[(SETTINGS_H3_DATAGRAM, 0)]); + assert_eq!(checker.check(&token), ZeroRttCheckResult::Accept); + + // Token with invalid datagram value (>1): fails decode. + let token = make_0rtt_token(&[(SETTINGS_H3_DATAGRAM, 2)]); + assert_eq!(checker.check(&token), ZeroRttCheckResult::Fail); + + // Token requests connect=1 but server doesn't have it: rejected. + let token = make_0rtt_token(&[(SETTINGS_ENABLE_CONNECT_PROTOCOL, 1)]); + assert_eq!(checker.check(&token), ZeroRttCheckResult::Reject); + + // Server with connect enabled. + let params = Http3Parameters::default().connect(true); + let checker = HttpZeroRttChecker::new(params); + let token = make_0rtt_token(&[(SETTINGS_ENABLE_CONNECT_PROTOCOL, 1)]); + assert_eq!(checker.check(&token), ZeroRttCheckResult::Accept); + + // Invalid token (truncated): rejected (remaining bytes interpreted as wrong version). + assert_eq!(checker.check(&token[1..]), ZeroRttCheckResult::Reject); + + // Empty token: fails. + assert_eq!(checker.check(&[]), ZeroRttCheckResult::Fail); + } } diff --git a/third_party/rust/neqo-qpack/.cargo-checksum.json b/third_party/rust/neqo-qpack/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"86f4b7907e79a72f73653ea509f3c49b6c286e3a3dd1f3c9926be33d783abf7c","src/decoder.rs":"74fe07840aec0f1aaa42311bb5168d16806674aebc77df41ace08bf4b7d88eb7","src/decoder_instructions.rs":"6b36eea01fdf92088ddac6b6988a239c28ddeb3cc7ecb16abf302f5d1ca8191a","src/encoder.rs":"10295dd855d18a18cbd7fe7d160337ce984d923be0976e456244b48a4379e14e","src/encoder_instructions.rs":"1cf1ba5ab2bbfc8f77ecfbc2bc59e40f77e12f85af5c10d0db2652000a8ff102","src/fuzz.rs":"c9ee149dab0b30a2850f530096274e05475920b43fbff1104def7e789f3e5c6c","src/header_block.rs":"6ed65adefdd251178bdfac0601db4ee8bbd00221de89947e52dc9335c0ac149b","src/huffman.rs":"c3740084c71580a5270c73cae4b7c5035fae913f533474f4a9cbc39b1f29adb7","src/huffman_decode_helper.rs":"3b983bafc69f58810ae93890f101f82148d5d6dbbce8bc232f58bcb50f3946a1","src/huffman_table.rs":"aaa9ee17b8bceb47877d41fdf12fd29d49662a12db183acdb6b06c6e2ad182d9","src/lib.rs":"2177e51a81c85746dd7e3a626c8fc912bd554b5d7fac3a3086e3b480fe6c3a8b","src/prefix.rs":"31bfb11d334a6df619bcc2720621e44a656be2514fad9033531a712d47dbe672","src/qlog.rs":"1ca9bdbc974024b32515af6b6529f5a69e80eae3f7d74445af304dc341a0eda1","src/qpack_send_buf.rs":"cec9b34cc0f2cd3a38eb15111c5f0418e31875d3ee20ecc1ed14f076da80979d","src/reader.rs":"146c4d1963390ffad2491fb74d9c588e316893c3062d9a7663d40233d800b937","src/static_table.rs":"6e5ec26e2b6bd63375d2d77e72748151d430d1629a8e497ec0d0ea21c078524a","src/stats.rs":"cb01723249f60e15a5cd7efd9cbab409fddc588d1df655ed06ba8c80e3d5d28e","src/table.rs":"7f4c59650f262cbae6e776beb3be2f2c3a52aa22f7a912e6e1df2af3188e3f13"},"package":null} -\ No newline at end of file +{"files":{"Cargo.toml":"78484e0ae82fe6f91877266edad1caf9b27a41f0208bf49f3f48f51a0cdecc63","src/decoder.rs":"355af4550ac0e61aa6e8d2fd72d39599e0c064669bc6114593dea00728c81b4a","src/decoder_instructions.rs":"ea1696ad8eb9f0bd3e990318bfd4b5e07ebe38738736c7e8937bdb15580e96d2","src/encoder.rs":"243c1765a680b925266602a0e83f2200ee9d0c3b7a3c9d7f69e3271c44a32121","src/encoder_instructions.rs":"f475638d34b775df1f4afea59aad8f447fd6b79674c58fba3d5f3d7197b7ee6a","src/fuzz.rs":"c9ee149dab0b30a2850f530096274e05475920b43fbff1104def7e789f3e5c6c","src/header_block.rs":"26829419ddac89dc8e6baf4fd1c3fe542a63132297be078d501578c89c4827f9","src/huffman.rs":"c3740084c71580a5270c73cae4b7c5035fae913f533474f4a9cbc39b1f29adb7","src/huffman_decode_helper.rs":"3b983bafc69f58810ae93890f101f82148d5d6dbbce8bc232f58bcb50f3946a1","src/huffman_table.rs":"aaa9ee17b8bceb47877d41fdf12fd29d49662a12db183acdb6b06c6e2ad182d9","src/lib.rs":"2177e51a81c85746dd7e3a626c8fc912bd554b5d7fac3a3086e3b480fe6c3a8b","src/prefix.rs":"31bfb11d334a6df619bcc2720621e44a656be2514fad9033531a712d47dbe672","src/qlog.rs":"6a5c2e6b4e040f9cf58f1b6e2fc0548176e3944a0b13d47dbf319128dd17af60","src/qpack_send_buf.rs":"2b402929e2f8de32b9651c2df95d0cbfa2cc24e38aad67fc62a3f06b890ca2d3","src/reader.rs":"16b2b71677b2b680a2c3d2605dd711a71eaead8d1f324efbd523cbfe6dfdb7bf","src/static_table.rs":"6e5ec26e2b6bd63375d2d77e72748151d430d1629a8e497ec0d0ea21c078524a","src/stats.rs":"cb01723249f60e15a5cd7efd9cbab409fddc588d1df655ed06ba8c80e3d5d28e","src/table.rs":"fac72adde5d1f7866f18ca4c9813f3e1f5c4cf82fff701c0b7552cbb03e89190"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/neqo-qpack/Cargo.toml b/third_party/rust/neqo-qpack/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.81.0" name = "neqo-qpack" -version = "0.20.0" +version = "0.21.0" authors = ["The Neqo Authors <necko@mozilla.com>"] build = false autolib = false diff --git a/third_party/rust/neqo-qpack/src/decoder.rs b/third_party/rust/neqo-qpack/src/decoder.rs @@ -6,14 +6,13 @@ use std::fmt::{self, Display, Formatter}; -use neqo_common::{qdebug, Header}; +use neqo_common::{qdebug, Encoder, Header}; use neqo_transport::{Connection, StreamId}; use crate::{ decoder_instructions::DecoderInstruction, encoder_instructions::{DecodedEncoderInstruction, EncoderInstructionReader}, header_block::{HeaderDecoder, HeaderDecoderResult}, - qpack_send_buf::Data, reader::{ReadByte, Reader, ReceiverConnWrapper}, stats::Stats, table::HeaderTable, @@ -28,7 +27,7 @@ pub struct Decoder { table: HeaderTable, acked_inserts: u64, max_entries: u64, - send_buf: Data, + send_buf: Encoder, local_stream_id: Option<StreamId>, max_table_size: u64, max_blocked_streams: usize, @@ -43,7 +42,7 @@ impl Decoder { #[must_use] pub fn new(qpack_settings: &Settings) -> Self { qdebug!("Decoder: creating a new qpack decoder"); - let mut send_buf = Data::default(); + let mut send_buf = Encoder::default(); send_buf.encode_varint(QPACK_UNI_STREAM_TYPE_DECODER); let max_blocked_streams = usize::from(qpack_settings.max_blocked_streams); Self { @@ -179,11 +178,11 @@ impl Decoder { let r = conn .stream_send( self.local_stream_id.ok_or(Error::Internal)?, - &self.send_buf[..], + self.send_buf.as_ref(), ) .map_err(|_| Error::DecoderStream)?; qdebug!("[{self}] {r} bytes sent"); - self.send_buf.read(r); + self.send_buf.skip(r); } Ok(()) } @@ -221,17 +220,13 @@ impl Decoder { if self.blocked_streams.len() > self.max_blocked_streams { Err(Error::Decompression) } else { - let r = self + let found = self .blocked_streams .iter() - .filter_map(|(id, req)| (*id == stream_id).then_some(*req)) - .collect::<Vec<_>>(); - if !r.is_empty() { - debug_assert!(r.len() == 1); - debug_assert!(r[0] == req_insert_cnt); - return Ok(None); + .any(|(id, _req)| *id == stream_id); + if !found { + self.blocked_streams.push((stream_id, req_insert_cnt)); } - self.blocked_streams.push((stream_id, req_insert_cnt)); Ok(None) } } diff --git a/third_party/rust/neqo-qpack/src/decoder_instructions.rs b/third_party/rust/neqo-qpack/src/decoder_instructions.rs @@ -14,7 +14,7 @@ use neqo_transport::StreamId; use crate::{ prefix::{DECODER_HEADER_ACK, DECODER_INSERT_COUNT_INCREMENT, DECODER_STREAM_CANCELLATION}, - qpack_send_buf::Data, + qpack_send_buf::Encoder, reader::{IntReader, ReadByte}, Res, }; @@ -44,7 +44,7 @@ impl DecoderInstruction { } } - pub(crate) fn marshal(&self, enc: &mut Data) { + pub(crate) fn marshal<T: Encoder>(&self, enc: &mut T) { match self { Self::InsertCountIncrement { increment } => { enc.encode_prefixed_encoded_int(DECODER_INSERT_COUNT_INCREMENT, *increment); @@ -144,16 +144,17 @@ impl DecoderInstructionReader { #[cfg_attr(coverage_nightly, coverage(off))] mod test { + use neqo_common::Encoder; use neqo_transport::StreamId; - use super::{Data, DecoderInstruction, DecoderInstructionReader}; + use super::{DecoderInstruction, DecoderInstructionReader}; use crate::{reader::test_receiver::TestReceiver, Error}; fn test_encoding_decoding(instruction: DecoderInstruction) { - let mut buf = Data::default(); + let mut buf = Encoder::default(); instruction.marshal(&mut buf); let mut test_receiver: TestReceiver = TestReceiver::default(); - test_receiver.write(&buf); + test_receiver.write(buf.as_ref()); let mut decoder = DecoderInstructionReader::new(); assert_eq!( decoder.read_instructions(&mut test_receiver).unwrap(), @@ -182,18 +183,18 @@ mod test { } fn test_encoding_decoding_slow_reader(instruction: DecoderInstruction) { - let mut buf = Data::default(); + let mut buf = Encoder::default(); instruction.marshal(&mut buf); let mut test_receiver: TestReceiver = TestReceiver::default(); let mut decoder = DecoderInstructionReader::new(); for i in 0..buf.len() - 1 { - test_receiver.write(&buf[i..=i]); + test_receiver.write(&buf.as_ref()[i..=i]); assert_eq!( decoder.read_instructions(&mut test_receiver), Err(Error::NeedMoreData) ); } - test_receiver.write(&buf[buf.len() - 1..buf.len()]); + test_receiver.write(&buf.as_ref()[buf.len() - 1..buf.len()]); assert_eq!( decoder.read_instructions(&mut test_receiver).unwrap(), instruction diff --git a/third_party/rust/neqo-qpack/src/encoder.rs b/third_party/rust/neqo-qpack/src/encoder.rs @@ -20,7 +20,6 @@ use crate::{ encoder_instructions::EncoderInstruction, header_block::HeaderEncoder, qlog, - qpack_send_buf::Data, reader::ReceiverConnWrapper, stats::Stats, table::{HeaderTable, LookupResult, ADDITIONAL_TABLE_ENTRY_SIZE}, @@ -225,7 +224,7 @@ impl Encoder { fn call_instruction( &mut self, instruction: DecoderInstruction, - qlog: &Qlog, + qlog: &mut Qlog, now: Instant, ) -> Res<()> { qdebug!("[{self}] call instruction {instruction:?}"); @@ -281,14 +280,14 @@ impl Encoder { return Err(Error::DynamicTableFull); } - let mut buf = Data::default(); + let mut buf = neqo_common::Encoder::default(); EncoderInstruction::InsertWithNameLiteral { name, value } .marshal(&mut buf, self.use_huffman); let stream_id = self.local_stream.stream_id().ok_or(Error::Internal)?; let sent = conn - .stream_send_atomic(stream_id, &buf) + .stream_send_atomic(stream_id, buf.as_ref()) .map_err(|e| map_stream_send_atomic_error(&e))?; if !sent { return Err(Error::EncoderStreamBlocked); @@ -321,9 +320,9 @@ impl Encoder { if cap < self.table.capacity() && !self.table.can_evict_to(cap) { return Err(Error::DynamicTableFull); } - let mut buf = Data::default(); + let mut buf = neqo_common::Encoder::default(); EncoderInstruction::Capacity { value: cap }.marshal(&mut buf, self.use_huffman); - if !conn.stream_send_atomic(stream_id, &buf)? { + if !conn.stream_send_atomic(stream_id, buf.as_ref())? { return Err(Error::EncoderStreamBlocked); } if self.table.set_capacity(cap).is_err() { @@ -351,9 +350,9 @@ impl Encoder { Ok(()) } LocalStreamState::Uninitialized(stream_id) => { - let mut buf = Data::default(); + let mut buf = neqo_common::Encoder::default(); buf.encode_varint(QPACK_UNI_STREAM_TYPE_ENCODER); - if !conn.stream_send_atomic(stream_id, &buf[..])? { + if !conn.stream_send_atomic(stream_id, buf.as_ref())? { return Err(Error::EncoderStreamBlocked); } self.local_stream = LocalStreamState::Initialized(stream_id); @@ -596,7 +595,7 @@ mod tests { let buf = self .encoder .encode_header_block(&mut self.conn, headers, stream_id); - assert_eq!(&buf[..], expected_encoding); + assert_eq!(buf.as_ref(), expected_encoding); self.send_instructions(inst); } @@ -822,7 +821,7 @@ mod tests { let buf = encoder .encoder .encode_header_block(&mut encoder.conn, &t.headers, STREAM_1); - assert_eq!(&buf[..], t.header_block); + assert_eq!(buf.as_ref(), t.header_block); encoder.send_instructions(t.encoder_inst); } } @@ -896,7 +895,7 @@ mod tests { let buf = encoder .encoder .encode_header_block(&mut encoder.conn, &t.headers, STREAM_1); - assert_eq!(&buf[..], t.header_block); + assert_eq!(buf.as_ref(), t.header_block); encoder.send_instructions(t.encoder_inst); } } @@ -968,7 +967,7 @@ mod tests { &[Header::new("content-length", "1234")], STREAM_1, ); - assert_eq!(&buf[..], ENCODE_INDEXED_REF_DYNAMIC); + assert_eq!(buf.as_ref(), ENCODE_INDEXED_REF_DYNAMIC); encoder.send_instructions(&[]); // insert "content-length: 12345 which will fail because the entry in the table cannot be diff --git a/third_party/rust/neqo-qpack/src/encoder_instructions.rs b/third_party/rust/neqo-qpack/src/encoder_instructions.rs @@ -16,7 +16,7 @@ use crate::{ ENCODER_CAPACITY, ENCODER_DUPLICATE, ENCODER_INSERT_WITH_NAME_LITERAL, ENCODER_INSERT_WITH_NAME_REF_DYNAMIC, ENCODER_INSERT_WITH_NAME_REF_STATIC, NO_PREFIX, }, - qpack_send_buf::Data, + qpack_send_buf::Encoder, reader::{IntReader, LiteralReader, ReadByte, Reader}, Res, }; @@ -52,7 +52,7 @@ pub enum EncoderInstruction<'a> { } impl EncoderInstruction<'_> { - pub(crate) fn marshal(&self, enc: &mut Data, use_huffman: bool) { + pub(crate) fn marshal<T: Encoder>(&self, enc: &mut T, use_huffman: bool) { match self { Self::Capacity { value } => { enc.encode_prefixed_encoded_int(ENCODER_CAPACITY, *value); @@ -297,14 +297,14 @@ impl EncoderInstructionReader { #[cfg_attr(coverage_nightly, coverage(off))] mod test { - use super::{Data, EncoderInstruction, EncoderInstructionReader}; + use super::{EncoderInstruction, EncoderInstructionReader}; use crate::{reader::test_receiver::TestReceiver, Error}; fn test_encoding_decoding(instruction: &EncoderInstruction, use_huffman: bool) { - let mut buf = Data::default(); + let mut buf = neqo_common::Encoder::default(); instruction.marshal(&mut buf, use_huffman); let mut test_receiver: TestReceiver = TestReceiver::default(); - test_receiver.write(&buf); + test_receiver.write(buf.as_ref()); let mut reader = EncoderInstructionReader::new(); assert_eq!( reader.read_instructions(&mut test_receiver).unwrap(), @@ -395,18 +395,18 @@ mod test { } fn test_encoding_decoding_slow_reader(instruction: &EncoderInstruction, use_huffman: bool) { - let mut buf = Data::default(); + let mut buf = neqo_common::Encoder::default(); instruction.marshal(&mut buf, use_huffman); let mut test_receiver: TestReceiver = TestReceiver::default(); let mut decoder = EncoderInstructionReader::new(); for i in 0..buf.len() - 1 { - test_receiver.write(&buf[i..=i]); + test_receiver.write(&buf.as_ref()[i..=i]); assert_eq!( decoder.read_instructions(&mut test_receiver), Err(Error::NeedMoreData) ); } - test_receiver.write(&buf[buf.len() - 1..buf.len()]); + test_receiver.write(&buf.as_ref()[buf.len() - 1..buf.len()]); assert_eq!( decoder.read_instructions(&mut test_receiver).unwrap(), instruction.into() diff --git a/third_party/rust/neqo-qpack/src/header_block.rs b/third_party/rust/neqo-qpack/src/header_block.rs @@ -20,7 +20,7 @@ use crate::{ HEADER_FIELD_LITERAL_NAME_REF_DYNAMIC_POST, HEADER_FIELD_LITERAL_NAME_REF_STATIC, NO_PREFIX, }, - qpack_send_buf::Data, + qpack_send_buf::Encoder as _, reader::{parse_utf8, ReceiverBufferWrapper}, table::HeaderTable, Error, Res, @@ -28,7 +28,7 @@ use crate::{ #[derive(Default, Debug, PartialEq, Eq)] pub struct HeaderEncoder { - buf: Data, + buf: neqo_common::Encoder, base: u64, use_huffman: bool, max_entries: u64, @@ -44,7 +44,7 @@ impl Display for HeaderEncoder { impl HeaderEncoder { pub fn new(base: u64, use_huffman: bool, max_entries: u64) -> Self { Self { - buf: Data::default(), + buf: neqo_common::Encoder::default(), base, use_huffman, max_entries, @@ -139,14 +139,14 @@ impl HeaderEncoder { .encode_prefixed_encoded_int(NO_PREFIX, enc_insert_cnt); self.buf.encode_prefixed_encoded_int(prefix, delta); - self.buf.write_bytes(&tmp); + self.buf.encode(tmp); } } impl Deref for HeaderEncoder { type Target = [u8]; fn deref(&self) -> &Self::Target { - &self.buf + self.buf.as_ref() } } diff --git a/third_party/rust/neqo-qpack/src/qlog.rs b/third_party/rust/neqo-qpack/src/qlog.rs @@ -15,12 +15,12 @@ use qlog::events::{ }; pub fn qpack_read_insert_count_increment_instruction( - qlog: &Qlog, + qlog: &mut Qlog, increment: u64, data: &[u8], now: Instant, ) { - qlog.add_event_data_with_instant( + qlog.add_event_at( || { let raw = RawInfo { length: Some(8), diff --git a/third_party/rust/neqo-qpack/src/qpack_send_buf.rs b/third_party/rust/neqo-qpack/src/qpack_send_buf.rs @@ -4,27 +4,26 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::ops::Deref; - -use neqo_common::Encoder; - use crate::{huffman, prefix::Prefix}; -#[derive(Default, Debug, PartialEq, Eq)] -pub struct Data { - buf: Vec<u8>, +/// Extension trait providing QPACK-specific encoding methods for `Encoder`. +/// +/// This trait extends the standard [`neqo_common::Encoder`] with QPACK-specific +/// methods for encoding integers with prefixes and literal strings with +/// optional Huffman encoding. +pub trait Encoder { + /// Encode an integer with a QPACK prefix according to RFC 7541 Section 5.1. + fn encode_prefixed_encoded_int(&mut self, prefix: Prefix, val: u64) -> usize; + + /// Encode a literal string with optional Huffman encoding according to RFC 7541 Section 5.2. + fn encode_literal(&mut self, use_huffman: bool, prefix: Prefix, value: &[u8]); } -impl Data { - fn write_byte(&mut self, b: u8) { - self.buf.push(b); - } - - pub fn encode_varint(&mut self, i: u64) { - Encoder::new_borrowed_vec(&mut self.buf).encode_varint(i); - } - - pub(crate) fn encode_prefixed_encoded_int(&mut self, prefix: Prefix, mut val: u64) -> usize { +impl<B> Encoder for neqo_common::Encoder<B> +where + B: neqo_common::Buffer, +{ + fn encode_prefixed_encoded_int(&mut self, prefix: Prefix, mut val: u64) -> usize { let first_byte_max: u8 = if prefix.len() == 0 { 0xff } else { @@ -33,11 +32,11 @@ impl Data { if val < u64::from(first_byte_max) { let v = u8::try_from(val).expect("first_byte_max is a u8 and val is smaller"); - self.write_byte((prefix.prefix() & !first_byte_max) | v); + self.encode_byte((prefix.prefix() & !first_byte_max) | v); return 1; } - self.write_byte(prefix.prefix() | first_byte_max); + self.encode_byte(prefix.prefix() | first_byte_max); val -= u64::from(first_byte_max); let mut written = 1; @@ -51,13 +50,13 @@ impl Data { done = true; } - self.write_byte(b); + self.encode_byte(b); written += 1; } written } - pub fn encode_literal(&mut self, use_huffman: bool, prefix: Prefix, value: &[u8]) { + fn encode_literal(&mut self, use_huffman: bool, prefix: Prefix, value: &[u8]) { let real_prefix = Prefix::new( if use_huffman { prefix.prefix() | (0x80 >> prefix.len()) @@ -73,68 +72,51 @@ impl Data { real_prefix, u64::try_from(encoded.len()).expect("usize fits in u64"), ); - self.write_bytes(&encoded); + self.encode(&encoded); } else { self.encode_prefixed_encoded_int( real_prefix, u64::try_from(value.len()).expect("usize fits in u64"), ); - self.write_bytes(value); + self.encode(value); } } - - pub fn write_bytes(&mut self, buf: &[u8]) { - self.buf.extend_from_slice(buf); - } - - pub fn read(&mut self, r: usize) { - assert!( - r <= self.buf.len(), - "want to set more bytes read than remain in the buffer" - ); - self.buf = self.buf.split_off(r); - } -} - -impl Deref for Data { - type Target = [u8]; - fn deref(&self) -> &Self::Target { - &self.buf - } } #[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] mod tests { - use super::{Data, Prefix}; + use neqo_common::Encoder; + + use super::{Encoder as _, Prefix}; #[test] fn encode_prefixed_encoded_int_1() { - let mut d = Data::default(); + let mut d = Encoder::default(); d.encode_prefixed_encoded_int(Prefix::new(0xC0, 2), 5); - assert_eq!(d[..], [0xc5]); + assert_eq!(d.as_ref(), [0xc5]); } #[test] fn encode_prefixed_encoded_int_2() { - let mut d = Data::default(); + let mut d = Encoder::default(); d.encode_prefixed_encoded_int(Prefix::new(0xC0, 2), 65); - assert_eq!(d[..], [0xff, 0x02]); + assert_eq!(d.as_ref(), [0xff, 0x02]); } #[test] fn encode_prefixed_encoded_int_3() { - let mut d = Data::default(); + let mut d = Encoder::default(); d.encode_prefixed_encoded_int(Prefix::new(0xC0, 2), 100_000); - assert_eq!(d[..], [0xff, 0xe1, 0x8c, 0x06]); + assert_eq!(d.as_ref(), [0xff, 0xe1, 0x8c, 0x06]); } #[test] fn max_int() { - let mut d = Data::default(); + let mut d = Encoder::default(); d.encode_prefixed_encoded_int(Prefix::new(0x80, 1), u64::MAX); assert_eq!( - d[..], + d.as_ref(), [0xff, 0x80, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01] ); } @@ -148,15 +130,15 @@ mod tests { #[test] fn encode_literal() { - let mut d = Data::default(); + let mut d = Encoder::default(); d.encode_literal(false, Prefix::new(0xC0, 2), VALUE); - assert_eq!(&&d[..], &LITERAL); + assert_eq!(d.as_ref(), LITERAL); } #[test] fn encode_literal_huffman() { - let mut d = Data::default(); + let mut d = Encoder::default(); d.encode_literal(true, Prefix::new(0xC0, 2), VALUE); - assert_eq!(&&d[..], &LITERAL_HUFFMAN); + assert_eq!(d.as_ref(), LITERAL_HUFFMAN); } } diff --git a/third_party/rust/neqo-qpack/src/reader.rs b/third_party/rust/neqo-qpack/src/reader.rs @@ -252,6 +252,19 @@ pub struct LiteralReader { } impl LiteralReader { + /// Maximum length for a literal string in QPACK encoding. + /// + /// RFC 9204 requires implementations to set their own limits for string literal + /// lengths to prevent denial-of-service attacks. The RFC does not mandate a + /// specific value, stating only that limits "SHOULD be large enough to process + /// the largest individual field the HTTP implementation can be configured to + /// accept." + /// + /// The Gecko limit is in `network.http.max_response_header_size` and defaults to + /// 393216 bytes (384 KB), see `modules/libpref/init/StaticPrefList.yaml`. We use + /// the same limit. + const MAX_LEN: usize = 384 * 1024; + /// Creates `LiteralReader` with the first byte. This constructor is always used /// when a literal has a prefix. /// For literals without a prefix please use the default constructor. @@ -299,9 +312,11 @@ impl LiteralReader { }; } LiteralReaderState::ReadLength { reader } => { - let v = reader.read(s)?; - self.literal - .resize(v.try_into().or(Err(Error::Decoding))?, 0x0); + let v = usize::try_from(reader.read(s)?) + .ok() + .filter(|&l| l <= Self::MAX_LEN) + .ok_or(Error::Decoding)?; + self.literal.resize(v, 0x0); self.state = LiteralReaderState::ReadLiteral { offset: 0 }; } LiteralReaderState::ReadLiteral { offset } => { @@ -380,12 +395,14 @@ pub(crate) mod test_receiver { #[cfg_attr(coverage_nightly, coverage(off))] mod tests { + use neqo_common::Encoder; use test_receiver::TestReceiver; use super::{ huffman, test_receiver, Error, IntReader, LiteralReader, ReadByte as _, ReceiverBufferWrapper, Res, }; + use crate::{prefix::Prefix, qpack_send_buf::Encoder as _}; const TEST_CASES_NUMBERS: [(&[u8], u8, u64); 7] = [ (&[0xEA], 3, 10), @@ -636,4 +653,39 @@ mod tests { let result = buffer.read_literal_from_buffer(3).unwrap(); assert_eq!(result, non_utf8_data); } + + /// Create a [`LiteralReader`] and [`TestReceiver`] for a literal with the given length. + fn literal_reader_for_test(literal_len: usize) -> (LiteralReader, TestReceiver) { + const PREFIX_LEN: u8 = 3; + let mut data = Encoder::default(); + data.encode_literal( + false, + Prefix::new(0x00, PREFIX_LEN), + &vec![b'a'; literal_len], + ); + let reader = LiteralReader::new_with_first_byte(data.as_ref()[0], PREFIX_LEN); + let mut test_receiver = TestReceiver::default(); + test_receiver.write(&data.as_ref()[1..]); + (reader, test_receiver) + } + + /// Test that [`LiteralReader`] rejects literals exceeding [`MAX_LEN`]. + /// + /// This prevents denial-of-service attacks where a malicious QPACK encoder + /// sends an extremely large length value to trigger excessive memory allocation. + /// RFC 9204 requires implementations to set their own limits for string literal + /// lengths. + #[test] + fn literal_exceeding_max_len_rejected() { + let (mut reader, mut test_receiver) = literal_reader_for_test(LiteralReader::MAX_LEN + 1); + assert_eq!(reader.read(&mut test_receiver), Err(Error::Decoding)); + } + + /// Test that [`LiteralReader`] accepts literals at exactly [`MAX_LEN`]. + #[test] + fn literal_at_max_len_accepted() { + let (mut reader, mut test_receiver) = literal_reader_for_test(LiteralReader::MAX_LEN); + let result = reader.read(&mut test_receiver).unwrap(); + assert_eq!(result.len(), LiteralReader::MAX_LEN); + } } diff --git a/third_party/rust/neqo-qpack/src/table.rs b/third_party/rust/neqo-qpack/src/table.rs @@ -134,10 +134,7 @@ impl HeaderTable { /// `HeaderLookup` if the index does not exist in the static table. pub fn get_static(index: u64) -> Res<&'static StaticTableEntry> { let inx = usize::try_from(index).or(Err(Error::HeaderLookup))?; - if inx >= HEADER_STATIC_TABLE.len() { - return Err(Error::HeaderLookup); - } - Ok(&HEADER_STATIC_TABLE[inx]) + HEADER_STATIC_TABLE.get(inx).ok_or(Error::HeaderLookup) } fn get_dynamic_with_abs_index(&mut self, index: u64) -> Res<&mut DynamicTableEntry> { @@ -147,18 +144,12 @@ impl HeaderTable { } let inx = self.base - index - 1; let inx = usize::try_from(inx).or(Err(Error::HeaderLookup))?; - if inx >= self.dynamic.len() { - return Err(Error::HeaderLookup); - } - Ok(&mut self.dynamic[inx]) + self.dynamic.get_mut(inx).ok_or(Error::HeaderLookup) } fn get_dynamic_with_relative_index(&self, index: u64) -> Res<&DynamicTableEntry> { let inx = usize::try_from(index).or(Err(Error::HeaderLookup))?; - if inx >= self.dynamic.len() { - return Err(Error::HeaderLookup); - } - Ok(&self.dynamic[inx]) + self.dynamic.get(inx).ok_or(Error::HeaderLookup) } /// Get a entry in the dynamic table. @@ -254,14 +245,15 @@ impl HeaderTable { "[{self}] reduce table to {reduce}, currently used:{}", self.used, ); - while (!self.dynamic.is_empty()) && self.used > reduce { - if let Some(e) = self.dynamic.back() { - if !e.can_reduce(self.acked_inserts_cnt) { - return false; - } - self.used -= u64::try_from(e.size()).expect("usize fits in u64"); - self.dynamic.pop_back(); + while let Some(e) = self.dynamic.back() { + if self.used <= reduce { + break; + } + if !e.can_reduce(self.acked_inserts_cnt) { + return false; } + self.used -= u64::try_from(e.size()).expect("usize fits in u64"); + self.dynamic.pop_back(); } true } diff --git a/third_party/rust/neqo-transport/.cargo-checksum.json b/third_party/rust/neqo-transport/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"f1bd4eed982713c99f98f95085fc786c089119249c4c3f32d0fbbf170b7b8224","benches/min_bandwidth.rs":"f81c8bccc45cbbecb5a3c4c28b406c5c49319dee3a9caed58feb43b2a2871955","benches/range_tracker.rs":"546c4701424a8f286a459af10d93bf591b3912f1273125272b32485a27e5631d","benches/rx_stream_orderer.rs":"23c4646e91c4e20d27a81544c73a83dc02b2362bab870136e39d52a9ddced01b","benches/sent_packets.rs":"1e6cde25a8047f5c63629ae2b80950ee8b08b19d8db216a871eee04a59049f5b","benches/transfer.rs":"c5b0a538a8f9ecaf623e85e0b78b3cc4d37ecdab46c142218f068a503e0c5360","build.rs":"78ec79c93bf13c3a40ceef8bba1ea2eada61c8f2dfc15ea7bf117958d367949c","src/ackrate.rs":"08f7f4777b7ac1ec85afb4c8cd1cab99a39bd2cb023628193e99325c8c0e34e2","src/addr_valid.rs":"904f5a32f1026f6e0d684c3b3611886f60d566cd108cf3027ceb91c2d3f5b71b","src/cc/classic_cc.rs":"6ba58c166a1dca926b499920cdd67d4faa66a2f0bfe010787dd06e944415f005","src/cc/cubic.rs":"5969163642c2d7ac8593f8603fba93c80940e6eccfb2602f2912628e5c66d64c","src/cc/mod.rs":"c7e30bf6aff8cbd888e83f0c593188238dc3baec8d88d3393f068c1bcfa88c50","src/cc/new_reno.rs":"278e6caa6164e69a30e84f1eb32358d5f4fac05a05addfa129cc6006c1e856c9","src/cc/tests/cubic.rs":"53ff669eb2cc5052f2475db1c6ddc1dc5663da78b432259d191248c592b69ec3","src/cc/tests/mod.rs":"017bf402a9a8c71b5c43343677635644babb57a849d81d0affc328b4b4b9cebb","src/cc/tests/new_reno.rs":"a0aa6b8c6369f42237edbb2597bb0c68e80ae49bf282b6e06c2bb478e7ea5a0e","src/cid.rs":"e8d4437663d1a954b3af75b40ea6fa5552802556c461665647584f1bea5f8bbf","src/connection/idle.rs":"7fffb026530f17604ac41b28c71cf3b3554e6b1958db96436478d9332b65217c","src/connection/mod.rs":"4944eb8975816d984d54c9c44d8871e635eed0a8414685495105a254c782f6ac","src/connection/params.rs":"1007bf962dbae8629746c4e18e8651424b0cf41ea01c1fd46eae40bdd0d9faa2","src/connection/state.rs":"c1f18f21b8729cd523e4165bf7072f056c3b26ba8094f3c1adfcdd7beb3c0979","src/connection/test_internal.rs":"25814d143b745c50c67708cd82aeecbc1e6f123e067b69c5021b9ba653c0c6c2","src/connection/tests/ackrate.rs":"4b7325b5798817d971c14f76ca79c1d7f677aee8e579c97623f49eb87727bd6f","src/connection/tests/cc.rs":"7b113bb1713d440a03bf34bcd42b146dd201c2110140650ee1d98b136d7cec48","src/connection/tests/close.rs":"3844fb0fa626db47a1f914189923bd71737c5331e87cd0e293d10f556405c204","src/connection/tests/datagram.rs":"35c865e9bdfeef2d58f9421370447f627210825f6a3b66a44e84fbce3a22efa3","src/connection/tests/ecn.rs":"f7ac4bab2f518132e0da6e46709c21a5c643a475c49026343fc99bd9bec9b8c4","src/connection/tests/handshake.rs":"cc539ccd4ee21e717f187da9edcfd56e6e8c6f73de3c997df93cadbe844c70b4","src/connection/tests/idle.rs":"f8607ee48bd28486c83e497ce9882239cd8dcf9bdf3a5b1e475d5f50836e6223","src/connection/tests/keys.rs":"3481eb3b24fd034b2b51bfaa02ce58d1890bc10213d3f9d0effdd4d7fceb677c","src/connection/tests/migration.rs":"a97dc409c0573eab65fa9ec7b6d4843d491a2942ce2a90e65a6c6cf750b09714","src/connection/tests/mod.rs":"48c04e7cfe70114f014ac4599a4a4ed5c059a9d56e742a1e975676863e34114a","src/connection/tests/null.rs":"d39d34c895c40ea88bcc137cba43c34386ef9759c6f66f3487ffd41a5099feb8","src/connection/tests/pmtud.rs":"39e9aa38c4d5ecfe8d3040ce268ce33fa63ad87a774f7cad45f5dcc6e43aaf82","src/connection/tests/priority.rs":"00fb90881299fb93b9ad628840585852422676a6db0dbeee348a51ea3029456d","src/connection/tests/recovery.rs":"b527fceb06ac306ac7504c494a831bf2997148a2b79b76fd890346764bf848d2","src/connection/tests/resumption.rs":"93ec8439d8cc05df77e21566b06c2948ab7ea4e8c7518228de328bc19a88815b","src/connection/tests/stream.rs":"772faea8f971d7b091345ccca3bc7070202f288c3acf6a8e98bfc8c0a5bce5e4","src/connection/tests/vn.rs":"94709b10922b31d518bb7afe8d10489ee5a6e52a51c2fe46b54dac2382997c59","src/connection/tests/zerortt.rs":"94a5a705283c31f50f68a74c49d4bba4ba2a51d8122d9e50a14a831a902f8578","src/crypto.rs":"18bf84e34fd05fd4c596f5db3b8f122b6685fbc4051ef0a9218e4542dc7618ed","src/ecn.rs":"66411888a79d3fbe7445418cafcc1ad7c1ca2a466341fb8de6240ff2cef25a77","src/events.rs":"0548408fd72c1d6c2458c9a682b6d63e2ce6dc699bce6fd7b72818b04bb9d61c","src/fc.rs":"fbc4b3c355459214408174a75bb0024063f32fcebd33db619d5aa2ac1c72002f","src/frame.rs":"fefd63e3b1fed22a606ca95538f21ad32e7222406d5a2b61ea297c165cf61101","src/lib.rs":"0c3c3610343593f10d0c75e75a01fcf9844c0a082d5a6071150358594cb91743","src/pace.rs":"e566b177c86edf8a8fda43863edef9058b74ed3c43109a5fa7e6a7a5e5f87b43","src/packet/metadata.rs":"7a3f520af2932e1ba26197df37bcc706293600cf94c7b559a5192a6f05f6b728","src/packet/mod.rs":"a2349bce7b55990896692a8646a434d058ae2f8f73507dbee5ca2c1af4cc8fc5","src/packet/retry.rs":"df1c7874ff3d3c478ca95a6c7adcb9b1d988894bf6467cee9afe9607ef0a7a20","src/path.rs":"7d4c01ebcbb901c11fb73fddb766c73c7a2ad64b74e02c565440f00e9c7140a2","src/pmtud.rs":"3d11140c7ef9661f3deba48218c7802c2ce89155692cd7f824fafb5c52613e72","src/qlog.rs":"e5a0a93f19959b235dab7f741b088d4a37aabde62b18d54493eac1418bae6321","src/quic_datagrams.rs":"4c2d37b30d202aac8c7bfbf433645338868a21c9bb7dd3bb746fef8351453557","src/recovery/mod.rs":"623f29a103c911ef78a6895b1d57b3c559b7c52f2919cb3a66211824a2bc459c","src/recovery/sent.rs":"882a105a30d09f43a059fb7472a4cfdeedd8dc1ea9607981ec0ffe09a2e42420","src/recovery/token.rs":"b8866bd2605e59d0c0eaa767275f972563c2fe58804e210b1bf78445c114f05b","src/recv_stream.rs":"40802b5ed4bcf301626a5ba447b05e61d0a222390d97448ed580c09a1a86b967","src/rtt.rs":"ab74c7618f1c200c095bb6f9feda5e8e164738de9196561673d0ea52e7117c90","src/saved.rs":"4ec4b1f45b95527fa37881e6542dc714855f9c66ca435d45de9343532bf89dbe","src/send_stream.rs":"ab8927cc461831cc3af86fad2d2a450bf548d74bc5301fe218549edfc768ac2d","src/sender.rs":"21f55fa440e3365ff8cc08e31868a45e5b4d9979be37579f4f5b23916d291821","src/server.rs":"bda1165f157cfdc4c7b9a02f8e2c32775cdb6a8b2962ee2524ae242869e378e6","src/sni.rs":"34361fd7a19fa7c8b33519ba1bdefc32a2cadec0b9cbcf1073a2e75f0c1730ae","src/stateless_reset.rs":"626e3c90682418d6390b18b0d6823171aa117d994f9762b7350eefd81a9ab4d8","src/stats.rs":"8aa2dd839a2669e2f8790b33509ac07dad56b955adcf86aa0b187ddd9b6167e8","src/stream_id.rs":"a6a44d6f02967feeed3250dadc658861f20e00373a0f239685776b2fa72a59e4","src/streams.rs":"b0727b9ba66b1064a6cf1c13bdae49e0b153728216f56eafbcbb96dfe7b9d012","src/tparams.rs":"7f8c4bd93bf7c6cdc78d8f6ac45ec4b52ca46d958d70543dba144e4e7f5148d0","src/tracking.rs":"991772d9be37740175fb16c5682591694182d51049c1f5d3135bd56ccb844aa1","src/version.rs":"780ae2ce70b8424d9a2a2cdd3cc7b8b040f4588ecf7f1aaf091ea80b8c28a54d","tests/common/mod.rs":"d1f96c024543cab68d09e2847990ffdf83568835c6ac740bf9cb765e7a661e1d","tests/conn_vectors.rs":"0e4a1b92c02b527842c127b789e70f6c4372c2b61b1a59a8e695f744ce155e2a","tests/connection.rs":"93c1982a6f049f735634ba5cfee6677f919a5f59694b6375e118efb20a69c09f","tests/network.rs":"2173d8347543d457a236817d219d2865315dbd005e1d19c2277e3a34d5cca759","tests/retry.rs":"81c9e6979cbd62c038bb6145e4c8b2e2cb7050d382c935f8a8a53277600f7cac","tests/server.rs":"4fd24a308ceb5b0469a3fb147a6d8a3db9dbdb1c8c8a16d0b8f880d7ecd92f4b","tests/sni.rs":"5cf5c6ec0601712596fdfd589eb346af599e45c80849adf31879ba4ba3640f38","tests/stats.rs":"af8c1da46e984b55b172118aff4ad33be2375443f405e297d40981e65eb4d0cf"},"package":null} -\ No newline at end of file +{"files":{"Cargo.toml":"01c3f6b87cfaad184ca90020321950e61367fe82b90d0f0f8016b4682d842b58","benches/min_bandwidth.rs":"f81c8bccc45cbbecb5a3c4c28b406c5c49319dee3a9caed58feb43b2a2871955","benches/range_tracker.rs":"546c4701424a8f286a459af10d93bf591b3912f1273125272b32485a27e5631d","benches/rx_stream_orderer.rs":"23c4646e91c4e20d27a81544c73a83dc02b2362bab870136e39d52a9ddced01b","benches/sent_packets.rs":"1e6cde25a8047f5c63629ae2b80950ee8b08b19d8db216a871eee04a59049f5b","benches/transfer.rs":"d3501525cffe7bbf2b89144be39081974a4e74c6f7465ac1bc36125809f4be3c","build.rs":"78ec79c93bf13c3a40ceef8bba1ea2eada61c8f2dfc15ea7bf117958d367949c","src/ackrate.rs":"08f7f4777b7ac1ec85afb4c8cd1cab99a39bd2cb023628193e99325c8c0e34e2","src/addr_valid.rs":"2e8fb7cc77e97dc709b6b0be5bf2dbc4dccfb2341fc4a49971b3883af481ff4b","src/cc/classic_cc.rs":"b01f987b0b717e787c66bdc10b609767ec0cfbd769ab9fa6e07d228f7b2a5d45","src/cc/cubic.rs":"f2f767439c1b3b6848cfeb5fc973315e64d67d3d796375172b777cd4d177e8ff","src/cc/mod.rs":"614986a1a064e45c06d9c6baf5eef5edc03515bc30defac4e5a78ba4b7e2f1c5","src/cc/new_reno.rs":"ec467ef02177dcee5a8c66e694601364ab06cffcc7fb25ea53dc9408f7055258","src/cc/tests/cubic.rs":"2ee04576bf17d3234b73e6828df23ac7184fde149c24913f2596bf6e9c10e9a0","src/cc/tests/mod.rs":"017bf402a9a8c71b5c43343677635644babb57a849d81d0affc328b4b4b9cebb","src/cc/tests/new_reno.rs":"a0aa6b8c6369f42237edbb2597bb0c68e80ae49bf282b6e06c2bb478e7ea5a0e","src/cid.rs":"e8d4437663d1a954b3af75b40ea6fa5552802556c461665647584f1bea5f8bbf","src/connection/idle.rs":"7fffb026530f17604ac41b28c71cf3b3554e6b1958db96436478d9332b65217c","src/connection/mod.rs":"dbe3ad7c4014c51fd9aee7d491da3a6d97123d9132b31d3aabddd72b1b52ea0c","src/connection/params.rs":"1007bf962dbae8629746c4e18e8651424b0cf41ea01c1fd46eae40bdd0d9faa2","src/connection/state.rs":"c1f18f21b8729cd523e4165bf7072f056c3b26ba8094f3c1adfcdd7beb3c0979","src/connection/test_internal.rs":"25814d143b745c50c67708cd82aeecbc1e6f123e067b69c5021b9ba653c0c6c2","src/connection/tests/ackrate.rs":"4b7325b5798817d971c14f76ca79c1d7f677aee8e579c97623f49eb87727bd6f","src/connection/tests/cc.rs":"7b113bb1713d440a03bf34bcd42b146dd201c2110140650ee1d98b136d7cec48","src/connection/tests/close.rs":"3844fb0fa626db47a1f914189923bd71737c5331e87cd0e293d10f556405c204","src/connection/tests/datagram.rs":"35c865e9bdfeef2d58f9421370447f627210825f6a3b66a44e84fbce3a22efa3","src/connection/tests/ecn.rs":"f03f2c661a14ca56e4dc6b6c1273352b3a747bb06452cecd3956ae3cc7df743b","src/connection/tests/handshake.rs":"246787d882e87bd427eb9b0c54c4722afd5930e5e8263252975f01f36fbe3277","src/connection/tests/idle.rs":"f8607ee48bd28486c83e497ce9882239cd8dcf9bdf3a5b1e475d5f50836e6223","src/connection/tests/keys.rs":"3481eb3b24fd034b2b51bfaa02ce58d1890bc10213d3f9d0effdd4d7fceb677c","src/connection/tests/migration.rs":"a97dc409c0573eab65fa9ec7b6d4843d491a2942ce2a90e65a6c6cf750b09714","src/connection/tests/mod.rs":"48c04e7cfe70114f014ac4599a4a4ed5c059a9d56e742a1e975676863e34114a","src/connection/tests/null.rs":"d39d34c895c40ea88bcc137cba43c34386ef9759c6f66f3487ffd41a5099feb8","src/connection/tests/pmtud.rs":"77d910b74a8e424a7df930610bd68881cbc3d1246241820d668f83dda09fe74f","src/connection/tests/priority.rs":"00fb90881299fb93b9ad628840585852422676a6db0dbeee348a51ea3029456d","src/connection/tests/recovery.rs":"b527fceb06ac306ac7504c494a831bf2997148a2b79b76fd890346764bf848d2","src/connection/tests/resumption.rs":"93ec8439d8cc05df77e21566b06c2948ab7ea4e8c7518228de328bc19a88815b","src/connection/tests/stream.rs":"772faea8f971d7b091345ccca3bc7070202f288c3acf6a8e98bfc8c0a5bce5e4","src/connection/tests/vn.rs":"94709b10922b31d518bb7afe8d10489ee5a6e52a51c2fe46b54dac2382997c59","src/connection/tests/zerortt.rs":"94a5a705283c31f50f68a74c49d4bba4ba2a51d8122d9e50a14a831a902f8578","src/crypto.rs":"6b1aaf79a65e73761e136f58c11456f9915d5325b4595a2478591a565a9b8ee0","src/ecn.rs":"66411888a79d3fbe7445418cafcc1ad7c1ca2a466341fb8de6240ff2cef25a77","src/events.rs":"379b3d0eec4d1f4f8a67a3fc1bb2c51e8a328b3733e75e083285d1a87bcf0f04","src/fc.rs":"e535adf852136fb5ddbf72b941eefe761e0675e67279504615731b233e3955e7","src/frame.rs":"fefd63e3b1fed22a606ca95538f21ad32e7222406d5a2b61ea297c165cf61101","src/lib.rs":"9a2fb5c3f91891571bdb3e0863825d4a671cb9ce2300a280d6da147099a40da2","src/pace.rs":"e566b177c86edf8a8fda43863edef9058b74ed3c43109a5fa7e6a7a5e5f87b43","src/packet/metadata.rs":"7a3f520af2932e1ba26197df37bcc706293600cf94c7b559a5192a6f05f6b728","src/packet/mod.rs":"73c77d5605f6725c01a5f4c79b731a8397b6ff04a51a2def3473096cf3c9b927","src/packet/retry.rs":"df1c7874ff3d3c478ca95a6c7adcb9b1d988894bf6467cee9afe9607ef0a7a20","src/path.rs":"b1a2279cfa8b6c4cf275123ebe202f3d2ba0db6fb6058cc3dae17eba063fcaf0","src/pmtud.rs":"006165fbd495e38b20b67b9077b94a146cc31e73c1c2fda445f80ddce786a4fd","src/qlog.rs":"10ff457d97c1f78651e0d4479c383492f5386cc7ccde10e3c2d80f0e0f7afe11","src/quic_datagrams.rs":"4c2d37b30d202aac8c7bfbf433645338868a21c9bb7dd3bb746fef8351453557","src/recovery/mod.rs":"ddf09eaffead48b5dc6d8af319bc73d2fd247dd2b44144e0984bfa4ae05cde51","src/recovery/sent.rs":"5edc6a76a9ca2ca676a031083957eb1ca683f02fef21b2a62c064efdb4ecb9db","src/recovery/token.rs":"00eb1565333d2528a5b95b85700c900c0a25f66c85906325681829cc2e24f493","src/recv_stream.rs":"40802b5ed4bcf301626a5ba447b05e61d0a222390d97448ed580c09a1a86b967","src/rtt.rs":"9fad8a50ea1ec979f657ff369b647106586b02ea2a4d3112c9ba1745776e4c15","src/saved.rs":"4ec4b1f45b95527fa37881e6542dc714855f9c66ca435d45de9343532bf89dbe","src/send_stream.rs":"ab8927cc461831cc3af86fad2d2a450bf548d74bc5301fe218549edfc768ac2d","src/sender.rs":"21f55fa440e3365ff8cc08e31868a45e5b4d9979be37579f4f5b23916d291821","src/server.rs":"4cee0cc7b2ed41de76a126ec0c3610d121b3a46b7f4f36ac7c866b1c920e2508","src/sni.rs":"34361fd7a19fa7c8b33519ba1bdefc32a2cadec0b9cbcf1073a2e75f0c1730ae","src/stateless_reset.rs":"626e3c90682418d6390b18b0d6823171aa117d994f9762b7350eefd81a9ab4d8","src/stats.rs":"8d90f92f7f8267e290492af756f9330bbf4b93169fe28bdf2b85ec2ea475cfee","src/stream_id.rs":"1b998b93f2cb57cf5ccda999439ffc40794b1caa9795cfce139ae4269ae7f6cc","src/streams.rs":"b0727b9ba66b1064a6cf1c13bdae49e0b153728216f56eafbcbb96dfe7b9d012","src/tparams.rs":"7f8c4bd93bf7c6cdc78d8f6ac45ec4b52ca46d958d70543dba144e4e7f5148d0","src/tracking.rs":"991772d9be37740175fb16c5682591694182d51049c1f5d3135bd56ccb844aa1","src/version.rs":"780ae2ce70b8424d9a2a2cdd3cc7b8b040f4588ecf7f1aaf091ea80b8c28a54d","tests/common/mod.rs":"d1f96c024543cab68d09e2847990ffdf83568835c6ac740bf9cb765e7a661e1d","tests/conn_vectors.rs":"0e4a1b92c02b527842c127b789e70f6c4372c2b61b1a59a8e695f744ce155e2a","tests/connection.rs":"93c1982a6f049f735634ba5cfee6677f919a5f59694b6375e118efb20a69c09f","tests/network.rs":"2173d8347543d457a236817d219d2865315dbd005e1d19c2277e3a34d5cca759","tests/retry.rs":"81c9e6979cbd62c038bb6145e4c8b2e2cb7050d382c935f8a8a53277600f7cac","tests/server.rs":"82743df2c087e2f1a3efd0e81a7a66ae49e3648da04d991f1ce9cb25e2503075","tests/sni.rs":"5cf5c6ec0601712596fdfd589eb346af599e45c80849adf31879ba4ba3640f38","tests/stats.rs":"af8c1da46e984b55b172118aff4ad33be2375443f405e297d40981e65eb4d0cf"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/neqo-transport/Cargo.toml b/third_party/rust/neqo-transport/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.81.0" name = "neqo-transport" -version = "0.20.0" +version = "0.21.0" authors = ["The Neqo Authors <necko@mozilla.com>"] build = "build.rs" autolib = false diff --git a/third_party/rust/neqo-transport/benches/transfer.rs b/third_party/rust/neqo-transport/benches/transfer.rs @@ -17,13 +17,12 @@ use test_fixture::{ boxed, sim::{ connection::{Node, ReachState, ReceiveData, SendData}, - network::{RandomDelay, TailDrop}, + network::{Delay, TailDrop}, Simulator, }, }; -const ZERO: Duration = Duration::from_millis(0); -const JITTER: Duration = Duration::from_millis(10); +const DELAY: Duration = Duration::from_millis(10); const TRANSFER_AMOUNT: usize = 1 << 22; // 4Mbyte #[expect( @@ -43,7 +42,7 @@ fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option<impl AsRef<st boxed![SendData::new(TRANSFER_AMOUNT)] ), TailDrop::dsl_uplink(), - RandomDelay::new(ZERO..JITTER), + Delay::new(DELAY), Node::new_server( ConnectionParameters::default() .pmtud(true) @@ -53,7 +52,7 @@ fn benchmark_transfer(c: &mut Criterion, label: &str, seed: Option<impl AsRef<st boxed![ReceiveData::new(TRANSFER_AMOUNT)] ), TailDrop::dsl_downlink(), - RandomDelay::new(ZERO..JITTER), + Delay::new(DELAY), ]; let mut sim = Simulator::new(label, nodes); if let Some(seed) = &seed { diff --git a/third_party/rust/neqo-transport/src/addr_valid.rs b/third_party/rust/neqo-transport/src/addr_valid.rs @@ -17,6 +17,7 @@ use neqo_crypto::{ selfencrypt::SelfEncrypt, }; use smallvec::SmallVec; +use static_assertions::const_assert; use crate::{ cid::ConnectionId, @@ -33,6 +34,8 @@ const TOKEN_IDENTIFIER_RETRY: &[u8] = &[0x52, 0x65, 0x74, 0x72, 0x79]; /// corruption of individual bits in transit. const TOKEN_IDENTIFIER_NEW_TOKEN: &[u8] = &[0xad, 0x9a, 0x8b, 0x8d, 0x86]; +const_assert!(TOKEN_IDENTIFIER_RETRY.len() == TOKEN_IDENTIFIER_NEW_TOKEN.len()); + /// The maximum number of tokens we'll save from `NEW_TOKEN` frames. /// This should be the same as the value of `MAX_TICKETS` in neqo-crypto. const MAX_NEW_TOKEN: usize = 4; @@ -131,9 +134,21 @@ impl AddressValidation { // Include the token identifier ("Retry"/~) in the AAD, then keep it for plaintext. let mut buf = Self::encode_aad(peer_address, retry); let encrypted = self.self_encrypt.seal(buf.as_ref(), data.as_ref())?; + #[cfg(feature = "build-fuzzing-corpus")] + let mut corpus_data = buf.as_ref()[TOKEN_IDENTIFIER_RETRY.len()..].to_vec(); buf.truncate(TOKEN_IDENTIFIER_RETRY.len()); buf.encode(&encrypted); - Ok(buf.into()) + let token: Vec<u8> = buf.into(); + #[cfg(feature = "build-fuzzing-corpus")] + { + if !retry { + // Port is not validated for NEW_TOKEN, so use 0 as a placeholder. + corpus_data.extend_from_slice(&[0, 0]); + } + corpus_data.extend_from_slice(&token); + neqo_common::write_item_to_fuzzing_corpus("addr_valid", &corpus_data); + } + Ok(token) } /// This generates a token for use with Retry. diff --git a/third_party/rust/neqo-transport/src/cc/classic_cc.rs b/third_party/rust/neqo-transport/src/cc/classic_cc.rs @@ -18,7 +18,7 @@ use rustc_hash::FxHashMap as HashMap; use super::CongestionControl; use crate::{ - packet, qlog, recovery::sent, rtt::RttEstimate, sender::PACING_BURST_SIZE, + cc::CongestionEvent, packet, qlog, recovery::sent, rtt::RttEstimate, sender::PACING_BURST_SIZE, stats::CongestionControlStats, Pmtud, }; @@ -94,6 +94,7 @@ pub trait WindowAdjustment: Display + Debug { curr_cwnd: usize, acked_bytes: usize, max_datagram_size: usize, + congestion_event: CongestionEvent, ) -> (usize, usize); /// Cubic needs this signal to reset its epoch. fn on_app_limited(&mut self); @@ -228,7 +229,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { if self.state.in_recovery() { self.set_state(State::CongestionAvoidance, now); - qlog::metrics_updated(&self.qlog, &[qlog::Metric::InRecovery(false)], now); + qlog::metrics_updated(&mut self.qlog, &[qlog::Metric::InRecovery(false)], now); } new_acked += pkt.len(); @@ -282,7 +283,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { self.acked_bytes = min(bytes_for_increase, self.acked_bytes); } qlog::metrics_updated( - &self.qlog, + &mut self.qlog, &[ qlog::Metric::CongestionWindow(self.congestion_window), qlog::Metric::BytesInFlight(self.bytes_in_flight), @@ -328,15 +329,14 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { } qlog::metrics_updated( - &self.qlog, + &mut self.qlog, &[qlog::Metric::BytesInFlight(self.bytes_in_flight)], now, ); - let is_pmtud_probe = self.pmtud.is_probe_filter(); let mut lost_packets = lost_packets .iter() - .filter(|pkt| !is_pmtud_probe(pkt)) + .filter(|pkt| !pkt.is_pmtud_probe()) .rev() .peekable(); @@ -345,7 +345,8 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { return false; }; - let congestion = self.on_congestion_event(last_lost_packet, false, now, cc_stats); + let congestion = + self.on_congestion_event(last_lost_packet, CongestionEvent::Loss, now, cc_stats); let persistent_congestion = self.detect_persistent_congestion( first_rtt_sample_time, prev_largest_acked_sent, @@ -372,7 +373,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { now: Instant, cc_stats: &mut CongestionControlStats, ) -> bool { - self.on_congestion_event(largest_acked_pkt, true, now, cc_stats) + self.on_congestion_event(largest_acked_pkt, CongestionEvent::Ecn, now, cc_stats) } fn discard(&mut self, pkt: &sent::Packet, now: Instant) { @@ -380,7 +381,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { assert!(self.bytes_in_flight >= pkt.len()); self.bytes_in_flight -= pkt.len(); qlog::metrics_updated( - &self.qlog, + &mut self.qlog, &[qlog::Metric::BytesInFlight(self.bytes_in_flight)], now, ); @@ -391,7 +392,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { fn discard_in_flight(&mut self, now: Instant) { self.bytes_in_flight = 0; qlog::metrics_updated( - &self.qlog, + &mut self.qlog, &[qlog::Metric::BytesInFlight(self.bytes_in_flight)], now, ); @@ -422,7 +423,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> { pkt.len() ); qlog::metrics_updated( - &self.qlog, + &mut self.qlog, &[qlog::Metric::BytesInFlight(self.bytes_in_flight)], now, ); @@ -489,7 +490,7 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> { if self.state != state { qdebug!("[{self}] state -> {state:?}"); let old_state = self.state; - self.qlog.add_event_data_with_instant( + self.qlog.add_event_at( || { // No need to tell qlog about exit from transient states. if old_state.transient() { @@ -531,7 +532,7 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> { // If all of them have been removed we detected a spurious congestion event. if self.maybe_lost_packets.is_empty() { - cc_stats.congestion_events_spurious += 1; + cc_stats.congestion_events[CongestionEvent::Spurious] += 1; // TODO: Implement spurious congestion event handling: <https://github.com/mozilla/neqo/issues/2694> } } @@ -592,7 +593,7 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> { self.acked_bytes = 0; self.set_state(State::PersistentCongestion, now); qlog::metrics_updated( - &self.qlog, + &mut self.qlog, &[qlog::Metric::CongestionWindow(self.congestion_window)], now, ); @@ -621,7 +622,7 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> { fn on_congestion_event( &mut self, last_packet: &sent::Packet, - ecn: bool, + congestion_event: CongestionEvent, now: Instant, cc_stats: &mut CongestionControlStats, ) -> bool { @@ -635,6 +636,7 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> { self.congestion_window, self.acked_bytes, self.max_datagram_size(), + congestion_event, ); self.congestion_window = max(cwnd, self.cwnd_min()); self.acked_bytes = acked_bytes; @@ -645,15 +647,11 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> { self.ssthresh ); - if ecn { - cc_stats.congestion_events_ecn += 1; - } else { - cc_stats.congestion_events_loss += 1; - } + cc_stats.congestion_events[congestion_event] += 1; cc_stats.slow_start_exited |= self.state.in_slow_start(); qlog::metrics_updated( - &self.qlog, + &mut self.qlog, &[ qlog::Metric::CongestionWindow(self.congestion_window), qlog::Metric::SsThresh(self.ssthresh), @@ -697,7 +695,7 @@ mod tests { cubic::Cubic, new_reno::NewReno, tests::{IP_ADDR, MTU, RTT}, - CongestionControl, CongestionControlAlgorithm, CWND_INITIAL_PKTS, + CongestionControl, CongestionControlAlgorithm, CongestionEvent, CWND_INITIAL_PKTS, }, packet, recovery::{self, sent}, @@ -1380,7 +1378,7 @@ mod tests { #[test] fn ecn_ce() { let now = now(); - let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); + let mut cc = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); let mut cc_stats = CongestionControlStats::default(); let p_ce = sent::Packet::new( packet::Type::Short, @@ -1391,15 +1389,17 @@ mod tests { cc.max_datagram_size(), ); cc.on_packet_sent(&p_ce, now); - cwnd_is_default(&cc); + assert_eq!(cc.cwnd(), cc.cwnd_initial()); + assert_eq!(cc.ssthresh(), usize::MAX); assert_eq!(cc.state, State::SlowStart); - assert_eq!(cc_stats.congestion_events_ecn, 0); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Ecn], 0); // Signal congestion (ECN CE) and thus change state to recovery start. cc.on_ecn_ce_received(&p_ce, now, &mut cc_stats); - cwnd_is_halved(&cc); + assert_eq!(cc.cwnd(), cc.cwnd_initial() * 85 / 100); + assert_eq!(cc.ssthresh(), cc.cwnd_initial() * 85 / 100); assert_eq!(cc.state, State::RecoveryStart); - assert_eq!(cc_stats.congestion_events_ecn, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Ecn], 1); } /// This tests spurious congestion event detection and stat counting @@ -1426,8 +1426,8 @@ mod tests { // Verify initial state assert_eq!(cc.state, State::SlowStart); - assert_eq!(cc_stats.congestion_events_loss, 0); - assert_eq!(cc_stats.congestion_events_spurious, 0); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 0); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Spurious], 0); let mut lost_pkt1 = pkt1.clone(); let mut lost_pkt2 = pkt2.clone(); @@ -1445,13 +1445,13 @@ mod tests { // Verify congestion event assert_eq!(cc.state, State::RecoveryStart); - assert_eq!(cc_stats.congestion_events_loss, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); let pkt3 = sent::make_packet(3, now, 1000); cc.on_packet_sent(&pkt3, now); assert_eq!(cc.state, State::Recovery); - assert_eq!(cc_stats.congestion_events_loss, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); cc.on_packets_acked( &[pkt3], @@ -1461,7 +1461,7 @@ mod tests { ); assert_eq!(cc.state, State::CongestionAvoidance); - assert_eq!(cc_stats.congestion_events_loss, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); cc.on_packets_acked( &[pkt1], @@ -1471,8 +1471,8 @@ mod tests { ); assert_eq!(cc.state, State::CongestionAvoidance); - assert_eq!(cc_stats.congestion_events_loss, 1); - assert_eq!(cc_stats.congestion_events_spurious, 0); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Spurious], 0); cc.on_packets_acked( &[pkt2], @@ -1482,8 +1482,8 @@ mod tests { ); assert_eq!(cc.state, State::CongestionAvoidance); - assert_eq!(cc_stats.congestion_events_loss, 1); - assert_eq!(cc_stats.congestion_events_spurious, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Spurious], 1); } #[test] @@ -1532,7 +1532,7 @@ mod tests { assert!(cc.maybe_lost_packets.is_empty()); } - fn slow_start_exit_stats(ecn: bool) { + fn slow_start_exit_stats(congestion_event: CongestionEvent) { let mut cc = ClassicCongestionControl::new(NewReno::default(), Pmtud::new(IP_ADDR, MTU)); let now = now(); let mut cc_stats = CongestionControlStats::default(); @@ -1543,10 +1543,14 @@ mod tests { let pkt1 = sent::make_packet(1, now, 1000); cc.on_packet_sent(&pkt1, now); - if ecn { - cc.on_ecn_ce_received(&pkt1, now, &mut cc_stats); - } else { - cc.on_packets_lost(Some(now), None, PTO, &[pkt1], now, &mut cc_stats); + match congestion_event { + CongestionEvent::Ecn => { + cc.on_ecn_ce_received(&pkt1, now, &mut cc_stats); + } + CongestionEvent::Loss => { + cc.on_packets_lost(Some(now), None, PTO, &[pkt1], now, &mut cc_stats); + } + CongestionEvent::Spurious => panic!("unsupported congestion event"), } assert!(!cc.state.in_slow_start()); @@ -1555,11 +1559,11 @@ mod tests { #[test] fn slow_start_exit_stats_loss() { - slow_start_exit_stats(false); + slow_start_exit_stats(CongestionEvent::Loss); } #[test] fn slow_start_exit_stats_ecn_ce() { - slow_start_exit_stats(true); + slow_start_exit_stats(CongestionEvent::Ecn); } } diff --git a/third_party/rust/neqo-transport/src/cc/cubic.rs b/third_party/rust/neqo-transport/src/cc/cubic.rs @@ -13,7 +13,7 @@ use std::{ use neqo_common::qtrace; -use crate::cc::classic_cc::WindowAdjustment; +use crate::cc::{classic_cc::WindowAdjustment, CongestionEvent}; /// Convert an integer congestion window value into a floating point value. /// This has the effect of reducing larger values to `1<<53`. @@ -121,7 +121,7 @@ impl Cubic { /// <https://datatracker.ietf.org/doc/html/rfc9438#name-reno-friendly-region> pub const ALPHA: f64 = 3.0 * (1.0 - 0.7) / (1.0 + 0.7); // with CUBIC_BETA = 0.7 - /// `CUBIC_BETA` = 0.7; + /// > CUBIC multiplicative decrease factor /// /// <https://datatracker.ietf.org/doc/html/rfc9438#name-constants-of-interest> /// @@ -133,9 +133,10 @@ impl Cubic { /// /// <https://datatracker.ietf.org/doc/html/rfc9438#name-principle-4-for-the-cubic-d> /// - /// For implementation reasons neqo uses a dividend and divisor approach with `usize` typing to - /// construct `CUBIC_BETA = 0.7`. - pub const BETA_USIZE_DIVIDEND: usize = 7; + /// For implementation reasons neqo uses a dividend and divisor approach with `usize` typing. + /// The divisor is set to `100` to also accommodate the `0.85` beta value for ECN induced + /// congestion events. + pub const BETA_USIZE_DIVISOR: usize = 100; /// > CUBIC multiplicative decrease factor /// @@ -145,14 +146,24 @@ impl Cubic { /// > window /// > decrease factor to 0.7 while Standard TCP uses 0.5. While this improves the scalability of /// > CUBIC, a side effect of this decision is slower convergence, especially under low - /// > statistical - /// > multiplexing environments. + /// > statistical multiplexing environments. /// /// <https://datatracker.ietf.org/doc/html/rfc9438#name-principle-4-for-the-cubic-d> /// /// For implementation reasons neqo uses a dividend and divisor approach with `usize` typing to - /// construct `CUBIC_BETA = 0.7` - pub const BETA_USIZE_DIVISOR: usize = 10; + /// construct `CUBIC_BETA = 0.7` from `70/100`. + pub const BETA_USIZE_DIVIDEND: usize = 70; + + /// As per RFC 8511 it makes sense to have a different decrease factor for ECN-CE congestion + /// events than for loss induced congestion events. + /// + /// > CUBIC connections benefit from beta_{ecn} of 0.85. + /// + /// <https://www.rfc-editor.org/rfc/rfc8511.html#section-3.1> + /// + /// For implementation reasons neqo uses a dividend and divisor approach with `usize` typing to + /// construct the beta value from `85/100`. + pub const BETA_USIZE_DIVIDEND_ECN: usize = 85; /// This is the factor that is used by fast convergence to further reduce the next `W_max` when /// a congestion event occurs while `cwnd < W_max`. This speeds up the bandwidth release for @@ -385,6 +396,7 @@ impl WindowAdjustment for Cubic { curr_cwnd: usize, acked_bytes: usize, max_datagram_size: usize, + congestion_event: CongestionEvent, ) -> (usize, usize) { let curr_cwnd_f64 = convert_to_f64(curr_cwnd); // Fast Convergence @@ -411,9 +423,14 @@ impl WindowAdjustment for Cubic { // Reducing the congestion window and resetting time self.t_epoch = None; + let beta_dividend = if congestion_event == CongestionEvent::Ecn { + Self::BETA_USIZE_DIVIDEND_ECN + } else { + Self::BETA_USIZE_DIVIDEND + }; ( - curr_cwnd * Self::BETA_USIZE_DIVIDEND / Self::BETA_USIZE_DIVISOR, - acked_bytes * Self::BETA_USIZE_DIVIDEND / Self::BETA_USIZE_DIVISOR, + curr_cwnd * beta_dividend / Self::BETA_USIZE_DIVISOR, + acked_bytes * beta_dividend / Self::BETA_USIZE_DIVISOR, ) } diff --git a/third_party/rust/neqo-transport/src/cc/mod.rs b/third_party/rust/neqo-transport/src/cc/mod.rs @@ -12,6 +12,7 @@ use std::{ time::{Duration, Instant}, }; +use enum_map::Enum; use neqo_common::qlog::Qlog; use crate::{recovery::sent, rtt::RttEstimate, stats::CongestionControlStats, Error, Pmtud}; @@ -26,6 +27,13 @@ pub use classic_cc::CWND_INITIAL_PKTS; pub use cubic::Cubic; pub use new_reno::NewReno; +#[derive(Clone, Copy, PartialEq, Eq, Enum, Debug)] +pub enum CongestionEvent { + Loss, + Ecn, + Spurious, +} + pub trait CongestionControl: Display + Debug { fn set_qlog(&mut self, qlog: Qlog); diff --git a/third_party/rust/neqo-transport/src/cc/new_reno.rs b/third_party/rust/neqo-transport/src/cc/new_reno.rs @@ -11,7 +11,7 @@ use std::{ time::{Duration, Instant}, }; -use crate::cc::classic_cc::WindowAdjustment; +use crate::cc::{classic_cc::WindowAdjustment, CongestionEvent}; #[derive(Debug, Default)] pub struct NewReno {} @@ -41,6 +41,7 @@ impl WindowAdjustment for NewReno { curr_cwnd: usize, acked_bytes: usize, _max_datagram_size: usize, + _congestion_event: CongestionEvent, ) -> (usize, usize) { (curr_cwnd / 2, acked_bytes / 2) } diff --git a/third_party/rust/neqo-transport/src/cc/tests/cubic.rs b/third_party/rust/neqo-transport/src/cc/tests/cubic.rs @@ -22,11 +22,10 @@ use crate::{ cc::{ classic_cc::ClassicCongestionControl, cubic::{convert_to_f64, Cubic}, - CongestionControl as _, + CongestionControl as _, CongestionEvent, }, - packet, pmtud::Pmtud, - recovery::{self, sent}, + recovery::sent, rtt::RttEstimate, stats::CongestionControlStats, }; @@ -39,16 +38,34 @@ const fn cwnd_after_loss_slow_start(cwnd: usize, mtu: usize) -> usize { (cwnd + mtu) * Cubic::BETA_USIZE_DIVIDEND / Cubic::BETA_USIZE_DIVISOR } +/// Sets up a Cubic congestion controller in congestion avoidance phase. +/// +/// If `fast_convergence` is true, sets `w_max` higher than cwnd to trigger fast convergence. +/// If false, sets `w_max` lower than cwnd to prevent fast convergence. +fn setup_congestion_avoidance( + fast_convergence: bool, +) -> (ClassicCongestionControl<Cubic>, CongestionControlStats) { + let mut cc = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); + let mut cc_stats = CongestionControlStats::default(); + // Enter congestion avoidance phase. + cc.set_ssthresh(1); + // Configure fast convergence behavior. + let cwnd_f64 = convert_to_f64(cc.cwnd_initial()); + let w_max = if fast_convergence { + cwnd_f64 * 10.0 + } else { + convert_to_f64(cc.max_datagram_size()) * 3.0 + }; + cc.cc_algorithm_mut().set_w_max(w_max); + // Fill cwnd and ack one packet to establish baseline. + _ = fill_cwnd(&mut cc, 0, now()); + ack_packet(&mut cc, 0, now(), &mut cc_stats); + (cc, cc_stats) +} + fn fill_cwnd(cc: &mut ClassicCongestionControl<Cubic>, mut next_pn: u64, now: Instant) -> u64 { while cc.bytes_in_flight() < cc.cwnd() { - let sent = sent::Packet::new( - packet::Type::Short, - next_pn, - now, - true, - recovery::Tokens::new(), - cc.max_datagram_size(), - ); + let sent = sent::make_packet(next_pn, now, cc.max_datagram_size()); cc.on_packet_sent(&sent, now); next_pn += 1; } @@ -61,14 +78,7 @@ fn ack_packet( now: Instant, cc_stats: &mut CongestionControlStats, ) { - let acked = sent::Packet::new( - packet::Type::Short, - pn, - now, - true, - recovery::Tokens::new(), - cc.max_datagram_size(), - ); + let acked = sent::make_packet(pn, now, cc.max_datagram_size()); cc.on_packets_acked(&[acked], &RttEstimate::new(RTT), now, cc_stats); } @@ -78,15 +88,19 @@ fn packet_lost( cc_stats: &mut CongestionControlStats, ) { const PTO: Duration = Duration::from_millis(120); - let p_lost = sent::Packet::new( - packet::Type::Short, - pn, - now(), - true, - recovery::Tokens::new(), - cc.max_datagram_size(), - ); - cc.on_packets_lost(None, None, PTO, &[p_lost], now(), cc_stats); + let now = now(); + let p_lost = sent::make_packet(pn, now, cc.max_datagram_size()); + cc.on_packets_lost(None, None, PTO, &[p_lost], now, cc_stats); +} + +fn ecn_ce( + cc: &mut ClassicCongestionControl<Cubic>, + pn: u64, + now: Instant, + cc_stats: &mut CongestionControlStats, +) { + let pkt = sent::make_packet(pn, now, cc.max_datagram_size()); + cc.on_ecn_ce_received(&pkt, now, cc_stats); } fn expected_tcp_acks(cwnd_rtt_start: usize, mtu: usize) -> u64 { @@ -290,26 +304,12 @@ fn congestion_event_slow_start() { cubic.cwnd(), cwnd_after_loss_slow_start(cubic.cwnd_initial(), cubic.max_datagram_size()) ); - assert_eq!(cc_stats.congestion_events_loss, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); } #[test] fn congestion_event_congestion_avoidance() { - let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); - let mut cc_stats = CongestionControlStats::default(); - - // Set ssthresh to something small to make sure that cc is in the congection avoidance phase. - cubic.set_ssthresh(1); - - // Set w_max to something smaller than cwnd so that the fast convergence is not - // triggered. - let max_datagram_size_f64 = convert_to_f64(cubic.max_datagram_size()); - cubic - .cc_algorithm_mut() - .set_w_max(3.0 * max_datagram_size_f64); - - _ = fill_cwnd(&mut cubic, 0, now()); - ack_packet(&mut cubic, 0, now(), &mut cc_stats); + let (mut cubic, mut cc_stats) = setup_congestion_avoidance(false); assert_eq!(cubic.cwnd(), cubic.cwnd_initial()); @@ -319,24 +319,56 @@ fn congestion_event_congestion_avoidance() { let cwnd_initial_f64 = convert_to_f64(cubic.cwnd_initial()); assert_within(cubic.cc_algorithm().w_max(), cwnd_initial_f64, f64::EPSILON); assert_eq!(cubic.cwnd(), cwnd_after_loss(cubic.cwnd_initial())); - assert_eq!(cc_stats.congestion_events_loss, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); } -#[test] -fn congestion_event_congestion_avoidance_fast_convergence() { - let mut cubic = ClassicCongestionControl::new(Cubic::default(), Pmtud::new(IP_ADDR, MTU)); - let mut cc_stats = CongestionControlStats::default(); +/// Verify that `acked_bytes` is correctly reduced on a congestion event. +fn acked_bytes_reduced_on_congestion_event( + trigger: impl FnOnce(&mut ClassicCongestionControl<Cubic>, Instant, &mut CongestionControlStats), + beta: usize, +) { + let (mut cubic, mut cc_stats) = setup_congestion_avoidance(false); - // Set ssthresh to something small to make sure that cc is in the congection avoidance phase. - cubic.set_ssthresh(1); + // The helper acked packet 0. Ack one more to accumulate acked_bytes. + let now = now() + RTT / 10; + _ = fill_cwnd(&mut cubic, 10, now); + ack_packet(&mut cubic, 1, now, &mut cc_stats); - // Set w_max to something higher than cwnd so that the fast convergence is triggered. - let cwnd_initial_f64 = convert_to_f64(cubic.cwnd_initial()); - cubic.cc_algorithm_mut().set_w_max(cwnd_initial_f64 * 10.0); + // Verify cwnd hasn't increased (so acked_bytes wasn't reset). + assert_eq!(cubic.cwnd(), cubic.cwnd_initial()); - _ = fill_cwnd(&mut cubic, 0, now()); - ack_packet(&mut cubic, 0, now(), &mut cc_stats); + let acked_bytes_before = cubic.acked_bytes(); + assert!(acked_bytes_before > 0); + + // Trigger the congestion event. + trigger(&mut cubic, now, &mut cc_stats); + + // Verify acked_bytes was reduced by the correct factor. + let expected = acked_bytes_before * beta / Cubic::BETA_USIZE_DIVISOR; + assert_eq!(cubic.acked_bytes(), expected); +} +#[test] +fn acked_bytes_reduced_on_loss() { + acked_bytes_reduced_on_congestion_event( + |cc, _, stats| packet_lost(cc, 2, stats), + Cubic::BETA_USIZE_DIVIDEND, + ); +} + +#[test] +fn acked_bytes_reduced_on_ecn_ce() { + acked_bytes_reduced_on_congestion_event( + |cc, now, stats| ecn_ce(cc, 2, now, stats), + Cubic::BETA_USIZE_DIVIDEND_ECN, + ); +} + +#[test] +fn congestion_event_congestion_avoidance_fast_convergence() { + let (mut cubic, mut cc_stats) = setup_congestion_avoidance(true); + + let cwnd_initial_f64 = convert_to_f64(cubic.cwnd_initial()); assert_within( cubic.cc_algorithm().w_max(), cwnd_initial_f64 * 10.0, @@ -353,7 +385,7 @@ fn congestion_event_congestion_avoidance_fast_convergence() { f64::EPSILON, ); assert_eq!(cubic.cwnd(), cwnd_after_loss(cubic.cwnd_initial())); - assert_eq!(cc_stats.congestion_events_loss, 1); + assert_eq!(cc_stats.congestion_events[CongestionEvent::Loss], 1); } #[test] diff --git a/third_party/rust/neqo-transport/src/connection/mod.rs b/third_party/rust/neqo-transport/src/connection/mod.rs @@ -445,7 +445,7 @@ impl Connection { role, version: conn_params.get_versions().initial(), state: State::Init, - paths: Paths::default(), + paths: Paths::new(conn_params.pmtud_enabled()), cid_manager, tps: Rc::clone(&tphandler), zero_rtt_state: ZeroRttState::Init, @@ -1059,7 +1059,7 @@ impl Connection { if let Some(path) = self.paths.primary() { let lost = self.loss_recovery.timeout(&path, now); self.handle_lost_packets(&lost); - qlog::packets_lost(&self.qlog, &lost, now); + qlog::packets_lost(&mut self.qlog, &lost, now); } if self.release_resumption_token_timer.is_some() { @@ -1447,7 +1447,7 @@ impl Connection { .set_initial(self.conn_params.get_versions().initial()); mem::swap(self, &mut c); qlog::client_version_information_negotiated( - &self.qlog, + &mut self.qlog, self.conn_params.get_versions().all(), supported, version, @@ -1636,7 +1636,7 @@ impl Connection { path: &PathRef, tos: Tos, remote: SocketAddr, - packet: &packet::Public, + packet: &packet::Decrypted, packet_number: packet::Number, migrate: bool, now: Instant, @@ -1733,7 +1733,7 @@ impl Connection { self.stats.borrow_mut().packets_rx += 1; self.stats.borrow_mut().dscp_rx[tos.into()] += 1; let slc_len = slc.len(); - let (mut packet, remainder) = + let (packet, remainder) = match packet::Public::decode(slc, self.cid_manager.decoder().as_ref()) { Ok((packet, remainder)) => { #[cfg(feature = "build-fuzzing-corpus")] @@ -1784,11 +1784,11 @@ impl Connection { match self.process_packet(path, &payload, now) { Ok(migrate) => { self.postprocess_packet( - path, tos, remote, &packet, pn, migrate, now, + path, tos, remote, &payload, pn, migrate, now, ); } Err(e) => { - self.ensure_error_path(path, &packet, now); + self.ensure_error_path(path, &payload, now); return Err(e); } } @@ -1800,9 +1800,10 @@ impl Connection { ); return Err(Error::ProtocolViolation); } + dcid = Some(ConnectionId::from(payload.dcid())); } Err(e) => { - match e { + match e.error { Error::KeysPending(epoch) => { // This packet can't be decrypted because we don't have the keys yet. // Don't check this packet for a stateless reset, just return. @@ -1812,7 +1813,7 @@ impl Connection { } Error::KeysExhausted => { // Exhausting read keys is fatal. - return Err(e); + return Err(e.error); } Error::KeysDiscarded(epoch) => { // This was a valid-appearing Initial packet: maybe probe with @@ -1825,13 +1826,13 @@ impl Connection { // Decryption failure, or not having keys is not fatal. // If the state isn't available, or we can't decrypt the packet, drop // the rest of the datagram on the floor, but don't generate an error. - self.check_stateless_reset(path, packet.data(), dcid.is_none(), now)?; + self.check_stateless_reset(path, e.data, dcid.is_none(), now)?; self.stats.borrow_mut().pkt_dropped("Decryption failure"); - qlog::packet_dropped(&self.qlog, &packet, now); + qlog::packet_dropped(&mut self.qlog, &e, now); + dcid = Some(e.dcid); } } slc = remainder; - dcid = Some(ConnectionId::from(packet.dcid())); } self.check_stateless_reset(path, &d, dcid.is_none(), now)?; Ok(()) @@ -1972,7 +1973,7 @@ impl Connection { /// After an error, a permanent path is needed to send the `CONNECTION_CLOSE`. /// This attempts to ensure that this exists. As the connection is now /// temporary, there is no reason to do anything special here. - fn ensure_error_path(&mut self, path: &PathRef, packet: &packet::Public, now: Instant) { + fn ensure_error_path(&mut self, path: &PathRef, packet: &packet::Decrypted, now: Instant) { path.borrow_mut().set_valid(now); if self.paths.is_temporary(path) { // First try to fill in handshake details. @@ -1986,7 +1987,7 @@ impl Connection { } } - fn start_handshake(&mut self, path: &PathRef, packet: &packet::Public, now: Instant) { + fn start_handshake(&mut self, path: &PathRef, packet: &packet::Decrypted, now: Instant) { qtrace!("[{self}] starting handshake"); debug_assert_eq!(packet.packet_type(), packet::Type::Initial); self.remote_initial_source_cid = Some(ConnectionId::from(packet.scid())); @@ -2256,13 +2257,9 @@ impl Connection { fn can_grease_quic_bit(&self) -> bool { let tph = self.tps.borrow(); - tph.remote_handshake().as_ref().map_or_else( - || { - tph.remote_0rtt() - .is_some_and(|r| r.get_empty(GreaseQuicBit)) - }, - |r| r.get_empty(GreaseQuicBit), - ) + tph.remote_handshake() + .as_ref() + .is_some_and(|r| r.get_empty(GreaseQuicBit)) } /// Write the frames that are exchanged in the application data space. @@ -2463,9 +2460,11 @@ impl Connection { && !coalesced // Only send PMTUD probes using non-coalesced packets. && full_mtu { - path.borrow_mut() - .pmtud_mut() - .send_probe(builder, &mut self.stats.borrow_mut()); + path.borrow_mut().pmtud_mut().send_probe( + builder, + &mut tokens, + &mut self.stats.borrow_mut(), + ); ack_eliciting = true; } self.write_appdata_frames(builder, &mut tokens, now); @@ -2590,15 +2589,10 @@ impl Connection { break; } - // Determine how we are sending packets (PTO, etc..). - let profile = self.loss_recovery.send_profile(&path.borrow(), now); - qdebug!("[{self}] output_path send_profile {profile:?}"); - match self.output_dgram_on_path( path, now, closing_frame.take(), - &profile, Encoder::new_borrowed_vec(&mut send_buffer), packet_tos, )? { @@ -2651,7 +2645,6 @@ impl Connection { path: &PathRef, now: Instant, closing_frame: Option<&ClosingFrame>, - profile: &SendProfile, mut encoder: Encoder<&mut Vec<u8>>, packet_tos: Tos, ) -> Res<SendOption> { @@ -2660,6 +2653,10 @@ impl Connection { let grease_quic_bit = self.can_grease_quic_bit(); let version = self.version(); + // Determine how we are sending packets (PTO, etc..). + let profile = self.loss_recovery.send_profile(&path.borrow(), now); + qdebug!("[{self}] output_dgram_on_path send_profile {profile:?}"); + // Determine the size limit and padding for this UDP datagram. let limit = if path.borrow().pmtud().needs_probe() { needs_padding = true; @@ -2714,7 +2711,7 @@ impl Connection { self.write_closing_frames(close, &mut builder, space, now, path, &mut tokens); } else { (tokens, ack_eliciting, padded) = - self.write_frames(path, space, profile, &mut builder, header_start != 0, now); + self.write_frames(path, space, &profile, &mut builder, header_start != 0, now); } if builder.packet_empty() { // Nothing to include in this packet. @@ -2853,10 +2850,10 @@ impl Connection { qdebug!("[{self}] client_start"); debug_assert_eq!(self.role, Role::Client); if let Some(path) = self.paths.primary() { - qlog::client_connection_started(&self.qlog, &path, now); + qlog::client_connection_started(&mut self.qlog, &path, now); } qlog::client_version_information_initiated( - &self.qlog, + &mut self.qlog, self.conn_params.get_versions(), now, ); @@ -2956,7 +2953,7 @@ impl Connection { self.cid_manager.set_limit(max_active_cids); } self.set_initial_limits(); - qlog::connection_tparams_set(&self.qlog, &self.tps.borrow(), now); + qlog::connection_tparams_set(&mut self.qlog, &self.tps.borrow(), now); Ok(()) } @@ -3299,6 +3296,13 @@ impl Connection { // Report an error if we don't have enough connection IDs. self.ensure_permanent(path, now)?; path.borrow_mut().challenged(data); + // A PATH_CHALLENGE indicates the peer sees a different path, + // so start PMTUD to discover any MTU changes. + if self.conn_params.pmtud_enabled() { + path.borrow_mut() + .pmtud_mut() + .start(now, &mut self.stats.borrow_mut()); + } } Frame::PathResponse { data } => { self.stats.borrow_mut().frame_rx.path_response += 1; @@ -3407,6 +3411,8 @@ impl Connection { self.stats.borrow_mut().datagram_tx.lost += 1; } recovery::Token::EcnEct0 => self.paths.lost_ecn(&mut self.stats.borrow_mut()), + // PMTUD probe loss is handled by the PMTUD state machine. + recovery::Token::PmtudProbe => (), } } } @@ -3474,13 +3480,13 @@ impl Connection { .events .datagram_outcome(dgram_tracker, OutgoingDatagramOutcome::Acked), recovery::Token::EcnEct0 => self.paths.acked_ecn(), - // We only worry when these are lost - recovery::Token::HandshakeDone => (), + // We don't care about these being ACK'ed + recovery::Token::HandshakeDone | recovery::Token::PmtudProbe => (), } } } self.handle_lost_packets(&lost_packets); - qlog::packets_lost(&self.qlog, &lost_packets, now); + qlog::packets_lost(&mut self.qlog, &lost_packets, now); let stats = &mut self.stats.borrow_mut().frame_rx; stats.ack += 1; if let Some(largest_acknowledged) = largest_acknowledged { @@ -3529,7 +3535,7 @@ impl Connection { let path = self.paths.primary().ok_or(Error::NoAvailablePath)?; path.borrow_mut().set_valid(now); // Generate a qlog event that the server connection started. - qlog::server_connection_started(&self.qlog, &path, now); + qlog::server_connection_started(&mut self.qlog, &path, now); } else { self.zero_rtt_state = if self .crypto @@ -3571,7 +3577,7 @@ impl Connection { self.streams.clear_streams(); } self.events.connection_state_change(state); - qlog::connection_state_updated(&self.qlog, &self.state, now); + qlog::connection_state_updated(&mut self.qlog, &self.state, now); } else if mem::discriminant(&state) != mem::discriminant(&self.state) { // Only tolerate a regression in state if the new state is closing // and the connection is already closed. @@ -3879,7 +3885,7 @@ impl Connection { self.paths.primary().unwrap().borrow().plpmtu() } - fn log_packet(&self, meta: packet::MetaData, now: Instant) { + fn log_packet(&mut self, meta: packet::MetaData, now: Instant) { if log::log_enabled!(log::Level::Debug) { let mut s = String::new(); let mut d = Decoder::from(meta.payload()); @@ -3896,7 +3902,7 @@ impl Connection { qdebug!("[{self}] {meta}{s}"); } - qlog::packet_io(&self.qlog, meta, now); + qlog::packet_io(&mut self.qlog, meta, now); } } diff --git a/third_party/rust/neqo-transport/src/connection/tests/ecn.rs b/third_party/rust/neqo-transport/src/connection/tests/ecn.rs @@ -219,7 +219,7 @@ fn debug() { tx: 0 lost 0 lateack 0 ptoack 0 unackdrop 0 cc: ce_loss 0 ce_ecn 0 ce_spurious 0 ss_exit: false - pmtud: 0 sent 0 acked 0 lost 0 change 0 iface_mtu 0 pmtu + pmtud: 0 sent 0 acked 0 lost 0 iface_mtu 0 pmtu resumed: false frames rx: crypto 0 done 0 token 0 close 0 diff --git a/third_party/rust/neqo-transport/src/connection/tests/handshake.rs b/third_party/rust/neqo-transport/src/connection/tests/handshake.rs @@ -38,7 +38,8 @@ use crate::{ stats::FrameStats, tparams::{TransportParameter, TransportParameterId::*}, tracking::DEFAULT_LOCAL_ACK_DELAY, - CloseReason, ConnectionParameters, Error, Pmtud, StreamType, Version, + CloseReason, ConnectionParameters, EmptyConnectionIdGenerator, Error, Pmtud, StreamType, + Version, }; const ECH_CONFIG_ID: u8 = 7; @@ -1564,3 +1565,43 @@ fn zero_rtt_with_ech() { assert!(client.tls_info().unwrap().early_data_accepted()); assert!(server.tls_info().unwrap().early_data_accepted()); } + +/// RFC 9287 Section 3.1 states: "A server MUST NOT remember that a client negotiated +/// the extension in a previous connection and set the QUIC Bit to 0 based on that information." +/// +/// This test verifies that the client complies with RFC 9287 Section 3.1 by ensuring +/// it does not grease the QUIC Bit based on cached (0-RTT) transport parameters. +/// Regression test for the `handshakeloss` interop test failure, where client Initial +/// packets with the fixed bit cleared (due to cached parameters) were discarded by the server. +#[test] +fn grease_quic_bit_respects_current_handshake() { + fixture_init(); + + // Create a client connection. + let client = Connection::new_client( + test_fixture::DEFAULT_SERVER_NAME, + test_fixture::DEFAULT_ALPN, + Rc::new(RefCell::new(EmptyConnectionIdGenerator::default())), + DEFAULT_ADDR, + DEFAULT_ADDR, + ConnectionParameters::default(), + now(), + ) + .unwrap(); + + // Simulate having cached 0-RTT transport parameters that include grease_quic_bit. + // In reality, this would come from a previous connection's session ticket. + let mut tp = crate::tparams::TransportParameters::default(); + tp.set_empty(GreaseQuicBit); + client.tps.borrow_mut().set_remote_0rtt(Some(tp)); + + // At this point: + // - We have remote_0rtt params with GreaseQuicBit + // - We do NOT have remote_handshake params (no current handshake confirmation) + + // With only cached 0-RTT params, no greasing is allowed. + assert!( + !client.can_grease_quic_bit(), + "Must not grease with only cached 0-RTT params (RFC 9287 Section 3.1)" + ); +} diff --git a/third_party/rust/neqo-transport/src/connection/tests/pmtud.rs b/third_party/rust/neqo-transport/src/connection/tests/pmtud.rs @@ -4,14 +4,22 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::{cell::RefCell, rc::Rc}; +use std::{ + cell::RefCell, + net::{IpAddr, Ipv6Addr, SocketAddr}, + rc::Rc, +}; +use neqo_common::Datagram; use test_fixture::{fixture_init, now, DEFAULT_ADDR_V4}; use super::Connection; use crate::{ - connection::tests::{connect, default_server, fill_stream, CountingConnectionIdGenerator}, - ConnectionParameters, StreamType, + connection::tests::{ + connect, default_server, fill_stream, new_client, new_server, send_something, + CountingConnectionIdGenerator, DEFAULT_RTT, + }, + ConnectionParameters, Output, Pmtud, StreamType, }; /// Test that one can reach the maximum MTU with GSO enabled. @@ -55,3 +63,131 @@ fn gso_with_max_mtu() { client.process_input(ack.unwrap(), now()); } } + +/// Simulates VPN by changing the source address of a datagram to an IPv6 VPN endpoint. +fn via_vpn(d: &Datagram) -> Datagram { + // Use an IPv6 address since the default test connection uses IPv6. + const VPN_ADDR: SocketAddr = SocketAddr::new( + IpAddr::V6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)), + 12345, + ); + Datagram::new(VPN_ADDR, d.destination(), d.tos(), &d[..]) +} + +/// Exchanges packets between sender and receiver until PMTUD settles, +/// dropping any packets larger than `mtu`. +fn drive_pmtud( + sender: &mut Connection, + receiver: &mut Connection, + mtu: usize, + mut now: std::time::Instant, +) -> std::time::Instant { + if let Ok(stream_id) = sender.stream_create(StreamType::UniDi) { + fill_stream(sender, stream_id); + } + loop { + match sender.process_output(now) { + Output::Datagram(d) => { + if d.len() <= mtu { + receiver.process_input(d, now); + } + } + Output::Callback(t) => { + // Get ACKs from receiver. + while let Some(d) = receiver.process_output(now).dgram() { + if d.len() <= mtu { + sender.process_input(d, now); + } + } + if t >= DEFAULT_RTT { + break; // PMTUD has settled (waiting for PTO or raise timer). + } + now += t; + } + Output::None => break, + } + } + now +} + +/// Tests that when a client goes through a VPN (packets arrive from different IP), +/// the server initiates a path challenge, and both client and server run PMTUD +/// to discover the VPN's lower MTU. +/// +/// Scenario: +/// 1. Connection established, both sides discover initial MTU (1500) +/// 2. VPN is brought up - all traffic now flows through VPN with lower MTU (1400) +/// 3. Server sees packets from new IP, sends `PATH_CHALLENGE`, creates new path, runs PMTUD +/// 4. Client receives `PATH_CHALLENGE`, which triggers PMTUD on its path +/// 5. Both sides discover the VPN's lower MTU +#[test] +fn vpn_migration_triggers_pmtud() { + fixture_init(); + let mut now = now(); + let mut client = new_client(ConnectionParameters::default().pmtud(true)); + let mut server = new_server(ConnectionParameters::default().pmtud(true)); + let header_size = Pmtud::header_size( + client + .paths + .primary() + .unwrap() + .borrow() + .local_address() + .ip(), + ); + let initial_path_mtu = 1500 - header_size; + let vpn_path_mtu = 1400 - header_size; + + connect(&mut client, &mut server); + assert_eq!(client.plpmtu(), 1232, "PMTU should be IPv6 default"); + assert_eq!(server.plpmtu(), 1232, "PMTU should be IPv6 default"); + + // Drive PMTUD on the initial path. + now = drive_pmtud(&mut client, &mut server, initial_path_mtu, now); + now = drive_pmtud(&mut server, &mut client, initial_path_mtu, now); + assert_eq!(client.plpmtu(), initial_path_mtu); + assert_eq!(server.plpmtu(), initial_path_mtu); + + // VPN is now brought up; client sends data, but from the server's perspective, it now arrives + // from the VPN tunnel endpoint address. + let c1 = send_something(&mut client, now); + let c1_via_vpn = via_vpn(&c1); + + // Server receives packet from "new" source IP (VPN endpoint). + // This triggers path validation (PATH_CHALLENGE) on a new path. + let before_challenge = server.stats().frame_tx.path_challenge; + let s1 = server.process(Some(c1_via_vpn), now).dgram(); + assert!(s1.is_some(), "Server should respond"); + assert_eq!(server.stats().frame_tx.path_challenge, before_challenge + 1); + + // Client receives the PATH_CHALLENGE. This triggers PMTUD restart on its path. + let s1 = s1.unwrap(); + let s1_to_client = Datagram::new(s1.source(), c1.source(), s1.tos(), &s1[..]); + let client_pmtud_tx_before_challenge = client.stats().pmtud_tx; + let before_response = client.stats().frame_tx.path_response; + let c2 = client.process(Some(s1_to_client), now).dgram(); + assert!(c2.is_some(), "Client should respond with PATH_RESPONSE"); + assert_eq!(client.stats().frame_tx.path_response, before_response + 1); + + // Server receives PATH_RESPONSE via VPN. + let c2 = c2.unwrap(); + let c2_via_vpn = via_vpn(&c2); + server.process_input(c2_via_vpn, now); + + // Record PMTUD probe counts before driving traffic on VPN path. + let server_pmtud_tx_before = server.stats().pmtud_tx; + + // Drive PMTUD probing for both sides on the VPN path with smaller MTU. + let now = drive_pmtud(&mut client, &mut server, vpn_path_mtu, now); + drive_pmtud(&mut server, &mut client, vpn_path_mtu, now); + + // Verify server and client sent PMTUD probes on the new path. + assert!(server.stats().pmtud_tx > server_pmtud_tx_before); + assert!(client.stats().pmtud_tx > client_pmtud_tx_before_challenge); + + // Verify both sides' PMTU reflects the VPN path's smaller MTU. + // vpn_path_mtu is 1400; the largest IPv6 search table entry <= 1400 is 1380. + let expected_vpn_mtu = 1380 - header_size; + assert_eq!(server.plpmtu(), expected_vpn_mtu); + assert_eq!(client.plpmtu(), expected_vpn_mtu); +} diff --git a/third_party/rust/neqo-transport/src/crypto.rs b/third_party/rust/neqo-transport/src/crypto.rs @@ -671,12 +671,12 @@ impl CryptoDxState { self.used_pn.end } - pub fn encrypt<'a>( + pub fn encrypt( &mut self, pn: packet::Number, hdr: Range<usize>, - data: &'a mut [u8], - ) -> Res<&'a mut [u8]> { + data: &mut [u8], + ) -> Res<usize> { debug_assert_eq!(self.direction, CryptoDxDirection::Write); qtrace!( "[{self}] encrypt_in_place pn={pn} hdr={} body={}", @@ -699,12 +699,12 @@ impl CryptoDxState { let (prev, data) = data.split_at_mut(hdr.end); // `prev` may have already-encrypted packets this one is being coalesced with. // Use only the actual current header for AAD. - let data = self.aead.encrypt_in_place(pn, &prev[hdr], data)?; + let len = self.aead.encrypt_in_place(pn, &prev[hdr], data)?; - qtrace!("[{self}] encrypt ct={}", hex(&data)); + qtrace!("[{self}] encrypt ct={}", hex(&data[..len])); debug_assert_eq!(pn, self.next_pn()); self.used(pn)?; - Ok(data) + Ok(len) } #[must_use] @@ -712,12 +712,12 @@ impl CryptoDxState { self.aead.expansion() } - pub fn decrypt<'a>( + pub fn decrypt( &mut self, pn: packet::Number, hdr: Range<usize>, - data: &'a mut [u8], - ) -> Res<&'a mut [u8]> { + data: &mut [u8], + ) -> Res<usize> { debug_assert_eq!(self.direction, CryptoDxDirection::Read); qtrace!( "[{self}] decrypt_in_place pn={pn} hdr={} body={}", @@ -726,9 +726,9 @@ impl CryptoDxState { ); self.invoked()?; let (hdr, data) = data.split_at_mut(hdr.end); - let data = self.aead.decrypt_in_place(pn, hdr, data)?; + let len = self.aead.decrypt_in_place(pn, hdr, data)?; self.used(pn)?; - Ok(data) + Ok(len) } #[cfg(not(feature = "disable-encryption"))] diff --git a/third_party/rust/neqo-transport/src/events.rs b/third_party/rust/neqo-transport/src/events.rs @@ -127,9 +127,12 @@ impl ConnectionEvents { } pub fn send_stream_complete(&self, stream_id: StreamId) { - self.remove(|evt| matches!(evt, ConnectionEvent::SendStreamWritable { stream_id: x } if *x == stream_id)); - - self.remove(|evt| matches!(evt, ConnectionEvent::SendStreamStopSending { stream_id: x, .. } if *x == stream_id.as_u64())); + self.remove(|evt| { + matches!(evt, + ConnectionEvent::SendStreamWritable { stream_id: x } | + ConnectionEvent::SendStreamStopSending { stream_id: x, .. } + if *x == stream_id) + }); self.insert(ConnectionEvent::SendStreamComplete { stream_id }); } diff --git a/third_party/rust/neqo-transport/src/fc.rs b/third_party/rust/neqo-transport/src/fc.rs @@ -15,6 +15,7 @@ use std::{ time::{Duration, Instant}, }; +use enum_map::EnumMap; use neqo_common::{qdebug, qtrace, Buffer, Role, MAX_VARINT}; use crate::{ @@ -423,6 +424,12 @@ where max_window, ); + // Debug <https://github.com/mozilla/neqo/issues/3208>. + debug_assert!( + self.max_active >= prev_max_active, + "expect no decrease, self: {self:?}, now: {now:?}, rtt: {rtt:?}, max_window: {max_window}, subject: {subject}" + ); + let increase = self.max_active - prev_max_active; if increase > 0 { qdebug!( @@ -658,17 +665,15 @@ impl DerefMut for RemoteStreamLimit { } } -pub struct RemoteStreamLimits { - bidirectional: RemoteStreamLimit, - unidirectional: RemoteStreamLimit, -} +pub struct RemoteStreamLimits(EnumMap<StreamType, RemoteStreamLimit>); impl RemoteStreamLimits { pub const fn new(local_max_stream_bidi: u64, local_max_stream_uni: u64, role: Role) -> Self { - Self { - bidirectional: RemoteStreamLimit::new(StreamType::BiDi, local_max_stream_bidi, role), - unidirectional: RemoteStreamLimit::new(StreamType::UniDi, local_max_stream_uni, role), - } + // Array order must match StreamType enum order: BiDi, UniDi + Self(EnumMap::from_array([ + RemoteStreamLimit::new(StreamType::BiDi, local_max_stream_bidi, role), + RemoteStreamLimit::new(StreamType::UniDi, local_max_stream_uni, role), + ])) } } @@ -676,50 +681,41 @@ impl Index<StreamType> for RemoteStreamLimits { type Output = RemoteStreamLimit; fn index(&self, index: StreamType) -> &Self::Output { - match index { - StreamType::BiDi => &self.bidirectional, - StreamType::UniDi => &self.unidirectional, - } + &self.0[index] } } impl IndexMut<StreamType> for RemoteStreamLimits { fn index_mut(&mut self, index: StreamType) -> &mut Self::Output { - match index { - StreamType::BiDi => &mut self.bidirectional, - StreamType::UniDi => &mut self.unidirectional, - } + &mut self.0[index] } } pub struct LocalStreamLimits { - bidirectional: SenderFlowControl<StreamType>, - unidirectional: SenderFlowControl<StreamType>, + limits: EnumMap<StreamType, SenderFlowControl<StreamType>>, role_bit: u64, } impl LocalStreamLimits { pub const fn new(role: Role) -> Self { Self { - bidirectional: SenderFlowControl::new(StreamType::BiDi, 0), - unidirectional: SenderFlowControl::new(StreamType::UniDi, 0), + // Array order must match StreamType enum order: BiDi, UniDi + limits: EnumMap::from_array([ + SenderFlowControl::new(StreamType::BiDi, 0), + SenderFlowControl::new(StreamType::UniDi, 0), + ]), role_bit: StreamId::role_bit(role), } } pub fn take_stream_id(&mut self, stream_type: StreamType) -> Option<StreamId> { - let fc = match stream_type { - StreamType::BiDi => &mut self.bidirectional, - StreamType::UniDi => &mut self.unidirectional, - }; + let fc = &mut self.limits[stream_type]; if fc.available() > 0 { let new_stream = fc.used(); fc.consume(1); - let type_bit = match stream_type { - StreamType::BiDi => 0, - StreamType::UniDi => 2, - }; - Some(StreamId::from((new_stream << 2) + type_bit + self.role_bit)) + Some(StreamId::from( + (new_stream << 2) + stream_type as u64 + self.role_bit, + )) } else { fc.blocked(); None @@ -731,19 +727,13 @@ impl Index<StreamType> for LocalStreamLimits { type Output = SenderFlowControl<StreamType>; fn index(&self, index: StreamType) -> &Self::Output { - match index { - StreamType::BiDi => &self.bidirectional, - StreamType::UniDi => &self.unidirectional, - } + &self.limits[index] } } impl IndexMut<StreamType> for LocalStreamLimits { fn index_mut(&mut self, index: StreamType) -> &mut Self::Output { - match index { - StreamType::BiDi => &mut self.bidirectional, - StreamType::UniDi => &mut self.unidirectional, - } + &mut self.limits[index] } } @@ -1234,7 +1224,7 @@ mod test { /// Allow auto-tuning algorithm to be off from actual bandwidth-delay /// product by up to 1KiB. const TOLERANCE: u64 = 1024; - const BW_TOLERANCE: f64 = 0.8; + const BW_TOLERANCE: f64 = 0.6; test_fixture::fixture_init(); diff --git a/third_party/rust/neqo-transport/src/lib.rs b/third_party/rust/neqo-transport/src/lib.rs @@ -11,6 +11,9 @@ use neqo_crypto::Error as CryptoError; use thiserror::Error; mod ackrate; +#[cfg(fuzzing)] +pub mod addr_valid; +#[cfg(not(fuzzing))] mod addr_valid; mod cc; mod cid; @@ -58,7 +61,7 @@ mod tracking; pub mod version; pub use self::{ - cc::CongestionControlAlgorithm, + cc::{CongestionControlAlgorithm, CongestionEvent}, cid::{ ConnectionId, ConnectionIdDecoder, ConnectionIdGenerator, ConnectionIdRef, EmptyConnectionIdGenerator, RandomConnectionIdGenerator, diff --git a/third_party/rust/neqo-transport/src/packet/mod.rs b/third_party/rust/neqo-transport/src/packet/mod.rs @@ -499,10 +499,10 @@ impl<B: Buffer> Builder<B> { self.pad_to(data_end + crypto.expansion(), 0); // Calculate the mask. - let ciphertext = crypto.encrypt(self.pn, self.header.clone(), self.encoder.as_mut())?; - let offset = SAMPLE_OFFSET - self.offsets.pn.len(); + crypto.encrypt(self.pn, self.header.clone(), self.encoder.as_mut())?; // `decode()` already checked that `decoder.remaining() >= SAMPLE_OFFSET + SAMPLE_SIZE`. - let sample = ciphertext[offset..offset + SAMPLE_SIZE] + let sample_start = self.header.end + SAMPLE_OFFSET - self.offsets.pn.len(); + let sample = self.encoder.as_ref()[sample_start..sample_start + SAMPLE_SIZE] .try_into() .map_err(|_| Error::Internal)?; let mask = crypto.compute_mask(sample)?; @@ -794,6 +794,7 @@ impl<'a> Public<'a> { self.data.len() } + #[cfg(feature = "build-fuzzing-corpus")] #[must_use] pub fn data(&self) -> &[u8] { self.data @@ -870,45 +871,65 @@ impl<'a> Public<'a> { /// /// This will return an error if the packet cannot be decrypted. pub fn decrypt( - &mut self, + mut self, crypto: &mut CryptoStates, release_at: Instant, - ) -> Res<Decrypted<'_>> { - let epoch: Epoch = self.packet_type.try_into()?; + ) -> Result<Decrypted<'a>, DecryptionError<'a>> { + let epoch = match self.packet_type.try_into() { + Ok(e) => e, + Err(e) => return Err((self, e).into()), + }; // When we don't have a version, the crypto code doesn't need a version // for lookup, so use the default, but fix it up if decryption succeeds. let version = self.version().unwrap_or_default(); // This has to work in two stages because we need to remove header protection // before picking the keys to use. - if let Some(rx) = crypto.rx_hp(version, epoch) { - // Note that this will dump early, which creates a side-channel. - // This is OK in this case because we the only reason this can - // fail is if the cryptographic module is bad or the packet is - // too small (which is public information). - let (key_phase, pn, header) = self.decrypt_header(rx)?; - let Some(rx) = crypto.rx(version, epoch, key_phase) else { - return Err(Error::Decrypt); - }; - let version = rx.version(); // Version fixup; see above. - let d = rx.decrypt(pn, header, self.data)?; - // If this is the first packet ever successfully decrypted - // using `rx`, make sure to initiate a key update. - if rx.needs_update() { - crypto.key_update_received(release_at)?; + let Some(rx) = crypto.rx_hp(version, epoch) else { + if crypto.rx_pending(epoch) { + return Err((self, Error::KeysPending(epoch)).into()); } - crypto.check_pn_overlap()?; - Ok(Decrypted { - version, - pt: self.packet_type, - pn, - data: d, - }) - } else if crypto.rx_pending(epoch) { - Err(Error::KeysPending(epoch)) - } else { qtrace!("keys for {epoch:?} already discarded"); - Err(Error::KeysDiscarded(epoch)) + return Err((self, Error::KeysDiscarded(epoch)).into()); + }; + // Note that this will dump early, which creates a side-channel. + // This is OK in this case because we the only reason this can + // fail is if the cryptographic module is bad or the packet is + // too small (which is public information). + let (key_phase, pn, header) = match self.decrypt_header(rx) { + Ok(v) => v, + Err(e) => return Err((self, e).into()), + }; + let Some(rx) = crypto.rx(version, epoch, key_phase) else { + return Err((self, Error::Decrypt).into()); + }; + let version = rx.version(); // Version fixup; see above. + let header_end = header.end; + let payload_len = match rx.decrypt(pn, header, self.data) { + Ok(v) => v, + Err(e) => return Err((self, e).into()), + }; + let data = &self.data[header_end..header_end + payload_len]; + // Helper for late errors where `self` is partially borrowed. + let make_err = |error| DecryptionError { + error, + data: self.data, + dcid: self.dcid.clone(), + packet_type: self.packet_type, + }; + // If this is the first packet ever successfully decrypted + // using `rx`, make sure to initiate a key update. + if rx.needs_update() { + crypto.key_update_received(release_at).map_err(make_err)?; } + crypto.check_pn_overlap().map_err(make_err)?; + Ok(Decrypted { + version, + pt: self.packet_type, + pn, + dcid: self.dcid, + scid: self.scid, + data, + }) } /// # Errors @@ -941,11 +962,58 @@ impl fmt::Debug for Public<'_> { } } +/// Error information from a failed decryption attempt. +/// Contains minimal packet information needed for error handling. +#[derive(Debug)] +pub struct DecryptionError<'a> { + /// The error that occurred. + pub error: Error, + /// The original packet data (unchanged since decryption failed). + pub data: &'a [u8], + /// The destination connection ID. + pub dcid: ConnectionId, + /// The packet type. + pub packet_type: Type, +} + +impl<'a> From<(Public<'a>, Error)> for DecryptionError<'a> { + fn from((packet, error): (Public<'a>, Error)) -> Self { + Self { + error, + data: packet.data, + dcid: packet.dcid, + packet_type: packet.packet_type, + } + } +} + +impl DecryptionError<'_> { + #[must_use] + pub const fn len(&self) -> usize { + self.data.len() + } + + // The packet module is made public when the `bench` feature is enabled or we're fuzzing, which + // triggers the `clippy::len_without_is_empty` lint without this. + #[cfg(any(fuzzing, feature = "bench"))] + #[must_use] + pub const fn is_empty(&self) -> bool { + self.data.is_empty() + } + + #[must_use] + pub const fn packet_type(&self) -> Type { + self.packet_type + } +} + pub struct Decrypted<'a> { version: Version, pt: Type, pn: Number, data: &'a [u8], + dcid: ConnectionId, + scid: Option<ConnectionId>, } impl Decrypted<'_> { @@ -963,6 +1031,22 @@ impl Decrypted<'_> { pub const fn pn(&self) -> Number { self.pn } + + #[must_use] + pub fn dcid(&self) -> ConnectionIdRef<'_> { + self.dcid.as_cid_ref() + } + + /// # Panics + /// + /// This will panic if called for a short header packet. + #[must_use] + pub fn scid(&self) -> ConnectionIdRef<'_> { + self.scid + .as_ref() + .expect("should only be called for long header packets") + .as_cid_ref() + } } impl Deref for Decrypted<'_> { @@ -1051,7 +1135,7 @@ mod tests { fixture_init(); let mut padded = SAMPLE_INITIAL.to_vec(); padded.extend_from_slice(EXTRA); - let (mut packet, remainder) = Public::decode(&mut padded, &cid_mgr()).unwrap(); + let (packet, remainder) = Public::decode(&mut padded, &cid_mgr()).unwrap(); assert_eq!(packet.packet_type(), Type::Initial); assert_eq!(&packet.dcid()[..], &[] as &[u8]); assert_eq!(&packet.scid()[..], SERVER_CID); @@ -1141,7 +1225,7 @@ mod tests { fn decode_short() { fixture_init(); let mut sample_short = SAMPLE_SHORT.to_vec(); - let (mut packet, remainder) = Public::decode(&mut sample_short, &cid_mgr()).unwrap(); + let (packet, remainder) = Public::decode(&mut sample_short, &cid_mgr()).unwrap(); assert_eq!(packet.packet_type(), Type::Short); assert!(remainder.is_empty()); let decrypted = packet @@ -1156,7 +1240,7 @@ mod tests { fn decode_short_bad_cid() { fixture_init(); let mut sample_short = SAMPLE_SHORT.to_vec(); - let (mut packet, remainder) = Public::decode( + let (packet, remainder) = Public::decode( &mut sample_short, &RandomConnectionIdGenerator::new(SERVER_CID.len() - 1), ) @@ -1503,7 +1587,7 @@ mod tests { let mut damaged_retry = SAMPLE_RETRY_V1.to_vec(); let last = damaged_retry.len() - 1; - damaged_retry[last] ^= 66; + damaged_retry[last] ^= 0b100_0010; // 66 let (packet, remainder) = Public::decode(&mut damaged_retry, &cid_mgr).unwrap(); assert!(remainder.is_empty()); assert!(!packet.is_valid_retry(&odcid)); @@ -1609,7 +1693,7 @@ mod tests { ]; fixture_init(); let mut packet = PACKET.to_vec(); - let (mut packet, slice) = + let (packet, slice) = Public::decode(&mut packet, &EmptyConnectionIdGenerator::default()).unwrap(); assert!(slice.is_empty()); let decrypted = packet diff --git a/third_party/rust/neqo-transport/src/path.rs b/third_party/rust/neqo-transport/src/path.rs @@ -44,7 +44,7 @@ pub type PathRef = Rc<RefCell<Path>>; /// processing a packet. /// This structure limits its storage and will forget about paths if it /// is exposed to too many paths. -#[derive(Debug, Default)] +#[derive(Debug)] pub struct Paths { /// All of the paths. All of these paths will be permanent. #[expect(clippy::struct_field_names, reason = "This is the best name.")] @@ -62,9 +62,24 @@ pub struct Paths { /// `QLog` handler. qlog: Qlog, + + /// Whether PMTUD is enabled for this connection. + pmtud: bool, } impl Paths { + #[must_use] + pub fn new(pmtud: bool) -> Self { + Self { + paths: Vec::new(), + primary: None, + migration_target: None, + to_retire: Vec::new(), + qlog: Qlog::disabled(), + pmtud, + } + } + /// Find the path for the given addresses. /// This might be a temporary path. pub fn find_path( @@ -264,6 +279,7 @@ impl Paths { /// Set the identified path to be primary. /// This panics if `make_permanent` hasn't been called. + /// If PMTUD is enabled, it will be started on the new primary path. pub fn handle_migration( &mut self, path: &PathRef, @@ -288,6 +304,10 @@ impl Paths { old_path.borrow_mut().probe(stats); // TODO(mt) - suppress probing if the path was valid within 3PTO. } + + if self.pmtud { + path.borrow_mut().pmtud_mut().start(now, stats); + } } /// Select a path to send on. This will select the first path that has @@ -301,6 +321,7 @@ impl Paths { /// A `PATH_RESPONSE` was received. /// Returns `true` if migration occurred. + /// If PMTUD is enabled and migration occurs, it will be started on the new primary path. #[must_use] pub fn path_response(&mut self, response: [u8; 8], now: Instant, stats: &mut Stats) -> bool { // TODO(mt) consider recording an RTT measurement here as we don't train @@ -314,6 +335,9 @@ impl Paths { .take_if(|target| Rc::ptr_eq(target, p)) { drop(self.select_primary(&primary, now)); + if self.pmtud { + primary.borrow_mut().pmtud_mut().start(now, stats); + } return true; } break; @@ -991,7 +1015,7 @@ impl Path { ); stats.rtt_init_guess = true; self.rtt.update( - &self.qlog, + &mut self.qlog, now - sent.time_sent(), Duration::new(0, 0), RttSource::Guesstimate, diff --git a/third_party/rust/neqo-transport/src/pmtud.rs b/third_party/rust/neqo-transport/src/pmtud.rs @@ -5,7 +5,6 @@ // except according to those terms. use std::{ - iter::zip, net::IpAddr, time::{Duration, Instant}, }; @@ -16,7 +15,7 @@ use static_assertions::const_assert; use crate::{ frame::{FrameEncoder as _, FrameType}, packet, - recovery::sent, + recovery::{self, sent}, Stats, }; @@ -55,7 +54,6 @@ pub struct Pmtud { probe_index: usize, probe_count: usize, probe_state: Probe, - loss_counts: [usize; SEARCH_TABLE_LEN], raise_timer: Option<Instant>, } @@ -69,7 +67,8 @@ impl Pmtud { } /// Size of the IPv4/IPv6 and UDP headers, in bytes. - const fn header_size(remote_ip: IpAddr) -> usize { + #[must_use] + pub const fn header_size(remote_ip: IpAddr) -> usize { match remote_ip { IpAddr::V4(_) => 20 + 8, IpAddr::V6(_) => 40 + 8, @@ -88,7 +87,6 @@ impl Pmtud { probe_index, probe_count: 0, probe_state: Probe::NotNeeded, - loss_counts: [0; SEARCH_TABLE_LEN], raise_timer: None, } } @@ -98,7 +96,7 @@ impl Pmtud { if self.probe_state == Probe::NotNeeded && self.raise_timer.is_some_and(|t| now >= t) { qdebug!("PMTUD raise timer fired"); self.raise_timer = None; - self.start(now, stats); + self.next(now, stats); } } @@ -122,10 +120,16 @@ impl Pmtud { } /// Sends a PMTUD probe. - pub fn send_probe<B: Buffer>(&mut self, builder: &mut packet::Builder<B>, stats: &mut Stats) { + pub fn send_probe<B: Buffer>( + &mut self, + builder: &mut packet::Builder<B>, + tokens: &mut recovery::Tokens, + stats: &mut Stats, + ) { // The packet may include ACK-eliciting data already, but rather than check for that, it // seems OK to burn one byte here to simply include a PING. builder.encode_frame(FrameType::Ping, |_| {}); + tokens.push(recovery::Token::PmtudProbe); stats.frame_tx.ping += 1; stats.pmtud_tx += 1; self.probe_count += 1; @@ -137,18 +141,6 @@ impl Pmtud { ); } - #[expect(rustdoc::private_intra_doc_links, reason = "Nicer docs.")] - /// Provides a [`Fn`] that returns true if the packet is a PMTUD probe. - /// - /// Allows filtering packets without holding a reference to [`Pmtud`]. When - /// in doubt, use [`Pmtud::is_probe`]. - pub fn is_probe_filter(&self) -> impl Fn(&sent::Packet) -> bool { - let probe_state = self.probe_state; - let probe_size = self.probe_size(); - - move |p: &sent::Packet| -> bool { probe_state == Probe::Sent && p.len() == probe_size } - } - /// Returns the maximum Packetization Layer Path MTU for the configured /// address family. Note that this ignores the interface MTU. #[expect(clippy::missing_panics_doc, reason = "search table is never empty")] @@ -157,14 +149,9 @@ impl Pmtud { *self.search_table.last().expect("search table is empty") } - /// Returns true if the packet is a PMTUD probe. - fn is_probe(&self, p: &sent::Packet) -> bool { - self.is_probe_filter()(p) - } - /// Count the PMTUD probes included in `pkts`. - fn count_probes(&self, pkts: &[sent::Packet]) -> usize { - pkts.iter().filter(|p| self.is_probe(p)).count() + fn count_probes(pkts: &[sent::Packet]) -> usize { + pkts.iter().filter(|p| p.is_pmtud_probe()).count() } /// Checks whether a PMTUD probe has been acknowledged, and if so, updates the PMTUD state. @@ -175,33 +162,17 @@ impl Pmtud { now: Instant, stats: &mut Stats, ) { - // Reset the loss counts for all packets sizes <= the size of the largest ACKed packet. - let Some(max_len) = acked_pkts.iter().map(sent::Packet::len).max() else { - // No packets were ACKed, nothing to do. - return; - }; - - let idx = self - .search_table - .iter() - .position(|&mtu| mtu > max_len + self.header_size) - .unwrap_or(SEARCH_TABLE_LEN); - self.loss_counts.iter_mut().take(idx).for_each(|c| *c = 0); - - let acked = self.count_probes(acked_pkts); + let acked = Self::count_probes(acked_pkts); if acked == 0 { return; } // A probe was ACKed, confirm the new MTU and try to probe upwards further. - // - // TODO: Maybe we should be tracking stats on a per-probe-size basis rather than just the - // total number of successful probes. stats.pmtud_ack += acked; self.mtu = self.search_table[self.probe_index]; stats.pmtud_pmtu = self.mtu; qdebug!("PMTUD probe of size {} succeeded", self.mtu); - self.start(now, stats); + self.next(now, stats); } /// Stops the PMTUD process, setting the MTU to the largest successful probe size. @@ -211,7 +182,6 @@ impl Pmtud { self.mtu = self.search_table[idx]; // Leading to this MTU stats.pmtud_pmtu = self.mtu; self.probe_count = 0; // Reset the count - self.loss_counts.fill(0); // Reset the loss counts self.raise_timer = Some(now + PMTU_RAISE_TIMER); qinfo!( "PMTUD stopped, PLPMTU is now {}, raise timer {:?}", @@ -221,105 +191,46 @@ impl Pmtud { } /// Checks whether a PMTUD probe has been lost. If it has been lost more than `MAX_PROBES` - /// times, the PMTUD process is stopped. + /// times, the PMTUD process is stopped at the current MTU. pub fn on_packets_lost( &mut self, lost_packets: &[sent::Packet], stats: &mut Stats, now: Instant, ) { - if lost_packets.is_empty() { - return; - } - - let mut increase = [0; SEARCH_TABLE_LEN]; - let mut loss_counts_updated = false; - for p in lost_packets { - let Some(idx) = self - .search_table - .iter() - .position(|&mtu| p.len() + self.header_size <= mtu) - else { - continue; - }; - // Count each lost packet size <= the current MTU only once. Otherwise a burst loss of - // >= MAX_PROBES MTU-sized packets triggers a PMTUD restart. Counting only one of them - // here requires three consecutive loss instances of such sizes to trigger a PMTUD - // restart. - // - // Also, ignore losses of packets <= the minimum QUIC packet size, (`searchtable[0]`), - // since they just increase loss counts across the board, adding to spurious - // PMTUD restarts. - if idx > 0 && (increase[idx] == 0 || p.len() > self.plpmtu()) { - loss_counts_updated = true; - increase[idx] += 1; - } - } - - if !loss_counts_updated { + let lost = Self::count_probes(lost_packets); + if lost == 0 { return; } - - let mut accum = 0; - for (c, incr) in zip(&mut self.loss_counts, increase) { - accum += incr; - *c += accum; - } - - // Track lost probes - let lost = self.count_probes(lost_packets); stats.pmtud_lost += lost; - // Check if any packet sizes have been lost MAX_PROBES times or more. - // - // TODO: It's not clear that MAX_PROBES is the right number for losses of packets that - // aren't PMTU probes. We might want to be more conservative, to avoid spurious PMTUD - // restarts. - let Some(first_failed) = self.loss_counts.iter().position(|&c| c >= MAX_PROBES) else { - // If not, keep going. - if lost > 0 { - // Don't stop the PMTUD process. - self.probe_state = Probe::Needed; - } - return; - }; - - let largest_ok_idx = first_failed - 1; - let largest_ok_mtu = self.search_table[largest_ok_idx]; - qdebug!( - "PMTUD Packet of size > {largest_ok_mtu} lost >= {MAX_PROBES} times, state {:?}", - self.probe_state - ); - if largest_ok_mtu < self.mtu { - // We saw multiple losses of packets <= the current MTU discovery, - // so we need to probe again. To limit connectivity disruptions, we - // start the PMTU discovery from the smallest packet up, rather than - // the failed packet size down. - // - // TODO: If we are declaring losses, that means that we're getting - // packets through. The size of those will put a floor on the MTU. - // We're currently conservative and start from scratch, but we don't - // strictly need to do that. - self.reset(stats); - qdebug!("PMTUD reset and restarting, PLPMTU is now {}", self.mtu); - self.start(now, stats); + if self.probe_count >= MAX_PROBES { + // We've sent MAX_PROBES probes and they were all lost. Stop probing at the + // previous successful MTU. + let ok_idx = self.probe_index.saturating_sub(1); + qdebug!( + "PMTUD probe of size {} failed after {MAX_PROBES} attempts", + self.search_table[self.probe_index] + ); + self.stop(ok_idx, now, stats); } else { - self.stop(largest_ok_idx, now, stats); + // Probe was lost but we haven't exhausted retries yet. + self.probe_state = Probe::Needed; } } - /// Resets the PMTUD process, starting from the smallest probe size. - fn reset(&mut self, stats: &mut Stats) { + /// Starts PMTUD from the minimum MTU, probing upward. + pub fn start(&mut self, now: Instant, stats: &mut Stats) { self.probe_index = 0; self.mtu = self.search_table[self.probe_index]; stats.pmtud_pmtu = self.mtu; - self.loss_counts.fill(0); self.raise_timer = None; - stats.pmtud_change += 1; + qdebug!("PMTUD started, PLPMTU is now {}", self.mtu); + self.next(now, stats); } /// Starts the next upward PMTUD probe. - pub fn start(&mut self, now: Instant, stats: &mut Stats) { + pub fn next(&mut self, now: Instant, stats: &mut Stats) { if self.probe_index == SEARCH_TABLE_LEN - 1 { qdebug!( "PMTUD reached end of search table, i.e. {}, stopping upwards search", @@ -360,7 +271,6 @@ impl Pmtud { mod tests { use std::{ cmp::min, - iter::zip, net::{IpAddr, Ipv4Addr, Ipv6Addr}, time::Instant, }; @@ -372,10 +282,22 @@ mod tests { crypto::CryptoDxState, packet, pmtud::{Probe, PMTU_RAISE_TIMER, SEARCH_TABLE_LEN}, - recovery::{sent, SendProfile}, + recovery::{self, sent, SendProfile}, Pmtud, Stats, }; + /// Test helper to create a sent PMTUD probe packet. + fn make_pmtud_probe(pn: packet::Number, sent_time: Instant, len: usize) -> sent::Packet { + sent::Packet::new( + packet::Type::Short, + pn, + sent_time, + true, + vec![recovery::Token::PmtudProbe], + len, + ) + } + const V4: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED); const V6: IpAddr = IpAddr::V6(Ipv6Addr::UNSPECIFIED); const IFACE_MTUS: &[Option<usize>] = &[ @@ -399,7 +321,6 @@ mod tests { assert!(mtu < pmtud.search_table[idx + 1]); } assert_eq!(Probe::NotNeeded, pmtud.probe_state); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); } #[cfg(test)] @@ -426,14 +347,14 @@ mod tests { let pn = prot.next_pn(); builder.pn(pn, 4); builder.enable_padding(true); - pmtud.send_probe(&mut builder, stats); + pmtud.send_probe(&mut builder, &mut Vec::new(), stats); builder.pad(); let encoder = builder.build(prot).unwrap(); assert_eq!(encoder.len(), pmtud.probe_size()); assert!(!pmtud.needs_probe()); assert_eq!(stats_before.pmtud_tx + 1, stats.pmtud_tx); - let packet = sent::make_packet(pn, now, encoder.len()); + let packet = make_pmtud_probe(pn, now, encoder.len()); if encoder.len() + Pmtud::header_size(addr) <= mtu { pmtud.on_packets_acked(&[packet], now, stats); assert_eq!(stats_before.pmtud_ack + 1, stats.pmtud_ack); @@ -454,7 +375,7 @@ mod tests { let mut stats = Stats::default(); let mut prot = CryptoDxState::test_default(); - pmtud.start(now, &mut stats); + pmtud.next(now, &mut stats); if let Some(iface_mtu) = iface_mtu { assert!(iface_mtu <= pmtud.search_table[1] || pmtud.needs_probe()); @@ -472,20 +393,37 @@ mod tests { (pmtud, stats, prot, now) } - fn find_pmtu_with_reduction(addr: IpAddr, mtu: usize, smaller_mtu: usize) { - assert!(mtu > smaller_mtu); - let (mut pmtud, mut stats, mut prot, now) = find_pmtu(addr, mtu, None); + /// Tests that when the path MTU decreases, PMTUD does not automatically reprobe downward. + /// The raise timer only triggers probing for *larger* MTUs. MTU reductions are not + /// automatically detected by PMTUD; the connection will continue using the old MTU + /// and packets will be lost until the raise timer fires and probing completes at + /// the same or a higher MTU (depending on path conditions). + fn find_pmtu_no_reduction_detection(addr: IpAddr, mtu: usize) { + let (mut pmtud, mut stats, _prot, now) = find_pmtu(addr, mtu, None); - qdebug!("Reducing MTU to {smaller_mtu}"); - while !pmtud.needs_probe() { - pmtud_step(&mut pmtud, &mut stats, &mut prot, addr, smaller_mtu, now); - } + // The current MTU is set. + let current_mtu = pmtud.mtu; + assert_eq!(Probe::NotNeeded, pmtud.probe_state); - // Drive second PMTUD process to completion. - while pmtud.needs_probe() { - pmtud_step(&mut pmtud, &mut stats, &mut prot, addr, smaller_mtu, now); + // Fire the raise timer - this only triggers probing for *higher* MTUs. + qdebug!("Firing raise timer after reaching MTU {current_mtu}"); + let now = now + PMTU_RAISE_TIMER; + pmtud.maybe_fire_raise_timer(now, &mut stats); + + // If we're not at the max MTU, the timer should trigger a probe for a higher MTU. + // If we're at the max MTU (or interface limit), no probe is needed. + if pmtud.probe_index < SEARCH_TABLE_LEN - 1 + && pmtud.search_table[pmtud.probe_index + 1] <= pmtud.iface_mtu + { + // Timer should have started probing for a larger MTU. + assert_eq!(Probe::Needed, pmtud.probe_state); + } else { + // At max MTU, timer doesn't change state. + assert_eq!(Probe::NotNeeded, pmtud.probe_state); } - assert_mtu(&pmtud, smaller_mtu); + + // Regardless, the current MTU should be unchanged. + assert_eq!(current_mtu, pmtud.mtu); } fn find_pmtu_with_increase(addr: IpAddr, mtu: usize, larger_mtu: usize) { @@ -493,7 +431,7 @@ mod tests { let (mut pmtud, mut stats, mut prot, now) = find_pmtu(addr, mtu, None); assert!(larger_mtu >= pmtud.search_table[0]); - pmtud.start(now, &mut stats); + pmtud.next(now, &mut stats); assert!(pmtud.needs_probe()); while pmtud.needs_probe() { @@ -526,16 +464,13 @@ mod tests { } } + /// Tests that the raise timer only probes upward, not downward. #[test] - fn pmtud_with_reduction() { + fn raise_timer_probes_upward_only() { for &addr in &[V4, V6] { for path_mtu in path_mtus() { - let path_mtus = path_mtus(); - let smaller_mtus = path_mtus.iter().filter(|&mtu| *mtu < path_mtu); - for &smaller_mtu in smaller_mtus { - qinfo!("PMTUD for {addr}, path MTU {path_mtu}, smaller path MTU {smaller_mtu}"); - find_pmtu_with_reduction(addr, path_mtu, smaller_mtu); - } + qinfo!("Testing raise timer behavior for {addr}, path MTU {path_mtu}"); + find_pmtu_no_reduction_detection(addr, path_mtu); } } } @@ -554,33 +489,15 @@ mod tests { } } - /// Increments the loss counts for the given search table, based on the given packet size. - fn search_table_inc(pmtud: &Pmtud, loss_counts: &[usize], lost_size: usize) -> Vec<usize> { - zip(pmtud.search_table, loss_counts.iter()) - .map(|(&size, &count)| { - if size >= lost_size + pmtud.header_size { - count + 1 - } else { - count - } - }) - .collect() - } - - /// Asserts that the PMTUD process has restarted. - fn assert_pmtud_restarted(pmtud: &Pmtud) { - assert_eq!(Probe::Needed, pmtud.probe_state); - assert_eq!(pmtud.mtu, pmtud.search_table[0]); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); - } - + /// Tests that losing non-probe packets does not affect PMTUD state. #[test] - fn pmtud_on_packets_lost() { + fn non_probe_loss_ignored() { const MTU: usize = 1500; let now = now(); let mut pmtud = Pmtud::new(V4, Some(MTU)); let mut stats = Stats::default(); - // Start with completed PMTUD with MTU 1500. + + // Complete PMTUD at MTU 1500. pmtud.stop( pmtud .search_table @@ -591,93 +508,31 @@ mod tests { &mut stats, ); assert_mtu(&pmtud, MTU); + let initial_lost = stats.pmtud_lost; - // No packets lost, nothing should change. + // Lose various non-probe packets - should not change PMTUD state. pmtud.on_packets_lost(&[], &mut stats, now); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); + assert_eq!(Probe::NotNeeded, pmtud.probe_state); - // A packet of size 100 was lost, which is smaller than all probe sizes. - // Loss counts should be unchanged. pmtud.on_packets_lost(&[sent::make_packet(0, now, 100)], &mut stats, now); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); - - // A packet of size 100_000 was lost, which is larger than all probe sizes. - // Loss counts should be unchanged. - pmtud.on_packets_lost(&[sent::make_packet(0, now, 100_000)], &mut stats, now); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); - - pmtud.loss_counts.fill(0); // Reset the loss counts. - - // A packet of size 1500 was lost, which should increase loss counts >= 1500 by one. - let plen = MTU - pmtud.header_size; - let mut expected_lc = search_table_inc(&pmtud, &pmtud.loss_counts, plen); - pmtud.on_packets_lost(&[sent::make_packet(0, now, plen)], &mut stats, now); - assert_eq!(expected_lc, pmtud.loss_counts); - - // A packet of size 2000 was lost, which should increase loss counts >= 2000 by one. - expected_lc = search_table_inc(&pmtud, &expected_lc, 2000); - pmtud.on_packets_lost(&[sent::make_packet(0, now, 2000)], &mut stats, now); - assert_eq!(expected_lc, pmtud.loss_counts); - - // A packet of size 5000 was lost, which should increase loss counts >= 5000 by one. There - // have now been MAX_PROBES losses of packets >= 5000. That should stop PMTUD. - expected_lc = search_table_inc(&pmtud, &expected_lc, 5000); - pmtud.on_packets_lost(&[sent::make_packet(0, now, 5000)], &mut stats, now); - assert_mtu(&pmtud, 4095); - expected_lc.fill(0); // Reset the loss counts. - - // Two packets of size 4000 were lost, which should increase loss counts >= 4000 by one. - expected_lc = search_table_inc(&pmtud, &expected_lc, 4000); - pmtud.on_packets_lost( - &[ - sent::make_packet(0, now, 4000), - sent::make_packet(1, now, 4000), - ], - &mut stats, - now, - ); - assert_eq!(expected_lc, pmtud.loss_counts); - - // Two packets of size 2000 were lost, which should increase loss counts >= 2000 by one. - expected_lc = search_table_inc(&pmtud, &expected_lc, 2000); - pmtud.on_packets_lost( - &[ - sent::make_packet(0, now, 2000), - sent::make_packet(1, now, 2000), - ], - &mut stats, - now, - ); - assert_eq!(expected_lc, pmtud.loss_counts); - - // Two more packet of size 1500 were lost. There have now been MAX_PROBES losses of packets - // >= 1500. That should restart PMTUD. - let plen = MTU - pmtud.header_size; - pmtud.on_packets_lost( - &[ - sent::make_packet(0, now, plen), - sent::make_packet(1, now, plen), - ], - &mut stats, - now, - ); - assert_pmtud_restarted(&pmtud); - } + assert_eq!(Probe::NotNeeded, pmtud.probe_state); - /// Zeros the loss counts for the given search table, below the given packet size. - fn search_table_zero(pmtud: &Pmtud, loss_counts: &[usize], sz: usize) -> Vec<usize> { - zip(pmtud.search_table, loss_counts.iter()) - .map(|(&s, &c)| if s <= sz + pmtud.header_size { 0 } else { c }) - .collect() + pmtud.on_packets_lost(&[sent::make_packet(1, now, 1000)], &mut stats, now); + assert_eq!(Probe::NotNeeded, pmtud.probe_state); + + // No probe losses should have been recorded. + assert_eq!(initial_lost, stats.pmtud_lost); } + /// Tests that `ACK`ing non-probe packets does not affect PMTUD state. #[test] - fn pmtud_on_packets_lost_and_acked() { + fn non_probe_ack_ignored() { const MTU: usize = 1500; let now = now(); let mut pmtud = Pmtud::new(V4, Some(MTU)); let mut stats = Stats::default(); - // Start with completed PMTUD with MTU 1500. + + // Complete PMTUD at MTU 1500. pmtud.stop( pmtud .search_table @@ -688,61 +543,19 @@ mod tests { &mut stats, ); assert_mtu(&pmtud, MTU); + let initial_ack = stats.pmtud_ack; - // A packet of size 100 was ACKed, which is smaller than all probe sizes. - // Loss counts should be unchanged. - pmtud.on_packets_acked(&[sent::make_packet(0, now, 100)], now, &mut stats); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); - - // A packet of size 100_000 was ACKed, which is larger than all probe sizes. - // Loss counts should be unchanged. - pmtud.on_packets_acked(&[sent::make_packet(0, now, 100_000)], now, &mut stats); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); - - pmtud.loss_counts.fill(0); // Reset the loss counts. - - // No packets ACKed, nothing should change. + // ACK various non-probe packets - should not change PMTUD state. pmtud.on_packets_acked(&[], now, &mut stats); - assert_eq!([0; SEARCH_TABLE_LEN], pmtud.loss_counts); - - // One packet of size 4000 was lost, which should increase loss counts >= 4000 by one. - let mut expected_lc = search_table_inc(&pmtud, &pmtud.loss_counts, 4000); - pmtud.on_packets_lost(&[sent::make_packet(0, now, 4000)], &mut stats, now); - assert_eq!(expected_lc, pmtud.loss_counts); - - // Now a packet of size 5000 is ACKed, which should reset all loss counts <= 5000. - pmtud.on_packets_acked(&[sent::make_packet(0, now, 5000)], now, &mut stats); - expected_lc = search_table_zero(&pmtud, &pmtud.loss_counts, 5000); - assert_eq!(expected_lc, pmtud.loss_counts); - - // Now, one more packets of size 4000 was lost, which should increase loss counts >= 4000 - // by one. - expected_lc = search_table_inc(&pmtud, &expected_lc, 4000); - pmtud.on_packets_lost(&[sent::make_packet(0, now, 4000)], &mut stats, now); - assert_eq!(expected_lc, pmtud.loss_counts); - - // Now a packet of size 8000 is ACKed, which should reset all loss counts <= 8000. - pmtud.on_packets_acked(&[sent::make_packet(0, now, 8000)], now, &mut stats); - expected_lc = search_table_zero(&pmtud, &pmtud.loss_counts, 8000); - assert_eq!(expected_lc, pmtud.loss_counts); - - // Now, one more packets of size 9000 was lost, which should increase loss counts >= 9000 - // by one. There have now been MAX_PROBES losses of packets >= 8191, but that is larger than - // the current MTU, so nothing will happen. - pmtud.on_packets_lost(&[sent::make_packet(0, now, 9000)], &mut stats, now); - - for _ in 0..2 { - // One packet of size 1400 was lost, which should increase loss counts >= 1400 by one. - expected_lc = search_table_inc(&pmtud, &pmtud.loss_counts, 1400); - pmtud.on_packets_lost(&[sent::make_packet(0, now, 1400)], &mut stats, now); - assert_eq!(expected_lc, pmtud.loss_counts); - } + assert_eq!(Probe::NotNeeded, pmtud.probe_state); - // One packet of size 1400 was lost, which should increase loss counts >= 1400 by one. - pmtud.on_packets_lost(&[sent::make_packet(0, now, 1400)], &mut stats, now); + pmtud.on_packets_acked(&[sent::make_packet(0, now, 100)], now, &mut stats); + assert_eq!(Probe::NotNeeded, pmtud.probe_state); + + pmtud.on_packets_acked(&[sent::make_packet(1, now, 5000)], now, &mut stats); + assert_eq!(Probe::NotNeeded, pmtud.probe_state); - // This was the third loss of a packet <= the current MTU, which should trigger a PMTUD - // restart. - assert_pmtud_restarted(&pmtud); + // No probe ACKs should have been recorded. + assert_eq!(initial_ack, stats.pmtud_ack); } } diff --git a/third_party/rust/neqo-transport/src/qlog.rs b/third_party/rust/neqo-transport/src/qlog.rs @@ -41,8 +41,8 @@ use crate::{ version::{self, Version}, }; -pub fn connection_tparams_set(qlog: &Qlog, tph: &TransportParametersHandler, now: Instant) { - qlog.add_event_data_with_instant( +pub fn connection_tparams_set(qlog: &mut Qlog, tph: &TransportParametersHandler, now: Instant) { + qlog.add_event_at( || { let remote = tph.remote(); #[expect(clippy::cast_possible_truncation, reason = "These are OK.")] @@ -89,16 +89,16 @@ pub fn connection_tparams_set(qlog: &Qlog, tph: &TransportParametersHandler, now ); } -pub fn server_connection_started(qlog: &Qlog, path: &PathRef, now: Instant) { +pub fn server_connection_started(qlog: &mut Qlog, path: &PathRef, now: Instant) { connection_started(qlog, path, now); } -pub fn client_connection_started(qlog: &Qlog, path: &PathRef, now: Instant) { +pub fn client_connection_started(qlog: &mut Qlog, path: &PathRef, now: Instant) { connection_started(qlog, path, now); } -fn connection_started(qlog: &Qlog, path: &PathRef, now: Instant) { - qlog.add_event_data_with_instant( +fn connection_started(qlog: &mut Qlog, path: &PathRef, now: Instant) { + qlog.add_event_at( || { let p = path.deref().borrow(); let ev_data = EventData::ConnectionStarted(ConnectionStarted { @@ -127,8 +127,8 @@ fn connection_started(qlog: &Qlog, path: &PathRef, now: Instant) { clippy::similar_names, reason = "FIXME: 'new and now are similar' hits on MSRV <1.91." )] -pub fn connection_state_updated(qlog: &Qlog, new: &State, now: Instant) { - qlog.add_event_data_with_instant( +pub fn connection_state_updated(qlog: &mut Qlog, new: &State, now: Instant) { + qlog.add_event_at( || { let ev_data = EventData::ConnectionStateUpdated(ConnectionStateUpdated { old: None, @@ -150,11 +150,11 @@ pub fn connection_state_updated(qlog: &Qlog, new: &State, now: Instant) { } pub fn client_version_information_initiated( - qlog: &Qlog, + qlog: &mut Qlog, version_config: &version::Config, now: Instant, ) { - qlog.add_event_data_with_instant( + qlog.add_event_at( || { Some(EventData::VersionInformation(VersionInformation { client_versions: Some( @@ -173,13 +173,13 @@ pub fn client_version_information_initiated( } pub fn client_version_information_negotiated( - qlog: &Qlog, + qlog: &mut Qlog, client: &[Version], server: &[version::Wire], chosen: Version, now: Instant, ) { - qlog.add_event_data_with_instant( + qlog.add_event_at( || { Some(EventData::VersionInformation(VersionInformation { client_versions: Some( @@ -197,12 +197,12 @@ pub fn client_version_information_negotiated( } pub fn server_version_information_failed( - qlog: &Qlog, + qlog: &mut Qlog, server: &[Version], client: version::Wire, now: Instant, ) { - qlog.add_event_data_with_instant( + qlog.add_event_at( || { Some(EventData::VersionInformation(VersionInformation { client_versions: Some(vec![format!("{client:02x}")]), @@ -219,8 +219,8 @@ pub fn server_version_information_failed( ); } -pub fn packet_io(qlog: &Qlog, meta: packet::MetaData, now: Instant) { - qlog.add_event_data_with_instant( +pub fn packet_io(qlog: &mut Qlog, meta: packet::MetaData, now: Instant) { + qlog.add_event_at( || { let mut d = Decoder::from(meta.payload()); let raw = RawInfo { @@ -257,14 +257,13 @@ pub fn packet_io(qlog: &Qlog, meta: packet::MetaData, now: Instant) { now, ); } - -pub fn packet_dropped(qlog: &Qlog, public_packet: &packet::Public, now: Instant) { - qlog.add_event_data_with_instant( +pub fn packet_dropped(qlog: &mut Qlog, decrypt_err: &packet::DecryptionError, now: Instant) { + qlog.add_event_at( || { let header = - PacketHeader::with_type(public_packet.packet_type().into(), None, None, None, None); + PacketHeader::with_type(decrypt_err.packet_type().into(), None, None, None, None); let raw = RawInfo { - length: Some(public_packet.len() as u64), + length: Some(decrypt_err.len() as u64), ..Default::default() }; @@ -280,7 +279,7 @@ pub fn packet_dropped(qlog: &Qlog, public_packet: &packet::Public, now: Instant) ); } -pub fn packets_lost(qlog: &Qlog, pkts: &[sent::Packet], now: Instant) { +pub fn packets_lost(qlog: &mut Qlog, pkts: &[sent::Packet], now: Instant) { qlog.add_event_with_stream(|stream| { for pkt in pkts { let header = @@ -313,10 +312,10 @@ pub enum Metric { PacingRate(u64), } -pub fn metrics_updated(qlog: &Qlog, updated_metrics: &[Metric], now: Instant) { +pub fn metrics_updated(qlog: &mut Qlog, updated_metrics: &[Metric], now: Instant) { debug_assert!(!updated_metrics.is_empty()); - qlog.add_event_data_with_instant( + qlog.add_event_at( || { let mut min_rtt: Option<f32> = None; let mut smoothed_rtt: Option<f32> = None; diff --git a/third_party/rust/neqo-transport/src/recovery/mod.rs b/third_party/rust/neqo-transport/src/recovery/mod.rs @@ -561,7 +561,7 @@ impl Loss { /// Record an RTT sample. fn rtt_sample( - &self, + &mut self, rtt: &mut RttEstimate, send_time: Instant, now: Instant, @@ -573,7 +573,7 @@ impl Loss { RttSource::Ack }; if let Some(sample) = now.checked_duration_since(send_time) { - rtt.update(&self.qlog, sample, ack_delay, source, now); + rtt.update(&mut self.qlog, sample, ack_delay, source, now); } } @@ -853,7 +853,7 @@ impl Loss { if let Some(st) = &mut self.pto_state { st.count_pto(&mut self.stats.borrow_mut()); - qlog::metrics_updated(&self.qlog, &[qlog::Metric::PtoCount(st.count())], now); + qlog::metrics_updated(&mut self.qlog, &[qlog::Metric::PtoCount(st.count())], now); } } diff --git a/third_party/rust/neqo-transport/src/recovery/sent.rs b/third_party/rust/neqo-transport/src/recovery/sent.rs @@ -74,6 +74,14 @@ impl Packet { .any(|t| matches!(t, recovery::Token::EcnEct0)) } + /// Returns `true` if this packet is a PMTUD probe. + #[must_use] + pub fn is_pmtud_probe(&self) -> bool { + self.tokens + .iter() + .any(|t| matches!(t, recovery::Token::PmtudProbe)) + } + /// The time that this packet was sent. #[must_use] pub const fn time_sent(&self) -> Instant { diff --git a/third_party/rust/neqo-transport/src/recovery/token.rs b/third_party/rust/neqo-transport/src/recovery/token.rs @@ -67,4 +67,6 @@ pub enum Token { Datagram(DatagramTracking), /// A packet marked with [`neqo_common::Ecn::Ect0`]. EcnEct0, + /// A PMTUD probe packet. + PmtudProbe, } diff --git a/third_party/rust/neqo-transport/src/rtt.rs b/third_party/rust/neqo-transport/src/rtt.rs @@ -97,7 +97,7 @@ impl RttEstimate { pub fn update( &mut self, - qlog: &Qlog, + qlog: &mut Qlog, mut rtt_sample: Duration, ack_delay: Duration, source: RttSource, diff --git a/third_party/rust/neqo-transport/src/server.rs b/third_party/rust/neqo-transport/src/server.rs @@ -204,6 +204,27 @@ impl Server { self.ech_config.as_ref().map_or(&[], |cfg| &cfg.encoded) } + /// Writes address validation fuzzing corpus data. + #[cfg(feature = "build-fuzzing-corpus")] + fn write_addr_valid_corpus(peer: std::net::SocketAddr, token: &[u8]) { + let mut d = Vec::new(); + match peer.ip() { + std::net::IpAddr::V4(ip) => { + let bytes = ip.octets(); + d.push(u8::try_from(bytes.len()).expect("IP address len fits in u8")); + d.extend_from_slice(&bytes); + } + std::net::IpAddr::V6(ip) => { + let bytes = ip.octets(); + d.push(u8::try_from(bytes.len()).expect("IP address len fits in u8")); + d.extend_from_slice(&bytes); + } + } + d.extend_from_slice(&peer.port().to_be_bytes()); + d.extend_from_slice(token); + neqo_common::write_item_to_fuzzing_corpus("addr_valid", &d); + } + fn handle_initial( &mut self, initial: InitialDetails, @@ -211,6 +232,8 @@ impl Server { now: Instant, ) -> Output { qdebug!("[{self}] Handle initial"); + #[cfg(feature = "build-fuzzing-corpus")] + Self::write_addr_valid_corpus(dgram.source(), &initial.token); let res = self .address_validation .borrow() @@ -351,7 +374,7 @@ impl Server { qwarn!("[{self}] Unable to create connection"); if e == crate::Error::VersionNegotiation { crate::qlog::server_version_information_failed( - &self.create_qlog_trace( + &mut self.create_qlog_trace( orig_dcid.unwrap_or(initial.dst_cid).as_cid_ref(), now, ), @@ -462,7 +485,7 @@ impl Server { ); crate::qlog::server_version_information_failed( - &self.create_qlog_trace(packet.dcid(), now), + &mut self.create_qlog_trace(packet.dcid(), now), self.conn_params.get_versions().all(), packet.wire_version(), now, diff --git a/third_party/rust/neqo-transport/src/stats.rs b/third_party/rust/neqo-transport/src/stats.rs @@ -18,7 +18,7 @@ use enum_map::EnumMap; use neqo_common::{qdebug, Dscp, Ecn}; use strum::IntoEnumIterator as _; -use crate::{ecn, packet}; +use crate::{cc::CongestionEvent, ecn, packet}; #[derive(Default, Clone, PartialEq, Eq)] pub struct FrameStats { @@ -137,14 +137,13 @@ pub struct DatagramStats { /// Congestion Control stats #[derive(Default, Clone, PartialEq, Eq)] pub struct CongestionControlStats { - /// Total number of congestion events caused by packet loss. - pub congestion_events_loss: usize, - /// Total number of congestion events caused by ECN-CE marked packets. - pub congestion_events_ecn: usize, - /// Number of spurious congestion events, where congestion was incorrectly inferred due to - /// packets initially considered lost but subsequently acknowledged. This indicates - /// instances where the congestion control algorithm overreacted to perceived losses. - pub congestion_events_spurious: usize, + /// Total number of congestion events caused by packet loss, total number of + /// congestion events caused by ECN-CE marked packets, and number of + /// spurious congestion events, where congestion was incorrectly inferred + /// due to packets initially considered lost but subsequently acknowledged. + /// The latter indicates instances where the congestion control algorithm + /// overreacted to perceived losses. + pub congestion_events: EnumMap<CongestionEvent, usize>, /// Whether this connection has exited slow start. pub slow_start_exited: bool, } @@ -276,8 +275,6 @@ pub struct Stats { pub pmtud_ack: usize, /// Number of PMTUD probes lost. pub pmtud_lost: usize, - /// Number of times a path MTU changed unexpectedly. - pub pmtud_change: usize, /// MTU of the local interface used for the most recent path. pub pmtud_iface_mtu: usize, /// Probed PMTU of the current path. @@ -388,20 +385,15 @@ impl Debug for Stats { writeln!( f, " cc: ce_loss {} ce_ecn {} ce_spurious {}", - self.cc.congestion_events_loss, - self.cc.congestion_events_ecn, - self.cc.congestion_events_spurious, + self.cc.congestion_events[CongestionEvent::Loss], + self.cc.congestion_events[CongestionEvent::Ecn], + self.cc.congestion_events[CongestionEvent::Spurious], )?; writeln!(f, " ss_exit: {}", self.cc.slow_start_exited)?; writeln!( f, - " pmtud: {} sent {} acked {} lost {} change {} iface_mtu {} pmtu", - self.pmtud_tx, - self.pmtud_ack, - self.pmtud_lost, - self.pmtud_change, - self.pmtud_iface_mtu, - self.pmtud_pmtu + " pmtud: {} sent {} acked {} lost {} iface_mtu {} pmtu", + self.pmtud_tx, self.pmtud_ack, self.pmtud_lost, self.pmtud_iface_mtu, self.pmtud_pmtu )?; writeln!(f, " resumed: {}", self.resumed)?; writeln!(f, " frames rx:")?; diff --git a/third_party/rust/neqo-transport/src/stream_id.rs b/third_party/rust/neqo-transport/src/stream_id.rs @@ -8,13 +8,16 @@ use std::fmt::{self, Display, Formatter}; +use enum_map::Enum; use neqo_common::Role; /// The type of stream, either Bi-Directional or Uni-Directional. -#[derive(PartialEq, Debug, Copy, Clone, PartialOrd, Eq, Ord, Hash)] +/// The discriminant values match the QUIC stream type bits. +#[derive(PartialEq, Debug, Copy, Clone, PartialOrd, Eq, Ord, Hash, Enum)] +#[repr(u64)] pub enum StreamType { - BiDi, - UniDi, + BiDi = 0, + UniDi = 2, } #[derive(Debug, Eq, PartialEq, Clone, Copy, Ord, PartialOrd, Hash, Default)] @@ -28,11 +31,7 @@ impl StreamId { #[must_use] pub const fn init(stream_type: StreamType, role: Role) -> Self { - let type_val = match stream_type { - StreamType::BiDi => 0, - StreamType::UniDi => 2, - }; - Self(type_val + Self::role_bit(role)) + Self(stream_type as u64 + Self::role_bit(role)) } #[must_use] diff --git a/third_party/rust/neqo-transport/tests/server.rs b/third_party/rust/neqo-transport/tests/server.rs @@ -157,7 +157,7 @@ fn duplicate_initial_new_path() { assert_eq!(*client.state(), State::Init); let initial = client.process_output(now()).dgram().unwrap(); let other = Datagram::new( - SocketAddr::new(initial.source().ip(), initial.source().port() ^ 23), + SocketAddr::new(initial.source().ip(), initial.source().port() ^ 0b1_01110), // 23 initial.destination(), initial.tos(), &initial[..], diff --git a/third_party/rust/neqo-udp/.cargo-checksum.json b/third_party/rust/neqo-udp/.cargo-checksum.json @@ -1 +1 @@ -{"files":{"Cargo.toml":"db443fbc30c0baf702a98b4611d16828055df2d167817a55d0f75830536e26e7","build.rs":"bf57cd35a78f636c14c442c1926abc2deca3d137e9d207e4f2f960f5b8363b07","src/lib.rs":"bb87c16ab8587eb2e3a68b948de6cc0d36a469194f243bfd6b33fcf31c9e6a2d"},"package":null} -\ No newline at end of file +{"files":{"Cargo.toml":"2f595650572db01bd5ec0d45a900298030bafe6fb7166e90f0472597231ccc7d","build.rs":"bf57cd35a78f636c14c442c1926abc2deca3d137e9d207e4f2f960f5b8363b07","src/lib.rs":"ad49b59a5d96f8fc6f73fa98704da1be8212a7c1c24bfdb206eec44ee0a93638"},"package":null} +\ No newline at end of file diff --git a/third_party/rust/neqo-udp/Cargo.toml b/third_party/rust/neqo-udp/Cargo.toml @@ -13,7 +13,7 @@ edition = "2021" rust-version = "1.81.0" name = "neqo-udp" -version = "0.20.0" +version = "0.21.0" authors = ["The Neqo Authors <necko@mozilla.com>"] build = "build.rs" autolib = false diff --git a/third_party/rust/neqo-udp/src/lib.rs b/third_party/rust/neqo-udp/src/lib.rs @@ -241,7 +241,7 @@ impl<S: SocketRef> Socket<S> { send_inner(&self.state, (&self.inner).into(), d) } - // TODO: Not used in neqo, but Gecko calls it. Needs a test to call it. + /// Returns the maximum number of GSO segments supported by this socket. pub fn max_gso_segments(&self) -> usize { self.state.max_gso_segments() } @@ -349,6 +349,55 @@ mod tests { Ok(()) } + #[test] + #[cfg(unix)] + fn is_emsgsize_true_for_emsgsize() { + let err = io::Error::from_raw_os_error(libc::EMSGSIZE); + assert!(is_emsgsize(&err)); + } + + #[test] + #[cfg(unix)] + fn is_emsgsize_false_for_other_errors() { + let err = io::Error::from_raw_os_error(libc::EAGAIN); + assert!(!is_emsgsize(&err)); + } + + #[test] + #[cfg(windows)] + fn is_emsgsize_true_for_wsaemsgsize() { + let err = io::Error::from_raw_os_error(windows::Win32::Networking::WinSock::WSAEMSGSIZE.0); + assert!(is_emsgsize(&err)); + } + + #[test] + #[cfg(windows)] + fn is_emsgsize_true_for_wsaeinval() { + let err = io::Error::from_raw_os_error(windows::Win32::Networking::WinSock::WSAEINVAL.0); + assert!(is_emsgsize(&err)); + } + + #[test] + #[cfg(windows)] + fn is_emsgsize_false_for_other_windows_errors() { + let err = + io::Error::from_raw_os_error(windows::Win32::Networking::WinSock::WSAEWOULDBLOCK.0); + assert!(!is_emsgsize(&err)); + } + + #[test] + fn is_emsgsize_false_for_non_os_error() { + let err = io::Error::other("test error"); + assert!(!is_emsgsize(&err)); + } + + #[test] + fn max_gso_segments_returns_at_least_one() -> Result<(), io::Error> { + let s = socket()?; + assert!(s.max_gso_segments() >= 1); + Ok(()) + } + /// Expect [`Socket::recv`] to handle multiple [`Datagram`]s on GRO read. #[test] #[cfg_attr( @@ -364,7 +413,7 @@ mod tests { let receiver = socket()?; let receiver_addr: SocketAddr = "127.0.0.1:0".parse().unwrap(); - let max_gso_segments = sender.state.max_gso_segments(); + let max_gso_segments = sender.max_gso_segments(); let msg = vec![0xAB; SEGMENT_SIZE * max_gso_segments]; let batch = DatagramBatch::new( sender.inner.local_addr()?,