Add default intention policy (#20544)

.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
+```

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 {

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)
 	})
 }

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)

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
 	}
 

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) {

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),

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"`

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.

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,

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":

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
 	}
 

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
 }