From 5e692d38a5ccbd86d6623b2f2e500edf7a54e899 Mon Sep 17 00:00:00 2001 From: Brian Picciano Date: Tue, 31 Dec 2024 11:46:49 +0100 Subject: [PATCH] Include 'internal_inbound' and 'internal_outbound' in 'vpn firewall show' output --- docs/operator/firewalls.md | 51 ++-- go/cmd/entrypoint/vpn_firewall.go | 60 ++--- go/cmd/entrypoint/vpn_firewall_test.go | 218 ++++++++++++------ go/daemon/children/nebula.go | 19 +- go/daemon/daecommon/config.go | 24 ++ tasks/bugs/include-garage-firewall-entries.md | 9 - tasks/docs/clarify-firewalls.md | 12 - 7 files changed, 230 insertions(+), 163 deletions(-) delete mode 100644 tasks/bugs/include-garage-firewall-entries.md delete mode 100644 tasks/docs/clarify-firewalls.md diff --git a/docs/operator/firewalls.md b/docs/operator/firewalls.md index 99a00d3..154e845 100644 --- a/docs/operator/firewalls.md +++ b/docs/operator/firewalls.md @@ -6,20 +6,21 @@ When providing resources on your host, whether host's firewall is configured correctly to do so. To make matters even more confusing, there are actually two firewalls at play: -the host's firewall, and the VPN firewall. +the host's firewall, and Isle's own VPN firewall. -## Host Firewall - -The host you are running isle on will almost definitely have a firewall -running, separate from the VPN firewall. If you wish to provide services for -your Isle network from your host, you will need to allow their ports in your -host's firewall. +Your host's firewall filters all traffic across all network interfaces, while +Isle's VPN firewall filters traffic only across the network interfaces it +creates itself. This means there is some duplication of responsibility across +the two, and so configuring both is required for providing resources. **isle does _not_ automatically configure your host's firewall to any extent!** -One option is to open your host to all traffic from your Isle network, and -allow the VPN firewall to be fully responsible for filtering traffic. To do this -on Linux using iptables, for example, you would add something like this to your +## Configuring the Host Firewall + +By default Isle's VPN firewall will reject all inbound traffic on VPN +interfaces. This is a safe default, and so for simplicity it is recommended to +configure the host firewall to allow all traffic on Isle networks. To do this on +Linux using iptables, for example, you would add something like this to your iptables configuration: ``` @@ -33,32 +34,31 @@ you will need to manually determine and add the ports for each service to your host's firewall. You will need to manually specify any configured storage allocation ports if this is the approach you take. -## VPN Firewall +## Configuring the VPN Firewall + +See the [Configuring Networks](./configuring-networks.md) document for notes on +how to configure Isle networks. This guide assumes configuration using the CLI. Isle uses the [nebula][nebula] project to provide its VPN layer. Nebula ships with its own [builtin firewall][nebulafirewall], which only applies to -connections coming in over the virtual network interface which it creates. This +connections coming in over the VPN interfaces which it creates for Isle. This firewall can be manually configured using the `isle vpn firewall` set of sub-commands, or using the [configuration file][configfile]. -Any storage allocations which are defined will have their network ports -automatically added to the VPN firewall by Isle. This means that you only need -to configure the VPN firewall if you are hosting services for your isle network -besides storage. - [nebula]: https://github.com/slackhq/nebula [nebulafirewall]: https://nebula.defined.net/docs/config/firewall [configfile]: ./configuring-networks.md -### Configuring the VPN Firewall - -See the [Configuring Networks](./configuring-networks.md) document for notes on -how to configure Isle networks. This guide assumes configuration using the CLI. - The `isle vpn firewall` sub-commands are used to configure the VPN's firewall. Without any flags the `isle vpn firewall show` command will display the currently active firewall. +Isle will automatically open inbound ports on its firewall for services it +provides, for example those necessary for storage allocations. When viewing open +ports using `isle vpn firewall show` these automatically opened ports will +appear separately under the `internal_inbound` section and are not configurable +by the user. + ```bash isle vpn firewall show # outbound: @@ -75,6 +75,13 @@ isle vpn firewall show # port: "22" # proto: tcp # host: my-laptop +# internal_inbound: +# - port: "3901" +# proto: tcp +# host: any +# - port: "3900" +# proto: tcp +# host: any ``` When making changes to the firewall, all changes are first applied to a staging diff --git a/go/cmd/entrypoint/vpn_firewall.go b/go/cmd/entrypoint/vpn_firewall.go index 05357fd..159e5ee 100644 --- a/go/cmd/entrypoint/vpn_firewall.go +++ b/go/cmd/entrypoint/vpn_firewall.go @@ -292,14 +292,22 @@ func newFirewallRuleViews( } type firewallView struct { - Outbound []firewallRuleView `yaml:"outbound"` - Inbound []firewallRuleView `yaml:"inbound"` + Outbound []firewallRuleView `yaml:"outbound"` + Inbound []firewallRuleView `yaml:"inbound"` + InternalOutbound []daecommon.ConfigFirewallRule `yaml:"internal_outbound,omitempty"` + InternalInbound []daecommon.ConfigFirewallRule `yaml:"internal_inbound,omitempty"` } -func newFirewallView(firewallConfig daecommon.ConfigFirewall) firewallView { +func newFirewallView(networkConfig daecommon.NetworkConfig) firewallView { + var ( + firewallConfig = networkConfig.VPN.Firewall + internalOutbound, internalInbound = networkConfig.InternalFirewallRules() + ) return firewallView{ - Outbound: newFirewallRuleViews(firewallConfig.Outbound), - Inbound: newFirewallRuleViews(firewallConfig.Inbound), + Outbound: newFirewallRuleViews(firewallConfig.Outbound), + Inbound: newFirewallRuleViews(firewallConfig.Inbound), + InternalOutbound: internalOutbound, + InternalInbound: internalInbound, } } @@ -318,35 +326,31 @@ var subCmdVPNFirewallShow = subCmd{ return nil, fmt.Errorf("parsing flags: %w", err) } - var ( - firewallConfig daecommon.ConfigFirewall - foundStaged bool - ) + daemonRPC, err := ctx.newDaemonRPC() + if err != nil { + return nil, fmt.Errorf("creating daemon RPC client: %w", err) + } + defer daemonRPC.Close() + + config, err := daemonRPC.GetConfig(ctx) + if err != nil { + return nil, fmt.Errorf("getting network config: %w", err) + } + if *staged { - var err error - if foundStaged, err = ctx.getChangeStager().get( + var firewallConfig daecommon.ConfigFirewall + foundStaged, err := ctx.getChangeStager().get( &firewallConfig, vpnFirewallConfigChangeStagerName, - ); err != nil { + ) + + if err != nil { return nil, fmt.Errorf("checking for staged changes: %w", err) + } else if foundStaged { + config.VPN.Firewall = firewallConfig } } - if !foundStaged { - daemonRPC, err := ctx.newDaemonRPC() - if err != nil { - return nil, fmt.Errorf("creating daemon RPC client: %w", err) - } - defer daemonRPC.Close() - - config, err := daemonRPC.GetConfig(ctx) - if err != nil { - return nil, fmt.Errorf("getting network config: %w", err) - } - - firewallConfig = config.VPN.Firewall - } - - return newFirewallView(firewallConfig), nil + return newFirewallView(config), nil }), } diff --git a/go/cmd/entrypoint/vpn_firewall_test.go b/go/cmd/entrypoint/vpn_firewall_test.go index 524560f..990952b 100644 --- a/go/cmd/entrypoint/vpn_firewall_test.go +++ b/go/cmd/entrypoint/vpn_firewall_test.go @@ -7,8 +7,6 @@ import ( "isle/daemon/daecommon" "isle/toolkit" "os" - "slices" - "strings" "testing" "github.com/stretchr/testify/assert" @@ -361,25 +359,43 @@ func TestVPNFirewallRemove(t *testing.T) { func TestVPNFirewallShow(t *testing.T) { t.Parallel() + emptyWant := map[string][]any{ + "outbound": { + map[string]any{ + "index": 0, + "port": "any", + "proto": "any", + "host": "any", + }, + }, + "inbound": { + map[string]any{ + "index": 0, + "port": "any", + "proto": "icmp", + "host": "any", + }, + }, + } + tests := []struct { - name string - outbound, inbound []string - staged string - flags []string - want map[string][]any + name string + config daecommon.NetworkConfig + staged string + flags []string + want map[string][]any }{ { name: "empty", - want: map[string][]any{ - "outbound": {}, - "inbound": {}, - }, + want: emptyWant, }, { name: "single", - outbound: []string{ - `{"port":"any","proto":"icmp","host":"any"}`, - }, + config: daecommon.NewNetworkConfig(func(c *daecommon.NetworkConfig) { + c.VPN.Firewall.Outbound = []daecommon.ConfigFirewallRule{ + {Port: "any", Proto: "icmp", Host: "any"}, + } + }), want: map[string][]any{ "outbound": { map[string]any{ @@ -389,18 +405,27 @@ func TestVPNFirewallShow(t *testing.T) { "host": "any", }, }, - "inbound": {}, + "inbound": { + map[string]any{ + "index": 0, + "port": "any", + "proto": "icmp", + "host": "any", + }, + }, }, }, { name: "multiple", - outbound: []string{ - `{"port":"any","proto":"icmp","host":"any"}`, - }, - inbound: []string{ - `{"port":"any","proto":"icmp","host":"any"}`, - `{"port":"22","proto":"tcp","host":"foo"}`, - }, + config: daecommon.NewNetworkConfig(func(c *daecommon.NetworkConfig) { + c.VPN.Firewall.Outbound = []daecommon.ConfigFirewallRule{ + {Port: "any", Proto: "icmp", Host: "any"}, + } + c.VPN.Firewall.Inbound = []daecommon.ConfigFirewallRule{ + {Port: "any", Proto: "icmp", Host: "any"}, + {Port: "22", Proto: "tcp", Host: "foo"}, + } + }), want: map[string][]any{ "outbound": { map[string]any{ @@ -427,28 +452,12 @@ func TestVPNFirewallShow(t *testing.T) { }, }, { - name: "staged/nothing staged", - outbound: []string{ - `{"port":"any","proto":"icmp","host":"any"}`, - }, + name: "staged/nothing staged", flags: []string{"--staged"}, - want: map[string][]any{ - "outbound": { - map[string]any{ - "index": 0, - "port": "any", - "proto": "icmp", - "host": "any", - }, - }, - "inbound": {}, - }, + want: emptyWant, }, { name: "staged/staged but no flag", - outbound: []string{ - `{"port":"any","proto":"icmp","host":"any"}`, - }, staged: `{ "Inbound": [ { @@ -458,23 +467,10 @@ func TestVPNFirewallShow(t *testing.T) { } ] }`, - want: map[string][]any{ - "outbound": { - map[string]any{ - "index": 0, - "port": "any", - "proto": "icmp", - "host": "any", - }, - }, - "inbound": {}, - }, + want: emptyWant, }, { name: "staged/staged with flag", - outbound: []string{ - `{"port":"any","proto":"icmp","host":"any"}`, - }, staged: `{ "Inbound": [ { @@ -497,17 +493,102 @@ func TestVPNFirewallShow(t *testing.T) { }, }, }, - } + { + name: "with alloc/no staged", + config: daecommon.NewNetworkConfig(func(c *daecommon.NetworkConfig) { + c.Storage.Allocations = []daecommon.ConfigStorageAllocation{{ + DataPath: "/data", + MetaPath: "/meta", + S3APIPort: 3901, + RPCPort: 3900, + AdminPort: 3902, + }} + }), + want: map[string][]any{ + "outbound": { + map[string]any{ + "index": 0, + "port": "any", + "proto": "any", + "host": "any", + }, + }, + "inbound": { + map[string]any{ + "index": 0, + "port": "any", + "proto": "icmp", + "host": "any", + }, + }, + "internal_inbound": { + map[string]any{ + "port": "3901", + "proto": "tcp", + "host": "any", + }, + map[string]any{ + "port": "3900", + "proto": "tcp", + "host": "any", + }, + }, + }, + }, + { + name: "with alloc/with staged", + config: daecommon.NewNetworkConfig(func(c *daecommon.NetworkConfig) { + c.Storage.Allocations = []daecommon.ConfigStorageAllocation{{ + DataPath: "/data", + MetaPath: "/meta", + S3APIPort: 3901, + RPCPort: 3900, + AdminPort: 3902, + }} + }), + staged: `{ + "Inbound": [ + { + "Port":"80", + "Proto":"tcp", + "Host":"some-host" + } + ] + }`, + flags: []string{"--staged"}, + want: map[string][]any{ + "outbound": {}, + "inbound": { + map[string]any{ + "index": 0, + "port": "80", + "proto": "tcp", + "host": "some-host", + }, + }, + "internal_inbound": { + map[string]any{ + "port": "3901", + "proto": "tcp", + "host": "any", + }, + map[string]any{ + "port": "3900", + "proto": "tcp", + "host": "any", + }, + }, + }, + }} for _, test := range tests { t.Run(test.name, func(t *testing.T) { - var ( - h = newRunHarness(t) - config daecommon.NetworkConfig + h := newRunHarness(t) - outboundRawJSON = "[" + strings.Join(test.outbound, ",") + "]" - inboundRawJSON = "[" + strings.Join(test.inbound, ",") + "]" - ) + h.daemonRPC. + On("GetConfig", toolkit.MockArg[context.Context]()). + Return(test.config, nil). + Once() if test.staged != "" { require.True(t, json.Valid([]byte(test.staged))) @@ -518,21 +599,6 @@ func TestVPNFirewallShow(t *testing.T) { )) } - require.NoError(t, json.Unmarshal( - []byte(outboundRawJSON), &config.VPN.Firewall.Outbound, - )) - - require.NoError(t, json.Unmarshal( - []byte(inboundRawJSON), &config.VPN.Firewall.Inbound, - )) - - if !slices.Contains(test.flags, "--staged") || test.staged == "" { - h.daemonRPC. - On("GetConfig", toolkit.MockArg[context.Context]()). - Return(config, nil). - Once() - } - args := append([]string{"vpn", "firewall", "show"}, test.flags...) h.runAssertStdout(t, test.want, args...) }) diff --git a/go/daemon/children/nebula.go b/go/daemon/children/nebula.go index c7bb968..b80902f 100644 --- a/go/daemon/children/nebula.go +++ b/go/daemon/children/nebula.go @@ -10,7 +10,6 @@ import ( "isle/yamlutil" "net" "path/filepath" - "strconv" "code.betamike.com/micropelago/pmux/pmuxlib" "dev.mediocregopher.com/mediocre-go-lib.git/mctx" @@ -89,21 +88,9 @@ func nebulaConfig( ) firewall := networkConfig.VPN.Firewall - for _, alloc := range networkConfig.Storage.Allocations { - firewall.Inbound = append( - firewall.Inbound, - daecommon.ConfigFirewallRule{ - Port: strconv.Itoa(alloc.S3APIPort), - Proto: "tcp", - Host: "any", - }, - daecommon.ConfigFirewallRule{ - Port: strconv.Itoa(alloc.RPCPort), - Proto: "tcp", - Host: "any", - }, - ) - } + internalOutbound, internalInbound := networkConfig.InternalFirewallRules() + firewall.Outbound = append(firewall.Outbound, internalOutbound...) + firewall.Inbound = append(firewall.Inbound, internalInbound...) type m = yamlutil.OrderedMap[string, any] diff --git a/go/daemon/daecommon/config.go b/go/daemon/daecommon/config.go index 8d3197c..7ded765 100644 --- a/go/daemon/daecommon/config.go +++ b/go/daemon/daecommon/config.go @@ -10,6 +10,7 @@ import ( "isle/yamlutil" "net" "sort" + "strconv" _ "embed" @@ -121,6 +122,29 @@ func NewNetworkConfig(fn func(*NetworkConfig)) NetworkConfig { return c } +// InternalFirewallRules returns the firewall rules which should be added to the +// NetworkConfig automatically, beyond those which are managed by the user. +func (c *NetworkConfig) InternalFirewallRules() ( + outbound, inbound []ConfigFirewallRule, +) { + for _, alloc := range c.Storage.Allocations { + inbound = append( + inbound, + ConfigFirewallRule{ + Port: strconv.Itoa(alloc.S3APIPort), + Proto: "tcp", + Host: "any", + }, + ConfigFirewallRule{ + Port: strconv.Itoa(alloc.RPCPort), + Proto: "tcp", + Host: "any", + }, + ) + } + return +} + func (c *NetworkConfig) fillDefaults() { if c.DNS.Resolvers == nil { c.DNS.Resolvers = []string{ diff --git a/tasks/bugs/include-garage-firewall-entries.md b/tasks/bugs/include-garage-firewall-entries.md deleted file mode 100644 index 738ba9b..0000000 --- a/tasks/bugs/include-garage-firewall-entries.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -type: task ---- - -When listing firewall entries using `isle vpn firewall list`, any firewall -entries which are automatically included for garage (and all other services in -the future) should be included in the returned list as well. These should be -annotated in such a way that the user understands they are automatically -generated and can't be changed. diff --git a/tasks/docs/clarify-firewalls.md b/tasks/docs/clarify-firewalls.md deleted file mode 100644 index 3c0eeb6..0000000 --- a/tasks/docs/clarify-firewalls.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -type: task ---- - -The Firewalls doc page should be extra clear that adding the line - -``` --A INPUT --source --jump ACCEPT -``` - -will not expose the host to the network entirely, as the nebula firewall will -still block all traffic by default.