Skip to content

Commit

Permalink
[v15] Dont allow cloud tenants to update certain cluster networking c…
Browse files Browse the repository at this point in the history 
…onfig fields (#41247)

* Dont allow cloud tenants to update certain cluster networking config fields

I'm bringing #28634 back from the dead, looks like it got nuked from
orbit in a bad merge conflict resolution in #27873

Dont allow cloud tenants to update certain cluster networking fields

* share validate impl

* add godoc comment

---------

Co-authored-by: Alex McGrath <alex.mcgrath@goteleport.com>
  • Loading branch information
@nklaassen @lxea
nklaassen and lxea committed May 8, 2024
1 parent b0f5739 commit 4b21267
Show file tree
Hide file tree
Showing 3 changed files with 222 additions and 2 deletions.
19 changes: 18 additions & 1 deletion lib/auth/auth_with_roles.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import (
"github.com/gravitational/teleport/api/types/wrappers"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib/auth/clusterconfig/clusterconfigv1"
"github.com/gravitational/teleport/lib/auth/okta"
"github.com/gravitational/teleport/lib/auth/trust/trustv1"
"github.com/gravitational/teleport/lib/authz"
Expand Down Expand Up @@ -4519,6 +4520,15 @@ func (a *ServerWithRoles) SetClusterNetworkingConfig(ctx context.Context, newNet
return trace.AccessDenied("proxy peering is an enterprise-only feature")
}

oldNetConf, err := a.authServer.GetClusterNetworkingConfig(ctx)
if err != nil {
return trace.Wrap(err)
}

if err := clusterconfigv1.ValidateCloudNetworkConfigUpdate(a.context, newNetConfig, oldNetConf); err != nil {
return trace.Wrap(err)
}

_, err = a.authServer.UpsertClusterNetworkingConfig(ctx, newNetConfig)
var msg string
if err != nil {
Expand All @@ -4539,7 +4549,6 @@ func (a *ServerWithRoles) SetClusterNetworkingConfig(ctx context.Context, newNet
}); auditErr != nil {
log.WithError(auditErr).Warn("Failed to emit cluster networking config update event event.")
}

return trace.Wrap(err)

}
Expand All @@ -4559,6 +4568,14 @@ func (a *ServerWithRoles) ResetClusterNetworkingConfig(ctx context.Context) erro
return trace.Wrap(err)
}
}
oldNetConf, err := a.authServer.GetClusterNetworkingConfig(ctx)
if err != nil {
return trace.Wrap(err)
}

if err := clusterconfigv1.ValidateCloudNetworkConfigUpdate(a.context, types.DefaultClusterNetworkingConfig(), oldNetConf); err != nil {
return trace.Wrap(err)
}

if err := a.context.AuthorizeAdminAction(); err != nil {
return trace.Wrap(err)
Expand Down
136 changes: 136 additions & 0 deletions lib/auth/auth_with_roles_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1263,6 +1263,142 @@ func TestAuthPreferenceRBAC(t *testing.T) {
})
}

