From ccd40f0224691b35b28802ab1969c727e30c1c27 Mon Sep 17 00:00:00 2001 From: Eric Wendland Date: Sun, 5 Jul 2026 18:22:07 +0200 Subject: [PATCH] fix: bound remote iroh line reads --- crates/geth-node/src/lib.rs | 67 +++++++++------------------- crates/geth-node/src/peer_client.rs | 16 +++---- crates/geth-node/src/wire.rs | 60 ++++++++++++++++++++++--- docs/architecture.md | 6 +++ docs/production-readiness-roadmap.md | 10 ++--- 5 files changed, 91 insertions(+), 68 deletions(-) diff --git a/crates/geth-node/src/lib.rs b/crates/geth-node/src/lib.rs index e22164e..1b8884c 100644 --- a/crates/geth-node/src/lib.rs +++ b/crates/geth-node/src/lib.rs @@ -76,7 +76,10 @@ use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::{TcpListener, TcpStream, UnixListener, UnixStream}; use tokio::sync::{mpsc, oneshot}; use tun_rs::{DeviceBuilder, Layer}; -use wire::{finish_iroh_send, read_iroh_line}; +use wire::{ + PEER_CONTROL_LINE_MAX, STREAM_HANDSHAKE_LINE_MAX, WIRE_REQUEST_LINE_MAX, finish_iroh_send, + read_iroh_line, +}; #[derive(Debug, thiserror::Error)] pub enum NodeError { @@ -1128,11 +1131,8 @@ async fn peer_ping(node: &LocalNode, peer_node: &str) -> Result { @@ -5179,14 +5172,9 @@ async fn handle_ssh_proxy_wire_connection( node: LocalNode, remote_endpoint_id: String, mut send: iroh::endpoint::SendStream, - recv: iroh::endpoint::RecvStream, + mut recv: iroh::endpoint::RecvStream, ) -> Result<(), NodeError> { - let mut reader = BufReader::new(recv); - let mut line = String::new(); - reader - .read_line(&mut line) - .await - .map_err(|error| NodeError::IrohPeer(error.to_string()))?; + let line = read_iroh_line(&mut recv, STREAM_HANDSHAKE_LINE_MAX).await?; let request = geth_control::decode_peer_request(&line)?; let PeerControlRequest::SshProxyConnect { peer_card, @@ -5291,7 +5279,6 @@ async fn handle_ssh_proxy_wire_connection( .await .map_err(|error| NodeError::IrohPeer(error.to_string()))?; - let mut recv = reader.into_inner(); let (mut tcp_read, mut tcp_write) = tcp.into_split(); let inbound = async { tokio::io::copy(&mut recv, &mut tcp_write) @@ -5314,14 +5301,9 @@ async fn handle_pipe_wire_connection( node: LocalNode, remote_endpoint_id: String, mut send: iroh::endpoint::SendStream, - recv: iroh::endpoint::RecvStream, + mut recv: iroh::endpoint::RecvStream, ) -> Result<(), NodeError> { - let mut reader = BufReader::new(recv); - let mut line = String::new(); - reader - .read_line(&mut line) - .await - .map_err(|error| NodeError::IrohPeer(error.to_string()))?; + let line = read_iroh_line(&mut recv, WIRE_REQUEST_LINE_MAX).await?; match geth_control::decode_pipe_wire_request(&line)? { PipeWireRequest::Send { peer_card, @@ -5361,7 +5343,7 @@ async fn handle_pipe_wire_connection( bearer_proof, }, send, - reader.into_inner(), + recv, ) .await } @@ -5381,7 +5363,7 @@ async fn handle_pipe_wire_connection( bearer_proof, }, send, - reader.into_inner(), + recv, ) .await } @@ -5392,14 +5374,9 @@ async fn handle_overlay_wire_connection( node: LocalNode, remote_endpoint_id: String, mut send: iroh::endpoint::SendStream, - recv: iroh::endpoint::RecvStream, + mut recv: iroh::endpoint::RecvStream, ) -> Result<(), NodeError> { - let mut reader = BufReader::new(recv); - let mut line = String::new(); - reader - .read_line(&mut line) - .await - .map_err(|error| NodeError::IrohPeer(error.to_string()))?; + let line = read_iroh_line(&mut recv, WIRE_REQUEST_LINE_MAX).await?; let response = match geth_control::decode_overlay_wire_request(&line)? { OverlayWireRequest::Packet { peer_card, diff --git a/crates/geth-node/src/peer_client.rs b/crates/geth-node/src/peer_client.rs index 1ecde06..d735ef1 100644 --- a/crates/geth-node/src/peer_client.rs +++ b/crates/geth-node/src/peer_client.rs @@ -1,6 +1,6 @@ //! Outbound Iroh peer request helpers. -use crate::wire::{finish_iroh_send, read_iroh_line}; +use crate::wire::{PEER_CONTROL_LINE_MAX, WIRE_REQUEST_LINE_MAX, finish_iroh_send, read_iroh_line}; use crate::{LocalNode, NodeError}; use geth_control::{ OverlayWireRequest, OverlayWireResponse, PeerControlRequest, PeerControlResponse, @@ -8,7 +8,6 @@ use geth_control::{ }; use geth_discovery::{DiscoverySource, PeerCard}; use geth_store::Store; -use tokio::io::{AsyncBufReadExt, BufReader}; pub(crate) async fn request_peer_control( node: &LocalNode, @@ -43,7 +42,7 @@ pub(crate) async fn request_peer_control( .connect(node_addr, geth_iroh::ALPN_CONTROL) .await .map_err(|error| NodeError::IrohPeer(error.to_string()))?; - let (mut send, recv) = conn + let (mut send, mut recv) = conn .open_bi() .await .map_err(|error| NodeError::IrohPeer(error.to_string()))?; @@ -56,12 +55,7 @@ pub(crate) async fn request_peer_control( .await .map_err(|error| NodeError::IrohPeer(error.to_string()))?; drop(send); - let mut response_line = String::new(); - let mut reader = BufReader::new(recv); - reader - .read_line(&mut response_line) - .await - .map_err(|error| NodeError::IrohPeer(error.to_string()))?; + let response_line = read_iroh_line(&mut recv, PEER_CONTROL_LINE_MAX).await?; let response = geth_control::decode_peer_response(&response_line)?; match &response { PeerControlResponse::SshCertSynced { @@ -176,7 +170,7 @@ pub(crate) async fn request_pipe_wire( .map_err(|error| NodeError::IrohPeer(error.to_string()))?; finish_iroh_send(&mut send).await?; drop(send); - let text = read_iroh_line(&mut recv, 16 * 1024 * 1024).await?; + let text = read_iroh_line(&mut recv, WIRE_REQUEST_LINE_MAX).await?; let response = geth_control::decode_pipe_wire_response(&text)?; match &response { PipeWireResponse::Sent { @@ -232,7 +226,7 @@ pub(crate) async fn request_overlay_wire( .map_err(|error| NodeError::IrohPeer(error.to_string()))?; finish_iroh_send(&mut send).await?; drop(send); - let text = read_iroh_line(&mut recv, 16 * 1024 * 1024).await?; + let text = read_iroh_line(&mut recv, WIRE_REQUEST_LINE_MAX).await?; let response = geth_control::decode_overlay_wire_response(&text)?; match &response { OverlayWireResponse::PacketAccepted { diff --git a/crates/geth-node/src/wire.rs b/crates/geth-node/src/wire.rs index ce36b74..f74460b 100644 --- a/crates/geth-node/src/wire.rs +++ b/crates/geth-node/src/wire.rs @@ -1,19 +1,34 @@ //! Shared Iroh stream helpers. use crate::NodeError; +use std::time::Duration; + +pub(crate) const PEER_CONTROL_LINE_MAX: usize = 16 * 1024 * 1024; +pub(crate) const WIRE_REQUEST_LINE_MAX: usize = 16 * 1024 * 1024; +pub(crate) const STREAM_HANDSHAKE_LINE_MAX: usize = 64 * 1024; +pub(crate) const IROH_LINE_READ_TIMEOUT: Duration = Duration::from_secs(30); pub(crate) async fn read_iroh_line( recv: &mut iroh::endpoint::RecvStream, max_len: usize, +) -> Result { + tokio::time::timeout(IROH_LINE_READ_TIMEOUT, read_iroh_line_inner(recv, max_len)) + .await + .map_err(|_| { + NodeError::IrohPeer(format!( + "iroh line read timed out after {} seconds", + IROH_LINE_READ_TIMEOUT.as_secs() + )) + })? +} + +async fn read_iroh_line_inner( + recv: &mut iroh::endpoint::RecvStream, + max_len: usize, ) -> Result { let mut bytes = Vec::new(); let mut byte = [0_u8; 1]; loop { - if bytes.len() >= max_len { - return Err(NodeError::IrohPeer(format!( - "iroh response line exceeded {max_len} bytes" - ))); - } let Some(n) = recv .read(&mut byte) .await @@ -29,14 +44,23 @@ pub(crate) async fn read_iroh_line( if n == 0 { continue; } - bytes.push(byte[0]); - if byte[0] == b'\n' { + if push_line_byte(&mut bytes, byte[0], max_len)? { break; } } String::from_utf8(bytes).map_err(|error| NodeError::IrohPeer(error.to_string())) } +fn push_line_byte(bytes: &mut Vec, byte: u8, max_len: usize) -> Result { + if bytes.len() >= max_len { + return Err(NodeError::IrohPeer(format!( + "iroh line exceeded {max_len} bytes" + ))); + } + bytes.push(byte); + Ok(byte == b'\n') +} + pub(crate) async fn finish_iroh_send( send: &mut iroh::endpoint::SendStream, ) -> Result<(), NodeError> { @@ -47,3 +71,25 @@ pub(crate) async fn finish_iroh_send( .map_err(|error| NodeError::IrohPeer(error.to_string()))?; Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn bounded_line_rejects_byte_past_limit_without_appending() { + let mut bytes = b"abc".to_vec(); + let error = push_line_byte(&mut bytes, b'd', 3).expect_err("line should exceed limit"); + + assert_eq!(bytes, b"abc"); + assert!(error.to_string().contains("iroh line exceeded 3 bytes")); + } + + #[test] + fn bounded_line_accepts_newline_within_limit() { + let mut bytes = b"abc".to_vec(); + + assert!(push_line_byte(&mut bytes, b'\n', 4).expect("push newline")); + assert_eq!(bytes, b"abc\n"); + } +} diff --git a/docs/architecture.md b/docs/architecture.md index 91e3221..c4fe1d2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -92,6 +92,12 @@ authenticates the Iroh endpoint and peer-card signature, but it does not authorize any resource module. Protected peer control requests must also prove that the signed peer card binds the observed Iroh EndpointID, then reduce resource auth ops; an EndpointID alone is not accepted as a resource principal. +Remote geth JSONL messages over Iroh are bounded before decoding: +peer-control and module wire requests/responses are limited to 16 MiB, +streaming handshakes for SSH proxy and TCP/Unix pipe forwarding are limited to +64 KiB, and each remote line read has a 30 second timeout. Oversized remote +lines are rejected before the request is decoded or dispatched to a resource +handler. The daemon starts this endpoint during `geth daemon run` and keeps it alive for the daemon lifetime. When endpoint startup succeeds, the Iroh EndpointID is diff --git a/docs/production-readiness-roadmap.md b/docs/production-readiness-roadmap.md index 4c89b40..a92c647 100644 --- a/docs/production-readiness-roadmap.md +++ b/docs/production-readiness-roadmap.md @@ -169,12 +169,12 @@ Goal: finish the authorization and remote-input audit before deployment. - `[ ]` Tests cover bearer secrets with wrong capabilities. - `[ ]` Tests cover bearer attempts to mutate trust graph state. -- `[ ]` Bound all remote input paths. +- `[x]` Bound all remote input paths. Acceptance criteria: - - `[ ]` Remote request payloads have documented size limits. - - `[ ]` Remote stream reads have timeouts or bounded behavior. - - `[ ]` Oversized messages are rejected without state mutation. - - `[ ]` Tests cover oversized payload denial for representative protocols. + - `[x]` Remote request payloads have documented size limits. + - `[x]` Remote stream reads have timeouts or bounded behavior. + - `[x]` Oversized messages are rejected without state mutation. + - `[x]` Tests cover oversized payload denial for representative protocols. - `[ ]` Audit host-opening paths. Acceptance criteria: