Merge branch 'release/1.20.x' into backport/patch-1/surely-united-jennet

hashicorp · Dec 18, 2024 · 52d8a63 · 52d8a63
2 parents b4419b1 + 7e52d43
commit 52d8a63
Show file tree

Hide file tree

Showing 20 changed files with 123 additions and 76 deletions.
diff --git a/.changelog/21908.txt b/.changelog/21908.txt
@@ -0,0 +1,3 @@
+```release-note:security
+Resolved issue where hcl would allow duplicates of the same key in acl policy configuration.
+```
diff --git a/.release/security-scan.hcl b/.release/security-scan.hcl
@@ -37,13 +37,6 @@ container {
 	triage {
 		suppress {
 			vulnerabilities = [
-				"CVE-2024-8096", # [email protected],
-				"CVE-2024-9143", # [email protected],
-				"CVE-2024-3596", # [email protected],
-				"CVE-2024-2236", # [email protected],
-				"CVE-2024-26458", # [email protected],
-				"CVE-2024-2511", # [email protected],
-				#the above can be resolved when they're resolved in the alpine image
 			]
 			paths = [
 				"internal/tools/proto-gen-rpc-glue/e2e/consul/*",

diff --git a/Dockerfile b/Dockerfile
@@ -16,7 +16,7 @@
 # Official docker image that includes binaries from releases.hashicorp.com. This
 # downloads the release from releases.hashicorp.com and therefore requires that
 # the release is published before building the Docker image.
-FROM docker.mirror.hashicorp.services/alpine:3.20 as official
+FROM docker.mirror.hashicorp.services/alpine:3.21 as official
 
 # This is the release of Consul to pull in.
 ARG VERSION
@@ -112,7 +112,7 @@ CMD ["agent", "-dev", "-client", "0.0.0.0"]
 
 # Production docker image that uses CI built binaries.
 # Remember, this image cannot be built locally.
-FROM docker.mirror.hashicorp.services/alpine:3.20 as default
+FROM docker.mirror.hashicorp.services/alpine:3.21 as default
 
 ARG PRODUCT_VERSION
 ARG BIN_NAME

diff --git a/acl/acl.go b/acl/acl.go
@@ -22,6 +22,9 @@ type Config struct {
 	// WildcardName is the string that represents a request to authorize a wildcard permission
 	WildcardName string
 
+	//by default errors, but in certain instances we want to make sure to maintain backwards compatabilty
+	WarnOnDuplicateKey bool
+
 	// embedded enterprise configuration
 	EnterpriseConfig
 }

diff --git a/acl/policy.go b/acl/policy.go
@@ -310,8 +310,8 @@ func (pr *PolicyRules) Validate(conf *Config) error {
 	return nil
 }
 
-func parse(rules string, conf *Config, meta *EnterprisePolicyMeta) (*Policy, error) {
-	p, err := decodeRules(rules, conf, meta)
+func parse(rules string, warnOnDuplicateKey bool, conf *Config, meta *EnterprisePolicyMeta) (*Policy, error) {
+	p, err := decodeRules(rules, warnOnDuplicateKey, conf, meta)
 	if err != nil {
 		return nil, err
 	}
@@ -338,7 +338,11 @@ func NewPolicyFromSource(rules string, conf *Config, meta *EnterprisePolicyMeta)
 
 	var policy *Policy
 	var err error
-	policy, err = parse(rules, conf, meta)
+	warnOnDuplicateKey := false
+	if conf != nil {
+		warnOnDuplicateKey = conf.WarnOnDuplicateKey
+	}
+	policy, err = parse(rules, warnOnDuplicateKey, conf, meta)
 	return policy, err
 }
 

diff --git a/acl/policy_ce.go b/acl/policy_ce.go
@@ -7,8 +7,9 @@ package acl
 
 import (
 	"fmt"
-
+	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/hcl"
+	"strings"
 )
 
 // EnterprisePolicyMeta stub
@@ -30,12 +31,28 @@ func (r *EnterprisePolicyRules) Validate(*Config) error {
 	return nil
 }
 
-func decodeRules(rules string, _ *Config, _ *EnterprisePolicyMeta) (*Policy, error) {
+func decodeRules(rules string, warnOnDuplicateKey bool, _ *Config, _ *EnterprisePolicyMeta) (*Policy, error) {
 	p := &Policy{}
 
-	if err := hcl.Decode(p, rules); err != nil {
+	err := hcl.DecodeErrorOnDuplicates(p, rules)
+
+	if errIsDuplicateKey(err) && warnOnDuplicateKey {
+		//because the snapshot saves the unparsed rules we have to assume some snapshots exist that shouldn't fail, but
+		// have duplicates
+		if err := hcl.Decode(p, rules); err != nil {
+			hclog.Default().Warn("Warning- Duplicate key in ACL Policy ignored", "errorMessage", err.Error())
+			return nil, fmt.Errorf("Failed to parse ACL rules: %v", err)
+		}
+	} else if err != nil {
 		return nil, fmt.Errorf("Failed to parse ACL rules: %v", err)
 	}
 
 	return p, nil
 }
+
+func errIsDuplicateKey(err error) bool {
+	if err == nil {
+		return false
+	}
+	return strings.Contains(err.Error(), "was already set. Each argument can only be defined once")
+}
diff --git a/acl/policy_test.go b/acl/policy_test.go
@@ -342,6 +342,12 @@ func TestPolicySourceParse(t *testing.T) {
 			RulesJSON: `{ "acl": "list" }`, // there is no list policy but this helps to exercise another check in isPolicyValid
 			Err:       "Invalid acl policy",
 		},
+		{
+			Name: "Bad Policy - Duplicate ACL Key",
+			Rules: `acl="read"
+					acl="write"`,
+			Err: "Failed to parse ACL rules: The argument \"acl\" at",
+		},
 		{
 			Name:      "Bad Policy - Agent",
 			Rules:     `agent "foo" { policy = "nope" }`,

diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go
@@ -2169,6 +2169,22 @@ func TestACLEndpoint_PolicySet(t *testing.T) {
 		require.Error(t, err)
 	})
 
+	t.Run("Key Dup", func(t *testing.T) {
+		req := structs.ACLPolicySetRequest{
+			Datacenter: "dc1",
+			Policy: structs.ACLPolicy{
+				Description: "foobar",
+				Name:        "baz2",
+				Rules:       "service \"\" { policy = \"read\" policy = \"write\" }",
+			},
+			WriteRequest: structs.WriteRequest{Token: TestDefaultInitialManagementToken},
+		}
+		resp := structs.ACLPolicy{}
+
+		err := aclEp.PolicySet(&req, &resp)
+		require.Error(t, err)
+	})
+
 	t.Run("Update it", func(t *testing.T) {
 		req := structs.ACLPolicySetRequest{
 			Datacenter: "dc1",

diff --git a/agent/dns_test.go b/agent/dns_test.go
@@ -2367,7 +2367,7 @@ func TestDNS_trimUDPResponse_NoTrim(t *testing.T) {
 		},
 	}
 
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1"  `)
 	if trimmed := trimUDPResponse(req, resp, cfg.DNSUDPAnswerLimit); trimmed {
 		t.Fatalf("Bad %#v", *resp)
 	}
@@ -2400,7 +2400,7 @@ func TestDNS_trimUDPResponse_NoTrim(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimLimit(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp, expected := &dns.Msg{}, &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < cfg.DNSUDPAnswerLimit+1; i++ {
@@ -2439,7 +2439,7 @@ func TestDNS_trimUDPResponse_TrimLimit(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimLimitWithNS(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp, expected := &dns.Msg{}, &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < cfg.DNSUDPAnswerLimit+1; i++ {
@@ -2486,7 +2486,7 @@ func TestDNS_trimUDPResponse_TrimLimitWithNS(t *testing.T) {
 }
 
 func TestDNS_trimTCPResponse_TrimLimitWithNS(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp, expected := &dns.Msg{}, &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < 5000; i++ {
@@ -2542,7 +2542,7 @@ func loadRuntimeConfig(t *testing.T, hcl string) *config.RuntimeConfig {
 }
 
 func TestDNS_trimUDPResponse_TrimSize(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1"  `)
 
 	req, resp := &dns.Msg{}, &dns.Msg{}
 	for i := 0; i < 100; i++ {
@@ -2594,7 +2594,7 @@ func TestDNS_trimUDPResponse_TrimSize(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimSizeEDNS(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" `)
 
 	req, resp := &dns.Msg{}, &dns.Msg{}
 
@@ -2672,7 +2672,7 @@ func TestDNS_trimUDPResponse_TrimSizeEDNS(t *testing.T) {
 }
 
 func TestDNS_trimUDPResponse_TrimSizeMaxSize(t *testing.T) {
-	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1" node_name = "dummy" `)
+	cfg := loadRuntimeConfig(t, `node_name = "test" data_dir = "a" bind_addr = "127.0.0.1"             `)
 
 	resp := &dns.Msg{}
 

diff --git a/agent/routine-leak-checker/leak_test.go b/agent/routine-leak-checker/leak_test.go
@@ -64,9 +64,7 @@ func setupPrimaryServer(t *testing.T) *agent.TestAgent {
 
 	config := `
 	   server = true
-		datacenter = "primary"
-		primary_datacenter = "primary"
-		
+		datacenter = "primary"	
 		connect {
 			enabled = true
 		}

diff --git a/agent/structs/acl.go b/agent/structs/acl.go
@@ -800,6 +800,11 @@ func (policies ACLPolicies) resolveWithCache(cache *ACLCaches, entConf *acl.Conf
 			continue
 		}
 
+		//pulling from the cache, we don't want to break any rules that are already in the cache
+		if entConf == nil {
+			entConf = &acl.Config{}
+		}
+		entConf.WarnOnDuplicateKey = true
 		p, err := acl.NewPolicyFromSource(policy.Rules, entConf, policy.EnterprisePolicyMeta())
 		if err != nil {
 			return nil, fmt.Errorf("failed to parse %q: %v", policy.Name, err)

diff --git a/agent/structs/acl_test.go b/agent/structs/acl_test.go
@@ -403,7 +403,7 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 			ID:          "5d5653a1-2c2b-4b36-b083-fc9f1398eb7b",
 			Name:        "policy1",
 			Description: "policy1",
-			Rules:       `node_prefix "" { policy = "read" }`,
+			Rules:       `node_prefix "" { policy = "read", policy = "read", },`,
 			RaftIndex: RaftIndex{
 				CreateIndex: 1,
 				ModifyIndex: 2,
@@ -413,7 +413,7 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 			ID:          "b35541f0-a88a-48da-bc66-43553c60b628",
 			Name:        "policy2",
 			Description: "policy2",
-			Rules:       `agent_prefix "" { policy = "read" }`,
+			Rules:       `agent_prefix "" { policy = "read" } `,
 			RaftIndex: RaftIndex{
 				CreateIndex: 3,
 				ModifyIndex: 4,
@@ -433,7 +433,8 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 			ID:          "8bf38965-95e5-4e86-9be7-f6070cc0708b",
 			Name:        "policy4",
 			Description: "policy4",
-			Rules:       `service_prefix "" { policy = "read" }`,
+			//test should still pass even with the duplicate key since its resolving the cache
+			Rules: `service_prefix "" { policy = "read"  policy = "read" }`,
 			RaftIndex: RaftIndex{
 				CreateIndex: 7,
 				ModifyIndex: 8,

diff --git a/envoyextensions/xdscommon/ENVOY_VERSIONS b/envoyextensions/xdscommon/ENVOY_VERSIONS
@@ -8,7 +8,7 @@
 #
 # See https://www.consul.io/docs/connect/proxies/envoy#supported-versions for more information on Consul's Envoy
 # version support.
-1.31.2
-1.30.6
-1.29.9
+1.31.4
+1.30.8
+1.29.11
 1.28.7
diff --git a/go.mod b/go.mod
@@ -68,13 +68,13 @@ require (
 	github.com/hashicorp/go-version v1.2.1
 	github.com/hashicorp/golang-lru v0.5.4
 	github.com/hashicorp/hcdiag v0.5.1
-	github.com/hashicorp/hcl v1.0.0
+	github.com/hashicorp/hcl v1.0.1-vault-7
 	github.com/hashicorp/hcl/v2 v2.14.1
 	github.com/hashicorp/hcp-scada-provider v0.2.4
 	github.com/hashicorp/hcp-sdk-go v0.80.0
 	github.com/hashicorp/hil v0.0.0-20200423225030-a18a1cd20038
 	github.com/hashicorp/memberlist v0.5.0
-	github.com/hashicorp/raft v1.5.0
+	github.com/hashicorp/raft v1.7.0
 	github.com/hashicorp/raft-autopilot v0.1.6
 	github.com/hashicorp/raft-boltdb/v2 v2.2.2
 	github.com/hashicorp/raft-wal v0.4.1
@@ -118,7 +118,7 @@ require (
 	golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63
 	golang.org/x/net v0.24.0
 	golang.org/x/oauth2 v0.15.0
-	golang.org/x/sync v0.3.0
+	golang.org/x/sync v0.4.0
 	golang.org/x/sys v0.20.0
 	golang.org/x/time v0.3.0
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98
@@ -201,7 +201,7 @@ require (
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.16.0 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-msgpack v0.5.5 // indirect
-	github.com/hashicorp/go-msgpack/v2 v2.0.0 // indirect
+	github.com/hashicorp/go-msgpack/v2 v2.1.1 // indirect
 	github.com/hashicorp/go-secure-stdlib/parseutil v0.1.6 // indirect
 	github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect
 	github.com/hashicorp/golang-lru/v2 v2.0.0 // indirect
@@ -263,10 +263,10 @@ require (
 	go.opencensus.io v0.24.0 // indirect
 	go.opentelemetry.io/otel/trace v1.17.0 // indirect
 	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect
-	golang.org/x/mod v0.12.0 // indirect
+	golang.org/x/mod v0.13.0 // indirect
 	golang.org/x/term v0.19.0 // indirect
 	golang.org/x/text v0.14.0 // indirect
-	golang.org/x/tools v0.12.1-0.20230815132531-74c255bcf846 // indirect
+	golang.org/x/tools v0.14.0 // indirect
 	google.golang.org/api v0.126.0 // indirect
 	google.golang.org/appengine v1.6.8 // indirect
 	google.golang.org/genproto v0.0.0-20230711160842-782d3b101e98 // indirect
-Original file line number
+Diff line change
@@ Expand Up / @@ -8,7 +8,7 @@ @@
     #
     # See https://www.consul.io/docs/connect/proxies/envoy#supported-versions for more information on Consul's Envoy
     # version support.
-.31.2
-.30.6
-.29.9
+.31.4
+.30.8
+.29.11
 .28.7