backport of commit 2c88125 (#14582)

This pull request was automerged via backport-assistant

Author: hc-github-team-consul-core

.changelog/14577.txt

@@ -0,0 +1,3 @@
+```release-note:security
+auto-config: Added input validation for auto-config JWT authorization checks. Prior to this change, it was possible for malicious actors to construct requests which incorrectly pass custom JWT claim validation for the `AutoConfig.InitialConfiguration` endpoint. Now, only a subset of characters are allowed for the input before evaluating the bexpr.
+```

agent/consul/auto_config_endpoint.go

@@ -5,13 +5,15 @@ import (
 	"crypto/x509"
 	"encoding/base64"
 	"fmt"
+	"regexp"
 
 	"github.com/hashicorp/consul/acl"
 
 	bexpr "github.com/hashicorp/go-bexpr"
 
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/authmethod/ssoauth"
+	"github.com/hashicorp/consul/agent/dns"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib/template"
 	"github.com/hashicorp/consul/proto/pbautoconf"
@@ -51,6 +53,11 @@ type jwtAuthorizer struct {
 	claimAssertions []string
 }
 
+// Invalidate any quote or whitespace characters that could cause an escape with bexpr.
+// This includes an extra single-quote character not specified in the grammar for safety in case it is later added.
+// https://github.com/hashicorp/go-bexpr/blob/v0.1.11/grammar/grammar.peg#L188-L191
+var invalidSegmentName = regexp.MustCompile("[`'\"\\s]+")
+
 func (a *jwtAuthorizer) Authorize(req *pbautoconf.AutoConfigRequest) (AutoConfigOptions, error) {
 	// perform basic JWT Authorization
 	identity, err := a.validator.ValidateLogin(context.Background(), req.JWT)
@@ -59,6 +66,21 @@ func (a *jwtAuthorizer) Authorize(req *pbautoconf.AutoConfigRequest) (AutoConfig
 		return AutoConfigOptions{}, acl.PermissionDenied("Failed JWT authorization: %v", err)
 	}
 
+	// Ensure provided data cannot escape the RHS of a bexpr for security.
+	// This is not the cleanest way to prevent this behavior. Ideally, the bexpr would allow us to
+	// inject a variable on the RHS for comparison as well, but it would be a complex change to implement
+	// that would likely break backwards-compatibility in certain circumstances.
+	if dns.InvalidNameRe.MatchString(req.Node) {
+		return AutoConfigOptions{}, fmt.Errorf("Invalid request field. %v = `%v`", "node", req.Node)
+	}
+	if invalidSegmentName.MatchString(req.Segment) {
+		return AutoConfigOptions{}, fmt.Errorf("Invalid request field. %v = `%v`", "segment", req.Segment)
+	}
+	if req.Partition != "" && !dns.IsValidLabel(req.Partition) {
+		return AutoConfigOptions{}, fmt.Errorf("Invalid request field. %v = `%v`", "partition", req.Partition)
+	}
+
+	// Ensure that every value in this mapping is safe to interpolate before using it.
 	varMap := map[string]string{
 		"node":      req.Node,
 		"segment":   req.Segment,

agent/consul/auto_config_endpoint_test.go

@@ -92,9 +92,9 @@ func signJWTWithStandardClaims(t *testing.T, privKey string, claims interface{})
 // TestAutoConfigInitialConfiguration is really an integration test of all the moving parts of the AutoConfig.InitialConfiguration RPC.
 // Full testing of the individual parts will not be done in this test:
 //
-//  * Any implementations of the AutoConfigAuthorizer interface (although these test do use the jwtAuthorizer)
-//  * Each of the individual config generation functions. These can be unit tested separately and should NOT
-//    require running test servers
+//   - Any implementations of the AutoConfigAuthorizer interface (although these test do use the jwtAuthorizer)
+//   - Each of the individual config generation functions. These can be unit tested separately and should NOT
+//     require running test servers
 func TestAutoConfigInitialConfiguration(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")
@@ -236,6 +236,29 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {
 			},
 			err: "Permission denied: Failed JWT authorization: no known key successfully validated the token signature",
 		},
+		"bad-req-node": {
+			request: &pbautoconf.AutoConfigRequest{
+				Node: "bad node",
+				JWT:  signJWTWithStandardClaims(t, priv, map[string]interface{}{"consul_node_name": "test-node"}),
+			},
+			err: "Invalid request field. node =",
+		},
+		"bad-req-segment": {
+			request: &pbautoconf.AutoConfigRequest{
+				Node:    "test-node",
+				Segment: "bad segment",
+				JWT:     signJWTWithStandardClaims(t, priv, map[string]interface{}{"consul_node_name": "test-node"}),
+			},
+			err: "Invalid request field. segment =",
+		},
+		"bad-req-partition": {
+			request: &pbautoconf.AutoConfigRequest{
+				Node:      "test-node",
+				Partition: "bad partition",
+				JWT:       signJWTWithStandardClaims(t, priv, map[string]interface{}{"consul_node_name": "test-node"}),
+			},
+			err: "Invalid request field. partition =",
+		},
 		"claim-assertion-failed": {
 			request: &pbautoconf.AutoConfigRequest{
 				Node: "test-node",
@@ -850,3 +873,39 @@ func TestAutoConfig_updateJoinAddressesInConfig(t *testing.T) {
 
 	backend.AssertExpectations(t)
 }
+
+func TestAutoConfig_invalidSegmentName(t *testing.T) {
+	invalid := []string{
+		"\n",
+		"\r",
+		"\t",
+		"`",
+		`'`,
+		`"`,
+		` `,
+		`a b`,
+		`a'b`,
+		`a or b`,
+		`a and b`,
+		`segment name`,
+		`segment"name`,
+		`"segment"name`,
+		`"segment" name`,
+		`segment'name'`,
+	}
+	valid := []string{
+		``,
+		`a`,
+		`a.b`,
+		`a.b.c`,
+		`a-b-c`,
+		`segment.name`,
+	}
+
+	for _, s := range invalid {
+		require.True(t, invalidSegmentName.MatchString(s), "incorrect match: %v", s)
+	}
+	for _, s := range valid {
+		require.False(t, invalidSegmentName.MatchString(s), "incorrect match: %v", s)
+	}
+}