Enforce lowercase peer names. (#15697)

Enforce lowercase peer names.

Prior to this change peer names could be mixed case.
This can cause issues, as peer names are used as DNS labels
in various locations. It also caused issues with envoy
configuration.

.changelog/15697.txt

@@ -0,0 +1,3 @@
+```release-note:breaking-change
+peering: Newly created peering connections must use only lowercase characters in the `name` field. Existing peerings with uppercase characters will not be modified, but they may encounter issues in various circumstances. To maintain forward compatibility and avoid issues, it is recommended to destroy and re-create any invalid peering connections so that they do not have a name containing uppercase characters.
+```

agent/rpc/peering/service.go

@@ -24,7 +24,6 @@ import (
 	"github.com/hashicorp/consul/acl/resolver"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/consul/stream"
-	"github.com/hashicorp/consul/agent/dns"
 	external "github.com/hashicorp/consul/agent/grpc-external"
 	"github.com/hashicorp/consul/agent/grpc-external/services/peerstream"
 	"github.com/hashicorp/consul/agent/structs"
@@ -203,7 +202,7 @@ func (s *Server) GenerateToken(
 		return nil, grpcstatus.Error(codes.InvalidArgument, err.Error())
 	}
 	// validate prior to forwarding to the leader, this saves a network hop
-	if err := dns.ValidateLabel(req.PeerName); err != nil {
+	if err := validatePeerName(req.PeerName); err != nil {
 		return nil, fmt.Errorf("%s is not a valid peer name: %w", req.PeerName, err)
 	}
 
@@ -351,7 +350,7 @@ func (s *Server) Establish(
 	}
 
 	// validate prior to forwarding to the leader, this saves a network hop
-	if err := dns.ValidateLabel(req.PeerName); err != nil {
+	if err := validatePeerName(req.PeerName); err != nil {
 		return nil, fmt.Errorf("%s is not a valid peer name: %w", req.PeerName, err)
 	}
 	tok, err := s.Backend.DecodeToken([]byte(req.PeeringToken))

agent/rpc/peering/service_test.go

@@ -85,7 +85,7 @@ func TestPeeringService_GenerateToken(t *testing.T) {
 
 	// TODO(peering): for more failure cases, consider using a table test
 	// check meta tags
-	reqE := pbpeering.GenerateTokenRequest{PeerName: "peerB", Meta: generateTooManyMetaKeys()}
+	reqE := pbpeering.GenerateTokenRequest{PeerName: "peer-b", Meta: generateTooManyMetaKeys()}
 	_, errE := client.GenerateToken(ctx, &reqE)
 	require.EqualError(t, errE, "rpc error: code = Unknown desc = meta tags failed validation: Node metadata cannot contain more than 64 key/value pairs")
 
@@ -95,7 +95,7 @@ func TestPeeringService_GenerateToken(t *testing.T) {
 	)
 	testutil.RunStep(t, "peering token is generated with data", func(t *testing.T) {
 		req := pbpeering.GenerateTokenRequest{
-			PeerName: "peerB",
+			PeerName: "peer-b",
 			Meta:     map[string]string{"foo": "bar"},
 		}
 		resp, err := client.GenerateToken(ctx, &req)
@@ -135,7 +135,7 @@ func TestPeeringService_GenerateToken(t *testing.T) {
 		peers[0].CreateIndex = 0
 
 		expect := &pbpeering.Peering{
-			Name:      "peerB",
+			Name:      "peer-b",
 			Partition: acl.DefaultPartitionName,
 			ID:        peerID,
 			State:     pbpeering.PeeringState_PENDING,
@@ -153,7 +153,7 @@ func TestPeeringService_GenerateToken(t *testing.T) {
 	})
 
 	testutil.RunStep(t, "re-generating a peering token re-generates the secret", func(t *testing.T) {
-		req := pbpeering.GenerateTokenRequest{PeerName: "peerB", Meta: map[string]string{"foo": "bar"}}
+		req := pbpeering.GenerateTokenRequest{PeerName: "peer-b", Meta: map[string]string{"foo": "bar"}}
 		resp, err := client.GenerateToken(ctx, &req)
 		require.NoError(t, err)
 
@@ -197,7 +197,7 @@ func TestPeeringService_GenerateTokenExternalAddress(t *testing.T) {
 
 	externalAddresses := []string{"32.1.2.3:8502"}
 	// happy path
-	req := pbpeering.GenerateTokenRequest{PeerName: "peerB", Meta: map[string]string{"foo": "bar"}, ServerExternalAddresses: externalAddresses}
+	req := pbpeering.GenerateTokenRequest{PeerName: "peer-b", Meta: map[string]string{"foo": "bar"}, ServerExternalAddresses: externalAddresses}
 	resp, err := client.GenerateToken(ctx, &req)
 	require.NoError(t, err)
 
@@ -396,7 +396,7 @@ func TestPeeringService_Establish_serverNameConflict(t *testing.T) {
 	base64Token := base64.StdEncoding.EncodeToString(jsonToken)
 
 	establishReq := &pbpeering.EstablishRequest{
-		PeerName:     "peerTwo",
+		PeerName:     "peer-two",
 		PeeringToken: base64Token,
 	}
 
@@ -1304,7 +1304,7 @@ func TestPeeringService_validatePeer(t *testing.T) {
 	t.Cleanup(cancel)
 
 	testutil.RunStep(t, "generate a token", func(t *testing.T) {
-		req := pbpeering.GenerateTokenRequest{PeerName: "peerB"}
+		req := pbpeering.GenerateTokenRequest{PeerName: "peer-b"}
 		resp, err := client1.GenerateToken(ctx, &req)
 		require.NoError(t, err)
 		require.NotEmpty(t, resp)
@@ -1325,7 +1325,7 @@ func TestPeeringService_validatePeer(t *testing.T) {
 
 	testutil.RunStep(t, "send an establish request for a different peer name", func(t *testing.T) {
 		resp, err := client1.Establish(ctx, &pbpeering.EstablishRequest{
-			PeerName:     "peerC",
+			PeerName:     "peer-c",
 			PeeringToken: s2Token,
 		})
 		require.NoError(t, err)
@@ -1333,24 +1333,24 @@ func TestPeeringService_validatePeer(t *testing.T) {
 	})
 
 	testutil.RunStep(t, "attempt to generate token with the same name used as dialer", func(t *testing.T) {
-		req := pbpeering.GenerateTokenRequest{PeerName: "peerC"}
+		req := pbpeering.GenerateTokenRequest{PeerName: "peer-c"}
 		resp, err := client1.GenerateToken(ctx, &req)
 
 		require.Error(t, err)
 		require.Contains(t, err.Error(),
-			"cannot create peering with name: \"peerC\"; there is already an established peering")
+			"cannot create peering with name: \"peer-c\"; there is already an established peering")
 		require.Nil(t, resp)
 	})
 
 	testutil.RunStep(t, "attempt to establish the with the same name used as acceptor", func(t *testing.T) {
 		resp, err := client1.Establish(ctx, &pbpeering.EstablishRequest{
-			PeerName:     "peerB",
+			PeerName:     "peer-b",
 			PeeringToken: s2Token,
 		})
 
 		require.Error(t, err)
 		require.Contains(t, err.Error(),
-			"cannot create peering with name: \"peerB\"; there is an existing peering expecting to be dialed")
+			"cannot create peering with name: \"peer-b\"; there is an existing peering expecting to be dialed")
 		require.Nil(t, resp)
 	})
 }

agent/rpc/peering/validate.go

@@ -1,14 +1,32 @@
 package peering
 
 import (
+	"errors"
 	"fmt"
 	"net"
+	"regexp"
 	"strconv"
 
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 )
 
+// matches valid DNS labels according to RFC 1123, should be at most 63
+// characters according to the RFC. This does not allow uppercase letters, unlike
+// node / service validation. All lowercase is enforced to reduce potential issues
+// relating to case-mismatch throughout the codebase (state-store lookups,
+// envoy listeners, etc).
+var validPeeringName = regexp.MustCompile(`^[a-z0-9]([a-z0-9\-]{0,61}[a-z0-9])?$`)
+
+// validatePeerName returns an error if the peer name does not match
+// the expected format. Returns nil on valid names.
+func validatePeerName(name string) error {
+	if !validPeeringName.MatchString(name) {
+		return errors.New("a valid peering name must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character")
+	}
+	return nil
+}
+
 // validatePeeringToken ensures that the token has valid values.
 func validatePeeringToken(tok *structs.PeeringToken) error {
 	// the CA values here should be valid x509 certs