From bcb56630e711cdccb8734d3e4aeb1ba8a26cf08e Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Fri, 20 Oct 2023 14:57:39 -0400 Subject: [PATCH] Backport of Fix consul token revocation with namespace specific policies into release/1.14.x (#23778) * backport of commit 72d66e28132aa783ffe530275fb555f2a028ab08 --------- Co-authored-by: davidadeleon <56207066+davidadeleon@users.noreply.github.com> Co-authored-by: robmonte <17119716+robmonte@users.noreply.github.com> --- builtin/logical/consul/backend_test.go | 278 +++++++++++++++++++++++++ builtin/logical/consul/secret_token.go | 21 +- changelog/23010.txt | 3 + 3 files changed, 301 insertions(+), 1 deletion(-) create mode 100644 changelog/23010.txt diff --git a/builtin/logical/consul/backend_test.go b/builtin/logical/consul/backend_test.go index 94ce864d9..68c3472bd 100644 --- a/builtin/logical/consul/backend_test.go +++ b/builtin/logical/consul/backend_test.go @@ -849,6 +849,22 @@ func TestBackend_Roles(t *testing.T) { } } +func TestBackend_Enterprise_Diff_Namespace_Revocation(t *testing.T) { + if _, hasLicense := os.LookupEnv("CONSUL_LICENSE"); !hasLicense { + t.Skip("Skipping: No enterprise license found") + } + + testBackendEntDiffNamespaceRevocation(t) +} + +func TestBackend_Enterprise_Diff_Partition_Revocation(t *testing.T) { + if _, hasLicense := os.LookupEnv("CONSUL_LICENSE"); !hasLicense { + t.Skip("Skipping: No enterprise license found") + } + + testBackendEntDiffPartitionRevocation(t) +} + func TestBackend_Enterprise_Namespace(t *testing.T) { if _, hasLicense := os.LookupEnv("CONSUL_LICENSE"); !hasLicense { t.Skip("Skipping: No enterprise license found") @@ -865,6 +881,268 @@ func TestBackend_Enterprise_Partition(t *testing.T) { testBackendEntPartition(t) } +func testBackendEntDiffNamespaceRevocation(t *testing.T) { + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + b, err := Factory(context.Background(), config) + if err != nil { + t.Fatal(err) + } + + cleanup, consulConfig := consul.PrepareTestContainer(t, "", true, true) + defer cleanup() + + // Perform additional Consul configuration + consulapiConfig := consulapi.DefaultNonPooledConfig() + consulapiConfig.Address = consulConfig.Address() + consulapiConfig.Token = consulConfig.Token + client, err := consulapi.NewClient(consulapiConfig) + if err != nil { + t.Fatal(err) + } + + // Create Policy in default namespace to manage ACLs in a different + // namespace + nsPol := &consulapi.ACLPolicy{ + Name: "diff-ns-test", + Description: "policy to test management of ACLs in one ns from another", + Rules: `namespace "ns1" { + acl="write" + } + `, + } + pol, _, err := client.ACL().PolicyCreate(nsPol, nil) + if err != nil { + t.Fatal(err) + } + + // Create new Token in default namespace with new ACL + cToken, _, err := client.ACL().TokenCreate( + &consulapi.ACLToken{ + Policies: []*consulapi.ACLLink{{ID: pol.ID}}, + }, nil) + if err != nil { + t.Fatal(err) + } + + // Write backend config + connData := map[string]interface{}{ + "address": consulConfig.Address(), + "token": cToken.SecretID, + } + + req := &logical.Request{ + Storage: config.StorageView, + Operation: logical.UpdateOperation, + Path: "config/access", + Data: connData, + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + // Create the role in namespace "ns1" + req.Path = "roles/test-ns" + req.Data = map[string]interface{}{ + "consul_policies": []string{"ns-test"}, + "lease": "6h", + "consul_namespace": "ns1", + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + // Get Token + req.Operation = logical.ReadOperation + req.Path = "creds/test-ns" + resp, err := b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("resp nil") + } + if resp.IsError() { + t.Fatalf("resp is error: %v", resp.Error()) + } + + generatedSecret := resp.Secret + generatedSecret.TTL = 6 * time.Hour + + // Verify Secret + var d struct { + Token string `mapstructure:"token"` + Accessor string `mapstructure:"accessor"` + ConsulNamespace string `mapstructure:"consul_namespace"` + } + if err := mapstructure.Decode(resp.Data, &d); err != nil { + t.Fatal(err) + } + + if d.ConsulNamespace != "ns1" { + t.Fatalf("Failed to access namespace") + } + + // Revoke the credential + req.Operation = logical.RevokeOperation + req.Secret = generatedSecret + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("Revocation failed: %v", err) + } + + // Build a management client and verify that the token does not exist anymore + consulmgmtConfig := consulapi.DefaultNonPooledConfig() + consulmgmtConfig.Address = connData["address"].(string) + consulmgmtConfig.Token = connData["token"].(string) + mgmtclient, err := consulapi.NewClient(consulmgmtConfig) + if err != nil { + t.Fatal(err) + } + q := &consulapi.QueryOptions{ + Datacenter: "DC1", + Namespace: "ns1", + } + + _, _, err = mgmtclient.ACL().TokenRead(d.Accessor, q) + if err == nil { + t.Fatal("err: expected error") + } +} + +func testBackendEntDiffPartitionRevocation(t *testing.T) { + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + b, err := Factory(context.Background(), config) + if err != nil { + t.Fatal(err) + } + + cleanup, consulConfig := consul.PrepareTestContainer(t, "", true, true) + defer cleanup() + + // Perform additional Consul configuration + consulapiConfig := consulapi.DefaultNonPooledConfig() + consulapiConfig.Address = consulConfig.Address() + consulapiConfig.Token = consulConfig.Token + client, err := consulapi.NewClient(consulapiConfig) + if err != nil { + t.Fatal(err) + } + + // Create Policy in default partition to manage ACLs in a different + // partition + partPol := &consulapi.ACLPolicy{ + Name: "diff-part-test", + Description: "policy to test management of ACLs in one part from another", + Rules: `partition "part1" { + acl="write" + } + `, + } + pol, _, err := client.ACL().PolicyCreate(partPol, nil) + if err != nil { + t.Fatal(err) + } + + // Create new Token in default partition with new ACL + cToken, _, err := client.ACL().TokenCreate( + &consulapi.ACLToken{ + Policies: []*consulapi.ACLLink{{ID: pol.ID}}, + }, nil) + if err != nil { + t.Fatal(err) + } + + // Write backend config + connData := map[string]interface{}{ + "address": consulConfig.Address(), + "token": cToken.SecretID, + } + + req := &logical.Request{ + Storage: config.StorageView, + Operation: logical.UpdateOperation, + Path: "config/access", + Data: connData, + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + // Create the role in partition "part1" + req.Path = "roles/test-part" + req.Data = map[string]interface{}{ + "consul_policies": []string{"part-test"}, + "lease": "6h", + "partition": "part1", + } + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + + // Get Token + req.Operation = logical.ReadOperation + req.Path = "creds/test-part" + resp, err := b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("resp nil") + } + if resp.IsError() { + t.Fatalf("resp is error: %v", resp.Error()) + } + + generatedSecret := resp.Secret + generatedSecret.TTL = 6 * time.Hour + + // Verify Secret + var d struct { + Token string `mapstructure:"token"` + Accessor string `mapstructure:"accessor"` + Partition string `mapstructure:"partition"` + } + if err := mapstructure.Decode(resp.Data, &d); err != nil { + t.Fatal(err) + } + + if d.Partition != "part1" { + t.Fatalf("Failed to access partition") + } + + // Revoke the credential + req.Operation = logical.RevokeOperation + req.Secret = generatedSecret + _, err = b.HandleRequest(context.Background(), req) + if err != nil { + t.Fatalf("Revocation failed: %v", err) + } + + // Build a management client and verify that the token does not exist anymore + consulmgmtConfig := consulapi.DefaultNonPooledConfig() + consulmgmtConfig.Address = connData["address"].(string) + consulmgmtConfig.Token = connData["token"].(string) + mgmtclient, err := consulapi.NewClient(consulmgmtConfig) + if err != nil { + t.Fatal(err) + } + q := &consulapi.QueryOptions{ + Datacenter: "DC1", + Partition: "part1", + } + + _, _, err = mgmtclient.ACL().TokenRead(d.Accessor, q) + if err == nil { + t.Fatal("err: expected error") + } +} + func testBackendEntNamespace(t *testing.T) { config := logical.TestBackendConfig() config.StorageView = &logical.InmemStorage{} diff --git a/builtin/logical/consul/secret_token.go b/builtin/logical/consul/secret_token.go index f2f206b70..8c56e0a44 100644 --- a/builtin/logical/consul/secret_token.go +++ b/builtin/logical/consul/secret_token.go @@ -7,6 +7,7 @@ import ( "context" "fmt" + "github.com/hashicorp/consul/api" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) @@ -84,6 +85,24 @@ func (b *backend) secretTokenRevoke(ctx context.Context, req *logical.Request, d version = versionRaw.(string) } + // Extract Consul Namespace and Partition info from secret + var revokeWriteOptions *api.WriteOptions + var namespace, partition string + + namespaceRaw, ok := req.Data["consul_namespace"] + if ok { + namespace = namespaceRaw.(string) + } + partitionRaw, ok := req.Data["partition"] + if ok { + partition = partitionRaw.(string) + } + + revokeWriteOptions = &api.WriteOptions{ + Namespace: namespace, + Partition: partition, + } + switch version { case "": // Pre 1.4 tokens @@ -92,7 +111,7 @@ func (b *backend) secretTokenRevoke(ctx context.Context, req *logical.Request, d return nil, err } case tokenPolicyType: - _, err := c.ACL().TokenDelete(tokenRaw.(string), nil) + _, err := c.ACL().TokenDelete(tokenRaw.(string), revokeWriteOptions) if err != nil { return nil, err } diff --git a/changelog/23010.txt b/changelog/23010.txt new file mode 100644 index 000000000..f6a72ecf9 --- /dev/null +++ b/changelog/23010.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/consul: Fix revocations when Vault has an access token using specific namespace and admin partition policies +``` \ No newline at end of file