diff --git a/crates/shadowsocks/src/relay/mod.rs b/crates/shadowsocks/src/relay/mod.rs index 49d40ada..66b65d10 100644 --- a/crates/shadowsocks/src/relay/mod.rs +++ b/crates/shadowsocks/src/relay/mod.rs @@ -11,6 +11,10 @@ pub mod udprelay; const AEAD2022_MAX_PADDING_SIZE: usize = 900; /// Get a properly AEAD 2022 padding size according to payload's length +/// +/// SIP022: In a request header, either initial payload or padding MUST be present. +/// When making a request header, if payload is not available, add non-zero random length padding. +/// https://shadowsocks.org/doc/sip022.html #[cfg(feature = "aead-cipher-2022")] fn get_aead_2022_padding_size(payload: &[u8]) -> usize { use std::cell::RefCell; @@ -22,8 +26,32 @@ fn get_aead_2022_padding_size(payload: &[u8]) -> usize { } if payload.is_empty() { - PADDING_RNG.with(|rng| rng.borrow_mut().random_range::(0..=AEAD2022_MAX_PADDING_SIZE)) + // A padding size of 0 with an empty payload would be rejected by servers + // (and is a violation of the AEAD-2022 spec), so the range starts from 1. + PADDING_RNG.with(|rng| rng.borrow_mut().random_range::(1..=AEAD2022_MAX_PADDING_SIZE)) } else { 0 } } + +#[cfg(feature = "aead-cipher-2022")] +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_get_aead_2022_padding_size() { + // Non-empty payload needs no padding + assert_eq!(get_aead_2022_padding_size(b"payload"), 0); + + // Empty payload MUST take a non-zero random padding size + // https://github.com/shadowsocks/shadowsocks-rust/issues/2184 + for _ in 0..10000 { + let padding_size = get_aead_2022_padding_size(b""); + assert!( + (1..=AEAD2022_MAX_PADDING_SIZE).contains(&padding_size), + "padding size {padding_size} should be within 1..={AEAD2022_MAX_PADDING_SIZE}" + ); + } + } +} diff --git a/crates/shadowsocks/tests/tcp.rs b/crates/shadowsocks/tests/tcp.rs index 4ae6e1c3..1a6c7325 100644 --- a/crates/shadowsocks/tests/tcp.rs +++ b/crates/shadowsocks/tests/tcp.rs @@ -199,6 +199,45 @@ async fn tcp_tunnel_aead_2022_aes() { .unwrap(); } +// https://github.com/shadowsocks/shadowsocks-rust/issues/2184 +// +// Clients that trigger the handshake with an empty write (server-first protocols) must not +// generate a header with no payload and padding 0, which servers MUST reject according to +// the AEAD-2022 spec (SIP022). +#[cfg(feature = "aead-cipher-2022")] +#[tokio::test] +async fn tcp_aead_2022_handshake_with_empty_payload() { + let _ = env_logger::try_init(); + + const PASSWORD: &str = "3L69X4PF2eSL/JSLkoWnXg=="; + let method = CipherKind::AEAD2022_BLAKE3_AES_128_GCM; + + let target_addr = Address::from(("www.example.com".to_owned(), 80)); + + let svr_cfg = Arc::new( + ServerConfig::new("127.0.0.1:34001".parse::().unwrap(), PASSWORD, method).unwrap(), + ); + + let ctx_server = Context::new_shared(ServerType::Server); + let ctx_local = Context::new_shared(ServerType::Local); + + for _ in 0..1000 { + let (client_side, server_side) = tokio::io::duplex(64 * 1024); + + let mut server = ProxyServerStream::from_stream(ctx_server.clone(), server_side, method, svr_cfg.key()); + let mut client = + ProxyClientStream::from_stream(ctx_local.clone(), client_side, &svr_cfg, target_addr.clone()); + + // NOTE: write_all(&[]) would be a no-op, so call write() directly + // to trigger the handshake with an empty first payload. + let n = client.write(&[]).await.unwrap(); + assert_eq!(n, 0); + + let addr = server.handshake().await.unwrap(); + assert_eq!(addr, target_addr); + } +} + #[cfg(feature = "aead-cipher-2022")] #[tokio::test] async fn tcp_tunnel_aead_2022_chacha20() {