diff --git a/packages/orchestrator/orchestrator.proto b/packages/orchestrator/orchestrator.proto index b9ce038665..65b8870e8a 100644 --- a/packages/orchestrator/orchestrator.proto +++ b/packages/orchestrator/orchestrator.proto @@ -123,6 +123,17 @@ message SandboxNetworkEgressConfig { repeated string allowed_domains = 3; map rules = 4; + // Opt-in default-deny egress posture. When false (default) egress is + // default-allow and only denied_cidrs/denied_domains subtract from it, which + // is the historical behaviour. When true the sandbox is default-deny: only + // allowed_cidrs/allowed_domains (and the runtime's own control-plane path, + // which is redirected before the egress proxy and never judged here) may + // leave; everything else — TCP, non-DNS UDP and other non-TCP — is dropped. + bool default_deny = 9; + // Domains blocked even under default-allow; takes effect before the + // default-allow fallthrough and is subtractive like denied_cidrs. + repeated string denied_domains = 10; + // BYOP SOCKS5 egress proxy. string egress_proxy_address = 5; string egress_proxy_username = 6; diff --git a/packages/orchestrator/pkg/sandbox/network/default_deny_test.go b/packages/orchestrator/pkg/sandbox/network/default_deny_test.go new file mode 100644 index 0000000000..9b3cda199d --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/network/default_deny_test.go @@ -0,0 +1,71 @@ +package network + +import ( + "context" + "os" + "testing" + + "github.com/google/nftables/expr" + "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" +) + +// TestApplyRules_DefaultDeny verifies the opt-in default-deny posture: +// - the chain is recorded as posture "deny" with the non-DNS UDP mode forced +// to Drop (so the per-slot UDP tally is kept while UDP is dropped); +// - the chain ends in a terminal non-TCP drop, so any non-TCP the operator +// did not put in an allow set is dropped rather than hitting the default +// ACCEPT policy. TCP is never matched by it (TCP leaves via the REDIRECT to +// the userspace proxy, which enforces default-deny there). +// +// Needs root; skips otherwise. +func TestApplyRules_DefaultDeny(t *testing.T) { //nolint:paralleltest // netns + nftables + if os.Geteuid() != 0 { + t.Skip("requires root for netns + nftables") + } + + config, err := ParseConfig() + require.NoError(t, err) + + slot, err := NewSlot("default-deny-test", reserveNSTestIdx(t), config, NewNoopEgressProxy()) + require.NoError(t, err) + + require.NoError(t, slot.CreateNetwork(t.Context())) + t.Cleanup(func() { _ = slot.RemoveNetwork(context.WithoutCancel(t.Context())) }) + + // default-deny with one allowed CIDR; mode Off requested — must be forced to Drop. + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, []string{"203.0.113.0/24"}, nil, true)) + + require.Equal(t, ChainDescriptor{Mode: NonDNSUDPModeDrop, Posture: postureDeny}, slot.Firewall.Installed(), + "default-deny must record posture=deny and force the non-DNS UDP mode to Drop") + + rules := filterChainRules(t, slot.NamespaceID()) + last := rules[len(rules)-1] + + var hasL4ProtoNeqTCP, dropVerdict, ctMatch, setLookup bool + for _, e := range last.Exprs { + switch x := e.(type) { + case *expr.Meta: + if x.Key == expr.MetaKeyL4PROTO { + hasL4ProtoNeqTCP = true + } + case *expr.Cmp: + if x.Op == expr.CmpOpNeq && len(x.Data) == 1 && x.Data[0] == unix.IPPROTO_TCP { + hasL4ProtoNeqTCP = true + } + case *expr.Verdict: + if x.Kind == expr.VerdictDrop { + dropVerdict = true + } + case *expr.Ct: + ctMatch = true + case *expr.Lookup: + setLookup = true + } + } + + require.True(t, hasL4ProtoNeqTCP && dropVerdict, + "the last rule under default-deny must be a non-TCP drop") + require.False(t, ctMatch || setLookup, + "the terminal non-TCP drop must be unconditional on destination/state (only protocol-qualified)") +} diff --git a/packages/orchestrator/pkg/sandbox/network/deny_egress_test.go b/packages/orchestrator/pkg/sandbox/network/deny_egress_test.go index 3696846448..83ba9e2acb 100644 --- a/packages/orchestrator/pkg/sandbox/network/deny_egress_test.go +++ b/packages/orchestrator/pkg/sandbox/network/deny_egress_test.go @@ -97,7 +97,7 @@ func TestHeadFilterRules_LeadEveryPolicy(t *testing.T) { //nolint:paralleltest / require.True(t, summarizeFilterChain(t, slot.NamespaceID()).plainAcceptFirst, "the head rule leads the initial rules") - require.NoError(t, slot.Firewall.ApplyRules(t.Context(), true, NonDNSUDPModeCount, []string{"192.0.2.0/24"}, []string{"0.0.0.0/0"})) + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), true, NonDNSUDPModeCount, []string{"192.0.2.0/24"}, []string{"0.0.0.0/0"}, false)) require.True(t, summarizeFilterChain(t, slot.NamespaceID()).plainAcceptFirst, "the head rule leads a replaced policy, the non-DNS UDP rule included") require.NoError(t, slot.DenyEgress(t.Context())) diff --git a/packages/orchestrator/pkg/sandbox/network/firewall.go b/packages/orchestrator/pkg/sandbox/network/firewall.go index 89a7620d86..76d666b7ef 100644 --- a/packages/orchestrator/pkg/sandbox/network/firewall.go +++ b/packages/orchestrator/pkg/sandbox/network/firewall.go @@ -38,6 +38,7 @@ const ( postureRules = "rules" postureBYOP = "byop" postureIsolated = "isolated" + postureDeny = "deny" ) // NonDNSUDPMode selects what the filter chain does with guest UDP datagrams @@ -221,7 +222,7 @@ func NewFirewall(netNSPath string, tapIf string, orchestratorInternalIP string, conn.AddObj(counter.obj) } - installed := ChainDescriptor{Mode: nonDNSUDPMode, Posture: egressPosture(false, nil, nil)} + installed := ChainDescriptor{Mode: nonDNSUDPMode, Posture: egressPosture(false, nil, nil, false)} live := -1 if nonDNSUDPMode != NonDNSUDPModeOff { live = 0 @@ -249,7 +250,7 @@ func NewFirewall(netNSPath string, tapIf string, orchestratorInternalIP string, } // Install default rules and initial set data in a single flush. - fw.installRules(false, nonDNSUDPMode, counters[0].obj) + fw.installRules(false, nonDNSUDPMode, counters[0].obj, false) if err := fw.bufferUserRules(nil, nil); err != nil { return nil, fmt.Errorf("error while configuring initial data: %w", err) } @@ -273,8 +274,10 @@ func (fw *Firewall) Installed() ChainDescriptor { return fw.installed } -func egressPosture(byop bool, allowedCIDRs, deniedCIDRs []string) string { +func egressPosture(byop bool, allowedCIDRs, deniedCIDRs []string, defaultDeny bool) string { switch { + case defaultDeny: + return postureDeny case byop: return postureBYOP case len(allowedCIDRs)+len(deniedCIDRs) > 0: @@ -747,7 +750,7 @@ func notInSet(ipSet *nftables.Set) *expr.Lookup { // non-TCP only (TCP shifts to the userspace SOCKS5 proxy); otherwise it drops // all protocols. The non-DNS UDP rule counts on counter. Buffer-only; the // caller must Flush. -func (fw *Firewall) installRules(byop bool, nonDNSUDPMode NonDNSUDPMode, counter *nftables.CounterObj) { +func (fw *Firewall) installRules(byop bool, nonDNSUDPMode NonDNSUDPMode, counter *nftables.CounterObj, defaultDeny bool) { // FILTER CHAIN (PREROUTING, priority -150) // 0. EgressProxy.HeadFilterRules, if any // 0. IPv4 UDP to a port other than 53, destination in neither allow set → named counter, DROP in drop mode (absent in off mode) @@ -789,11 +792,41 @@ func (fw *Firewall) installRules(byop bool, nonDNSUDPMode NonDNSUDPMode, counter // Only non-TCP traffic is affected; TCP goes to proxy fw.addNonTCPSetFilterRule(fw.userDenySet.Set(), true) + // Rule 6 (default-deny only): terminal non-TCP drop. Established flows, + // the predefined allow set (orchestrator control IP + extra allowed CIDRs) + // and the user allow set were already accepted above, so what reaches here + // is non-TCP the operator did not allow — dropped. TCP is unaffected (it is + // REDIRECTed to the userspace proxy, which enforces default-deny there). + if defaultDeny { + fw.addNonTCPDefaultDenyRule() + } + // Default policy: ACCEPT - // - Non-TCP not in user sets: allowed (default policy) + // - Non-TCP not in user sets: allowed by default, OR dropped by Rule 6 under default-deny // - TCP: iptables REDIRECT handles TCP traffic to proxy } +// addNonTCPDefaultDenyRule buffers a terminal rule that drops every non-TCP +// packet from the tap interface. Installed last under the default-deny posture, +// after the established/allow-set accepts, so only traffic the operator did not +// explicitly allow reaches it. TCP is never matched (it leaves via the REDIRECT +// to the userspace proxy). Buffer-only; the caller flushes. +func (fw *Firewall) addNonTCPDefaultDenyRule() { + fw.conn.AddRule(&nftables.Rule{ + Table: fw.table, + Chain: fw.filterChain, + Exprs: append(fw.tapIfaceMatch(), + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpNeq, + Register: 1, + Data: []byte{unix.IPPROTO_TCP}, + }, + &expr.Verdict{Kind: expr.VerdictDrop}, + ), + }) +} + // bufferUserRules buffers a full replacement of every firewall set. // Buffer-only; the caller flushes. func (fw *Firewall) bufferUserRules(allowedCIDRs, deniedCIDRs []string) error { @@ -834,7 +867,7 @@ func (fw *Firewall) bufferUserRules(allowedCIDRs, deniedCIDRs []string) error { // // On any failure the conn is replaced via resetConn, so a poisoned batch can // never leak into a later flush. -func (fw *Firewall) ApplyRules(ctx context.Context, byop bool, nonDNSUDPMode NonDNSUDPMode, allowedCIDRs, deniedCIDRs []string) (err error) { +func (fw *Firewall) ApplyRules(ctx context.Context, byop bool, nonDNSUDPMode NonDNSUDPMode, allowedCIDRs, deniedCIDRs []string, defaultDeny bool) (err error) { fw.mu.Lock() defer fw.mu.Unlock() @@ -848,10 +881,17 @@ func (fw *Firewall) ApplyRules(ctx context.Context, byop bool, nonDNSUDPMode Non } }() - next := ChainDescriptor{Mode: nonDNSUDPMode, Posture: egressPosture(byop, allowedCIDRs, deniedCIDRs)} + // Default-deny forces the non-DNS UDP rule into drop mode: the terminal + // non-TCP drop below would drop those datagrams anyway, but keeping the + // counter rule in drop mode preserves the per-slot UDP tally. + if defaultDeny { + nonDNSUDPMode = NonDNSUDPModeDrop + } + + next := ChainDescriptor{Mode: nonDNSUDPMode, Posture: egressPosture(byop, allowedCIDRs, deniedCIDRs, defaultDeny)} return fw.replaceChainLocked(ctx, next, func(counter *nftables.CounterObj) error { - fw.installRules(byop, nonDNSUDPMode, counter) + fw.installRules(byop, nonDNSUDPMode, counter, defaultDeny) return fw.bufferUserRules(allowedCIDRs, deniedCIDRs) }) diff --git a/packages/orchestrator/pkg/sandbox/network/firewall_maxboundary_test.go b/packages/orchestrator/pkg/sandbox/network/firewall_maxboundary_test.go index 32f73ff1ea..cef291bac4 100644 --- a/packages/orchestrator/pkg/sandbox/network/firewall_maxboundary_test.go +++ b/packages/orchestrator/pkg/sandbox/network/firewall_maxboundary_test.go @@ -73,11 +73,11 @@ func TestApplyRules_MaxBoundaryCIDRs(t *testing.T) { //nolint:paralleltest // mu for _, cidr := range cidrs { t.Run("deny/"+cidr, func(t *testing.T) { - require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, nil, []string{cidr}), + require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, nil, []string{cidr}, false), "denyOut %s must apply without error", cidr) }) t.Run("allow/"+cidr, func(t *testing.T) { - require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, []string{cidr}, nil), + require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, []string{cidr}, nil, false), "allowOut %s must apply without error", cidr) }) } @@ -96,11 +96,11 @@ func TestApplyRules_MaxBoundaryCIDRs(t *testing.T) { //nolint:paralleltest // mu } for _, b := range batches { t.Run("deny-batch/"+strings.Join(b, ","), func(t *testing.T) { - require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, nil, b), + require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, nil, b, false), "denyOut %v must apply without error", b) }) t.Run("allow-batch/"+strings.Join(b, ","), func(t *testing.T) { - require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, b, nil), + require.NoErrorf(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, b, nil, false), "allowOut %v must apply without error", b) }) } @@ -133,13 +133,13 @@ func TestApplyRules_MaxBoundaryCIDRs(t *testing.T) { //nolint:paralleltest // mu for _, tc := range overlaps { //nolint:paralleltest // shares the test's single netns/nftables slot, cannot run in parallel name := strings.Join(tc.cidrs, ",") t.Run("deny-overlap/"+name, func(t *testing.T) { - require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, nil, tc.cidrs)) + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, nil, tc.cidrs, false)) elements, err := slot.Firewall.conn.GetSetElements(slot.Firewall.userDenySet.Set()) require.NoError(t, err) assertIPv4SetMembership(t, elements, tc.probes) }) t.Run("allow-overlap/"+name, func(t *testing.T) { - require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, tc.cidrs, nil)) + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeOff, tc.cidrs, nil, false)) elements, err := slot.Firewall.conn.GetSetElements(slot.Firewall.userAllowSet.Set()) require.NoError(t, err) assertIPv4SetMembership(t, elements, tc.probes) diff --git a/packages/orchestrator/pkg/sandbox/network/firewall_udp_packet_test.go b/packages/orchestrator/pkg/sandbox/network/firewall_udp_packet_test.go index 852394dc1b..6bab900016 100644 --- a/packages/orchestrator/pkg/sandbox/network/firewall_udp_packet_test.go +++ b/packages/orchestrator/pkg/sandbox/network/firewall_udp_packet_test.go @@ -312,7 +312,7 @@ func TestFirewall_NonDNSUDP_PacketPath(t *testing.T) { //nolint:paralleltest // fw, err := NewFirewall(h.fwNSPath(), h.tap, config.OrchestratorInSandboxIPAddress, []string{harnessPlatformIP + "/32"}, nil, NonDNSUDPModeCount) require.NoError(t, err) - require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, tenantAllowed, nil)) + require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, tenantAllowed, nil, false)) slot := &Slot{Idx: idx, Firewall: fw, config: config, sandboxID: "sandbox-packet-path"} t.Cleanup(func() { _ = slot.CloseFirewall(context.WithoutCancel(t.Context())) }) @@ -358,7 +358,7 @@ func TestFirewall_NonDNSUDP_PacketPath(t *testing.T) { //nolint:paralleltest // require.Positive(t, snapshot.Bytes) // Drop mode on the live flow: its conntrack entry must not keep it open. - require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeDrop, tenantAllowed, nil)) + require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeDrop, tenantAllowed, nil, false)) require.False(t, sendAndExpectReply(t, flow, "flow-3"), "drop mode drops an established flow") require.False(t, sendAndExpectReply(t, flow, "flow-4")) require.False(t, sendAndExpectReply(t, flow, "flow-5")) @@ -644,7 +644,7 @@ func TestFirewall_HostThreadOperatesOnSlotNamespace(t *testing.T) { //nolint:par require.Equal(t, defaultChain, filterChainShapes(t, h.fwNS), "the default chain is in the slot namespace") require.False(t, hostTableExists(t), "building the firewall adds nothing to the host namespace") - require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, []string{"8.8.8.8/32"})) + require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, []string{"8.8.8.8/32"}, false)) require.Equal(t, []ruleShape{nonDNSUDPCount, establishedNonUDPAccept, establishedReplyAccept, allowSetAccept, denySetDrop, nonTCPUserAllow, nonTCPUserDeny}, filterChainShapes(t, h.fwNS), "the update replaced the chain in the slot namespace") @@ -822,7 +822,7 @@ func TestFirewall_NonDNSUDP_CloseAfterFailedReopenExportsTally(t *testing.T) { / fw.conn.AddRule(&nftables.Rule{Table: missing, Chain: &nftables.Chain{Name: "missing", Table: missing}, Exprs: accept()}) restore, err := forbidNewFileDescriptors() require.NoError(t, err) - applyErr := fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, []string{"8.8.8.8/32"}) + applyErr := fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, []string{"8.8.8.8/32"}, false) restore() require.ErrorIs(t, applyErr, unix.EMFILE, "the reopen must have been the step that failed") @@ -847,7 +847,7 @@ func TestFirewall_FailedConnReopenKeepsOneConn(t *testing.T) { //nolint:parallel restore, err := forbidNewFileDescriptors() require.NoError(t, err) - applyErr := fw.ApplyRules(ctx, false, NonDNSUDPModeOff, nil, []string{"10.0.0.0/8"}) + applyErr := fw.ApplyRules(ctx, false, NonDNSUDPModeOff, nil, []string{"10.0.0.0/8"}, false) restore() require.ErrorIs(t, applyErr, unix.EMFILE, "the reopen must have been the step that failed") require.Same(t, before, fw.conn, "the firewall keeps its one connection") @@ -893,7 +893,7 @@ func TestFirewall_OperationsAfterCloseAreRefused(t *testing.T) { //nolint:parall require.Equal(t, errFirewallClosed, fw.Close(ctx), "a second close") _, err = fw.Harvest() require.Equal(t, errFirewallClosed, err, "a read") - require.Equal(t, errFirewallClosed, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, nil), "a rebuild") + require.Equal(t, errFirewallClosed, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, nil, false), "a rebuild") require.Equal(t, errFirewallClosed, fw.DenyEgress(ctx), "an isolation") } diff --git a/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go b/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go index 0e2405d7a2..693c952200 100644 --- a/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go +++ b/packages/orchestrator/pkg/sandbox/network/firewall_udp_test.go @@ -165,7 +165,7 @@ func TestFirewall_NonDNSUDP_RuleShape(t *testing.T) { //nolint:paralleltest // m {NonDNSUDPModeDrop, []ruleShape{nonDNSUDPDrop, establishedNonUDPAccept, establishedReplyAccept, allowSetAccept, denySetDrop, nonTCPUserAllow, nonTCPUserDeny}}, {NonDNSUDPModeOff, defaultChain}, } { - require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, tc.mode, nil, nil)) + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, tc.mode, nil, nil, false)) require.Equal(t, tc.want, filterChainShapes(t, slot.NamespaceID()), "mode %s", tc.mode) } } @@ -189,11 +189,11 @@ func TestParseNonDNSUDPMode(t *testing.T) { func TestFirewall_NonDNSUDP_BYOPKeepsTCPPath(t *testing.T) { //nolint:paralleltest // mutates the caller's netns via LockOSThread + netns.Set; cannot run in parallel slot := newFirewallTestSlot(t, "fw-non-dns-udp-byop") - require.NoError(t, slot.Firewall.ApplyRules(t.Context(), true, NonDNSUDPModeOff, nil, nil)) + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), true, NonDNSUDPModeOff, nil, nil, false)) byopChain := []ruleShape{establishedNonUDPAccept, establishedReplyAccept, allowSetAccept, nonTCPDenySetDrop, nonTCPUserAllow, nonTCPUserDeny} require.Equal(t, byopChain, filterChainShapes(t, slot.NamespaceID())) - require.NoError(t, slot.Firewall.ApplyRules(t.Context(), true, NonDNSUDPModeDrop, nil, nil)) + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), true, NonDNSUDPModeDrop, nil, nil, false)) got := filterChainShapes(t, slot.NamespaceID()) require.Equal(t, []ruleShape{nonDNSUDPDrop, establishedNonUDPAccept, establishedReplyAccept, allowSetAccept, nonTCPDenySetDrop, nonTCPUserAllow, nonTCPUserDeny}, got) @@ -309,7 +309,7 @@ func ruleHandles(rules []*nftables.Rule) []uint64 { func TestFirewall_DenyEgressReplacesInstalledChain(t *testing.T) { //nolint:paralleltest // mutates the caller's netns via LockOSThread + netns.Set; cannot run in parallel slot := newFirewallTestSlot(t, "fw-deny-egress-descriptor") - require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeDrop, nil, []string{"10.0.0.0/8"})) + require.NoError(t, slot.Firewall.ApplyRules(t.Context(), false, NonDNSUDPModeDrop, nil, []string{"10.0.0.0/8"}, false)) require.Equal(t, ChainDescriptor{Mode: NonDNSUDPModeDrop, Posture: postureRules}, slot.Firewall.Installed()) require.NoError(t, slot.DenyEgress(t.Context())) @@ -324,7 +324,7 @@ func TestFirewall_HarvestSnapshotIsOneChainGeneration(t *testing.T) { //nolint:p fw := slot.Firewall ctx := t.Context() - require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, nil)) + require.NoError(t, fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, nil, false)) const rounds = 50 done := make(chan error, 1) @@ -334,7 +334,7 @@ func TestFirewall_HarvestSnapshotIsOneChainGeneration(t *testing.T) { //nolint:p if i%2 == 1 { denied = []string{"10.0.0.0/8"} } - if err := fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, denied); err != nil { + if err := fw.ApplyRules(ctx, false, NonDNSUDPModeCount, nil, denied, false); err != nil { done <- err return diff --git a/packages/orchestrator/pkg/sandbox/network/slot.go b/packages/orchestrator/pkg/sandbox/network/slot.go index 07affff516..795137ef7f 100644 --- a/packages/orchestrator/pkg/sandbox/network/slot.go +++ b/packages/orchestrator/pkg/sandbox/network/slot.go @@ -414,7 +414,7 @@ func (s *Slot) reapplyNonDNSUDPMode(ctx context.Context) (bool, error) { } egress := s.tenantEgress - if err := fw.ApplyRules(ctx, egress.GetEgressProxyAddress() != "", mode, egress.GetAllowedCidrs(), egress.GetDeniedCidrs()); err != nil { + if err := fw.ApplyRules(ctx, egress.GetEgressProxyAddress() != "", mode, egress.GetAllowedCidrs(), egress.GetDeniedCidrs(), egress.GetDefaultDeny()); err != nil { return false, err } @@ -569,7 +569,9 @@ func (s *Slot) ConfigureInternet(ctx context.Context, network *orchestrator.Sand egress := network.GetEgress() hasUserRules := len(egress.GetAllowedCidrs()) != 0 || len(egress.GetDeniedCidrs()) != 0 || - len(egress.GetAllowedDomains()) != 0 + len(egress.GetAllowedDomains()) != 0 || + len(egress.GetDeniedDomains()) != 0 || + egress.GetDefaultDeny() hasBYOP := egress.GetEgressProxyAddress() != "" s.firewallMu.Lock() @@ -594,7 +596,7 @@ func (s *Slot) ConfigureInternet(ctx context.Context, network *orchestrator.Sand s.firewallCustomRules.Store(true) } - if err := fw.ApplyRules(ctx, hasBYOP, mode, egress.GetAllowedCidrs(), egress.GetDeniedCidrs()); err != nil { + if err := fw.ApplyRules(ctx, hasBYOP, mode, egress.GetAllowedCidrs(), egress.GetDeniedCidrs(), egress.GetDefaultDeny()); err != nil { return err } s.rememberTenant(ctx, egress) @@ -624,7 +626,7 @@ func (s *Slot) UpdateInternet(ctx context.Context, egress *orchestrator.SandboxN // Set before mutating: a partial failure must still trigger cleanup. s.firewallCustomRules.Store(true) - if err := fw.ApplyRules(ctx, hasBYOP, s.currentNonDNSUDPMode(ctx), allowedCIDRs, deniedCIDRs); err != nil { + if err := fw.ApplyRules(ctx, hasBYOP, s.currentNonDNSUDPMode(ctx), allowedCIDRs, deniedCIDRs, egress.GetDefaultDeny()); err != nil { return err } s.rememberTenant(ctx, egress) @@ -680,7 +682,7 @@ func (s *Slot) ResetInternet(ctx context.Context) error { if s.firewallCustomRules.Load() { // Revert BYOP so the next tenant can't inherit non-TCP-only deny rules. - if err := fw.ApplyRules(ctx, false, s.currentNonDNSUDPMode(ctx), nil, nil); err != nil { + if err := fw.ApplyRules(ctx, false, s.currentNonDNSUDPMode(ctx), nil, nil, false); err != nil { return err } } diff --git a/packages/orchestrator/pkg/tcpfirewall/handlers.go b/packages/orchestrator/pkg/tcpfirewall/handlers.go index c54b4bb129..7b078ae3af 100644 --- a/packages/orchestrator/pkg/tcpfirewall/handlers.go +++ b/packages/orchestrator/pkg/tcpfirewall/handlers.go @@ -178,7 +178,7 @@ func proxyWithDial(ctx context.Context, c egressConn, upstreamAddr string, dial // Priority order: // 1. Allow domain / Allow CIDR (if either matches → allow) // 2. Deny domain / Deny CIDR (if either matches → deny) -// 3. Default: allow +// 3. Default: allow, or deny when the egress config opts into default_deny func isEgressAllowed(sbx *sandbox.Sandbox, hostname string, ip net.IP) (bool, MatchType, error) { egress := sbx.Config.GetNetworkEgress() if egress == nil { @@ -207,6 +207,15 @@ func isEgressAllowed(sbx *sandbox.Sandbox, hostname string, ip net.IP) (bool, Ma } } + // Priority 2: Check denied domains + if hostname != noHostnameValue { + for _, domain := range egress.GetDeniedDomains() { + if matchDomain(hostname, domain) { + return false, MatchTypeDomain, nil // Explicitly denied by domain + } + } + } + // Priority 2: Check denied CIDRs for _, cidr := range egress.GetDeniedCidrs() { _, ipNet, err := net.ParseCIDR(cidr) @@ -219,7 +228,14 @@ func isEgressAllowed(sbx *sandbox.Sandbox, hostname string, ip net.IP) (bool, Ma } } - // Default: allow all traffic. + // Priority 3: default posture. Under opt-in default-deny, anything not + // explicitly allowed above is dropped — this also closes the no-SNI / no-Host + // blind pass (cidrOnlyHandler reaches here with no hostname, so only an + // allowed CIDR can let it through). Default-allow is the historical behaviour. + if egress.GetDefaultDeny() { + return false, MatchTypeNone, nil + } + return true, MatchTypeNone, nil } diff --git a/packages/orchestrator/pkg/tcpfirewall/handlers_test.go b/packages/orchestrator/pkg/tcpfirewall/handlers_test.go index 94b500f520..782975dd86 100644 --- a/packages/orchestrator/pkg/tcpfirewall/handlers_test.go +++ b/packages/orchestrator/pkg/tcpfirewall/handlers_test.go @@ -381,6 +381,71 @@ func TestIsEgressAllowed(t *testing.T) { want: true, wantError: false, }, + { + name: "default_deny drops traffic matching no allow rule", + network: &orchestrator.SandboxNetworkConfig{ + Egress: &orchestrator.SandboxNetworkEgressConfig{DefaultDeny: true}, + }, + hostname: "example.com", + ip: net.ParseIP("1.2.3.4"), + want: false, + }, + { + name: "default_deny still allows an explicitly allowed domain", + network: &orchestrator.SandboxNetworkConfig{ + Egress: &orchestrator.SandboxNetworkEgressConfig{ + DefaultDeny: true, + AllowedDomains: []string{"example.com"}, + }, + }, + hostname: "example.com", + ip: net.ParseIP("1.2.3.4"), + want: true, + }, + { + name: "default_deny still allows an explicitly allowed CIDR", + network: &orchestrator.SandboxNetworkConfig{ + Egress: &orchestrator.SandboxNetworkEgressConfig{ + DefaultDeny: true, + AllowedCidrs: []string{"1.2.3.0/24"}, + }, + }, + hostname: "", + ip: net.ParseIP("1.2.3.4"), + want: true, + }, + { + name: "default_deny drops a no-hostname (no-SNI) connection not in an allow CIDR", + network: &orchestrator.SandboxNetworkConfig{ + Egress: &orchestrator.SandboxNetworkEgressConfig{DefaultDeny: true}, + }, + hostname: "", + ip: net.ParseIP("9.9.9.9"), + want: false, + }, + { + name: "denied_domains blocks a domain even under default-allow", + network: &orchestrator.SandboxNetworkConfig{ + Egress: &orchestrator.SandboxNetworkEgressConfig{ + DeniedDomains: []string{"*.evil.com"}, + }, + }, + hostname: "c2.evil.com", + ip: net.ParseIP("1.2.3.4"), + want: false, + }, + { + name: "allowed_domains beats denied_domains (allow has priority)", + network: &orchestrator.SandboxNetworkConfig{ + Egress: &orchestrator.SandboxNetworkEgressConfig{ + AllowedDomains: []string{"safe.evil.com"}, + DeniedDomains: []string{"*.evil.com"}, + }, + }, + hostname: "safe.evil.com", + ip: net.ParseIP("1.2.3.4"), + want: true, + }, } for _, tt := range tests { diff --git a/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go b/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go index b4a0d01a72..21c60ea85a 100644 --- a/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go +++ b/packages/shared/pkg/grpc/orchestrator/orchestrator.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.36.11 -// protoc v7.34.1 +// protoc v3.21.12 // source: orchestrator.proto package orchestrator @@ -715,6 +715,16 @@ type SandboxNetworkEgressConfig struct { DeniedCidrs []string `protobuf:"bytes,2,rep,name=denied_cidrs,json=deniedCidrs,proto3" json:"denied_cidrs,omitempty"` AllowedDomains []string `protobuf:"bytes,3,rep,name=allowed_domains,json=allowedDomains,proto3" json:"allowed_domains,omitempty"` Rules map[string]*SandboxNetworkDomainRules `protobuf:"bytes,4,rep,name=rules,proto3" json:"rules,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` + // Opt-in default-deny egress posture. When false (default) egress is + // default-allow and only denied_cidrs/denied_domains subtract from it, which + // is the historical behaviour. When true the sandbox is default-deny: only + // allowed_cidrs/allowed_domains (and the runtime's own control-plane path, + // which is redirected before the egress proxy and never judged here) may + // leave; everything else — TCP, non-DNS UDP and other non-TCP — is dropped. + DefaultDeny bool `protobuf:"varint,9,opt,name=default_deny,json=defaultDeny,proto3" json:"default_deny,omitempty"` + // Domains blocked even under default-allow; takes effect before the + // default-allow fallthrough and is subtractive like denied_cidrs. + DeniedDomains []string `protobuf:"bytes,10,rep,name=denied_domains,json=deniedDomains,proto3" json:"denied_domains,omitempty"` // BYOP SOCKS5 egress proxy. EgressProxyAddress string `protobuf:"bytes,5,opt,name=egress_proxy_address,json=egressProxyAddress,proto3" json:"egress_proxy_address,omitempty"` EgressProxyUsername string `protobuf:"bytes,6,opt,name=egress_proxy_username,json=egressProxyUsername,proto3" json:"egress_proxy_username,omitempty"` @@ -782,6 +792,20 @@ func (x *SandboxNetworkEgressConfig) GetRules() map[string]*SandboxNetworkDomain return nil } +func (x *SandboxNetworkEgressConfig) GetDefaultDeny() bool { + if x != nil { + return x.DefaultDeny + } + return false +} + +func (x *SandboxNetworkEgressConfig) GetDeniedDomains() []string { + if x != nil { + return x.DeniedDomains + } + return nil +} + func (x *SandboxNetworkEgressConfig) GetEgressProxyAddress() string { if x != nil { return x.EgressProxyAddress @@ -1953,12 +1977,15 @@ const file_orchestrator_proto_rawDesc = "" + "\n" + "_transform\"F\n" + "\x19SandboxNetworkDomainRules\x12)\n" + - "\x05rules\x18\x01 \x03(\v2\x13.SandboxNetworkRuleR\x05rules\"\x9e\x04\n" + + "\x05rules\x18\x01 \x03(\v2\x13.SandboxNetworkRuleR\x05rules\"\xe8\x04\n" + "\x1aSandboxNetworkEgressConfig\x12#\n" + "\rallowed_cidrs\x18\x01 \x03(\tR\fallowedCidrs\x12!\n" + "\fdenied_cidrs\x18\x02 \x03(\tR\vdeniedCidrs\x12'\n" + "\x0fallowed_domains\x18\x03 \x03(\tR\x0eallowedDomains\x12<\n" + - "\x05rules\x18\x04 \x03(\v2&.SandboxNetworkEgressConfig.RulesEntryR\x05rules\x120\n" + + "\x05rules\x18\x04 \x03(\v2&.SandboxNetworkEgressConfig.RulesEntryR\x05rules\x12!\n" + + "\fdefault_deny\x18\t \x01(\bR\vdefaultDeny\x12%\n" + + "\x0edenied_domains\x18\n" + + " \x03(\tR\rdeniedDomains\x120\n" + "\x14egress_proxy_address\x18\x05 \x01(\tR\x12egressProxyAddress\x122\n" + "\x15egress_proxy_username\x18\x06 \x01(\tR\x13egressProxyUsername\x122\n" + "\x15egress_proxy_password\x18\a \x01(\tR\x13egressProxyPassword\x12L\n" +