func TestClusterNetworkingCloudUpdates(t *testing.T) {
srv := newTestTLSServer(t)
ctx := context.Background()
_, err := srv.Auth().UpsertClusterNetworkingConfig(ctx, types.DefaultClusterNetworkingConfig())
require.NoError(t, err)

user, _, err := CreateUserAndRole(srv.Auth(), "username", []string{}, []types.Rule{
{
Resources: []string{
types.KindClusterNetworkingConfig,
},
Verbs: services.RW(),
},
})
require.NoError(t, err)

for _, tc := range []struct {
cloud bool
identity TestIdentity
expectSetErr string
clusterNetworkingConfig types.ClusterNetworkingConfig
name string
}{
{
name: "non admin user can set existing values to the same value",
cloud: true,
identity: TestUser(user.GetName()),
clusterNetworkingConfig: types.DefaultClusterNetworkingConfig(),
},
{
name: "non admin user cannot set keep_alive_interval",
cloud: true,
identity: TestUser(user.GetName()),
expectSetErr: "keep_alive_interval",
clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
KeepAliveInterval: types.Duration(time.Second * 20),
}),
},
{
name: "non admin user cannot set tunnel_strategy",
cloud: true,
identity: TestUser(user.GetName()),
expectSetErr: "tunnel_strategy",
clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
TunnelStrategy: &types.TunnelStrategyV1{
Strategy: &types.TunnelStrategyV1_ProxyPeering{
ProxyPeering: types.DefaultProxyPeeringTunnelStrategy(),
},
},
}),
},
{
name: "non admin user cannot set proxy_listener_mode",
cloud: true,
identity: TestUser(user.GetName()),
expectSetErr: "proxy_listener_mode",
clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
ProxyListenerMode: types.ProxyListenerMode_Multiplex,
}),
},
{
name: "non admin user cannot set keep_alive_count_max",
cloud: true,
identity: TestUser(user.GetName()),
expectSetErr: "keep_alive_count_max",
clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
KeepAliveCountMax: 55,
}),
},
{
name: "non admin user can set client_idle_timeout",
cloud: true,
identity: TestUser(user.GetName()),
clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
ClientIdleTimeout: types.Duration(time.Second * 67),
}),
},
{
name: "admin user can set keep_alive_interval",
cloud: true,
identity: TestAdmin(),
clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
KeepAliveInterval: types.Duration(time.Second * 67),
}),
},
{
name: "non admin user can set keep_alive_interval on non cloud cluster",
cloud: false,
identity: TestUser(user.GetName()),
clusterNetworkingConfig: newClusterNetworkingConf(t, types.ClusterNetworkingConfigSpecV2{
KeepAliveInterval: types.Duration(time.Second * 67),
}),
},
} {
t.Run(tc.name, func(t *testing.T) {
modules.SetTestModules(t, &modules.TestModules{
TestBuildType: modules.BuildEnterprise,
TestFeatures: modules.Features{
Cloud: tc.cloud,
},
})

client, err := srv.NewClient(tc.identity)
require.NoError(t, err)

err = client.SetClusterNetworkingConfig(ctx, tc.clusterNetworkingConfig.(*types.ClusterNetworkingConfigV2))
if tc.expectSetErr != "" {
assert.ErrorContains(t, err, tc.expectSetErr)
} else {
assert.NoError(t, err)
}

_, err = client.UpsertClusterNetworkingConfig(ctx, tc.clusterNetworkingConfig)
if tc.expectSetErr != "" {
assert.ErrorContains(t, err, tc.expectSetErr)
} else {
assert.NoError(t, err)
}
})
}
}

func newClusterNetworkingConf(t *testing.T, spec types.ClusterNetworkingConfigSpecV2) *types.ClusterNetworkingConfigV2 {
c := &types.ClusterNetworkingConfigV2{
Metadata: types.Metadata{
Labels: map[string]string{
types.OriginLabel: types.OriginDynamic,
},
},
Spec: spec,
}
err := c.CheckAndSetDefaults()
require.NoError(t, err)
return c
}

