Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate trusted_cluster name #49169

Closed
wants to merge 2 commits into from
Closed

Validate trusted_cluster name #49169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7749ed
Validate trusted_cluster name
bernardjkim Nov 19, 2024
d296fa1
Merge branch 'master' into bernard/trusted-cluster-name
bernardjkim Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 9 additions & 23 deletions integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2785,10 +2785,6 @@ func testMapRoles(t *testing.T, suite *integrationTestSuite) {
{Remote: mainDevs, Local: []string{auxDevs}},
})

// modify trusted cluster resource name so it would not
// match the cluster name to check that it does not matter
trustedCluster.SetName(main.Secrets.SiteName + "-cluster")

require.NoError(t, main.Start())
require.NoError(t, aux.Start())

Expand Down Expand Up @@ -3118,15 +3114,20 @@ func trustedClusters(t *testing.T, suite *integrationTestSuite, test trustedClus
{Remote: mainOps, Local: []string{auxDevs}},
})

// modify trusted cluster resource name, so it would not
// match the cluster name to check that it does not matter
trustedCluster.SetName(main.Secrets.SiteName + "-cluster")

require.NoError(t, main.Start())
require.NoError(t, aux.Start())

require.NoError(t, services.CheckAndSetDefaults(trustedCluster))

// Note that the trusted cluster resource name must match the cluster name.
// Modify the trusted cluster resource name and expect the upsert to fail.
trustedCluster.SetName(main.Secrets.SiteName + "-cluster")
_, err = aux.Process.GetAuthServer().UpsertTrustedCluster(ctx, trustedCluster)
require.Error(t, err, "expected failure due to tc name mismatch")

// Modify the trusted cluster resource name back to what it was orignally.
trustedCluster.SetName(main.Secrets.SiteName)

// try and upsert a trusted cluster
helpers.TryCreateTrustedCluster(t, aux.Process.GetAuthServer(), trustedCluster)
helpers.WaitForTunnelConnections(t, main.Process.GetAuthServer(), clusterAux, 1)
Expand Down Expand Up @@ -3353,9 +3354,6 @@ func trustedDisabledCluster(t *testing.T, suite *integrationTestSuite, test trus
{Remote: mainOps, Local: []string{auxDevs}},
})

// modify trusted cluster resource name, so it would not
// match the cluster name to check that it does not matter
trustedCluster.SetName(main.Secrets.SiteName + "-cluster")
// disable cluster
trustedCluster.SetEnabled(false)

Expand Down Expand Up @@ -3493,10 +3491,6 @@ func trustedClustersRoleMapChanges(t *testing.T, suite *integrationTestSuite, te
{Remote: mainOps, Local: []string{auxDevs}},
})

// modify trusted cluster resource name, so it would not
// match the cluster name to check that it does not matter
trustedCluster.SetName(main.Secrets.SiteName + "-cluster")

require.NoError(t, main.Start())
require.NoError(t, aux.Start())

Expand Down Expand Up @@ -3594,10 +3588,6 @@ func testTrustedTunnelNode(t *testing.T, suite *integrationTestSuite) {
{Remote: mainDevs, Local: []string{auxDevs}},
})

// modify trusted cluster resource name, so it would not
// match the cluster name to check that it does not matter
trustedCluster.SetName(main.Secrets.SiteName + "-cluster")

require.NoError(t, main.Start())
require.NoError(t, aux.Start())

Expand Down Expand Up @@ -3778,10 +3768,6 @@ func testTrustedClusterAgentless(t *testing.T, suite *integrationTestSuite) {
{Remote: devsRoleName, Local: []string{devsRoleName}},
})

// modify trusted cluster resource name, so it would not
// match the cluster name to check that it does not matter
trustedCluster.SetName(main.Secrets.SiteName + "-cluster")

require.NoError(t, main.Start())
require.NoError(t, leaf.Start())

Expand Down
7 changes: 4 additions & 3 deletions lib/auth/trustedcluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,10 @@ func (a *Server) establishTrust(ctx context.Context, trustedCluster types.Truste
if remoteClusterName == domainName {
return nil, trace.BadParameter("remote cluster name can not be the same as local cluster name")
}
// TODO(klizhentas) in 2.5.0 prohibit adding trusted cluster resource name
// different from cluster name (we had no way of checking this before x509,
// because SSH CA was a public key, not a cert with metadata)
Comment on lines -325 to -327
Copy link
Contributor

@hugoShaka hugoShaka Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, congrats for addressing a TODO from Sasha in v2 😅

if trustedCluster.GetName() != remoteClusterName {
return nil, trace.CompareFailed("trusted cluster resource name must be the same as the remote cluster name. got: %q, expected: %q",
trustedCluster.GetName(), remoteClusterName)
}
}
}

Expand Down
Loading