net: relay/firewall robustness and hosted service timeouts#72
Merged
Conversation
The length-prefix reader accepted any uint32 before allocating and reading the declared frame payload. A misbehaving peer (malicious guest on the VM side, or a corrupt network-side stream) could send a 0xFFFFFFFF prefix and force a 4 GiB make/ReadFull allocation. Ethernet frames at the topology MTU never exceed a few KiB — cap at 65535 and treat overflow as a protocol violation that tears down the relay. Test writes an oversized length prefix without a payload and asserts the relay's Run returns with "exceeds maximum" within a short timeout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ParseHeaders returns nil for any EtherType other than IPv4 (0x0800). The relay previously only dropped hdr==nil frames when a DNS hook was attached; without one, IPv6 and other exotic EtherTypes slipped past even a deny-default filter. Callers setting FirewallDefaultAction=Deny expect a closed egress — honor that for v6 too. ARP (0x0806) continues to pass (it is required for VM bootstrap and harmless under deny). Test covers the new case: deny-default with no DNS hook, an IPv6-tagged frame is dropped while an ARP frame in the same session passes. The existing TestRelay_ARPPassthroughWithDenyAll remains green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
http.Server previously ran with no Read/Write/Idle timeouts. A guest that opened connections and stalled request headers or bodies could tie up server goroutines and file descriptors indefinitely — the classic Slowloris shape. Defaults are now applied: 10s ReadHeader, 30s Read, 30s Write, 60s Idle. Overridable per-Service for streaming handlers that legitimately exceed the defaults. Construction is extracted to an unexported newHTTPServer helper so timeout handling can be unit-tested without spinning up the full virtual-network stack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 4 of the hardening series. Three focused changes to the
network edge:
net/firewall/relay.go). The length-prefix reader accepted any
uint32and then allocated /ReadFull'd that many bytes. A misbehaving peer could send0xFFFFFFFFand force a 4 GiB allocation. Cap at 65535 andtreat overflow as a protocol violation that tears down the relay.
(
net/firewall/relay.go). Previously the relay only droppedhdr == nilframes when a DNS hook was attached. Without one,IPv6 and other exotic EtherTypes slipped past even a deny-default
filter. ARP (0x0806) continues to pass — required for VM
bootstrap and harmless under deny.
net/hosted/service.go).http.Serverpreviously ran with no Read/Write/Idle timeouts,giving a misbehaving guest a straightforward Slowloris vector
against the host process. Defaults (10s/30s/30s/60s) applied,
overridable per-Service via new fields. Construction extracted
to an unexported helper so timeouts can be unit-tested without
the full virtual-network stack.
Each commit is self-contained and includes its own tests.
task verifygreen at every commit.Test plan
task verifygreen at every commitTestRelay_RejectsOversizedLengthPrefix— oversized prefixtears down the relay within a short timeout without
attempting the allocation.
TestRelay_DropsNonIPv4UnderDenyDefault— v6 frame droppedunder deny-default without a DNS hook; ARP in the same
session still passes.
TestNewHTTPServer_AppliesDefaults/TestNewHTTPServer_RespectsOverrides— timeouts defaultcorrectly and per-Service overrides take effect.
brood-boxlocalreplace:bbox rebuilds, VM boots, DNS egress works (
api.anthropic.comresolves), HTTPS through the firewall returns a live response
(
api.github.com/zen200).🤖 Generated with Claude Code