From e1cff98a8feea217b424b8f74814169e6888b6e0 Mon Sep 17 00:00:00 2001 From: Paul Glass Date: Fri, 28 Apr 2023 12:51:36 -0500 Subject: [PATCH] Permissive mTLS: Config entry filtering and CLI warnings (#17183) This adds filtering for service-defaults: consul config list -filter 'MutualTLSMode == "permissive"'. It adds CLI warnings when the CLI writes a config entry and sees that either service-defaults or proxy-defaults contains MutualTLSMode=permissive, or sees that the mesh config entry contains AllowEnablingPermissiveMutualTLSMode=true. --- .changelog/17183.txt | 7 ++ agent/consul/config_endpoint.go | 25 +++++ agent/consul/config_endpoint_test.go | 125 ++++++++++++++++++++++ command/config/config.go | 49 +++++++++ command/config/list/config_list.go | 9 +- command/config/list/config_list_test.go | 56 ++++++++-- command/config/write/config_write.go | 6 ++ command/config/write/config_write_test.go | 92 ++++++++++++++++ 8 files changed, 356 insertions(+), 13 deletions(-) create mode 100644 .changelog/17183.txt diff --git a/.changelog/17183.txt b/.changelog/17183.txt new file mode 100644 index 000000000..1d4326c55 --- /dev/null +++ b/.changelog/17183.txt @@ -0,0 +1,7 @@ +```release-note:improvement +* cli: Add `-filter` option to `consul config list` for filtering config entries. +``` +```release-note:improvement +* api: Support filtering for config entries. +``` + diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index 4ed7c450a..4108eb20b 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -10,6 +10,7 @@ import ( metrics "github.com/armon/go-metrics" "github.com/armon/go-metrics/prometheus" + "github.com/hashicorp/go-bexpr" "github.com/hashicorp/go-hclog" memdb "github.com/hashicorp/go-memdb" hashstructure_v2 "github.com/mitchellh/hashstructure/v2" @@ -248,6 +249,22 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe } } + // Filtering. + // This is only supported for certain config entries. + var filter *bexpr.Filter + if args.Filter != "" { + switch args.Kind { + case structs.ServiceDefaults: + f, err := bexpr.CreateFilter(args.Filter, nil, []*structs.ServiceConfigEntry{}) + if err != nil { + return err + } + filter = f + default: + return fmt.Errorf("filtering not supported for config entry kind=%v", args.Kind) + } + } + var ( priorHash uint64 ranOnce bool @@ -283,6 +300,14 @@ func (c *ConfigEntry) List(args *structs.ConfigEntryQuery, reply *structs.Indexe return fmt.Errorf("error hashing reply for spurious wakeup suppression: %w", err) } + if filter != nil { + raw, err := filter.Execute(reply.Entries) + if err != nil { + return err + } + reply.Entries = raw.([]structs.ConfigEntry) + } + if ranOnce && priorHash == newHash { priorHash = newHash return errNotChanged diff --git a/agent/consul/config_endpoint_test.go b/agent/consul/config_endpoint_test.go index e5584d857..ad6dac744 100644 --- a/agent/consul/config_endpoint_test.go +++ b/agent/consul/config_endpoint_test.go @@ -633,6 +633,131 @@ func TestConfigEntry_ListAll(t *testing.T) { }) } +func TestConfigEntry_List_Filter(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + + dir1, s1 := testServer(t) + t.Cleanup(func() { os.RemoveAll(dir1) }) + t.Cleanup(func() { s1.Shutdown() }) + codec := rpcClient(t, s1) + t.Cleanup(func() { codec.Close() }) + + // Create some services + state := s1.fsm.State() + expected := structs.IndexedConfigEntries{ + Entries: []structs.ConfigEntry{ + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "svc1", + MutualTLSMode: structs.MutualTLSModeDefault, + }, + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "svc2", + MutualTLSMode: structs.MutualTLSModeStrict, + }, + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "svc3", + MutualTLSMode: structs.MutualTLSModePermissive, + }, + }, + } + + require.NoError(t, state.EnsureConfigEntry(1, &structs.MeshConfigEntry{ + AllowEnablingPermissiveMutualTLS: true, + })) + for i, e := range expected.Entries { + require.NoError(t, state.EnsureConfigEntry(uint64(i+2), e)) + } + + cases := []struct { + filter string + expected []structs.ConfigEntry + }{ + { + filter: `MutualTLSMode == ""`, + expected: expected.Entries[0:1], + }, + { + filter: `MutualTLSMode == "strict"`, + expected: expected.Entries[1:2], + }, + { + filter: `MutualTLSMode == "permissive"`, + expected: expected.Entries[2:3], + }, + } + for _, c := range cases { + c := c + t.Run(c.filter, func(t *testing.T) { + args := structs.ConfigEntryQuery{ + Kind: structs.ServiceDefaults, + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{ + Filter: c.filter, + }, + } + + var out structs.IndexedConfigEntries + require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &out)) + require.Equal(t, out.Entries, c.expected) + }) + } +} + +func TestConfigEntry_List_Filter_UnsupportedType(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + + dir1, s1 := testServer(t) + t.Cleanup(func() { os.RemoveAll(dir1) }) + t.Cleanup(func() { s1.Shutdown() }) + codec := rpcClient(t, s1) + t.Cleanup(func() { codec.Close() }) + + for _, kind := range []string{ + // Only service-defaults is supported for now. + structs.ProxyDefaults, + structs.ServiceRouter, + structs.ServiceSplitter, + structs.ServiceResolver, + structs.IngressGateway, + structs.TerminatingGateway, + structs.ServiceIntentions, + structs.MeshConfig, + structs.ExportedServices, + structs.SamenessGroup, + structs.APIGateway, + structs.BoundAPIGateway, + structs.InlineCertificate, + structs.HTTPRoute, + structs.TCPRoute, + structs.JWTProvider, + } { + args := structs.ConfigEntryQuery{ + Kind: kind, + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{ + Filter: `X == "y"`, + }, + } + + var out structs.IndexedConfigEntries + err := msgpackrpc.CallWithCodec(codec, "ConfigEntry.List", &args, &out) + require.Error(t, err) + require.Equal(t, "filtering not supported for config entry kind="+kind, err.Error()) + } + +} + func TestConfigEntry_List_ACLDeny(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") diff --git a/command/config/config.go b/command/config/config.go index 65088cdf5..e5fbe803a 100644 --- a/command/config/config.go +++ b/command/config/config.go @@ -4,6 +4,7 @@ package config import ( + "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" "github.com/mitchellh/cli" ) @@ -52,3 +53,51 @@ Usage: consul config [options] [args] For more examples, ask for subcommand help or view the documentation. ` + +const ( + // TODO(pglass): These warnings can go away when the UI provides visibility into + // permissive mTLS settings (expected 1.17). + WarningServiceDefaultsPermissiveMTLS = "MutualTLSMode=permissive is insecure. " + + "Set to `strict` when your service no longer needs to accept non-mTLS " + + "traffic. Check `tcp.permissive_public_listener` metrics in Envoy for " + + "non-mTLS traffic. Refer to Consul documentation for more information." + + WarningProxyDefaultsPermissiveMTLS = "MutualTLSMode=permissive is insecure. " + + "To keep your services secure, set MutualTLSMode to `strict` whenever possible " + + "and override with service-defaults only if necessary. To check which " + + "service-defaults are currently in permissive mode, run `consul config list " + + "-kind service-defaults -filter 'MutualTLSMode = \"permissive\"'`." + + WarningMeshAllowEnablingPermissiveMutualTLS = "AllowEnablingPermissiveMutualTLS=true " + + "allows insecure MutualTLSMode=permissive configurations in the proxy-defaults " + + "and service-defaults config entries. You can set " + + "AllowEnablingPermissiveMutualTLS=false at any time to disallow additional " + + "permissive configurations. To list services in permissive mode, run `consul " + + "config list -kind service-defaults -filter 'MutualTLSMode = \"permissive\"'`." +) + +// KindSpecificWriteWarning returns a warning message for the given config +// entry write. Use this to inform the user of (un)recommended settings when +// they read or write config entries with the CLI. +// +// Do not return a warning on default/zero values. Because the config +// entry is parsed, we cannot distinguish between an absent field in the +// user-provided content and a zero value, so we'd end up warning on +// every invocation. +func KindSpecificWriteWarning(reqEntry api.ConfigEntry) string { + switch req := reqEntry.(type) { + case *api.ServiceConfigEntry: + if req.MutualTLSMode == api.MutualTLSModePermissive { + return WarningServiceDefaultsPermissiveMTLS + } + case *api.ProxyConfigEntry: + if req.MutualTLSMode == api.MutualTLSModePermissive { + return WarningProxyDefaultsPermissiveMTLS + } + case *api.MeshConfigEntry: + if req.AllowEnablingPermissiveMutualTLS == true { + return WarningMeshAllowEnablingPermissiveMutualTLS + } + } + return "" +} diff --git a/command/config/list/config_list.go b/command/config/list/config_list.go index 46b098017..978804a19 100644 --- a/command/config/list/config_list.go +++ b/command/config/list/config_list.go @@ -7,6 +7,7 @@ import ( "flag" "fmt" + "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/flags" "github.com/mitchellh/cli" ) @@ -23,12 +24,14 @@ type cmd struct { http *flags.HTTPFlags help string - kind string + kind string + filter string } func (c *cmd) init() { c.flags = flag.NewFlagSet("", flag.ContinueOnError) c.flags.StringVar(&c.kind, "kind", "", "The kind of configurations to list.") + c.flags.StringVar(&c.filter, "filter", "", "Filter to use with the request.") c.http = &flags.HTTPFlags{} flags.Merge(c.flags, c.http.ClientFlags()) flags.Merge(c.flags, c.http.ServerFlags()) @@ -52,7 +55,9 @@ func (c *cmd) Run(args []string) int { return 1 } - entries, _, err := client.ConfigEntries().List(c.kind, nil) + entries, _, err := client.ConfigEntries().List(c.kind, &api.QueryOptions{ + Filter: c.filter, + }) if err != nil { c.UI.Error(fmt.Sprintf("Error listing config entries for kind %q: %v", c.kind, err)) return 1 diff --git a/command/config/list/config_list_test.go b/command/config/list/config_list_test.go index 79da797ed..bf188f51b 100644 --- a/command/config/list/config_list_test.go +++ b/command/config/list/config_list_test.go @@ -28,9 +28,6 @@ func TestConfigList(t *testing.T) { defer a.Shutdown() client := a.Client() - ui := cli.NewMockUi() - c := New(ui) - _, _, err := client.ConfigEntries().Set(&api.ServiceConfigEntry{ Kind: api.ServiceDefaults, Name: "web", @@ -48,21 +45,58 @@ func TestConfigList(t *testing.T) { _, _, err = client.ConfigEntries().Set(&api.ServiceConfigEntry{ Kind: api.ServiceDefaults, Name: "api", - Protocol: "tcp", + Protocol: "http", }, nil) require.NoError(t, err) - args := []string{ - "-http-addr=" + a.HTTPAddr(), - "-kind=" + api.ServiceDefaults, + cases := map[string]struct { + args []string + expected []string + errMsg string + }{ + "list service-defaults": { + args: []string{ + "-http-addr=" + a.HTTPAddr(), + "-kind=" + api.ServiceDefaults, + }, + expected: []string{"web", "foo", "api"}, + }, + "filter service-defaults": { + args: []string{ + "-http-addr=" + a.HTTPAddr(), + "-kind=" + api.ServiceDefaults, + "-filter=" + `Protocol == "http"`, + }, + expected: []string{"api"}, + }, + "filter unsupported kind": { + args: []string{ + "-http-addr=" + a.HTTPAddr(), + "-kind=" + api.ProxyDefaults, + "-filter", `Mode == "transparent"`, + }, + errMsg: "filtering not supported for config entry kind=proxy-defaults", + }, } + for name, c := range cases { + c := c + t.Run(name, func(t *testing.T) { + ui := cli.NewMockUi() + cmd := New(ui) - code := c.Run(args) - require.Equal(t, 0, code) + code := cmd.Run(c.args) - services := strings.Split(strings.Trim(ui.OutputWriter.String(), "\n"), "\n") + if c.errMsg == "" { + require.Equal(t, 0, code) + services := strings.Split(strings.Trim(ui.OutputWriter.String(), "\n"), "\n") + require.ElementsMatch(t, c.expected, services) + } else { + require.NotEqual(t, 0, code) + require.Contains(t, ui.ErrorWriter.String(), c.errMsg) + } - require.ElementsMatch(t, []string{"web", "foo", "api"}, services) + }) + } } func TestConfigList_InvalidArgs(t *testing.T) { diff --git a/command/config/write/config_write.go b/command/config/write/config_write.go index 44e4b1311..d6a0c188b 100644 --- a/command/config/write/config_write.go +++ b/command/config/write/config_write.go @@ -14,6 +14,7 @@ import ( "github.com/mitchellh/mapstructure" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/command/config" "github.com/hashicorp/consul/command/flags" "github.com/hashicorp/consul/command/helpers" "github.com/hashicorp/consul/lib/decode" @@ -100,6 +101,11 @@ func (c *cmd) Run(args []string) int { } c.UI.Info(fmt.Sprintf("Config entry written: %s/%s", entry.GetKind(), entry.GetName())) + + if msg := config.KindSpecificWriteWarning(entry); msg != "" { + c.UI.Warn("WARNING: " + msg) + } + return 0 } diff --git a/command/config/write/config_write_test.go b/command/config/write/config_write_test.go index cf8933106..3319084f4 100644 --- a/command/config/write/config_write_test.go +++ b/command/config/write/config_write_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/command/config" "github.com/hashicorp/consul/sdk/testutil" ) @@ -171,6 +172,97 @@ kind = "proxy-defaults" }) } +func TestConfigWrite_Warning(t *testing.T) { + t.Parallel() + + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := agent.NewTestAgent(t, ``) + defer a.Shutdown() + client := a.Client() + + cases := map[string]struct { + entry string + warning string + }{ + "service-defaults no warning": { + entry: ` + Kind = "service-defaults" + Name = "web" + MutualTLSMode = "strict" + `, + }, + "proxy-defaults no warning": { + entry: ` + Kind = "proxy-defaults" + Name = "global" + MutualTLSMode = "strict" + `, + }, + "mesh config entry no warning": { + entry: ` + Kind = "mesh" + AllowEnablingPermissiveMutualTLS = false + `, + }, + "service-defaults warning on MutualTLSMode=permissive": { + entry: ` + Kind = "service-defaults" + Name = "web" + MutualTLSMode = "permissive" + `, + warning: config.WarningServiceDefaultsPermissiveMTLS, + }, + "proxy-defaults warning on MutualTLSMode=permissive": { + entry: ` + Kind = "proxy-defaults" + Name = "global" + MutualTLSMode = "permissive" + `, + warning: config.WarningProxyDefaultsPermissiveMTLS, + }, + "mesh config entry warning on AllowEnablingPermissiveMutualTLS=true": { + entry: ` + Kind = "mesh" + AllowEnablingPermissiveMutualTLS = true + `, + warning: config.WarningMeshAllowEnablingPermissiveMutualTLS, + }, + } + for name, c := range cases { + c := c + t.Run(name, func(t *testing.T) { + // Always reset this setting to avoid causing validation errors. + _, _, err := client.ConfigEntries().Set(&api.MeshConfigEntry{ + AllowEnablingPermissiveMutualTLS: true, + }, nil) + require.NoError(t, err) + + f := testutil.TempFile(t, "config-write-warning-*.hcl") + _, err = f.WriteString(c.entry) + require.NoError(t, err) + + ui := cli.NewMockUi() + code := New(ui).Run([]string{ + "-http-addr=" + a.HTTPAddr(), + f.Name(), + }) + + require.Equal(t, 0, code) + require.Contains(t, ui.OutputWriter.String(), `Config entry written`) + + errMsg := ui.ErrorWriter.String() + if c.warning != "" { + require.Contains(t, errMsg, c.warning) + } else { + require.Empty(t, errMsg) + } + }) + } +} + func requireContainsLower(t *testing.T, haystack, needle string) { t.Helper() require.Contains(t, strings.ToLower(haystack), strings.ToLower(needle))