From ab7c48540a812994571775ead4841f828c9b178a Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Tue, 10 Mar 2020 17:02:25 -0400 Subject: [PATCH] Configure anonymous token policy for connect When running Consul Connect, cross-dc calls require that the anonymous token has read permissions on all services. This change updates the server-acl-init command to give the anonymous token those permissions if connect is enabled. Since we already set those permissions in the case of dns being enabled, the change was to also set those permissions in the case of connect being enabled. To detect connect being enabled, we used the presence of the -create-inject-auth-method flag since that's set when connect is enabled. The policy was renamed from dns-policy to anonymous-token-policy since it applies for more than just dns now. In existing installations, a new policy with that name will be created and attached to the anonymous token that will duplicate the old dns-policy but will have no detrimental effects. --- subcommand/server-acl-init/anonymous_token.go | 43 +++++ subcommand/server-acl-init/command.go | 13 +- .../server-acl-init/command_ent_test.go | 4 +- subcommand/server-acl-init/command_test.go | 165 ++++++++++-------- subcommand/server-acl-init/dns.go | 43 ----- subcommand/server-acl-init/rules.go | 22 ++- subcommand/server-acl-init/rules_test.go | 6 +- 7 files changed, 160 insertions(+), 136 deletions(-) create mode 100644 subcommand/server-acl-init/anonymous_token.go delete mode 100644 subcommand/server-acl-init/dns.go diff --git a/subcommand/server-acl-init/anonymous_token.go b/subcommand/server-acl-init/anonymous_token.go new file mode 100644 index 0000000000..763a815988 --- /dev/null +++ b/subcommand/server-acl-init/anonymous_token.go @@ -0,0 +1,43 @@ +package serveraclinit + +import ( + "github.com/hashicorp/consul/api" +) + +// configureAnonymousPolicy sets up policies and tokens so that Consul DNS and +// cross-datacenter Consul connect calls will work. +func (c *Command) configureAnonymousPolicy(consulClient *api.Client) error { + dnsRules, err := c.anonymousTokenRules() + if err != nil { + c.Log.Error("Error templating anonymous token rules", "err", err) + return err + } + + // Create policy for the anonymous token + anonPolicy := api.ACLPolicy{ + Name: "anonymous-token-policy", + Description: "Anonymous token Policy", + Rules: dnsRules, + } + + err = c.untilSucceeds("creating anonymous token policy - PUT /v1/acl/policy", + func() error { + return c.createOrUpdateACLPolicy(anonPolicy, consulClient) + }) + if err != nil { + return err + } + + // Create token to get sent to TokenUpdate + aToken := api.ACLToken{ + AccessorID: "00000000-0000-0000-0000-000000000002", + Policies: []*api.ACLTokenPolicyLink{{Name: anonPolicy.Name}}, + } + + // Update anonymous token to include this policy + return c.untilSucceeds("updating anonymous token with policy", + func() error { + _, _, err := consulClient.ACL().TokenUpdate(&aToken, &api.WriteOptions{}) + return err + }) +} diff --git a/subcommand/server-acl-init/command.go b/subcommand/server-acl-init/command.go index 1bcbf46a62..70af8e2bed 100644 --- a/subcommand/server-acl-init/command.go +++ b/subcommand/server-acl-init/command.go @@ -378,14 +378,15 @@ func (c *Command) Run(args []string) int { } } - // The DNS policy is attached to the anonymous token. - // If performing ACL replication, we assume that the primary datacenter - // has already created the DNS policy and attached it to the anonymous - // token. We don't want to modify the DNS policy in secondary datacenters + // The anonymous token policy needs to be configured if using Consul DNS + // or Consul Connect. If performing ACL replication, we assume that the + // primary datacenter has already created the anonymous policy and attached + // it to the anonymous token. + // We don't want to modify the anonymous policy in secondary datacenters // because it is global and we can't create separate tokens for each // secondary datacenter because the anonymous token is global. - if c.flagAllowDNS && c.flagACLReplicationTokenFile == "" { - err := c.configureDNSPolicies(consulClient) + if (c.flagAllowDNS || c.flagCreateInjectAuthMethod) && c.flagACLReplicationTokenFile == "" { + err := c.configureAnonymousPolicy(consulClient) if err != nil { c.Log.Error(err.Error()) return 1 diff --git a/subcommand/server-acl-init/command_ent_test.go b/subcommand/server-acl-init/command_ent_test.go index f287f278f9..971c2500de 100644 --- a/subcommand/server-acl-init/command_ent_test.go +++ b/subcommand/server-acl-init/command_ent_test.go @@ -249,7 +249,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { // Check that the expected policies were created. firstRunExpectedPolicies := []string{ - "dns-policy", + "anonymous-token-policy", "client-token", "catalog-sync-token", "connect-inject-token", @@ -295,7 +295,7 @@ func TestRun_ACLPolicyUpdates(t *testing.T) { // Check that the policies have all been updated. secondRunExpectedPolicies := []string{ - "dns-policy", + "anonymous-token-policy", "client-token", "catalog-sync-token", "connect-inject-token", diff --git a/subcommand/server-acl-init/command_test.go b/subcommand/server-acl-init/command_test.go index c943251590..fa02781722 100644 --- a/subcommand/server-acl-init/command_test.go +++ b/subcommand/server-acl-init/command_test.go @@ -439,56 +439,61 @@ func TestRun_TokensReplicatedDC(t *testing.T) { } } -func TestRun_AllowDNS(t *testing.T) { +func TestRun_AnonymousTokenPolicy(t *testing.T) { t.Parallel() - k8s, testSvr := completeSetup(t, resourcePrefix) - defer testSvr.Stop() - require := require.New(t) - // Run the command. - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - } - cmd.init() - cmdArgs := []string{ - "-server-label-selector=component=server,app=consul,release=" + releaseName, - "-resource-prefix=" + resourcePrefix, - "-k8s-namespace=" + ns, - "-expected-replicas=1", - "-allow-dns", - } - responseCode := cmd.Run(cmdArgs) - require.Equal(0, responseCode, ui.ErrorWriter.String()) + cases := []string{"-allow-dns", "-create-inject-auth-method"} + for _, flag := range cases { + t.Run(flag, func(t *testing.T) { + k8s, testSvr := completeSetup(t, resourcePrefix) + defer testSvr.Stop() + setUpK8sServiceAccount(t, k8s) - // Check that the dns policy was created. - bootToken := getBootToken(t, k8s, resourcePrefix, ns) - consul, err := api.NewClient(&api.Config{ - Address: testSvr.HTTPAddr, - Token: bootToken, - }) - require.NoError(err) - policy := policyExists(t, "dns-policy", consul) - // Should be a global policy. - require.Len(policy.Datacenters, 0) + // Run the command. + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } + cmd.init() + cmdArgs := append([]string{ + "-server-label-selector=component=server,app=consul,release=" + releaseName, + "-resource-prefix=" + resourcePrefix, + "-k8s-namespace=" + ns, + "-expected-replicas=1", + }, flag) + responseCode := cmd.Run(cmdArgs) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - // Check that the anonymous token has the DNS policy. - tokenData, _, err := consul.ACL().TokenReadSelf(&api.QueryOptions{Token: "anonymous"}) - require.NoError(err) - require.Equal("dns-policy", tokenData.Policies[0].Name) - - // Test that if the same command is re-run it doesn't error. - t.Run("retried", func(t *testing.T) { - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - } - cmd.init() - responseCode := cmd.Run(cmdArgs) - require.Equal(0, responseCode, ui.ErrorWriter.String()) - }) + // Check that the anonymous token policy was created. + bootToken := getBootToken(t, k8s, resourcePrefix, ns) + consul, err := api.NewClient(&api.Config{ + Address: testSvr.HTTPAddr, + Token: bootToken, + }) + require.NoError(t, err) + policy := policyExists(t, "anonymous-token-policy", consul) + // Should be a global policy. + require.Len(t, policy.Datacenters, 0) + + // Check that the anonymous token has the policy. + tokenData, _, err := consul.ACL().TokenReadSelf(&api.QueryOptions{Token: "anonymous"}) + require.NoError(t, err) + require.Equal(t, "anonymous-token-policy", tokenData.Policies[0].Name) + + // Test that if the same command is re-run it doesn't error. + t.Run("retried", func(t *testing.T) { + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } + cmd.init() + responseCode := cmd.Run(cmdArgs) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + }) + }) + } } func TestRun_ConnectInjectAuthMethod(t *testing.T) { @@ -1535,39 +1540,45 @@ func TestRun_ACLReplicationTokenValid(t *testing.T) { }) } -// Test that if acl replication is enabled, we don't create a dns policy. -func TestRun_AllowDNSFlag_IgnoredWithReplication(t *testing.T) { - bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" - tokenFile, fileCleanup := writeTempFile(t, bootToken) - defer fileCleanup() - k8s, consul, cleanup := mockReplicatedSetup(t, resourcePrefix, bootToken) - defer cleanup() +// Test that if acl replication is enabled, we don't create an anonymous token policy. +func TestRun_AnonPolicy_IgnoredWithReplication(t *testing.T) { + // The anonymous policy is configured when one of these flags is set. + cases := []string{"-allow-dns", "-create-inject-auth-method"} + for _, flag := range cases { + t.Run(flag, func(t *testing.T) { + bootToken := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + tokenFile, fileCleanup := writeTempFile(t, bootToken) + defer fileCleanup() + k8s, consul, cleanup := mockReplicatedSetup(t, resourcePrefix, bootToken) + setUpK8sServiceAccount(t, k8s) + defer cleanup() - // Run the command. - ui := cli.NewMockUi() - cmd := Command{ - UI: ui, - clientset: k8s, - } - cmd.init() - cmdArgs := []string{ - "-k8s-namespace=" + ns, - "-expected-replicas=1", - "-acl-replication-token-file", tokenFile, - "-server-label-selector=component=server,app=consul,release=" + releaseName, - "-resource-prefix=" + resourcePrefix, - "-allow-dns", - } - responseCode := cmd.Run(cmdArgs) - require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) + // Run the command. + ui := cli.NewMockUi() + cmd := Command{ + UI: ui, + clientset: k8s, + } + cmd.init() + cmdArgs := append([]string{ + "-k8s-namespace=" + ns, + "-expected-replicas=1", + "-acl-replication-token-file", tokenFile, + "-server-label-selector=component=server,app=consul,release=" + releaseName, + "-resource-prefix=" + resourcePrefix, + }, flag) + responseCode := cmd.Run(cmdArgs) + require.Equal(t, 0, responseCode, ui.ErrorWriter.String()) - // The DNS policy should not have been created. - policies, _, err := consul.ACL().PolicyList(nil) - require.NoError(t, err) - for _, p := range policies { - if p.Name == "dns-policy" { - require.Fail(t, "dns-policy exists") - } + // The anonymous token policy should not have been created. + policies, _, err := consul.ACL().PolicyList(nil) + require.NoError(t, err) + for _, p := range policies { + if p.Name == "anonymous-token-policy" { + require.Fail(t, "anonymous-token-policy exists") + } + } + }) } } diff --git a/subcommand/server-acl-init/dns.go b/subcommand/server-acl-init/dns.go deleted file mode 100644 index 6477ac0bea..0000000000 --- a/subcommand/server-acl-init/dns.go +++ /dev/null @@ -1,43 +0,0 @@ -package serveraclinit - -import ( - "github.com/hashicorp/consul/api" -) - -// configureDNSPolicies sets up policies and tokens so that Consul DNS will -// work. -func (c *Command) configureDNSPolicies(consulClient *api.Client) error { - dnsRules, err := c.dnsRules() - if err != nil { - c.Log.Error("Error templating dns rules", "err", err) - return err - } - - // Create policy for the anonymous token - dnsPolicy := api.ACLPolicy{ - Name: "dns-policy", - Description: "DNS Policy", - Rules: dnsRules, - } - - err = c.untilSucceeds("creating dns policy - PUT /v1/acl/policy", - func() error { - return c.createOrUpdateACLPolicy(dnsPolicy, consulClient) - }) - if err != nil { - return err - } - - // Create token to get sent to TokenUpdate - aToken := api.ACLToken{ - AccessorID: "00000000-0000-0000-0000-000000000002", - Policies: []*api.ACLTokenPolicyLink{{Name: dnsPolicy.Name}}, - } - - // Update anonymous token to include this policy - return c.untilSucceeds("updating anonymous token with DNS policy", - func() error { - _, _, err := consulClient.ACL().TokenUpdate(&aToken, &api.WriteOptions{}) - return err - }) -} diff --git a/subcommand/server-acl-init/rules.go b/subcommand/server-acl-init/rules.go index bbce5746a3..c8fac08722 100644 --- a/subcommand/server-acl-init/rules.go +++ b/subcommand/server-acl-init/rules.go @@ -54,10 +54,22 @@ namespace_prefix "" { return c.renderRules(agentRulesTpl) } -func (c *Command) dnsRules() (string, error) { - // DNS rules need to have access to all namespaces - // to be able to resolve services in any namespace. - dnsRulesTpl := ` +func (c *Command) anonymousTokenRules() (string, error) { + // For Consul DNS and cross-datacenter Consul Connect, + // the anonymous token needs to have read access to + // services in all namespaces. + // For Consul DNS this is needed because in a DNS request + // no token can be presented so the anonymous policy will + // be used and DNS needs to be able to resolve all services. + // For cross-dc Consul Connect, each Kubernetes pod has a + // local ACL token returned from the Kubernetes auth method. + // When making cross-dc requests, the sidecar proxies need read + // access to services in the other dc. When the API call + // to read cross-dc services is forwarded to the remote dc, the + // local ACL token is stripped and the request continues without + // ACL token. Thus the anonymous policy must + // allow reading all services. + anonTokenRulesTpl := ` {{- if .EnableNamespaces }} namespace_prefix "" { {{- end }} @@ -72,7 +84,7 @@ namespace_prefix "" { {{- end }} ` - return c.renderRules(dnsRulesTpl) + return c.renderRules(anonTokenRulesTpl) } // This assumes users are using the default name for the service, i.e. diff --git a/subcommand/server-acl-init/rules_test.go b/subcommand/server-acl-init/rules_test.go index 2a575946da..7a19ea4f7e 100644 --- a/subcommand/server-acl-init/rules_test.go +++ b/subcommand/server-acl-init/rules_test.go @@ -52,7 +52,7 @@ namespace_prefix "" { } } -func TestDNSRules(t *testing.T) { +func TestAnonymousTokenRules(t *testing.T) { cases := []struct { Name string EnableNamespaces bool @@ -92,10 +92,10 @@ namespace_prefix "" { flagEnableNamespaces: tt.EnableNamespaces, } - dnsRules, err := cmd.dnsRules() + rules, err := cmd.anonymousTokenRules() require.NoError(err) - require.Equal(tt.Expected, dnsRules) + require.Equal(tt.Expected, rules) }) } }