func TestClusterNetworkingConfigRBAC(t *testing.T) {
t.Parallel()
ctx := context.Background()
Expand Down
69 changes: 68 additions & 1 deletion lib/auth/clusterconfig/clusterconfigv1/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,17 @@ func (s *Service) UpdateClusterNetworkingConfig(ctx context.Context, req *cluste

req.ClusterNetworkConfig.SetOrigin(types.OriginDynamic)

oldCfg, err := s.cache.GetClusterNetworkingConfig(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

newCfg := req.GetClusterNetworkConfig()

if err := ValidateCloudNetworkConfigUpdate(*authzCtx, newCfg, oldCfg); err != nil {
return nil, trace.Wrap(err)
}

updated, err := s.backend.UpdateClusterNetworkingConfig(ctx, req.ClusterNetworkConfig)

if err := s.emitter.EmitAuditEvent(ctx, &apievents.ClusterNetworkingConfigUpdate{
Expand Down Expand Up @@ -487,7 +498,18 @@ func (s *Service) UpsertClusterNetworkingConfig(ctx context.Context, req *cluste

req.ClusterNetworkConfig.SetOrigin(types.OriginDynamic)

upserted, err := s.backend.UpsertClusterNetworkingConfig(ctx, req.GetClusterNetworkConfig())
oldCfg, err := s.cache.GetClusterNetworkingConfig(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

newCfg := req.GetClusterNetworkConfig()

if err := ValidateCloudNetworkConfigUpdate(*authzCtx, newCfg, oldCfg); err != nil {
return nil, trace.Wrap(err)
}

upserted, err := s.backend.UpsertClusterNetworkingConfig(ctx, newCfg)

if err := s.emitter.EmitAuditEvent(ctx, &apievents.ClusterNetworkingConfigUpdate{
Metadata: apievents.Metadata{
Expand Down Expand Up @@ -531,6 +553,16 @@ func (s *Service) ResetClusterNetworkingConfig(ctx context.Context, _ *clusterco
}

defaultConfig := types.DefaultClusterNetworkingConfig()

oldCfg, err := s.cache.GetClusterNetworkingConfig(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

if err := ValidateCloudNetworkConfigUpdate(*authzCtx, defaultConfig, oldCfg); err != nil {
return nil, trace.Wrap(err)
}

const iterationLimit = 3
// Attempt a few iterations in case the conditional update fails
// due to spurious networking conditions.
Expand Down Expand Up @@ -578,6 +610,41 @@ func (s *Service) ResetClusterNetworkingConfig(ctx context.Context, _ *clusterco
return nil, trace.LimitExceeded("failed to reset networking config within %v iterations", iterationLimit)
}

// ValidateCloudNetworkConfigUpdate validates that that [newConfig] is a valid update of [oldConfig]. Cloud
// customers are not allowed to edit certain fields of the cluster networking config, and even if they were,
// the edits would be overwritten by the values from the static config file every time an auth process starts
// up.
func ValidateCloudNetworkConfigUpdate(authzCtx authz.Context, newConfig, oldConfig types.ClusterNetworkingConfig) error {
if authz.HasBuiltinRole(authzCtx, string(types.RoleAdmin)) {
return nil
}

if !modules.GetModules().Features().Cloud {
return nil
}

const cloudUpdateFailureMsg = "cloud tenants cannot update %q"

if newConfig.GetProxyListenerMode() != oldConfig.GetProxyListenerMode() {
return trace.BadParameter(cloudUpdateFailureMsg, "proxy_listener_mode")
}
newtst, _ := newConfig.GetTunnelStrategyType()
oldtst, _ := oldConfig.GetTunnelStrategyType()
if newtst != oldtst {
return trace.BadParameter(cloudUpdateFailureMsg, "tunnel_strategy")
}

if newConfig.GetKeepAliveInterval() != oldConfig.GetKeepAliveInterval() {
return trace.BadParameter(cloudUpdateFailureMsg, "keep_alive_interval")
}

if newConfig.GetKeepAliveCountMax() != oldConfig.GetKeepAliveCountMax() {
return trace.BadParameter(cloudUpdateFailureMsg, "keep_alive_count_max")
}

return nil
}

// GetSessionRecordingConfig returns the locally cached networking configuration.
func (s *Service) GetSessionRecordingConfig(ctx context.Context, _ *clusterconfigpb.GetSessionRecordingConfigRequest) (*types.SessionRecordingConfigV2, error) {
authzCtx, err := s.authorizer.Authorize(ctx)
Expand Down

0 comments on commit 4b21267

Please sign in to comment.