Skip to content

Commit

Permalink
Remove role downgrade logic for insecure drop mode (#41371)
Browse files Browse the repository at this point in the history 
This logic was necessary in Teleport 15 and earlier, because some v14
(and earlier) clients did not support the insecure-drop mode.

Teleport 16 no longer needs to downgrade roles, as the minimum supported
client version is 15, and all v15 versions support insecure-drop.
  • Loading branch information
@zmb3
zmb3 authored May 9, 2024
1 parent f893ecb commit 19a89bc
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 232 deletions.
62 changes: 2 additions & 60 deletions lib/auth/grpcserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ 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/accessmonitoringrules/accessmonitoringrulesv1"
"github.com/gravitational/teleport/lib/auth/assist/assistv1"
"github.com/gravitational/teleport/lib/auth/clusterconfig/clusterconfigv1"
Expand Down Expand Up @@ -2140,68 +2139,11 @@ func (g *GRPCServer) DeleteAllKubernetesServers(ctx context.Context, req *authpb
// version for some features of the role returns a shallow copy of the given
// role downgraded for compatibility with the older version.
func maybeDowngradeRole(ctx context.Context, role *types.RoleV6) (*types.RoleV6, error) {
clientVersionString, ok := metadata.ClientVersionFromContext(ctx)
if !ok {
// This client is not reporting its version via gRPC metadata. Teleport
// clients have been reporting their version for long enough that older
// clients won't even support v6 roles at all, so this is likely a
// third-party client, and we shouldn't assume that downgrading the role
// will do more good than harm.
return role, nil
}

clientVersion, err := semver.NewVersion(clientVersionString)
if err != nil {
return nil, trace.BadParameter("unrecognized client version: %s is not a valid semver", clientVersionString)
}

role = maybeDowngradeRoleHostUserCreationMode(role, clientVersion)
// Teleport 16 supports all role features that Teleport 15 does,
// so no downgrade is necessary.
return role, nil
}

var minSupportedInsecureDropModeVersions = map[int64]semver.Version{
12: {Major: 12, Minor: 4, Patch: 30},
13: {Major: 13, Minor: 4, Patch: 11},
14: {Major: 14, Minor: 2, Patch: 2},
}

// maybeDowngradeRoleHostUserCreationMode tests the client version passed through the gRPC metadata, and
// if the client version is less than the minimum supported version for the insecure-drop
// host user creation mode returns a shallow copy of the role with mode drop. If the
// passed in role does not have mode insecure-drop or the client is a supported version,
// the role is returned unchanged.
func maybeDowngradeRoleHostUserCreationMode(role *types.RoleV6, clientVersion *semver.Version) *types.RoleV6 {
if role.GetOptions().CreateHostUserMode != types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP {
return role
}
minSupportedVersion, ok := minSupportedInsecureDropModeVersions[clientVersion.Major]
// Check if insecure-drop is supported. insecure-drop is not supported below
// v12, supported for some versions of v12 through v14, and supported for v15+.
if clientVersion.Major >= 12 && (!ok || !clientVersion.LessThan(minSupportedVersion)) {
return role
}

// 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)
options := role.GetOptions()
options.CreateHostUserMode = types.CreateHostUserMode_HOST_USER_MODE_DROP
role.SetOptions(options)

reason := fmt.Sprintf(`Client version %q does not support the host creation user mode `+
`'insecure-drop'. Role %q will be downgraded to use 'drop' mode instead. `+
`In order to support 'insecure-drop', all clients must be updated to version %q or higher.`,
clientVersion, role.GetName(), minSupportedVersion)
if role.Metadata.Labels == nil {
role.Metadata.Labels = make(map[string]string, 1)
}
role.Metadata.Labels[types.TeleportDowngradedLabel] = reason
return role
}

// GetRole retrieves a role by name.
func (g *GRPCServer) GetRole(ctx context.Context, req *authpb.GetRoleRequest) (*types.RoleV6, error) {
auth, err := g.authenticate(ctx)
Expand Down
172 changes: 0 additions & 172 deletions lib/auth/grpcserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import (
"google.golang.org/protobuf/types/known/emptypb"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/constants"
apidefaults "github.com/gravitational/teleport/api/defaults"
Expand Down Expand Up @@ -4267,177 +4266,6 @@ func TestPing_VersionCheck_AccessMonitoringFlag(t *testing.T) {
require.True(t, ping.ServerFeatures.AccessMonitoring.Enabled, "expected field AccessMonitoring.Enabled to be true")
}

func TestRoleVersions(t *testing.T) {
t.Parallel()
srv := newTestTLSServer(t)

newRole := func(name string, version string, spec types.RoleSpecV6) types.Role {
role, err := types.NewRoleWithVersion(name, version, spec)
meta := role.GetMetadata()
role.SetMetadata(meta)
require.NoError(t, err)
return role
}

role := newRole("test_role_1", types.V7, types.RoleSpecV6{
Allow: types.RoleConditions{
Rules: []types.Rule{
types.NewRule(types.KindRole, services.RW()),
},
},
Options: types.RoleOptions{
CreateHostUserMode: types.CreateHostUserMode_HOST_USER_MODE_INSECURE_DROP,
},
})

user, err := CreateUser(context.Background(), srv.Auth(), "user", role)
require.NoError(t, err)

client, err := srv.NewClient(TestUser(user.GetName()))
require.NoError(t, err)

for _, tc := range []struct {
desc string
clientVersions []string
expectError bool
inputRole types.Role
expectedRole types.Role
expectDowngraded bool
}{
{
desc: "up to date",
clientVersions: []string{
"15.1.2", api.Version, "",
},
inputRole: role,
expectedRole: role,
},
{
desc: "downgrade host user creation mode only",
clientVersions: []string{
"14.0.0-alpha.1",
},
inputRole: role,
expectedRole: newRole(role.GetName(), types.V7, types.RoleSpecV6{
Allow: types.RoleConditions{
Rules: []types.Rule{
types.NewRule(types.KindRole, services.RW()),
},
},
Options: types.RoleOptions{
CreateHostUserMode: types.CreateHostUserMode_HOST_USER_MODE_DROP,
},
}),
expectDowngraded: true,
},
} {
t.Run(tc.desc, func(t *testing.T) {
for _, clientVersion := range tc.clientVersions {
t.Run(clientVersion, func(t *testing.T) {
// setup client metadata
ctx := context.Background()
if clientVersion == "" {
ctx = context.WithValue(ctx, metadata.DisableInterceptors{}, struct{}{})
} else {
ctx = metadata.AddMetadataToContext(ctx, map[string]string{
metadata.VersionKey: clientVersion,
})
}

checkRole := func(gotRole types.Role) {
t.Helper()
if tc.expectError {
return
}
require.Empty(t, cmp.Diff(tc.expectedRole, gotRole,
cmpopts.IgnoreFields(types.Metadata{}, "ID", "Revision", "Labels")))
// The downgraded label value won't match exactly because it
// includes the client version, so just check it's not empty
// and ignore it in the role diff.
if tc.expectDowngraded {
require.NotEmpty(t, gotRole.GetMetadata().Labels[types.TeleportDowngradedLabel])
}
}
checkErr := func(err error) {
t.Helper()
if tc.expectError {
require.Error(t, err)
} else {
require.NoError(t, err)
}
}

// Test GetRole
gotRole, err := client.GetRole(ctx, tc.inputRole.GetName())
checkErr(err)
checkRole(gotRole)

// Test GetRoles
gotRoles, err := client.GetRoles(ctx)
checkErr(err)
if !tc.expectError {
foundTestRole := false
for _, gotRole := range gotRoles {
if gotRole.GetName() != tc.inputRole.GetName() {
continue
}
checkRole(gotRole)
foundTestRole = true
break
}
require.True(t, foundTestRole, "GetRoles result does not include expected role")
}

ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()

// Test WatchEvents
watcher, err := client.NewWatcher(ctx, types.Watch{Name: "roles", Kinds: []types.WatchKind{{Kind: types.KindRole}}})
require.NoError(t, err)
defer watcher.Close()

// Swallow the init event
e := <-watcher.Events()
require.Equal(t, types.OpInit, e.Type)

// Re-upsert the role so that the watcher sees it, do this
// on the auth server directly to avoid the
// TeleportDowngradedLabel check in ServerWithRoles
tc.inputRole, err = srv.Auth().UpsertRole(ctx, tc.inputRole)
require.NoError(t, err)

gotRole, err = func() (types.Role, error) {
for {
select {
case <-watcher.Done():
return nil, watcher.Error()
case e := <-watcher.Events():
if gotRole, ok := e.Resource.(types.Role); ok && gotRole.GetName() == tc.inputRole.GetName() {
return gotRole, nil
}
}
}
}()
checkErr(err)
checkRole(gotRole)

if !tc.expectError {
// Try to re-upsert the role we got. If it was
// downgraded, it should be rejected due to the
// TeleportDowngradedLabel
_, err = client.UpsertRole(ctx, gotRole)
if tc.expectDowngraded {
require.Error(t, err)
} else {
require.NoError(t, err)
}
}
})
}
})
}
}

func TestUpsertApplicationServerOrigin(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 19a89bc

Please sign in to comment.