Skip to content

Commit

Permalink
Permissive mTLS: Config entry filtering and CLI warnings (#17183)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Paul Glass authored Apr 28, 2023
1 parent 6b49869 commit e4a341c
Show file tree
Hide file tree
Showing 8 changed files with 356 additions and 13 deletions.
7 changes: 7 additions & 0 deletions .changelog/17183.txt
Original file line number Diff line number Diff line change
@@ -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.
```

25 changes: 25 additions & 0 deletions agent/consul/config_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
125 changes: 125 additions & 0 deletions agent/consul/config_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
49 changes: 49 additions & 0 deletions command/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package config

import (
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/flags"
"github.com/mitchellh/cli"
)
Expand Down Expand Up @@ -52,3 +53,51 @@ Usage: consul config <subcommand> [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 ""
}
9 changes: 7 additions & 2 deletions command/config/list/config_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"flag"
"fmt"

"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/flags"
"github.com/mitchellh/cli"
)
Expand All @@ -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())
Expand All @@ -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
Expand Down
56 changes: 45 additions & 11 deletions command/config/list/config_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions command/config/write/config_write.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down
Loading

0 comments on commit e4a341c

Please sign in to comment.