fix(websocket): validate chunk length on write side (WS-04, HY-09)
This commit is contained in:
@@ -41,6 +41,7 @@ use futures::channel::mpsc as futures_mpsc;
|
|||||||
use futures::{SinkExt, StreamExt};
|
use futures::{SinkExt, StreamExt};
|
||||||
use tokio::io::{AsyncRead, AsyncWrite};
|
use tokio::io::{AsyncRead, AsyncWrite};
|
||||||
use tokio::sync::mpsc;
|
use tokio::sync::mpsc;
|
||||||
|
use tokio::sync::oneshot;
|
||||||
|
|
||||||
/// Practical per-WS-message cap. Chunks larger than this are split
|
/// Practical per-WS-message cap. Chunks larger than this are split
|
||||||
/// across multiple WS messages — legal, since the receiver's boundary
|
/// across multiple WS messages — legal, since the receiver's boundary
|
||||||
@@ -57,6 +58,16 @@ const WRITE_SLOTS: usize = 64;
|
|||||||
/// Protocol-error close code for text messages (websocket.md §Framing).
|
/// Protocol-error close code for text messages (websocket.md §Framing).
|
||||||
pub const WS_PROTOCOL_ERROR: u16 = 1002;
|
pub const WS_PROTOCOL_ERROR: u16 = 1002;
|
||||||
|
|
||||||
|
/// Internal-error close code used when the write pump hits a protocol
|
||||||
|
/// violation it cannot recover from (WS-04/HY-09, WS-05).
|
||||||
|
pub const WS_INTERNAL_ERROR: u16 = 1011;
|
||||||
|
|
||||||
|
/// Maximum chunk payload length — the channels protocol's 16 MiB wire
|
||||||
|
/// bound (ADR-052 §5), re-exported from alkcall. The write-side chunk
|
||||||
|
/// parser rejects a header claiming a longer payload instead of waiting
|
||||||
|
/// to accumulate up to ~4 GiB from a misaligned offset.
|
||||||
|
pub use alkcall::channels::wire::MAX_CHUNK_LEN;
|
||||||
|
|
||||||
pub(crate) enum WriteMsg {
|
pub(crate) enum WriteMsg {
|
||||||
Bytes(Vec<u8>),
|
Bytes(Vec<u8>),
|
||||||
/// Close with the given code (e.g. the 1002 text-rejection).
|
/// Close with the given code (e.g. the 1002 text-rejection).
|
||||||
@@ -72,6 +83,8 @@ pub struct WsByteStream {
|
|||||||
eof: bool,
|
eof: bool,
|
||||||
write_tx: futures_mpsc::Sender<WriteMsg>,
|
write_tx: futures_mpsc::Sender<WriteMsg>,
|
||||||
write_open: bool,
|
write_open: bool,
|
||||||
|
write_error: oneshot::Receiver<&'static str>,
|
||||||
|
write_failed: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The WS pump tasks. Dropping this guard detaches them (tokio
|
/// The WS pump tasks. Dropping this guard detaches them (tokio
|
||||||
@@ -109,6 +122,7 @@ pub fn split_ws_to_bytes(socket: WebSocket) -> (WsByteStream, WsPumps) {
|
|||||||
let (mut ws_sink, mut ws_stream) = socket.split();
|
let (mut ws_sink, mut ws_stream) = socket.split();
|
||||||
let (read_tx, read_rx) = mpsc::channel::<Vec<u8>>(READ_SLOTS);
|
let (read_tx, read_rx) = mpsc::channel::<Vec<u8>>(READ_SLOTS);
|
||||||
let (write_tx, mut write_rx) = futures_mpsc::channel::<WriteMsg>(WRITE_SLOTS);
|
let (write_tx, mut write_rx) = futures_mpsc::channel::<WriteMsg>(WRITE_SLOTS);
|
||||||
|
let (write_error_tx, write_error_rx) = oneshot::channel::<&'static str>();
|
||||||
|
|
||||||
#[cfg(feature = "wss")]
|
#[cfg(feature = "wss")]
|
||||||
let read_eof = tokio::sync::watch::channel(false).0;
|
let read_eof = tokio::sync::watch::channel(false).0;
|
||||||
@@ -160,9 +174,19 @@ pub fn split_ws_to_bytes(socket: WebSocket) -> (WsByteStream, WsPumps) {
|
|||||||
if pending.len() < 8 {
|
if pending.len() < 8 {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
let len =
|
let len_bytes = [pending[4], pending[5], pending[6], pending[7]];
|
||||||
u32::from_be_bytes([pending[4], pending[5], pending[6], pending[7]]) as usize;
|
let len = u32::from_be_bytes(len_bytes);
|
||||||
let total = 8 + len;
|
if len > MAX_CHUNK_LEN {
|
||||||
|
let _ = write_error_tx.send("chunk length exceeds MAX_CHUNK_LEN");
|
||||||
|
let _ = ws_sink
|
||||||
|
.send(AxumMessage::Close(Some(CloseFrame {
|
||||||
|
code: WS_INTERNAL_ERROR,
|
||||||
|
reason: "chunk length exceeds MAX_CHUNK_LEN".into(),
|
||||||
|
})))
|
||||||
|
.await;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
let total = 8usize.saturating_add(len as usize);
|
||||||
if pending.len() < total {
|
if pending.len() < total {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@@ -189,6 +213,8 @@ pub fn split_ws_to_bytes(socket: WebSocket) -> (WsByteStream, WsPumps) {
|
|||||||
eof: false,
|
eof: false,
|
||||||
write_tx,
|
write_tx,
|
||||||
write_open: true,
|
write_open: true,
|
||||||
|
write_error: write_error_rx,
|
||||||
|
write_failed: false,
|
||||||
},
|
},
|
||||||
WsPumps {
|
WsPumps {
|
||||||
read_task,
|
read_task,
|
||||||
@@ -248,6 +274,9 @@ impl AsyncWrite for WsByteStream {
|
|||||||
"ws stream shut down",
|
"ws stream shut down",
|
||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
|
if let Some(err) = this.poll_write_error() {
|
||||||
|
return Poll::Ready(Err(err));
|
||||||
|
}
|
||||||
match this.write_tx.poll_ready(cx) {
|
match this.write_tx.poll_ready(cx) {
|
||||||
Poll::Ready(Ok(())) => match this.write_tx.try_send(WriteMsg::Bytes(buf.to_vec())) {
|
Poll::Ready(Ok(())) => match this.write_tx.try_send(WriteMsg::Bytes(buf.to_vec())) {
|
||||||
Ok(()) => Poll::Ready(Ok(buf.len())),
|
Ok(()) => Poll::Ready(Ok(buf.len())),
|
||||||
@@ -278,6 +307,40 @@ impl AsyncWrite for WsByteStream {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
impl WsByteStream {
|
||||||
|
/// Fail the stream with the write pump's protocol error, if one
|
||||||
|
/// has arrived on the error channel (WS-04/HY-09, WS-05) — checked
|
||||||
|
/// before every `poll_write`, so a failed write task surfaces
|
||||||
|
/// loudly instead of leaving the stream pending/flushed silently.
|
||||||
|
fn poll_write_error(&mut self) -> Option<io::Error> {
|
||||||
|
if self.write_failed {
|
||||||
|
return Some(io::Error::new(
|
||||||
|
io::ErrorKind::InvalidData,
|
||||||
|
"ws write pump failed (protocol violation)",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
match self.write_error.try_recv() {
|
||||||
|
Ok(reason) => {
|
||||||
|
self.write_failed = true;
|
||||||
|
self.write_open = false;
|
||||||
|
Some(io::Error::new(
|
||||||
|
io::ErrorKind::InvalidData,
|
||||||
|
format!("ws write pump failed: {reason}"),
|
||||||
|
))
|
||||||
|
}
|
||||||
|
Err(oneshot::error::TryRecvError::Closed) => {
|
||||||
|
self.write_failed = true;
|
||||||
|
self.write_open = false;
|
||||||
|
Some(io::Error::new(
|
||||||
|
io::ErrorKind::BrokenPipe,
|
||||||
|
"ws write pump ended",
|
||||||
|
))
|
||||||
|
}
|
||||||
|
Err(oneshot::error::TryRecvError::Empty) => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Client-side split for a tokio-tungstenite `WebSocketStream`
|
/// Client-side split for a tokio-tungstenite `WebSocketStream`
|
||||||
/// (`from_wss`, ADR-070): the same WS↔byte-stream seam as
|
/// (`from_wss`, ADR-070): the same WS↔byte-stream seam as
|
||||||
/// [`split_ws_to_bytes`], over the tungstenite socket instead of axum's
|
/// [`split_ws_to_bytes`], over the tungstenite socket instead of axum's
|
||||||
@@ -294,6 +357,7 @@ where
|
|||||||
let (mut ws_sink, mut ws_stream) = socket.split();
|
let (mut ws_sink, mut ws_stream) = socket.split();
|
||||||
let (read_tx, read_rx) = mpsc::channel::<Vec<u8>>(READ_SLOTS);
|
let (read_tx, read_rx) = mpsc::channel::<Vec<u8>>(READ_SLOTS);
|
||||||
let (write_tx, mut write_rx) = futures_mpsc::channel::<WriteMsg>(WRITE_SLOTS);
|
let (write_tx, mut write_rx) = futures_mpsc::channel::<WriteMsg>(WRITE_SLOTS);
|
||||||
|
let (write_error_tx, write_error_rx) = oneshot::channel::<&'static str>();
|
||||||
|
|
||||||
let read_eof = tokio::sync::watch::channel(false).0;
|
let read_eof = tokio::sync::watch::channel(false).0;
|
||||||
|
|
||||||
@@ -342,9 +406,21 @@ where
|
|||||||
if pending.len() < 8 {
|
if pending.len() < 8 {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
let len =
|
let len_bytes = [pending[4], pending[5], pending[6], pending[7]];
|
||||||
u32::from_be_bytes([pending[4], pending[5], pending[6], pending[7]]) as usize;
|
let len = u32::from_be_bytes(len_bytes);
|
||||||
let total = 8 + len;
|
if len > MAX_CHUNK_LEN {
|
||||||
|
let _ = write_error_tx.send("chunk length exceeds MAX_CHUNK_LEN");
|
||||||
|
let _ = ws_sink
|
||||||
|
.send(tokio_tungstenite::tungstenite::Message::Close(Some(
|
||||||
|
tokio_tungstenite::tungstenite::protocol::CloseFrame {
|
||||||
|
code: WS_INTERNAL_ERROR.into(),
|
||||||
|
reason: "chunk length exceeds MAX_CHUNK_LEN".into(),
|
||||||
|
},
|
||||||
|
)))
|
||||||
|
.await;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
let total = 8usize.saturating_add(len as usize);
|
||||||
if pending.len() < total {
|
if pending.len() < total {
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@@ -373,6 +449,8 @@ where
|
|||||||
eof: false,
|
eof: false,
|
||||||
write_tx,
|
write_tx,
|
||||||
write_open: true,
|
write_open: true,
|
||||||
|
write_error: write_error_rx,
|
||||||
|
write_failed: false,
|
||||||
},
|
},
|
||||||
WsPumps {
|
WsPumps {
|
||||||
read_task,
|
read_task,
|
||||||
@@ -382,3 +460,55 @@ where
|
|||||||
},
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use tokio::io::AsyncWriteExt;
|
||||||
|
|
||||||
|
/// An 8-byte bogus chunk header claiming `MAX_CHUNK_LEN + 1`
|
||||||
|
/// payload bytes (WS-04/HY-09 acceptance: the parser must fail the
|
||||||
|
/// stream loudly instead of silently waiting for ~4 GiB).
|
||||||
|
fn oversize_header() -> Vec<u8> {
|
||||||
|
let mut header = vec![0u8; 8];
|
||||||
|
header[4..8].copy_from_slice(&(MAX_CHUNK_LEN + 1).to_be_bytes());
|
||||||
|
header
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn tungstenite_write_side_rejects_oversized_chunk_header() {
|
||||||
|
let (client_io, _server_io) = tokio::io::duplex(64);
|
||||||
|
let ws = tokio_tungstenite::WebSocketStream::from_raw_socket(
|
||||||
|
client_io,
|
||||||
|
tokio_tungstenite::tungstenite::protocol::Role::Client,
|
||||||
|
None,
|
||||||
|
)
|
||||||
|
.await;
|
||||||
|
let (mut stream, _pumps) = split_tungstenite_to_bytes(ws);
|
||||||
|
|
||||||
|
stream.write_all(&oversize_header()).await.expect("write accepted");
|
||||||
|
stream.flush().await.expect("flush");
|
||||||
|
|
||||||
|
let err: io::Error;
|
||||||
|
let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5);
|
||||||
|
loop {
|
||||||
|
match stream.write_all(&[0u8; 8]).await {
|
||||||
|
Err(e) => {
|
||||||
|
err = e;
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
Ok(_) => {
|
||||||
|
assert!(
|
||||||
|
tokio::time::Instant::now() < deadline,
|
||||||
|
"stream never failed after the oversized header"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
assert_eq!(err.kind(), io::ErrorKind::InvalidData);
|
||||||
|
assert!(
|
||||||
|
err.to_string().contains("MAX_CHUNK_LEN"),
|
||||||
|
"error names the violation: {err}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user