From 2a4dbf9e332e6c295467bba37fbb5c7b5fb1a0b2 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Fri, 17 Apr 2026 09:17:38 +0300 Subject: [PATCH 1/3] Cap relay frame length at 65 KiB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- net/firewall/relay.go | 11 +++++++++++ net/firewall/relay_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/net/firewall/relay.go b/net/firewall/relay.go index 15205e4..427e61d 100644 --- a/net/firewall/relay.go +++ b/net/firewall/relay.go @@ -17,6 +17,12 @@ import ( "golang.org/x/sync/errgroup" ) +// maxFrameSize bounds the length prefix the relay will accept from either +// peer. Legitimate traffic at the topology MTU sits well under 64 KiB — +// this cap catches corrupt or malicious length prefixes that would +// otherwise trigger multi-GiB allocations in the subsequent make/ReadFull. +const maxFrameSize = 65535 + // InterceptAction tells the relay what to do with a DNS frame. type InterceptAction uint8 @@ -132,6 +138,11 @@ func (r *Relay) forward(ctx context.Context, src, dst net.Conn, dir Direction) e if frameLen == 0 { continue } + if frameLen > maxFrameSize { + return r.wrapError(ctx, fmt.Errorf( + "frame length %d exceeds maximum %d: peer protocol violation", + frameLen, maxFrameSize)) + } // Grow the frame buffer if needed, reuse otherwise. if uint32(cap(frameBuf)) < frameLen { diff --git a/net/firewall/relay_test.go b/net/firewall/relay_test.go index 1a10285..f030a67 100644 --- a/net/firewall/relay_test.go +++ b/net/firewall/relay_test.go @@ -98,6 +98,41 @@ func TestRelay_EndToEnd(t *testing.T) { <-errCh } +func TestRelay_RejectsOversizedLengthPrefix(t *testing.T) { + t.Parallel() + + filter := NewFilter(nil, Allow) + relay := NewRelay(filter) + + vmApp, vmRelay := net.Pipe() + netRelay, _ := net.Pipe() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + errCh := make(chan error, 1) + go func() { + errCh <- relay.Run(ctx, vmRelay, netRelay) + }() + + // Write a 4-byte big-endian length prefix claiming a 2 MiB frame — + // well above maxFrameSize. Do not send any payload. + var lenBuf [4]byte + binary.BigEndian.PutUint32(lenBuf[:], 2*1024*1024) + _, err := vmApp.Write(lenBuf[:]) + require.NoError(t, err) + + // The relay must terminate with a protocol-violation error rather + // than attempt a multi-MiB allocation and hang on ReadFull. + select { + case err := <-errCh: + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds maximum") + case <-time.After(2 * time.Second): + t.Fatal("relay did not terminate on oversized length prefix") + } +} + func TestRelay_DroppedFrame(t *testing.T) { t.Parallel() From b1b62aff9da15759250406066d351d10742978d5 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Fri, 17 Apr 2026 09:19:10 +0300 Subject: [PATCH 2/3] Drop non-IPv4 non-ARP frames under deny-default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- net/firewall/relay.go | 11 ++++----- net/firewall/relay_test.go | 46 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/net/firewall/relay.go b/net/firewall/relay.go index 427e61d..f3bfef1 100644 --- a/net/firewall/relay.go +++ b/net/firewall/relay.go @@ -184,11 +184,12 @@ func (r *Relay) forward(ctx context.Context, src, dst net.Conn, dir Direction) e } } - // When a DNS hook is active, drop non-IPv4 frames that are not - // ARP (EtherType 0x0806). This prevents IPv6 from bypassing the - // egress policy. Without a DNS hook, non-IPv4 frames pass through - // as before (needed for basic network bootstrapping). - if hdr == nil && r.dnsHook != nil { + // Non-IPv4 frames return hdr == nil from ParseHeaders. Under a + // DNS hook or a deny-default filter, drop them (except ARP) so + // that IPv6 and exotic EtherTypes cannot bypass the egress policy. + // With neither, non-IPv4 frames pass through as before (needed + // for basic network bootstrapping on allow-default setups). + if hdr == nil && (r.dnsHook != nil || r.filter.defaultAction == Deny) { if len(frameBuf) >= 14 { etherType := binary.BigEndian.Uint16(frameBuf[12:14]) if etherType != 0x0806 { // not ARP diff --git a/net/firewall/relay_test.go b/net/firewall/relay_test.go index f030a67..242fa5d 100644 --- a/net/firewall/relay_test.go +++ b/net/firewall/relay_test.go @@ -218,6 +218,52 @@ func TestRelay_ARPPassthroughWithDenyAll(t *testing.T) { <-errCh } +func TestRelay_DropsNonIPv4UnderDenyDefault(t *testing.T) { + t.Parallel() + + // Deny-default with no DNS hook. IPv6 (and any other non-IPv4, non-ARP + // EtherType) would previously pass through as "hdr == nil" without + // being checked against the filter. Callers who set FirewallDefault + // Deny expect a closed egress; honor that for v6 frames. + filter := NewFilter(nil, Deny) + relay := NewRelay(filter) + + vmApp, vmRelay := net.Pipe() + netRelay, netApp := net.Pipe() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + errCh := make(chan error, 1) + go func() { + errCh <- relay.Run(ctx, vmRelay, netRelay) + }() + + // Build a minimal IPv6-tagged frame (EtherType 0x86DD). + v6Frame := make([]byte, 60) + binary.BigEndian.PutUint16(v6Frame[12:14], 0x86DD) + + // Send the v6 frame first; it must be dropped. + _, err := vmApp.Write(buildPrefixedFrame(v6Frame)) + require.NoError(t, err) + + // Follow with an ARP frame; it must still pass (existing guarantee). + arpFrame := make([]byte, 42) + binary.BigEndian.PutUint16(arpFrame[12:14], 0x0806) + _, err = vmApp.Write(buildPrefixedFrame(arpFrame)) + require.NoError(t, err) + + got := readPrefixedFrame(t, netApp) + assert.Equal(t, arpFrame, got, "ARP should still pass under deny-default") + + m := relay.Metrics() + assert.Equal(t, uint64(1), m.FramesForwarded.Load()) + assert.Equal(t, uint64(1), m.FramesDropped.Load(), "v6 frame must have been dropped") + + cancel() + <-errCh +} + func TestRelay_Metrics(t *testing.T) { t.Parallel() From ed8892aab8c4e8fae2c7d57105f343ea66710eb0 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Fri, 17 Apr 2026 09:20:39 +0300 Subject: [PATCH 3/3] Set HTTP timeouts on hosted gateway services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- net/hosted/service.go | 54 +++++++++++++++++++++++++++++++++++--- net/hosted/service_test.go | 34 ++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/net/hosted/service.go b/net/hosted/service.go index 9127efd..f13b853 100644 --- a/net/hosted/service.go +++ b/net/hosted/service.go @@ -14,6 +14,19 @@ import ( "github.com/stacklok/go-microvm/net/topology" ) +// Default HTTP server timeouts for hosted services. These protect the +// host process from a misbehaving or hostile guest that opens +// connections but stalls the request — classic Slowloris / slow-body +// patterns — exhausting goroutines and file descriptors in the caller's +// process. Callers can override any of these per-Service if they ship a +// streaming handler that legitimately takes longer than the default. +const ( + defaultReadHeaderTimeout = 10 * time.Second + defaultReadTimeout = 30 * time.Second + defaultWriteTimeout = 30 * time.Second + defaultIdleTimeout = 60 * time.Second +) + // Service describes an HTTP service to expose inside the virtual network. // // Services always bind to the gateway IP ([topology.GatewayIP], 192.168.127.1) @@ -27,6 +40,43 @@ type Service struct { // Handler is the HTTP handler that serves requests. Handler http.Handler + + // ReadHeaderTimeout bounds the time the server will wait to finish + // reading request headers. Zero uses defaultReadHeaderTimeout. + ReadHeaderTimeout time.Duration + + // ReadTimeout bounds the total time reading a request including + // the body. Zero uses defaultReadTimeout. + ReadTimeout time.Duration + + // WriteTimeout bounds the total time writing the response. Zero + // uses defaultWriteTimeout. + WriteTimeout time.Duration + + // IdleTimeout bounds the time a keep-alive connection may remain + // idle between requests. Zero uses defaultIdleTimeout. + IdleTimeout time.Duration +} + +// timeoutOrDefault returns user if set, else the fallback default. +func timeoutOrDefault(user, fallback time.Duration) time.Duration { + if user > 0 { + return user + } + return fallback +} + +// newHTTPServer constructs an *http.Server for the given Service with +// Slowloris-bounding timeouts applied. Zero-valued timeout fields on +// svc fall back to defaults. +func newHTTPServer(svc Service) *http.Server { + return &http.Server{ + Handler: svc.Handler, + ReadHeaderTimeout: timeoutOrDefault(svc.ReadHeaderTimeout, defaultReadHeaderTimeout), + ReadTimeout: timeoutOrDefault(svc.ReadTimeout, defaultReadTimeout), + WriteTimeout: timeoutOrDefault(svc.WriteTimeout, defaultWriteTimeout), + IdleTimeout: timeoutOrDefault(svc.IdleTimeout, defaultIdleTimeout), + } } // runningService tracks a started service for graceful shutdown. @@ -63,9 +113,7 @@ func (p *Provider) startServices() error { return fmt.Errorf("listen on %s for service %d: %w", addr, i, err) } - srv := &http.Server{ - Handler: svc.Handler, - } + srv := newHTTPServer(svc) p.runningServices = append(p.runningServices, runningService{ server: srv, diff --git a/net/hosted/service_test.go b/net/hosted/service_test.go index bcbf1ce..3d62e97 100644 --- a/net/hosted/service_test.go +++ b/net/hosted/service_test.go @@ -7,6 +7,7 @@ import ( "context" "net/http" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,6 +16,39 @@ import ( propnet "github.com/stacklok/go-microvm/net" ) +func TestNewHTTPServer_AppliesDefaults(t *testing.T) { + t.Parallel() + + srv := newHTTPServer(Service{ + Port: 4483, + Handler: http.NotFoundHandler(), + }) + + assert.Equal(t, defaultReadHeaderTimeout, srv.ReadHeaderTimeout) + assert.Equal(t, defaultReadTimeout, srv.ReadTimeout) + assert.Equal(t, defaultWriteTimeout, srv.WriteTimeout) + assert.Equal(t, defaultIdleTimeout, srv.IdleTimeout) +} + +func TestNewHTTPServer_RespectsOverrides(t *testing.T) { + t.Parallel() + + override := 3 * time.Second + srv := newHTTPServer(Service{ + Port: 4483, + Handler: http.NotFoundHandler(), + ReadHeaderTimeout: override, + ReadTimeout: override, + WriteTimeout: override, + IdleTimeout: override, + }) + + assert.Equal(t, override, srv.ReadHeaderTimeout) + assert.Equal(t, override, srv.ReadTimeout) + assert.Equal(t, override, srv.WriteTimeout) + assert.Equal(t, override, srv.IdleTimeout) +} + func TestAddServiceBeforeStart(t *testing.T) { t.Parallel()