[v13] Correctly clone RoleV6 when downgrading

lib/auth/grpcserver.go

@@ -58,6 +58,7 @@ import (
 	apievents "github.com/gravitational/teleport/api/types/events"
 	"github.com/gravitational/teleport/api/types/installers"
 	"github.com/gravitational/teleport/api/types/wrappers"
+	apiutils "github.com/gravitational/teleport/api/utils"
 	"github.com/gravitational/teleport/lib/auth/assist/assistv1"
 	integrationService "github.com/gravitational/teleport/lib/auth/integration/integrationv1"
 	"github.com/gravitational/teleport/lib/auth/okta"
@@ -2053,6 +2054,12 @@ func maybeDowngradeRoleLabelExpressions(ctx context.Context, role *types.RoleV6,
 	if !clientVersion.LessThan(minSupportedLabelExpressionVersion) {
 		return role, nil
 	}
+	// Make a shallow copy of the role so that we don't mutate the original.
+	// This is necessary because the role is shared between multiple clients
+	// sessions when notifying about changes in watchers.  If we mutate the
+	// original role, it will be mutated for all clients which can cause panics
+	// since it causes a race condition.
+	role = apiutils.CloneProtoMsg(role)
 	hasLabelExpression := false
 	for _, kind := range types.LabelMatcherKinds {
 		allowLabelMatchers, err := role.GetLabelMatchers(types.Allow, kind)

lib/auth/grpcserver_test.go

@@ -31,6 +31,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/coreos/go-semver/semver"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/google/uuid"
@@ -44,6 +45,8 @@ import (
 	otlpcommonv1 "go.opentelemetry.io/proto/otlp/common/v1"
 	otlpresourcev1 "go.opentelemetry.io/proto/otlp/resource/v1"
 	otlptracev1 "go.opentelemetry.io/proto/otlp/trace/v1"
+	"golang.org/x/exp/maps"
+	grpcmetadata "google.golang.org/grpc/metadata"
 	"google.golang.org/protobuf/testing/protocmp"
 	"google.golang.org/protobuf/types/known/emptypb"
 
@@ -4185,9 +4188,13 @@ func TestRoleVersions(t *testing.T) {
 
 	wildcardLabels := types.Labels{types.Wildcard: {types.Wildcard}}
 
+	originalLabels := map[string]string{"env": "staging"}
 	newRole := func(spec types.RoleSpecV6) types.Role {
 		role, err := types.NewRole("test_rule", spec)
 		require.NoError(t, err)
+		metadata := role.GetMetadata()
+		metadata.Labels = maps.Clone(originalLabels)
+		role.SetMetadata(metadata)
 		return role
 	}
 
@@ -4365,6 +4372,17 @@ func TestRoleVersions(t *testing.T) {
 							require.NoError(t, err)
 						}
 					}
+
+					// Call maybeDowngrade directly to make sure the original
+					// role isn't modified.
+					if _, err := semver.NewVersion(clientVersion); err == nil {
+						ctx = grpcmetadata.NewIncomingContext(ctx, grpcmetadata.New(map[string]string{
+							metadata.VersionKey: clientVersion,
+						}))
+						_, err = maybeDowngradeRole(ctx, role.(*types.RoleV6))
+						require.NoError(t, err)
+						require.Equal(t, originalLabels, role.GetMetadata().Labels)
+					}
 				})
 			}
 		})