diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go index 8d8411969a5fa..685dd70217be0 100644 --- a/lib/auth/grpcserver.go +++ b/lib/auth/grpcserver.go @@ -61,6 +61,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" "github.com/gravitational/teleport/lib/auth/discoveryconfig/discoveryconfigv1" integrationService "github.com/gravitational/teleport/lib/auth/integration/integrationv1" @@ -2090,6 +2091,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) @@ -2163,11 +2170,13 @@ func downgradeRoleToV6(r *types.RoleV6) (*types.RoleV6, bool, error) { case types.V3, types.V4, types.V5, types.V6: return r, false, nil case types.V7: - var ( - downgraded types.RoleV6 - restricted bool - ) - downgraded = *r + var restricted bool + // 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. + downgraded := apiutils.CloneProtoMsg(r) downgraded.Version = types.V6 if len(downgraded.GetKubeResources(types.Deny)) > 0 { @@ -2225,7 +2234,7 @@ func downgradeRoleToV6(r *types.RoleV6) (*types.RoleV6, bool, error) { restricted = true } - return &downgraded, restricted, nil + return downgraded, restricted, nil default: return nil, false, trace.BadParameter("unrecognized role version %T", r.Version) } diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go index a5aee4568cee2..1c3c9c0646d09 100644 --- a/lib/auth/grpcserver_test.go +++ b/lib/auth/grpcserver_test.go @@ -23,6 +23,7 @@ import ( "encoding/pem" "fmt" "io" + "maps" "net" "net/http" "net/http/httptest" @@ -30,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" @@ -1808,10 +1810,9 @@ func TestIsMFARequired(t *testing.T) { // If auth pref or role require session MFA, and MFA is not already // verified according to private key policy, expect MFA required. - wantRequired := - (role.GetOptions().RequireMFAType.IsSessionMFARequired() || authPref.GetRequireMFAType().IsSessionMFARequired()) && - !role.GetPrivateKeyPolicy().MFAVerified() && - !authPref.GetPrivateKeyPolicy().MFAVerified() + wantRequired := (role.GetOptions().RequireMFAType.IsSessionMFARequired() || authPref.GetRequireMFAType().IsSessionMFARequired()) && + !role.GetPrivateKeyPolicy().MFAVerified() && + !authPref.GetPrivateKeyPolicy().MFAVerified() var wantMFARequired proto.MFARequired if wantRequired { wantMFARequired = proto.MFARequired_MFA_REQUIRED_YES @@ -3930,8 +3931,12 @@ func TestRoleVersions(t *testing.T) { wildcardLabels := types.Labels{types.Wildcard: {types.Wildcard}} + originalLabels := map[string]string{"env": "staging"} newRole := func(version string, spec types.RoleSpecV6) types.Role { role, err := types.NewRoleWithVersion("test_rule", version, spec) + meta := role.GetMetadata() + meta.Labels = maps.Clone(originalLabels) + role.SetMetadata(meta) require.NoError(t, err) return role } @@ -4150,6 +4155,15 @@ func TestRoleVersions(t *testing.T) { require.NoError(t, err) } } + + // Call maybeDowngrade directly to make sure the original + // role isn't modified + sv, err := semver.NewVersion(clientVersion) + if err == nil { + _, err := maybeDowngradeRoleToV6(ctx, role.(*types.RoleV6), sv) + require.NoError(t, err) + require.Equal(t, originalLabels, role.GetMetadata().Labels) + } }) } })