From e40c660c9a51c196df6344dfbe807f456c7a9ad5 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Thu, 17 Feb 2022 17:49:32 +0100 Subject: [PATCH 1/4] Enforce that a leaf cluster has sent us just a single host CA --- lib/auth/trustedcluster.go | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/auth/trustedcluster.go b/lib/auth/trustedcluster.go index ad67360a3b321..a0c0a5fabcd49 100644 --- a/lib/auth/trustedcluster.go +++ b/lib/auth/trustedcluster.go @@ -472,15 +472,28 @@ func (a *Server) validateTrustedCluster(ctx context.Context, validateRequest *Va return nil, trace.Wrap(err) } - // add remote cluster resource to keep track of the remote cluster - var remoteClusterName string - for _, certAuthority := range validateRequest.CAs { - // don't add a ca with the same as as local cluster name - if certAuthority.GetName() == domainName { - return nil, trace.AccessDenied("remote certificate authority has same name as cluster certificate authority: %v", domainName) - } - remoteClusterName = certAuthority.GetName() + if len(validateRequest.CAs) != 1 { + return nil, trace.AccessDenied("expected exactly one certificate authority, received %v", len(validateRequest.CAs)) + } + remoteCA := validateRequest.CAs[0] + err = remoteCA.CheckAndSetDefaults() + if err != nil { + return nil, trace.Wrap(err) } + + if remoteCA.GetType() != types.HostCA { + return nil, trace.AccessDenied("expected host certificate authority, received CA with type %q", remoteCA.GetType()) + } + + // a host CA shouldn't have a rolemap or roles in the first place + remoteCA.SetRoleMap(nil) + remoteCA.SetRoles(nil) + + remoteClusterName := remoteCA.GetName() + if remoteClusterName == domainName { + return nil, trace.AccessDenied("remote cluster has same name as this cluster: %v", domainName) + } + remoteCluster, err := types.NewRemoteCluster(remoteClusterName) if err != nil { return nil, trace.Wrap(err) @@ -498,12 +511,9 @@ func (a *Server) validateTrustedCluster(ctx context.Context, validateRequest *Va } } - // token has been validated, upsert the given certificate authority - for _, certAuthority := range validateRequest.CAs { - err = a.UpsertCertAuthority(certAuthority) - if err != nil { - return nil, trace.Wrap(err) - } + err = a.UpsertCertAuthority(remoteCA) + if err != nil { + return nil, trace.Wrap(err) } // export local cluster certificate authority and return it to the cluster From e730016d328839da88e35aa60960b0b1e56d8c70 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Fri, 18 Feb 2022 14:36:37 +0100 Subject: [PATCH 2/4] Test coverage for validateTrustedCluster --- lib/auth/trustedcluster_test.go | 109 ++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/lib/auth/trustedcluster_test.go b/lib/auth/trustedcluster_test.go index 9091451faf4e0..a3c0f26018dab 100644 --- a/lib/auth/trustedcluster_test.go +++ b/lib/auth/trustedcluster_test.go @@ -24,6 +24,7 @@ import ( authority "github.com/gravitational/teleport/lib/auth/testauthority" "github.com/gravitational/teleport/lib/backend/memory" "github.com/gravitational/teleport/lib/services" + "github.com/gravitational/teleport/lib/services/suite" "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" @@ -99,6 +100,114 @@ func TestRemoteClusterStatus(t *testing.T) { require.Empty(t, cmp.Diff(rc, gotRC)) } +func TestValidateTrustedCluster(t *testing.T) { + const localClusterName = "localcluster" + const validToken = "validtoken" + ctx := context.Background() + + testAuth, err := NewTestAuthServer(TestAuthServerConfig{ + ClusterName: localClusterName, + Dir: t.TempDir(), + }) + require.NoError(t, err) + a := testAuth.AuthServer + + tks, err := types.NewStaticTokens(types.StaticTokensSpecV2{ + StaticTokens: []types.ProvisionTokenV1{{ + Roles: []types.SystemRole{types.RoleTrustedCluster}, + Token: validToken, + }}, + }) + require.NoError(t, err) + a.SetStaticTokens(tks) + + _, err = a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ + Token: "invalidtoken", + CAs: []types.CertAuthority{}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid cluster token") + + _, err = a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ + Token: validToken, + CAs: []types.CertAuthority{}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "expected exactly one") + + _, err = a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ + Token: validToken, + CAs: []types.CertAuthority{ + suite.NewTestCA(types.HostCA, "rc1"), + suite.NewTestCA(types.HostCA, "rc2"), + }, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "expected exactly one") + + _, err = a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ + Token: validToken, + CAs: []types.CertAuthority{ + suite.NewTestCA(types.UserCA, "rc3"), + }, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "expected host certificate authority") + + _, err = a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ + Token: validToken, + CAs: []types.CertAuthority{ + suite.NewTestCA(types.HostCA, localClusterName), + }, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "same name as this cluster") + + leafClusterCA := types.CertAuthority(suite.NewTestCA(types.HostCA, "rc4")) + resp, err := a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ + Token: validToken, + CAs: []types.CertAuthority{leafClusterCA}, + }) + require.NoError(t, err) + + require.Len(t, resp.CAs, 2) + require.ElementsMatch(t, + []types.CertAuthType{types.HostCA, types.UserCA}, + []types.CertAuthType{resp.CAs[0].GetType(), resp.CAs[1].GetType()}, + ) + + for _, returnedCA := range resp.CAs { + localCA, err := a.GetCertAuthority(types.CertAuthID{ + Type: returnedCA.GetType(), + DomainName: localClusterName, + }, false) + require.NoError(t, err) + require.True(t, services.CertAuthoritiesEquivalent(localCA, returnedCA)) + } + + rcs, err := a.GetRemoteClusters() + require.NoError(t, err) + require.Len(t, rcs, 1) + require.Equal(t, leafClusterCA.GetName(), rcs[0].GetName()) + + hostCAs, err := a.GetCertAuthorities(types.HostCA, false) + require.NoError(t, err) + require.Len(t, hostCAs, 2) + require.ElementsMatch(t, + []string{localClusterName, leafClusterCA.GetName()}, + []string{hostCAs[0].GetName(), hostCAs[1].GetName()}, + ) + require.Empty(t, hostCAs[0].GetRoles()) + require.Empty(t, hostCAs[0].GetRoleMap()) + require.Empty(t, hostCAs[1].GetRoles()) + require.Empty(t, hostCAs[1].GetRoleMap()) + + userCAs, err := a.GetCertAuthorities(types.UserCA, false) + require.NoError(t, err) + require.Len(t, userCAs, 1) + require.Equal(t, localClusterName, userCAs[0].GetName()) +} + func newTestAuthServer(ctx context.Context, t *testing.T, name ...string) *Server { bk, err := memory.New(memory.Config{}) require.NoError(t, err) From 3930cb6b14ec0721b5e8deddd5ddc74923b036ed Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Fri, 18 Feb 2022 14:56:27 +0100 Subject: [PATCH 3/4] Add `tctl rm cert_authority/kind/name` --- tool/tctl/common/resource_command.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go index 7ef91868417db..6112c59df4dd9 100644 --- a/tool/tctl/common/resource_command.go +++ b/tool/tctl/common/resource_command.go @@ -770,6 +770,21 @@ func (rc *ResourceCommand) Delete(client auth.ClientI) (err error) { return err } fmt.Printf(fmts+"\n", deleted, rc.ref.Name) + case types.KindCertAuthority: + if rc.ref.SubKind == "" || rc.ref.Name == "" { + return trace.BadParameter( + "full %s path must be specified (e.g. '%s/%s/clustername')", + types.KindCertAuthority, types.KindCertAuthority, types.HostCA, + ) + } + err := client.DeleteCertAuthority(types.CertAuthID{ + Type: types.CertAuthType(rc.ref.SubKind), + DomainName: rc.ref.Name, + }) + if err != nil { + return trace.Wrap(err) + } + fmt.Printf("%s '%s/%s' has been deleted\n", types.KindCertAuthority, rc.ref.SubKind, rc.ref.Name) default: return trace.BadParameter("deleting resources of type %q is not supported", rc.ref.Kind) } From 6f04c6b42ffc493794da1b9d1fcd1787e33da41b Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Mon, 21 Feb 2022 09:26:58 +0100 Subject: [PATCH 4/4] Check against existing trusted clusters --- lib/auth/trustedcluster.go | 6 ++++++ lib/auth/trustedcluster_test.go | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/auth/trustedcluster.go b/lib/auth/trustedcluster.go index a0c0a5fabcd49..98167a8d62754 100644 --- a/lib/auth/trustedcluster.go +++ b/lib/auth/trustedcluster.go @@ -493,6 +493,12 @@ func (a *Server) validateTrustedCluster(ctx context.Context, validateRequest *Va if remoteClusterName == domainName { return nil, trace.AccessDenied("remote cluster has same name as this cluster: %v", domainName) } + _, err = a.GetTrustedCluster(ctx, remoteClusterName) + if err == nil { + return nil, trace.AccessDenied("remote cluster has same name as trusted cluster: %v", remoteClusterName) + } else if !trace.IsNotFound(err) { + return nil, trace.Wrap(err) + } remoteCluster, err := types.NewRemoteCluster(remoteClusterName) if err != nil { diff --git a/lib/auth/trustedcluster_test.go b/lib/auth/trustedcluster_test.go index a3c0f26018dab..f3fe988742b1b 100644 --- a/lib/auth/trustedcluster_test.go +++ b/lib/auth/trustedcluster_test.go @@ -163,7 +163,24 @@ func TestValidateTrustedCluster(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "same name as this cluster") - leafClusterCA := types.CertAuthority(suite.NewTestCA(types.HostCA, "rc4")) + trustedCluster, err := types.NewTrustedCluster("trustedcluster", + types.TrustedClusterSpecV2{Roles: []string{"nonempty"}}) + require.NoError(t, err) + // use the UpsertTrustedCluster in Presence as we just want the resource in + // the backend, we don't want to actually connect + _, err = a.Presence.UpsertTrustedCluster(ctx, trustedCluster) + require.NoError(t, err) + + _, err = a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ + Token: validToken, + CAs: []types.CertAuthority{ + suite.NewTestCA(types.HostCA, trustedCluster.GetName()), + }, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "same name as trusted cluster") + + leafClusterCA := types.CertAuthority(suite.NewTestCA(types.HostCA, "leafcluster")) resp, err := a.validateTrustedCluster(ctx, &ValidateTrustedClusterRequest{ Token: validToken, CAs: []types.CertAuthority{leafClusterCA},