From 91fb5863ab00c9f9eac85d4de383dcb79bf71ed2 Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Wed, 8 Apr 2020 18:35:06 +0200 Subject: [PATCH 1/4] Allow to restrict servers that can join a given Serf Consul cluster. Based on work done in https://github.com/hashicorp/memberlist/pull/196 this allows to restrict the IP ranges that can join a given Serf cluster and be a member of the cluster. Restrictions on IPs can be done separatly using 2 new differents flags and config options to restrict IPs for LAN and WAN Serf. --- agent/agent.go | 2 ++ agent/config/builder.go | 12 +++++++++++ agent/config/config.go | 2 ++ agent/config/flags.go | 2 ++ agent/config/runtime.go | 20 +++++++++++++++++++ agent/config/runtime_test.go | 30 +++++++++++++++++++++++++++- website/pages/docs/agent/options.mdx | 10 ++++++++++ 7 files changed, 77 insertions(+), 1 deletion(-) diff --git a/agent/agent.go b/agent/agent.go index 79c534756f97..76f07e6dad7f 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1177,6 +1177,8 @@ func (a *Agent) consulConfig() (*consul.Config, error) { base.SerfLANConfig.MemberlistConfig.BindAddr = a.config.SerfBindAddrLAN.IP.String() base.SerfLANConfig.MemberlistConfig.BindPort = a.config.SerfBindAddrLAN.Port + base.SerfLANConfig.MemberlistConfig.CIDRsAllowed = a.config.SerfAllowedCIDRsLAN + base.SerfWANConfig.MemberlistConfig.CIDRsAllowed = a.config.SerfAllowedCIDRsWAN base.SerfLANConfig.MemberlistConfig.AdvertiseAddr = a.config.SerfAdvertiseAddrLAN.IP.String() base.SerfLANConfig.MemberlistConfig.AdvertisePort = a.config.SerfAdvertiseAddrLAN.Port base.SerfLANConfig.MemberlistConfig.GossipVerifyIncoming = a.config.EncryptVerifyIncoming diff --git a/agent/config/builder.go b/agent/config/builder.go index e3b22e292fde..42c277974c95 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -24,6 +24,7 @@ import ( "github.com/hashicorp/consul/types" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-sockaddr/template" + "github.com/hashicorp/memberlist" "golang.org/x/time/rate" ) @@ -738,6 +739,15 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { } } + serfAllowedCIDRSLAN, err := memberlist.ParseCIDRs(c.SerfAllowedCIDRsLAN) + if err != nil { + return RuntimeConfig{}, fmt.Errorf("serf_lan_allowed_cidrs: %s", err) + } + serfAllowedCIDRSWAN, err := memberlist.ParseCIDRs(c.SerfAllowedCIDRsWAN) + if err != nil { + return RuntimeConfig{}, fmt.Errorf("serf_wan_allowed_cidrs: %s", err) + } + // ---------------------------------------------------------------- // build runtime config // @@ -960,6 +970,8 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { Segments: segments, SerfAdvertiseAddrLAN: serfAdvertiseAddrLAN, SerfAdvertiseAddrWAN: serfAdvertiseAddrWAN, + SerfAllowedCIDRsLAN: serfAllowedCIDRSLAN, + SerfAllowedCIDRsWAN: serfAllowedCIDRSWAN, SerfBindAddrLAN: serfBindAddrLAN, SerfBindAddrWAN: serfBindAddrWAN, SerfPortLAN: serfPortLAN, diff --git a/agent/config/config.go b/agent/config/config.go index b0b021965409..540a3f4159bc 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -270,6 +270,8 @@ type Config struct { RetryJoinMaxAttemptsLAN *int `json:"retry_max,omitempty" hcl:"retry_max" mapstructure:"retry_max"` RetryJoinMaxAttemptsWAN *int `json:"retry_max_wan,omitempty" hcl:"retry_max_wan" mapstructure:"retry_max_wan"` RetryJoinWAN []string `json:"retry_join_wan,omitempty" hcl:"retry_join_wan" mapstructure:"retry_join_wan"` + SerfAllowedCIDRsLAN []string `json:"serf_lan_allowed_cidrs,omitempty" hcl:"serf_lan_allowed_cidrs" mapstructure:"serf_lan_allowed_cidrs"` + SerfAllowedCIDRsWAN []string `json:"serf_wan_allowed_cidrs,omitempty" hcl:"serf_wan_allowed_cidrs" mapstructure:"serf_wan_allowed_cidrs"` SerfBindAddrLAN *string `json:"serf_lan,omitempty" hcl:"serf_lan" mapstructure:"serf_lan"` SerfBindAddrWAN *string `json:"serf_wan,omitempty" hcl:"serf_wan" mapstructure:"serf_wan"` ServerMode *bool `json:"server,omitempty" hcl:"server" mapstructure:"server"` diff --git a/agent/config/flags.go b/agent/config/flags.go index 9cbc0f26803e..fccee4c1d04b 100644 --- a/agent/config/flags.go +++ b/agent/config/flags.go @@ -105,6 +105,8 @@ func AddFlags(fs *flag.FlagSet, f *Flags) { add(&f.Config.RetryJoinWAN, "retry-join-wan", "Address of an agent to join -wan at start time with retries enabled. Can be specified multiple times.") add(&f.Config.RetryJoinMaxAttemptsLAN, "retry-max", "Maximum number of join attempts. Defaults to 0, which will retry indefinitely.") add(&f.Config.RetryJoinMaxAttemptsWAN, "retry-max-wan", "Maximum number of join -wan attempts. Defaults to 0, which will retry indefinitely.") + add(&f.Config.SerfAllowedCIDRsLAN, "serf-lan-allowed-cidrs", "Networks (eg: 192.168.1.0/24) allowed for Serf LAN. Can be specified multiple times.") + add(&f.Config.SerfAllowedCIDRsWAN, "serf-wan-allowed-cidrs", "Networks (eg: 192.168.1.0/24) allowed for Serf WAN (other datacenters). Can be specified multiple times.") add(&f.Config.SerfBindAddrLAN, "serf-lan-bind", "Address to bind Serf LAN listeners to.") add(&f.Config.Ports.SerfLAN, "serf-lan-port", "Sets the Serf LAN port to listen on.") add(&f.Config.SegmentName, "segment", "(Enterprise-only) Sets the network segment to join.") diff --git a/agent/config/runtime.go b/agent/config/runtime.go index f80d28de7939..9cdfcd1109d4 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -1153,6 +1153,18 @@ type RuntimeConfig struct { // hcl: bind_addr = string advertise_addr_wan = string ports { serf_wan = int } SerfAdvertiseAddrWAN *net.TCPAddr + // SerfAllowedCIDRsLAN if set to a non-empty value, will restrict which networks + // are allowed to connect to Serf on the LAN. + // hcl: serf_lan_allowed_cidrs = []string + // flag: serf-lan-allowed-cidrs string (can be specified multiple times) + SerfAllowedCIDRsLAN []net.IPNet + + // SerfAllowedCIDRsWAN if set to a non-empty value, will restrict which networks + // are allowed to connect to Serf on the WAN. + // hcl: serf_wan_allowed_cidrs = []string + // flag: serf-wan-allowed-cidrs string (can be specified multiple times) + SerfAllowedCIDRsWAN []net.IPNet + // SerfBindAddrLAN is the address to bind the Serf LAN TCP and UDP // listeners to. The ip address is either the default bind address or the // 'serf_lan' address which can be either an ip address or a go-sockaddr @@ -1794,6 +1806,14 @@ func sanitize(name string, v reflect.Value) reflect.Value { case isArray(typ) || isSlice(typ): ma := make([]interface{}, 0, v.Len()) + if strings.HasPrefix(name, "SerfAllowedCIDRs") { + for i := 0; i < v.Len(); i++ { + addr := v.Index(i).Addr() + ip := addr.Interface().(*net.IPNet) + ma = append(ma, ip.String()) + } + return reflect.ValueOf(ma) + } for i := 0; i < v.Len(); i++ { ma = append(ma, sanitize(fmt.Sprintf("%s[%d]", name, i), v.Index(i)).Interface()) } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 09be53060e84..98e97992fbef 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1538,6 +1538,28 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { }, }, + { + desc: "Serf Allowed CIDRS LAN, multiple values from flags", + args: []string{`-data-dir=` + dataDir, `-serf-lan-allowed-cidrs=127.0.0.0/4`, `-serf-lan-allowed-cidrs=192.168.0.0/24`}, + json: []string{}, + hcl: []string{}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.SerfAllowedCIDRsLAN = []net.IPNet{*(parseCIDR(t, "127.0.0.0/4")), *(parseCIDR(t, "192.168.0.0/24"))} + }, + }, + + { + desc: "Serf Allowed CIDRS WAN, multiple values from flags", + args: []string{`-data-dir=` + dataDir, `-serf-wan-allowed-cidrs=192.168.4.0/24`, `-serf-wan-allowed-cidrs=192.168.3.0/24`}, + json: []string{}, + hcl: []string{}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.SerfAllowedCIDRsWAN = []net.IPNet{*(parseCIDR(t, "192.168.4.0/24")), *(parseCIDR(t, "192.168.3.0/24"))} + }, + }, + // ------------------------------------------------------------ // validations // @@ -6201,7 +6223,11 @@ func TestSanitize(t *testing.T) { }, }, KVMaxValueSize: 1234567800000000, - TxnMaxReqLen: 5678000000000000, + SerfAllowedCIDRsLAN: []net.IPNet{ + *parseCIDR(t, "192.168.1.0/24"), + *parseCIDR(t, "127.0.0.0/8"), + }, + TxnMaxReqLen: 5678000000000000, } rtJSON := `{ @@ -6424,6 +6450,8 @@ func TestSanitize(t *testing.T) { "Segments": [], "SerfAdvertiseAddrLAN": "tcp://1.2.3.4:5678", "SerfAdvertiseAddrWAN": "", + "SerfAllowedCIDRsLAN": ["192.168.1.0/24", "127.0.0.0/8"], + "SerfAllowedCIDRsWAN": [], "SerfBindAddrLAN": "", "SerfBindAddrWAN": "", "SerfPortLAN": 0, diff --git a/website/pages/docs/agent/options.mdx b/website/pages/docs/agent/options.mdx index 185a060d1ede..55bbdc7c3e58 100644 --- a/website/pages/docs/agent/options.mdx +++ b/website/pages/docs/agent/options.mdx @@ -445,10 +445,20 @@ The options below are all specified on the command-line. more details. By default, this is an empty string, which is the default network segment. +- `-serf-lan-allowed-cidrs` - The Serf LAN allowed CIDRs allow to accept incoming + connections for Serf only from several networks (mutiple values are supported). + Those networks are specified with CIDR notation (eg: 192.168.1.0/24). + This is available in Consul 1.8 and later. + - `-serf-lan-port` ((#\_serf_lan_port)) - the Serf LAN port to listen on. This overrides the default Serf LAN port 8301. This is available in Consul 1.2.2 and later. +- `-serf-lan-allowed-cidrs` - he Serf LAN allowed CIDRs allow to accept incoming + connections for Serf only from several networks (mutiple values are supported). + Those networks are specified with CIDR notation (eg: 192.168.1.0/24). + This is available in Consul 1.8 and later. + - `-serf-wan-port` ((#\_serf_wan_port)) - the Serf WAN port to listen on. This overrides the default Serf WAN port 8302. This is available in Consul 1.2.2 and later. From 24f850f9bf8079aca233ad239cf6bbd51def00ee Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 10 Apr 2020 00:34:55 +0200 Subject: [PATCH 2/4] Added unit test `TestServer_JoinLAN_SerfAllowedCIDRs()` This test does check that blocking IPs works on LAN --- agent/consul/client_test.go | 9 +++++- agent/consul/server_test.go | 64 +++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/agent/consul/client_test.go b/agent/consul/client_test.go index cdaa7067722e..8b037b18b4fb 100644 --- a/agent/consul/client_test.go +++ b/agent/consul/client_test.go @@ -67,7 +67,7 @@ func testClientDC(t *testing.T, dc string) (string, *Client) { }) } -func testClientWithConfig(t *testing.T, cb func(c *Config)) (string, *Client) { +func testClientWithConfigWithErr(t *testing.T, cb func(c *Config)) (string, *Client, error) { dir, config := testClientConfig(t) if cb != nil { cb(config) @@ -91,6 +91,13 @@ func testClientWithConfig(t *testing.T, cb func(c *Config)) (string, *Client) { client, err := NewClientLogger(config, logger, tlsConf) if err != nil { config.NotifyShutdown() + } + return dir, client, err +} + +func testClientWithConfig(t *testing.T, cb func(c *Config)) (string, *Client) { + dir, client, err := testClientWithConfigWithErr(t, cb) + if err != nil { t.Fatalf("err: %v", err) } return dir, client diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 941fb7ea537a..f502cd2f2090 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -13,6 +13,7 @@ import ( "github.com/google/tcpproxy" "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/ipaddr" + "github.com/hashicorp/memberlist" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/metadata" @@ -28,6 +29,7 @@ import ( "github.com/hashicorp/go-uuid" "golang.org/x/time/rate" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -370,6 +372,68 @@ func TestServer_JoinLAN(t *testing.T) { }) } +// TestServer_JoinLAN_SerfAllowedCIDRs test that IPs might be blocked +// with Serf. +// To run properlly, this test requires to be able to bind and have access +// on 127.0.1.1 which is the case for most Linux machines and Windows, +// so Unit test will run in the CI. +// To run it on Mac OS, please run this commandd first, otherwise the +// test will be skipped: `sudo ifconfig lo0 alias 127.0.1.1 up` +func TestServer_JoinLAN_SerfAllowedCIDRs(t *testing.T) { + t.Parallel() + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.BootstrapExpect = 1 + lan, err := memberlist.ParseCIDRs([]string{"127.0.0.1/32"}) + assert.NoError(t, err) + c.SerfLANConfig.MemberlistConfig.CIDRsAllowed = lan + wan, err := memberlist.ParseCIDRs([]string{"127.0.0.0/24", "::1/128"}) + assert.NoError(t, err) + c.SerfWANConfig.MemberlistConfig.CIDRsAllowed = wan + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + + targetAddr := "127.0.1.1" + dir2, a2, err := testClientWithConfigWithErr(t, func(c *Config) { + c.SerfLANConfig.MemberlistConfig.BindAddr = targetAddr + }) + defer os.RemoveAll(dir2) + if err != nil { + t.Skipf("Cannot bind on %s, to run on Mac OS: `sudo ifconfig lo0 alias 127.0.1.1 up`", targetAddr) + } + defer a2.Shutdown() + + dir3, rs3 := testServerWithConfig(t, func(c *Config) { + c.BootstrapExpect = 1 + c.Datacenter = "dc2" + }) + defer os.RemoveAll(dir3) + defer rs3.Shutdown() + + leaderAddr := joinAddrLAN(s1) + if _, err := a2.JoinLAN([]string{leaderAddr}); err != nil { + t.Fatalf("Expected no error, had: %#v", err) + } + // Try to join + joinWAN(t, rs3, s1) + retry.Run(t, func(r *retry.R) { + if got, want := len(s1.LANMembers()), 1; got != want { + // LAN is blocked, should be 1 only + r.Fatalf("got %d s1 LAN members want %d", got, want) + } + if got, want := len(a2.LANMembers()), 2; got != want { + // LAN is blocked a2 can see s1, but not s1 + r.Fatalf("got %d a2 LAN members want %d", got, want) + } + if got, want := len(s1.WANMembers()), 2; got != want { + r.Fatalf("got %d s1 WAN members want %d", got, want) + } + if got, want := len(rs3.WANMembers()), 2; got != want { + r.Fatalf("got %d rs3 WAN members want %d", got, want) + } + }) +} + func TestServer_LANReap(t *testing.T) { t.Parallel() From bfdfde538af8a86bfd1e5f3910a3440b2466622e Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Fri, 10 Apr 2020 19:12:14 +0200 Subject: [PATCH 3/4] Added comments for unit test as it requires setup on some OSes --- agent/consul/server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index f502cd2f2090..a49d18e9265f 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -374,7 +374,7 @@ func TestServer_JoinLAN(t *testing.T) { // TestServer_JoinLAN_SerfAllowedCIDRs test that IPs might be blocked // with Serf. -// To run properlly, this test requires to be able to bind and have access +// To run properly, this test requires to be able to bind and have access // on 127.0.1.1 which is the case for most Linux machines and Windows, // so Unit test will run in the CI. // To run it on Mac OS, please run this commandd first, otherwise the From 14ad95c7180b1da498c98c8a65a3b87c3f45c1ff Mon Sep 17 00:00:00 2001 From: Pierre Souchay Date: Tue, 19 May 2020 11:48:15 +0200 Subject: [PATCH 4/4] Added a test for JSON/HCL serialization of parameters. --- agent/config/runtime_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 98e97992fbef..5081e1913624 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1548,7 +1548,19 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { rt.SerfAllowedCIDRsLAN = []net.IPNet{*(parseCIDR(t, "127.0.0.0/4")), *(parseCIDR(t, "192.168.0.0/24"))} }, }, - + { + desc: "Serf Allowed CIDRS LAN/WAN, multiple values from HCL/JSON", + args: []string{`-data-dir=` + dataDir}, + json: []string{`{"serf_lan_allowed_cidrs": ["127.0.0.0/4", "192.168.0.0/24"]}`, + `{"serf_wan_allowed_cidrs": ["10.228.85.46/25"]}`}, + hcl: []string{`serf_lan_allowed_cidrs=["127.0.0.0/4", "192.168.0.0/24"]`, + `serf_wan_allowed_cidrs=["10.228.85.46/25"]`}, + patch: func(rt *RuntimeConfig) { + rt.DataDir = dataDir + rt.SerfAllowedCIDRsLAN = []net.IPNet{*(parseCIDR(t, "127.0.0.0/4")), *(parseCIDR(t, "192.168.0.0/24"))} + rt.SerfAllowedCIDRsWAN = []net.IPNet{*(parseCIDR(t, "10.228.85.46/25"))} + }, + }, { desc: "Serf Allowed CIDRS WAN, multiple values from flags", args: []string{`-data-dir=` + dataDir, `-serf-wan-allowed-cidrs=192.168.4.0/24`, `-serf-wan-allowed-cidrs=192.168.3.0/24`},