From 26661a1c3bc3fa49acc06c3e5703c299d0a7d010 Mon Sep 17 00:00:00 2001 From: "Chris S. Kim" Date: Thu, 8 Feb 2024 15:25:42 -0500 Subject: [PATCH] Add default intention policy (#20544) --- .changelog/20544.txt | 3 + acl/authorizer.go | 15 +- acl/chained_authorizer.go | 1 + agent/agent.go | 13 +- agent/agent_endpoint.go | 4 + agent/agent_endpoint_test.go | 144 +++++++++------- agent/config/builder.go | 1 + agent/config/config.go | 1 + agent/config/runtime.go | 9 + .../TestRuntimeConfig_Sanitize.golden | 7 +- agent/consul/acl.go | 17 ++ agent/consul/client.go | 2 +- agent/consul/config.go | 29 ++-- agent/consul/intention_endpoint.go | 16 +- agent/consul/intention_endpoint_test.go | 157 ++++++++---------- agent/consul/internal_endpoint.go | 6 +- agent/consul/internal_endpoint_test.go | 38 ++++- agent/consul/server.go | 2 +- agent/consul/state/catalog.go | 6 +- agent/consul/state/intention.go | 12 +- agent/consul/state/intention_test.go | 102 ++++++------ agent/consul/util.go | 13 ++ agent/proxycfg-glue/glue.go | 2 +- agent/proxycfg-glue/intention_upstreams.go | 19 ++- .../proxycfg-glue/intention_upstreams_test.go | 43 ++++- agent/structs/intention.go | 5 +- 26 files changed, 400 insertions(+), 267 deletions(-) create mode 100644 .changelog/20544.txt diff --git a/.changelog/20544.txt b/.changelog/20544.txt new file mode 100644 index 000000000000..bfe3cbd6ff9d --- /dev/null +++ b/.changelog/20544.txt @@ -0,0 +1,3 @@ +```release-note:feature +agent: Introduces a new agent config default_intention_policy to decouple the default intention behavior from ACLs +``` diff --git a/acl/authorizer.go b/acl/authorizer.go index 9e5bacc25e86..39bac5f7b08b 100644 --- a/acl/authorizer.go +++ b/acl/authorizer.go @@ -93,6 +93,10 @@ type Authorizer interface { // IntentionDefaultAllow determines the default authorized behavior // when no intentions match a Connect request. + // + // Deprecated: Use DefaultIntentionPolicy under agent configuration. + // Moving forwards, intentions will not inherit default allow behavior + // from the ACL system. IntentionDefaultAllow(*AuthorizerContext) EnforcementDecision // IntentionRead determines if a specific intention can be read. @@ -297,17 +301,6 @@ func (a AllowAuthorizer) IdentityWriteAnyAllowed(ctx *AuthorizerContext) error { return nil } -// IntentionDefaultAllowAllowed determines the default authorized behavior -// when no intentions match a Connect request. -func (a AllowAuthorizer) IntentionDefaultAllowAllowed(ctx *AuthorizerContext) error { - if a.Authorizer.IntentionDefaultAllow(ctx) != Allow { - // This is a bit nuanced, in that this isn't set by a rule, but inherited globally - // TODO(acl-error-enhancements) revisit when we have full accessor info - return PermissionDeniedError{Cause: "Denied by intention default"} - } - return nil -} - // IntentionReadAllowed determines if a specific intention can be read. func (a AllowAuthorizer) IntentionReadAllowed(name string, ctx *AuthorizerContext) error { if a.Authorizer.IntentionRead(name, ctx) != Allow { diff --git a/acl/chained_authorizer.go b/acl/chained_authorizer.go index 76e973d2e9ed..26f0c2dfe7fd 100644 --- a/acl/chained_authorizer.go +++ b/acl/chained_authorizer.go @@ -113,6 +113,7 @@ func (c *ChainedAuthorizer) IdentityWriteAny(entCtx *AuthorizerContext) Enforcem // when no intentions match a Connect request. func (c *ChainedAuthorizer) IntentionDefaultAllow(entCtx *AuthorizerContext) EnforcementDecision { return c.executeChain(func(authz Authorizer) EnforcementDecision { + //nolint:staticcheck return authz.IntentionDefaultAllow(entCtx) }) } diff --git a/agent/agent.go b/agent/agent.go index ef73242cd960..781c36f4e0fa 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -813,6 +813,15 @@ func (a *Agent) Start(ctx context.Context) error { return fmt.Errorf("unexpected ACL default policy value of %q", a.config.ACLResolverSettings.ACLDefaultPolicy) } + // If DefaultIntentionPolicy is defined, it should override + // the values inherited from ACLDefaultPolicy. + switch a.config.DefaultIntentionPolicy { + case "allow": + intentionDefaultAllow = true + case "deny": + intentionDefaultAllow = false + } + go a.baseDeps.ViewStore.Run(&lib.StopChannelContext{StopCh: a.shutdownCh}) // Start the proxy config manager. @@ -4747,8 +4756,8 @@ func (a *Agent) proxyDataSources(server *consul.Server) proxycfg.DataSources { sources.Health = proxycfgglue.ServerHealthBlocking(deps, proxycfgglue.ClientHealth(a.rpcClientHealth)) sources.HTTPChecks = proxycfgglue.ServerHTTPChecks(deps, a.config.NodeName, proxycfgglue.CacheHTTPChecks(a.cache), a.State) sources.Intentions = proxycfgglue.ServerIntentions(deps) - sources.IntentionUpstreams = proxycfgglue.ServerIntentionUpstreams(deps) - sources.IntentionUpstreamsDestination = proxycfgglue.ServerIntentionUpstreamsDestination(deps) + sources.IntentionUpstreams = proxycfgglue.ServerIntentionUpstreams(deps, a.config.DefaultIntentionPolicy) + sources.IntentionUpstreamsDestination = proxycfgglue.ServerIntentionUpstreamsDestination(deps, a.config.DefaultIntentionPolicy) sources.InternalServiceDump = proxycfgglue.ServerInternalServiceDump(deps, proxycfgglue.CacheInternalServiceDump(a.cache)) sources.PeeringList = proxycfgglue.ServerPeeringList(deps) sources.PeeredUpstreams = proxycfgglue.ServerPeeredUpstreams(deps) diff --git a/agent/agent_endpoint.go b/agent/agent_endpoint.go index 2a1c23d99300..5360eafe285b 100644 --- a/agent/agent_endpoint.go +++ b/agent/agent_endpoint.go @@ -1760,8 +1760,12 @@ func (s *HTTPHandlers) AgentConnectAuthorize(resp http.ResponseWriter, req *http // This is an L7 intention, so DENY. authorized = false } + } else if s.agent.config.DefaultIntentionPolicy != "" { + reason = "Default intention policy" + authorized = s.agent.config.DefaultIntentionPolicy == structs.IntentionDefaultPolicyAllow } else { reason = "Default behavior configured by ACLs" + //nolint:staticcheck authorized = authz.IntentionDefaultAllow(nil) == acl.Allow } diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 51308a7945fa..9fd76fae4fa0 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -8015,76 +8015,104 @@ func TestAgentConnectAuthorize_serviceWrite(t *testing.T) { assert.Equal(t, http.StatusForbidden, resp.Code) } -// Test when no intentions match w/ a default deny policy -func TestAgentConnectAuthorize_defaultDeny(t *testing.T) { +func TestAgentConnectAuthorize_DefaultIntentionPolicy(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") } t.Parallel() - a := NewTestAgent(t, TestACLConfig()) - defer a.Shutdown() - testrpc.WaitForLeader(t, a.RPC, "dc1") - - args := &structs.ConnectAuthorizeRequest{ - Target: "foo", - ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(), - } - req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) - req.Header.Add("X-Consul-Token", "root") - resp := httptest.NewRecorder() - a.srv.h.ServeHTTP(resp, req) - assert.Equal(t, 200, resp.Code) - - dec := json.NewDecoder(resp.Body) - obj := &connectAuthorizeResp{} - require.NoError(t, dec.Decode(obj)) - assert.False(t, obj.Authorized) - assert.Contains(t, obj.Reason, "Default behavior") -} - -// Test when no intentions match w/ a default allow policy -func TestAgentConnectAuthorize_defaultAllow(t *testing.T) { - if testing.Short() { - t.Skip("too slow for testing.Short") - } - - t.Parallel() + agentConfig := `primary_datacenter = "dc1" +default_intention_policy = "%s" +` + aclBlock := `acl { + enabled = true + default_policy = "%s" + tokens { + initial_management = "root" + agent = "root" + agent_recovery = "towel" + } +} +` + + type testcase struct { + aclsEnabled bool + defaultACL string + defaultIxn string + expectAuthz bool + expectReason string + } + tcs := map[string]testcase{ + "no ACLs, default intention allow": { + aclsEnabled: false, + defaultIxn: "allow", + expectAuthz: true, + expectReason: "Default intention policy", + }, + "no ACLs, default intention deny": { + aclsEnabled: false, + defaultIxn: "deny", + expectAuthz: false, + expectReason: "Default intention policy", + }, + "ACL deny, no intention policy": { + aclsEnabled: true, + defaultACL: "deny", + expectAuthz: false, + expectReason: "Default behavior configured by ACLs", + }, + "ACL allow, no intention policy": { + aclsEnabled: true, + defaultACL: "allow", + expectAuthz: true, + expectReason: "Default behavior configured by ACLs", + }, + "ACL deny, default intentions allow": { + aclsEnabled: true, + defaultACL: "deny", + defaultIxn: "allow", + expectAuthz: true, + expectReason: "Default intention policy", + }, + "ACL allow, default intentions deny": { + aclsEnabled: true, + defaultACL: "allow", + defaultIxn: "deny", + expectAuthz: false, + expectReason: "Default intention policy", + }, + } + for name, tc := range tcs { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() - dc1 := "dc1" - a := NewTestAgent(t, ` - primary_datacenter = "`+dc1+`" + conf := fmt.Sprintf(agentConfig, tc.defaultIxn) + if tc.aclsEnabled { + conf += fmt.Sprintf(aclBlock, tc.defaultACL) + } + a := NewTestAgent(t, conf) - acl { - enabled = true - default_policy = "allow" + testrpc.WaitForLeader(t, a.RPC, "dc1") - tokens { - initial_management = "root" - agent = "root" - agent_recovery = "towel" + args := &structs.ConnectAuthorizeRequest{ + Target: "foo", + ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(), } - } - `) - defer a.Shutdown() - testrpc.WaitForTestAgent(t, a.RPC, dc1) + req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) + req.Header.Add("X-Consul-Token", "root") + resp := httptest.NewRecorder() + a.srv.h.ServeHTTP(resp, req) + assert.Equal(t, 200, resp.Code) - args := &structs.ConnectAuthorizeRequest{ - Target: "foo", - ClientCertURI: connect.TestSpiffeIDService(t, "web").URI().String(), + dec := json.NewDecoder(resp.Body) + obj := &connectAuthorizeResp{} + require.NoError(t, dec.Decode(obj)) + assert.Equal(t, tc.expectAuthz, obj.Authorized) + assert.Contains(t, obj.Reason, tc.expectReason) + }) } - req, _ := http.NewRequest("POST", "/v1/agent/connect/authorize", jsonReader(args)) - req.Header.Add("X-Consul-Token", "root") - resp := httptest.NewRecorder() - a.srv.h.ServeHTTP(resp, req) - assert.Equal(t, 200, resp.Code) - - dec := json.NewDecoder(resp.Body) - obj := &connectAuthorizeResp{} - require.NoError(t, dec.Decode(obj)) - assert.True(t, obj.Authorized) - assert.Contains(t, obj.Reason, "Default behavior") } func TestAgent_Host(t *testing.T) { diff --git a/agent/config/builder.go b/agent/config/builder.go index 3918daabd771..0ed6f0304fc7 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1000,6 +1000,7 @@ func (b *builder) build() (rt RuntimeConfig, err error) { DataDir: dataDir, Datacenter: datacenter, DefaultQueryTime: b.durationVal("default_query_time", c.DefaultQueryTime), + DefaultIntentionPolicy: stringVal(c.DefaultIntentionPolicy), DevMode: boolVal(b.opts.DevMode), DisableAnonymousSignature: boolVal(c.DisableAnonymousSignature), DisableCoordinates: boolVal(c.DisableCoordinates), diff --git a/agent/config/config.go b/agent/config/config.go index d620b8c2f4e4..013f14dabf1b 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -165,6 +165,7 @@ type Config struct { DataDir *string `mapstructure:"data_dir" json:"data_dir,omitempty"` Datacenter *string `mapstructure:"datacenter" json:"datacenter,omitempty"` DefaultQueryTime *string `mapstructure:"default_query_time" json:"default_query_time,omitempty"` + DefaultIntentionPolicy *string `mapstructure:"default_intention_policy" json:"default_intention_policy,omitempty"` DisableAnonymousSignature *bool `mapstructure:"disable_anonymous_signature" json:"disable_anonymous_signature,omitempty"` DisableCoordinates *bool `mapstructure:"disable_coordinates" json:"disable_coordinates,omitempty"` DisableHostNodeID *bool `mapstructure:"disable_host_node_id" json:"disable_host_node_id,omitempty"` diff --git a/agent/config/runtime.go b/agent/config/runtime.go index 9e4e4ace0066..2ac7ea19d9f1 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -564,6 +564,15 @@ type RuntimeConfig struct { // flag: -data-dir string DataDir string + // DefaultIntentionPolicy is used to define a default intention action for all + // sources and destinations. Possible values are "allow", "deny", or "" (blank). + // For compatibility, falls back to ACLResolverSettings.ACLDefaultPolicy (which + // itself has a default of "allow") if left blank. Future versions of Consul + // will default this field to "deny" to be secure by default. + // + // hcl: default_intention_policy = string + DefaultIntentionPolicy string + // DefaultQueryTime is the amount of time a blocking query will wait before // Consul will force a response. This value can be overridden by the 'wait' // query parameter. diff --git a/agent/config/testdata/TestRuntimeConfig_Sanitize.golden b/agent/config/testdata/TestRuntimeConfig_Sanitize.golden index 56397ad94d0f..da0eec7e247a 100644 --- a/agent/config/testdata/TestRuntimeConfig_Sanitize.golden +++ b/agent/config/testdata/TestRuntimeConfig_Sanitize.golden @@ -134,11 +134,11 @@ "ClientSecret": "hidden", "Hostname": "", "ManagementToken": "hidden", + "NodeID": "", + "NodeName": "", "ResourceID": "cluster1", "ScadaAddress": "", - "TLSConfig": null, - "NodeID": "", - "NodeName": "" + "TLSConfig": null }, "ConfigEntryBootstrap": [], "ConnectCAConfig": {}, @@ -185,6 +185,7 @@ "DNSUseCache": false, "DataDir": "", "Datacenter": "", + "DefaultIntentionPolicy": "", "DefaultQueryTime": "0s", "DevMode": false, "DisableAnonymousSignature": false, diff --git a/agent/consul/acl.go b/agent/consul/acl.go index ddba359d949f..cdfcad640a3c 100644 --- a/agent/consul/acl.go +++ b/agent/consul/acl.go @@ -221,6 +221,23 @@ type ACLResolverSettings struct { ACLDefaultPolicy string } +func (a ACLResolverSettings) CheckACLs() error { + switch a.ACLDefaultPolicy { + case "allow": + case "deny": + default: + return fmt.Errorf("Unsupported default ACL policy: %s", a.ACLDefaultPolicy) + } + switch a.ACLDownPolicy { + case "allow": + case "deny": + case "async-cache", "extend-cache": + default: + return fmt.Errorf("Unsupported down ACL policy: %s", a.ACLDownPolicy) + } + return nil +} + func (s ACLResolverSettings) IsDefaultAllow() (bool, error) { switch s.ACLDefaultPolicy { case "allow": diff --git a/agent/consul/client.go b/agent/consul/client.go index 5216ffe2d500..f47a2871d130 100644 --- a/agent/consul/client.go +++ b/agent/consul/client.go @@ -107,7 +107,7 @@ func NewClient(config *Config, deps Deps) (*Client, error) { if config.DataDir == "" { return nil, fmt.Errorf("Config must provide a DataDir") } - if err := config.CheckACL(); err != nil { + if err := config.CheckEnumStrings(); err != nil { return nil, err } diff --git a/agent/consul/config.go b/agent/consul/config.go index edc5423ca4fa..a5ab21f7311a 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -411,6 +411,13 @@ type Config struct { // datacenters should exclusively traverse mesh gateways. ConnectMeshGatewayWANFederationEnabled bool + // DefaultIntentionPolicy is used to define a default intention action for all + // sources and destinations. Possible values are "allow", "deny", or "" (blank). + // For compatibility, falls back to ACLResolverSettings.ACLDefaultPolicy (which + // itself has a default of "allow") if left blank. Future versions of Consul + // will default this field to "deny" to be secure by default. + DefaultIntentionPolicy string + // DisableFederationStateAntiEntropy solely exists for use in unit tests to // disable a background routine. DisableFederationStateAntiEntropy bool @@ -470,21 +477,17 @@ func (c *Config) CheckProtocolVersion() error { return nil } -// CheckACL validates the ACL configuration. -// TODO: move this to ACLResolverSettings -func (c *Config) CheckACL() error { - switch c.ACLResolverSettings.ACLDefaultPolicy { - case "allow": - case "deny": - default: - return fmt.Errorf("Unsupported default ACL policy: %s", c.ACLResolverSettings.ACLDefaultPolicy) +// CheckEnumStrings validates string configuration which must be specific values. +func (c *Config) CheckEnumStrings() error { + if err := c.ACLResolverSettings.CheckACLs(); err != nil { + return err } - switch c.ACLResolverSettings.ACLDownPolicy { - case "allow": - case "deny": - case "async-cache", "extend-cache": + switch c.DefaultIntentionPolicy { + case structs.IntentionDefaultPolicyAllow: + case structs.IntentionDefaultPolicyDeny: + case "": default: - return fmt.Errorf("Unsupported down ACL policy: %s", c.ACLResolverSettings.ACLDownPolicy) + return fmt.Errorf("Unsupported default intention policy: %s", c.DefaultIntentionPolicy) } return nil } diff --git a/agent/consul/intention_endpoint.go b/agent/consul/intention_endpoint.go index a3e4ad678b1d..df0542814597 100644 --- a/agent/consul/intention_endpoint.go +++ b/agent/consul/intention_endpoint.go @@ -752,19 +752,7 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In } } - // Note: the default intention policy is like an intention with a - // wildcarded destination in that it is limited to L4-only. - - // No match, we need to determine the default behavior. We do this by - // fetching the default intention behavior from the resolved authorizer. - // The default behavior if ACLs are disabled is to allow connections - // to mimic the behavior of Consul itself: everything is allowed if - // ACLs are disabled. - // - // NOTE(mitchellh): This is the same behavior as the agent authorize - // endpoint. If this behavior is incorrect, we should also change it there - // which is much more important. - defaultDecision := authz.IntentionDefaultAllow(nil) + defaultAllow := DefaultIntentionAllow(authz, s.srv.config.DefaultIntentionPolicy) store := s.srv.fsm.State() @@ -784,7 +772,7 @@ func (s *Intention) Check(args *structs.IntentionQueryRequest, reply *structs.In Partition: query.DestinationPartition, Intentions: intentions, MatchType: structs.IntentionMatchDestination, - DefaultDecision: defaultDecision, + DefaultAllow: defaultAllow, AllowPermissions: false, } decision, err := store.IntentionDecision(opts) diff --git a/agent/consul/intention_endpoint_test.go b/agent/consul/intention_endpoint_test.go index 6296b8479fde..08480501d7bf 100644 --- a/agent/consul/intention_endpoint_test.go +++ b/agent/consul/intention_endpoint_test.go @@ -1960,106 +1960,89 @@ func TestIntentionMatch_acl(t *testing.T) { } } -// Test the Check method defaults to allow with no ACL set. -func TestIntentionCheck_defaultNoACL(t *testing.T) { +func TestIntentionCheck(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") } t.Parallel() - dir1, s1 := testServer(t) - defer os.RemoveAll(dir1) - defer s1.Shutdown() - codec := rpcClient(t, s1) - defer codec.Close() - - waitForLeaderEstablishment(t, s1) - - // Test - req := &structs.IntentionQueryRequest{ - Datacenter: "dc1", - Check: &structs.IntentionQueryCheck{ - SourceName: "bar", - DestinationName: "qux", - SourceType: structs.IntentionSourceConsul, - }, - } - var resp structs.IntentionQueryCheckResponse - require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.Check", req, &resp)) - require.True(t, resp.Allowed) -} - -// Test the Check method defaults to deny with allowlist ACLs. -func TestIntentionCheck_defaultACLDeny(t *testing.T) { - if testing.Short() { - t.Skip("too slow for testing.Short") + type testcase struct { + aclsEnabled bool + defaultACL string + defaultIxn string + expectAllowed bool } - - t.Parallel() - - dir1, s1 := testServerWithConfig(t, func(c *Config) { - c.PrimaryDatacenter = "dc1" - c.ACLsEnabled = true - c.ACLInitialManagementToken = "root" - c.ACLResolverSettings.ACLDefaultPolicy = "deny" - }) - defer os.RemoveAll(dir1) - defer s1.Shutdown() - codec := rpcClient(t, s1) - defer codec.Close() - - waitForLeaderEstablishment(t, s1) - - // Check - req := &structs.IntentionQueryRequest{ - Datacenter: "dc1", - Check: &structs.IntentionQueryCheck{ - SourceName: "bar", - DestinationName: "qux", - SourceType: structs.IntentionSourceConsul, + tcs := map[string]testcase{ + "acls disabled, no default intention policy": { + aclsEnabled: false, + expectAllowed: true, + }, + "acls disabled, default intention allow": { + aclsEnabled: false, + defaultIxn: "allow", + expectAllowed: true, + }, + "acls disabled, default intention deny": { + aclsEnabled: false, + defaultIxn: "deny", + expectAllowed: false, + }, + "acls deny, no default intention policy": { + aclsEnabled: true, + defaultACL: "deny", + expectAllowed: false, + }, + "acls allow, no default intention policy": { + aclsEnabled: true, + defaultACL: "allow", + expectAllowed: true, + }, + "acls deny, default intention allow": { + aclsEnabled: true, + defaultACL: "deny", + defaultIxn: "allow", + expectAllowed: true, + }, + "acls allow, default intention deny": { + aclsEnabled: true, + defaultACL: "allow", + defaultIxn: "deny", + expectAllowed: false, }, } - req.Token = "root" - var resp structs.IntentionQueryCheckResponse - require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.Check", req, &resp)) - require.False(t, resp.Allowed) -} - -// Test the Check method defaults to deny with denylist ACLs. -func TestIntentionCheck_defaultACLAllow(t *testing.T) { - if testing.Short() { - t.Skip("too slow for testing.Short") - } - - t.Parallel() + for name, tc := range tcs { + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + _, s1 := testServerWithConfig(t, func(c *Config) { + if tc.aclsEnabled { + c.PrimaryDatacenter = "dc1" + c.ACLsEnabled = true + c.ACLInitialManagementToken = "root" + c.ACLResolverSettings.ACLDefaultPolicy = tc.defaultACL + } + c.DefaultIntentionPolicy = tc.defaultIxn + }) + codec := rpcClient(t, s1) - dir1, s1 := testServerWithConfig(t, func(c *Config) { - c.PrimaryDatacenter = "dc1" - c.ACLsEnabled = true - c.ACLInitialManagementToken = "root" - c.ACLResolverSettings.ACLDefaultPolicy = "allow" - }) - defer os.RemoveAll(dir1) - defer s1.Shutdown() - codec := rpcClient(t, s1) - defer codec.Close() + waitForLeaderEstablishment(t, s1) - waitForLeaderEstablishment(t, s1) + req := &structs.IntentionQueryRequest{ + Datacenter: "dc1", + Check: &structs.IntentionQueryCheck{ + SourceName: "bar", + DestinationName: "qux", + SourceType: structs.IntentionSourceConsul, + }, + } + req.Token = "root" - // Check - req := &structs.IntentionQueryRequest{ - Datacenter: "dc1", - Check: &structs.IntentionQueryCheck{ - SourceName: "bar", - DestinationName: "qux", - SourceType: structs.IntentionSourceConsul, - }, + var resp structs.IntentionQueryCheckResponse + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.Check", req, &resp)) + require.Equal(t, tc.expectAllowed, resp.Allowed) + }) } - req.Token = "root" - var resp structs.IntentionQueryCheckResponse - require.NoError(t, msgpackrpc.CallWithCodec(codec, "Intention.Check", req, &resp)) - require.True(t, resp.Allowed) } // Test the Check method requires service:read permission. diff --git a/agent/consul/internal_endpoint.go b/agent/consul/internal_endpoint.go index 84ea48d0563e..af27842d2045 100644 --- a/agent/consul/internal_endpoint.go +++ b/agent/consul/internal_endpoint.go @@ -306,7 +306,7 @@ func (m *Internal) ServiceTopology(args *structs.ServiceSpecificRequest, reply * &args.QueryOptions, &reply.QueryMeta, func(ws memdb.WatchSet, state *state.Store) error { - defaultAllow := authz.IntentionDefaultAllow(nil) + defaultAllow := DefaultIntentionAllow(authz, m.srv.config.DefaultIntentionPolicy) index, topology, err := state.ServiceTopology(ws, args.Datacenter, args.ServiceName, args.ServiceKind, defaultAllow, &args.EnterpriseMeta) if err != nil { @@ -375,10 +375,10 @@ func (m *Internal) internalUpstreams(args *structs.ServiceSpecificRequest, reply &args.QueryOptions, &reply.QueryMeta, func(ws memdb.WatchSet, state *state.Store) error { - defaultDecision := authz.IntentionDefaultAllow(nil) + defaultAllow := DefaultIntentionAllow(authz, m.srv.config.DefaultIntentionPolicy) sn := structs.NewServiceName(args.ServiceName, &args.EnterpriseMeta) - index, services, err := state.IntentionTopology(ws, sn, false, defaultDecision, intentionTarget) + index, services, err := state.IntentionTopology(ws, sn, false, defaultAllow, intentionTarget) if err != nil { return err } diff --git a/agent/consul/internal_endpoint_test.go b/agent/consul/internal_endpoint_test.go index c11f43494c33..a7f132810145 100644 --- a/agent/consul/internal_endpoint_test.go +++ b/agent/consul/internal_endpoint_test.go @@ -2384,14 +2384,12 @@ func TestInternal_ServiceTopology_ACL(t *testing.T) { } t.Parallel() - dir1, s1 := testServerWithConfig(t, func(c *Config) { + _, s1 := testServerWithConfig(t, func(c *Config) { c.PrimaryDatacenter = "dc1" c.ACLsEnabled = true c.ACLInitialManagementToken = TestDefaultInitialManagementToken c.ACLResolverSettings.ACLDefaultPolicy = "deny" }) - defer os.RemoveAll(dir1) - defer s1.Shutdown() testrpc.WaitForLeader(t, s1.RPC, "dc1") @@ -2473,6 +2471,40 @@ service "web" { policy = "read" } }) } +// Tests that default intention deny policy overrides the ACL allow policy. +// More comprehensive tests are done at the state store so this is minimal +// coverage to be confident that the override happens. +func TestInternal_ServiceTopology_DefaultIntentionPolicy(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + _, s1 := testServerWithConfig(t, func(c *Config) { + c.PrimaryDatacenter = "dc1" + c.ACLsEnabled = true + c.ACLInitialManagementToken = TestDefaultInitialManagementToken + c.ACLResolverSettings.ACLDefaultPolicy = "allow" + c.DefaultIntentionPolicy = "deny" + }) + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + codec := rpcClient(t, s1) + + registerTestTopologyEntries(t, codec, TestDefaultInitialManagementToken) + + args := structs.ServiceSpecificRequest{ + Datacenter: "dc1", + ServiceName: "redis", + QueryOptions: structs.QueryOptions{Token: TestDefaultInitialManagementToken}, + } + var out structs.IndexedServiceTopology + require.NoError(t, msgpackrpc.CallWithCodec(codec, "Internal.ServiceTopology", &args, &out)) + + webSN := structs.NewServiceName("web", acl.DefaultEnterpriseMeta()) + require.False(t, out.ServiceTopology.DownstreamDecisions[webSN.String()].DefaultAllow) +} + func TestInternal_IntentionUpstreams(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") diff --git a/agent/consul/server.go b/agent/consul/server.go index 9513b839f7f1..a3090dd81c0c 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -526,7 +526,7 @@ func NewServer(config *Config, flat Deps, externalGRPCServer *grpc.Server, if config.DataDir == "" && !config.DevMode { return nil, fmt.Errorf("Config must provide a DataDir") } - if err := config.CheckACL(); err != nil { + if err := config.CheckEnumStrings(); err != nil { return nil, err } diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 993dd9b0b283..8973381f2b08 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -4315,7 +4315,7 @@ func (s *Store) ServiceTopology( ws memdb.WatchSet, dc, service string, kind structs.ServiceKind, - defaultAllow acl.EnforcementDecision, + defaultAllow bool, entMeta *acl.EnterpriseMeta, ) (uint64, *structs.ServiceTopology, error) { tx := s.db.ReadTxn() @@ -4466,7 +4466,7 @@ func (s *Store) ServiceTopology( Partition: un.PartitionOrDefault(), Intentions: srcIntentions, MatchType: structs.IntentionMatchDestination, - DefaultDecision: defaultAllow, + DefaultAllow: defaultAllow, AllowPermissions: false, } decision, err := s.IntentionDecision(opts) @@ -4590,7 +4590,7 @@ func (s *Store) ServiceTopology( Partition: dn.PartitionOrDefault(), Intentions: dstIntentions, MatchType: structs.IntentionMatchSource, - DefaultDecision: defaultAllow, + DefaultAllow: defaultAllow, AllowPermissions: false, } decision, err := s.IntentionDecision(opts) diff --git a/agent/consul/state/intention.go b/agent/consul/state/intention.go index f36055522858..12c79cad5c94 100644 --- a/agent/consul/state/intention.go +++ b/agent/consul/state/intention.go @@ -743,7 +743,7 @@ type IntentionDecisionOpts struct { Peer string Intentions structs.SimplifiedIntentions MatchType structs.IntentionMatchType - DefaultDecision acl.EnforcementDecision + DefaultAllow bool AllowPermissions bool } @@ -763,7 +763,7 @@ func (s *Store) IntentionDecision(opts IntentionDecisionOpts) (structs.Intention } resp := structs.IntentionDecisionSummary{ - DefaultAllow: opts.DefaultDecision == acl.Allow, + DefaultAllow: opts.DefaultAllow, } if ixnMatch == nil { // No intention found, fall back to default @@ -1029,13 +1029,13 @@ func (s *Store) IntentionTopology( ws memdb.WatchSet, target structs.ServiceName, downstreams bool, - defaultDecision acl.EnforcementDecision, + defaultAllow bool, intentionTarget structs.IntentionTargetType, ) (uint64, structs.ServiceList, error) { tx := s.db.ReadTxn() defer tx.Abort() - idx, services, err := s.intentionTopologyTxn(tx, ws, target, downstreams, defaultDecision, intentionTarget) + idx, services, err := s.intentionTopologyTxn(tx, ws, target, downstreams, defaultAllow, intentionTarget) if err != nil { requested := "upstreams" if downstreams { @@ -1055,7 +1055,7 @@ func (s *Store) intentionTopologyTxn( tx ReadTxn, ws memdb.WatchSet, target structs.ServiceName, downstreams bool, - defaultDecision acl.EnforcementDecision, + defaultAllow bool, intentionTarget structs.IntentionTargetType, ) (uint64, []ServiceWithDecision, error) { @@ -1163,7 +1163,7 @@ func (s *Store) intentionTopologyTxn( Partition: candidate.PartitionOrDefault(), Intentions: intentions, MatchType: decisionMatchType, - DefaultDecision: defaultDecision, + DefaultAllow: defaultAllow, AllowPermissions: true, } decision, err := s.IntentionDecision(opts) diff --git a/agent/consul/state/intention_test.go b/agent/consul/state/intention_test.go index efa2ad6f7545..b1c7ccf7b89c 100644 --- a/agent/consul/state/intention_test.go +++ b/agent/consul/state/intention_test.go @@ -1966,27 +1966,27 @@ func TestStore_IntentionDecision(t *testing.T) { src string dst string matchType structs.IntentionMatchType - defaultDecision acl.EnforcementDecision + defaultAllow bool allowPermissions bool expect structs.IntentionDecisionSummary }{ { - name: "no matching intention and default deny", - src: "does-not-exist", - dst: "ditto", - matchType: structs.IntentionMatchDestination, - defaultDecision: acl.Deny, + name: "no matching intention and default deny", + src: "does-not-exist", + dst: "ditto", + matchType: structs.IntentionMatchDestination, + defaultAllow: false, expect: structs.IntentionDecisionSummary{ Allowed: false, DefaultAllow: false, }, }, { - name: "no matching intention and default allow", - src: "does-not-exist", - dst: "ditto", - matchType: structs.IntentionMatchDestination, - defaultDecision: acl.Allow, + name: "no matching intention and default allow", + src: "does-not-exist", + dst: "ditto", + matchType: structs.IntentionMatchDestination, + defaultAllow: true, expect: structs.IntentionDecisionSummary{ Allowed: true, DefaultAllow: true, @@ -2079,7 +2079,7 @@ func TestStore_IntentionDecision(t *testing.T) { Partition: acl.DefaultPartitionName, Intentions: intentions, MatchType: tc.matchType, - DefaultDecision: tc.defaultDecision, + DefaultAllow: tc.defaultAllow, AllowPermissions: tc.allowPermissions, } decision, err := s.IntentionDecision(opts) @@ -2161,7 +2161,7 @@ func TestStore_IntentionTopology(t *testing.T) { } tests := []struct { name string - defaultDecision acl.EnforcementDecision + defaultAllow bool intentions []structs.ServiceIntentionsConfigEntry discoveryChains []structs.ConfigEntry target structs.ServiceName @@ -2169,8 +2169,8 @@ func TestStore_IntentionTopology(t *testing.T) { expect expect }{ { - name: "(upstream) acl allow all but intentions deny one", - defaultDecision: acl.Allow, + name: "(upstream) default allow all but intentions deny one", + defaultAllow: true, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2196,8 +2196,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "(upstream) acl allow includes virtual service", - defaultDecision: acl.Allow, + name: "(upstream) default allow includes virtual service", + defaultAllow: true, discoveryChains: []structs.ConfigEntry{ &structs.ServiceResolverConfigEntry{ Kind: structs.ServiceResolver, @@ -2225,8 +2225,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "(upstream) acl deny all intentions allow virtual service", - defaultDecision: acl.Deny, + name: "(upstream) default deny intentions allow virtual service", + defaultAllow: false, discoveryChains: []structs.ConfigEntry{ &structs.ServiceResolverConfigEntry{ Kind: structs.ServiceResolver, @@ -2258,8 +2258,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "(upstream) acl deny all intentions allow one", - defaultDecision: acl.Deny, + name: "(upstream) default deny intentions allow one", + defaultAllow: false, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2285,8 +2285,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "(downstream) acl allow all but intentions deny one", - defaultDecision: acl.Allow, + name: "(downstream) default allow but intentions deny one", + defaultAllow: true, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2316,8 +2316,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "(downstream) acl deny all intentions allow one", - defaultDecision: acl.Deny, + name: "(downstream) default deny all intentions allow one", + defaultAllow: false, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2343,8 +2343,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "acl deny but intention allow all overrides it", - defaultDecision: acl.Deny, + name: "default deny but intention allow all overrides it", + defaultAllow: false, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2374,8 +2374,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "acl allow but intention deny all overrides it", - defaultDecision: acl.Allow, + name: "default allow but intention deny all overrides it", + defaultAllow: true, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2396,8 +2396,8 @@ func TestStore_IntentionTopology(t *testing.T) { }, }, { - name: "acl deny but intention allow all overrides it", - defaultDecision: acl.Deny, + name: "default deny but intention allow all overrides it", + defaultAllow: false, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2448,7 +2448,7 @@ func TestStore_IntentionTopology(t *testing.T) { idx++ } - idx, got, err := s.IntentionTopology(nil, tt.target, tt.downstreams, tt.defaultDecision, structs.IntentionTargetService) + idx, got, err := s.IntentionTopology(nil, tt.target, tt.downstreams, tt.defaultAllow, structs.IntentionTargetService) require.NoError(t, err) require.Equal(t, tt.expect.idx, idx) @@ -2502,16 +2502,16 @@ func TestStore_IntentionTopology_Destination(t *testing.T) { services structs.ServiceList } tests := []struct { - name string - defaultDecision acl.EnforcementDecision - intentions []structs.ServiceIntentionsConfigEntry - target structs.ServiceName - downstreams bool - expect expect + name string + defaultAllow bool + intentions []structs.ServiceIntentionsConfigEntry + target structs.ServiceName + downstreams bool + expect expect }{ { - name: "(upstream) acl allow all but intentions deny one, destination target", - defaultDecision: acl.Allow, + name: "(upstream) default allow all but intentions deny one, destination target", + defaultAllow: true, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2537,8 +2537,8 @@ func TestStore_IntentionTopology_Destination(t *testing.T) { }, }, { - name: "(upstream) acl deny all intentions allow one, destination target", - defaultDecision: acl.Deny, + name: "(upstream) default deny intentions allow one, destination target", + defaultAllow: false, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2564,8 +2564,8 @@ func TestStore_IntentionTopology_Destination(t *testing.T) { }, }, { - name: "(upstream) acl deny all check only destinations show, service target", - defaultDecision: acl.Deny, + name: "(upstream) default deny check only destinations show, service target", + defaultAllow: false, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2586,8 +2586,8 @@ func TestStore_IntentionTopology_Destination(t *testing.T) { }, }, { - name: "(upstream) acl allow all check only destinations show, service target", - defaultDecision: acl.Allow, + name: "(upstream) default allow check only destinations show, service target", + defaultAllow: true, intentions: []structs.ServiceIntentionsConfigEntry{ { Kind: structs.ServiceIntentions, @@ -2638,7 +2638,7 @@ func TestStore_IntentionTopology_Destination(t *testing.T) { idx++ } - idx, got, err := s.IntentionTopology(nil, tt.target, tt.downstreams, tt.defaultDecision, structs.IntentionTargetDestination) + idx, got, err := s.IntentionTopology(nil, tt.target, tt.downstreams, tt.defaultAllow, structs.IntentionTargetDestination) require.NoError(t, err) require.Equal(t, tt.expect.idx, idx) @@ -2665,7 +2665,7 @@ func TestStore_IntentionTopology_Watches(t *testing.T) { target := structs.NewServiceName("web", structs.DefaultEnterpriseMetaInDefaultPartition()) ws := memdb.NewWatchSet() - index, got, err := s.IntentionTopology(ws, target, false, acl.Deny, structs.IntentionTargetService) + index, got, err := s.IntentionTopology(ws, target, false, false, structs.IntentionTargetService) require.NoError(t, err) require.Equal(t, uint64(0), index) require.Empty(t, got) @@ -2687,7 +2687,7 @@ func TestStore_IntentionTopology_Watches(t *testing.T) { // Reset the WatchSet ws = memdb.NewWatchSet() - index, got, err = s.IntentionTopology(ws, target, false, acl.Deny, structs.IntentionTargetService) + index, got, err = s.IntentionTopology(ws, target, false, false, structs.IntentionTargetService) require.NoError(t, err) require.Equal(t, uint64(2), index) // Because API is a virtual service, it is included in this output. @@ -2709,7 +2709,7 @@ func TestStore_IntentionTopology_Watches(t *testing.T) { // require.False(t, watchFired(ws)) // Result should not have changed - index, got, err = s.IntentionTopology(ws, target, false, acl.Deny, structs.IntentionTargetService) + index, got, err = s.IntentionTopology(ws, target, false, false, structs.IntentionTargetService) require.NoError(t, err) require.Equal(t, uint64(3), index) require.Equal(t, structs.ServiceList{structs.NewServiceName("api", nil)}, got) @@ -2724,7 +2724,7 @@ func TestStore_IntentionTopology_Watches(t *testing.T) { require.True(t, watchFired(ws)) // Reset the WatchSet - index, got, err = s.IntentionTopology(nil, target, false, acl.Deny, structs.IntentionTargetService) + index, got, err = s.IntentionTopology(nil, target, false, false, structs.IntentionTargetService) require.NoError(t, err) require.Equal(t, uint64(4), index) diff --git a/agent/consul/util.go b/agent/consul/util.go index 0fd88e14c8eb..b5827f659a38 100644 --- a/agent/consul/util.go +++ b/agent/consul/util.go @@ -11,7 +11,9 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/serf/serf" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/metadata" + "github.com/hashicorp/consul/agent/structs" ) // CanServersUnderstandProtocol checks to see if all the servers in the given @@ -171,3 +173,14 @@ func isSerfMember(s *serf.Serf, nodeName string) bool { } return false } + +func DefaultIntentionAllow(authz acl.Authorizer, defaultIntentionPolicy string) bool { + // The default intention policy inherits from ACLs but + // is overridden by the agent's DefaultIntentionPolicy. + //nolint:staticcheck + defaultAllow := authz.IntentionDefaultAllow(nil) == acl.Allow + if defaultIntentionPolicy != "" { + defaultAllow = defaultIntentionPolicy == structs.IntentionDefaultPolicyAllow + } + return defaultAllow +} diff --git a/agent/proxycfg-glue/glue.go b/agent/proxycfg-glue/glue.go index a4fa538b9285..b88c7d2a419e 100644 --- a/agent/proxycfg-glue/glue.go +++ b/agent/proxycfg-glue/glue.go @@ -43,7 +43,7 @@ type Store interface { FederationStateList(ws memdb.WatchSet) (uint64, []*structs.FederationState, error) GatewayServices(ws memdb.WatchSet, gateway string, entMeta *acl.EnterpriseMeta) (uint64, structs.GatewayServices, error) IntentionMatchOne(ws memdb.WatchSet, entry structs.IntentionMatchEntry, matchType structs.IntentionMatchType, destinationType structs.IntentionTargetType) (uint64, structs.SimplifiedIntentions, error) - IntentionTopology(ws memdb.WatchSet, target structs.ServiceName, downstreams bool, defaultDecision acl.EnforcementDecision, intentionTarget structs.IntentionTargetType) (uint64, structs.ServiceList, error) + IntentionTopology(ws memdb.WatchSet, target structs.ServiceName, downstreams bool, defaultDecision bool, intentionTarget structs.IntentionTargetType) (uint64, structs.ServiceList, error) ReadResolvedServiceConfigEntries(ws memdb.WatchSet, serviceName string, entMeta *acl.EnterpriseMeta, upstreamIDs []structs.ServiceID, proxyMode structs.ProxyMode) (uint64, *configentry.ResolvedServiceConfigSet, error) ServiceDiscoveryChain(ws memdb.WatchSet, serviceName string, entMeta *acl.EnterpriseMeta, req discoverychain.CompileRequest) (uint64, *structs.CompiledDiscoveryChain, *configentry.DiscoveryChainSet, error) ServiceDump(ws memdb.WatchSet, kind structs.ServiceKind, useKind bool, entMeta *acl.EnterpriseMeta, peerName string) (uint64, structs.CheckServiceNodes, error) diff --git a/agent/proxycfg-glue/intention_upstreams.go b/agent/proxycfg-glue/intention_upstreams.go index dc6731372271..846eb2a72b26 100644 --- a/agent/proxycfg-glue/intention_upstreams.go +++ b/agent/proxycfg-glue/intention_upstreams.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/consul/agent/cache" cachetype "github.com/hashicorp/consul/agent/cache-types" + "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/consul/watch" "github.com/hashicorp/consul/agent/proxycfg" "github.com/hashicorp/consul/agent/structs" @@ -33,20 +34,21 @@ func CacheIntentionUpstreamsDestination(c *cache.Cache) proxycfg.IntentionUpstre // ServerIntentionUpstreams satisfies the proxycfg.IntentionUpstreams interface // by sourcing upstreams for the given service, inferred from intentions, from // the server's state store. -func ServerIntentionUpstreams(deps ServerDataSourceDeps) proxycfg.IntentionUpstreams { - return serverIntentionUpstreams{deps, structs.IntentionTargetService} +func ServerIntentionUpstreams(deps ServerDataSourceDeps, defaultIntentionPolicy string) proxycfg.IntentionUpstreams { + return serverIntentionUpstreams{deps, structs.IntentionTargetService, defaultIntentionPolicy} } // ServerIntentionUpstreamsDestination satisfies the proxycfg.IntentionUpstreams // interface by sourcing upstreams for the given destination, inferred from // intentions, from the server's state store. -func ServerIntentionUpstreamsDestination(deps ServerDataSourceDeps) proxycfg.IntentionUpstreams { - return serverIntentionUpstreams{deps, structs.IntentionTargetDestination} +func ServerIntentionUpstreamsDestination(deps ServerDataSourceDeps, defaultIntentionPolicy string) proxycfg.IntentionUpstreams { + return serverIntentionUpstreams{deps, structs.IntentionTargetDestination, defaultIntentionPolicy} } type serverIntentionUpstreams struct { - deps ServerDataSourceDeps - target structs.IntentionTargetType + deps ServerDataSourceDeps + target structs.IntentionTargetType + defaultIntentionPolicy string } func (s serverIntentionUpstreams) Notify(ctx context.Context, req *structs.ServiceSpecificRequest, correlationID string, ch chan<- proxycfg.UpdateEvent) error { @@ -58,9 +60,10 @@ func (s serverIntentionUpstreams) Notify(ctx context.Context, req *structs.Servi if err != nil { return 0, nil, err } - defaultDecision := authz.IntentionDefaultAllow(nil) - index, services, err := store.IntentionTopology(ws, target, false, defaultDecision, s.target) + defaultAllow := consul.DefaultIntentionAllow(authz, s.defaultIntentionPolicy) + + index, services, err := store.IntentionTopology(ws, target, false, defaultAllow, s.target) if err != nil { return 0, nil, err } diff --git a/agent/proxycfg-glue/intention_upstreams_test.go b/agent/proxycfg-glue/intention_upstreams_test.go index bfaeb55b3864..b1c77aaf3ae9 100644 --- a/agent/proxycfg-glue/intention_upstreams_test.go +++ b/agent/proxycfg-glue/intention_upstreams_test.go @@ -66,7 +66,7 @@ func TestServerIntentionUpstreams(t *testing.T) { dataSource := ServerIntentionUpstreams(ServerDataSourceDeps{ ACLResolver: newStaticResolver(authz), GetStore: func() Store { return store }, - }) + }, "") ch := make(chan proxycfg.UpdateEvent) err := dataSource.Notify(ctx, &structs.ServiceSpecificRequest{ServiceName: serviceName}, "", ch) @@ -84,6 +84,47 @@ func TestServerIntentionUpstreams(t *testing.T) { require.Equal(t, "db", result.Services[0].Name) } +// Variant of TestServerIntentionUpstreams where a default allow intention policy +// returns "db" service as an IntentionUpstream even if there are no explicit +// intentions for "db". +func TestServerIntentionUpstreams_DefaultIntentionPolicy(t *testing.T) { + const serviceName = "web" + + var index uint64 + getIndex := func() uint64 { + index++ + return index + } + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + store := state.NewStateStore(nil) + disableLegacyIntentions(t, store) + + require.NoError(t, store.EnsureRegistration(getIndex(), &structs.RegisterRequest{ + Node: "node-1", + Service: &structs.NodeService{ + Service: "db", + }, + })) + + // Ensures that "db" service will not be filtered due to ACLs + authz := policyAuthorizer(t, `service "db" { policy = "read" }`) + + dataSource := ServerIntentionUpstreams(ServerDataSourceDeps{ + ACLResolver: newStaticResolver(authz), + GetStore: func() Store { return store }, + }, "allow") + + ch := make(chan proxycfg.UpdateEvent) + require.NoError(t, dataSource.Notify(ctx, &structs.ServiceSpecificRequest{ServiceName: serviceName}, "", ch)) + + result := getEventResult[*structs.IndexedServiceList](t, ch) + require.Len(t, result.Services, 1) + require.Equal(t, "db", result.Services[0].Name) +} + func disableLegacyIntentions(t *testing.T, store *state.Store) { t.Helper() diff --git a/agent/structs/intention.go b/agent/structs/intention.go index 3bbc2584f70f..95e9d8388aae 100644 --- a/agent/structs/intention.go +++ b/agent/structs/intention.go @@ -30,6 +30,9 @@ const ( // fix up all the places where this was used with the proper namespace // value. IntentionDefaultNamespace = "default" + + IntentionDefaultPolicyAllow = "allow" + IntentionDefaultPolicyDeny = "deny" ) // Intention defines an intention for the Connect Service Graph. This defines @@ -727,7 +730,7 @@ type IntentionQueryCheckResponse struct { // - Whether the matching intention has L7 permissions attached // - Whether the intention is managed by an external source like k8s // - Whether there is an exact, or wildcard, intention referencing the two services -// - Whether ACLs are in DefaultAllow mode +// - Whether intentions are in DefaultAllow mode type IntentionDecisionSummary struct { Allowed bool HasPermissions bool