From 837718e4c8b015da0b1e464cb5ff7d54fc7ed5e3 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Wed, 2 Mar 2022 19:35:36 +0100 Subject: [PATCH] [v6.2] Sanitize leaf cluster CA (#10745) --- api/types/events/oneof.go | 2 + lib/auth/trustedcluster.go | 47 +++++++--- lib/auth/trustedcluster_test.go | 128 +++++++++++++++++++++++++++ tool/tctl/common/resource_command.go | 15 ++++ 4 files changed, 178 insertions(+), 14 deletions(-) diff --git a/api/types/events/oneof.go b/api/types/events/oneof.go index 4b3c8d870fe8c..758ed08c1a31f 100644 --- a/api/types/events/oneof.go +++ b/api/types/events/oneof.go @@ -339,6 +339,8 @@ func FromOneOf(in OneOf) (AuditEvent, error) { return e, nil } else if e := in.GetAccessRequestDelete(); e != nil { return e, nil + } else if e := in.GetCertificateCreate(); e != nil { + return e, nil } else { if in.Event == nil { return nil, trace.BadParameter("failed to parse event, session record is corrupted") diff --git a/lib/auth/trustedcluster.go b/lib/auth/trustedcluster.go index b38770ad89c08..b70706836c5f3 100644 --- a/lib/auth/trustedcluster.go +++ b/lib/auth/trustedcluster.go @@ -469,15 +469,37 @@ func (a *Server) validateTrustedCluster(validateRequest *ValidateTrustedClusterR 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.GetName() != remoteCA.GetClusterName() { + return nil, trace.AccessDenied("remote CA is inconsistently named, metadata name is %q and spec cluster name is %q", remoteCA.GetName(), remoteCA.GetClusterName()) + } + + if remoteCA.GetType() != services.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) + } + _, err = a.GetTrustedCluster(context.TODO(), 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 := services.NewRemoteCluster(remoteClusterName) if err != nil { return nil, trace.Wrap(err) @@ -495,12 +517,9 @@ func (a *Server) validateTrustedCluster(validateRequest *ValidateTrustedClusterR } } - // 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 diff --git a/lib/auth/trustedcluster_test.go b/lib/auth/trustedcluster_test.go index 280d4d92f0ffc..032b5b0fe1a43 100644 --- a/lib/auth/trustedcluster_test.go +++ b/lib/auth/trustedcluster_test.go @@ -1,13 +1,16 @@ package auth import ( + "context" "testing" "time" "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/types" 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" @@ -82,6 +85,131 @@ 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(&ValidateTrustedClusterRequest{ + Token: "invalidtoken", + CAs: []types.CertAuthority{}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid cluster token") + + _, err = a.validateTrustedCluster(&ValidateTrustedClusterRequest{ + Token: validToken, + CAs: []types.CertAuthority{}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "expected exactly one") + + _, err = a.validateTrustedCluster(&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(&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(&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") + + 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(&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(&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(t *testing.T, name ...string) *Server { bk, err := memory.New(memory.Config{}) require.NoError(t, err) diff --git a/tool/tctl/common/resource_command.go b/tool/tctl/common/resource_command.go index b0b065289061f..463bba70d9235 100644 --- a/tool/tctl/common/resource_command.go +++ b/tool/tctl/common/resource_command.go @@ -517,6 +517,21 @@ func (rc *ResourceCommand) Delete(client auth.ClientI) (err error) { return trace.Wrap(err) } fmt.Printf("kubernetes service %v has been deleted\n", rc.ref.Name) + case services.KindCertAuthority: + if rc.ref.SubKind == "" || rc.ref.Name == "" { + return trace.BadParameter( + "full %s path must be specified (e.g. '%s/%s/clustername')", + services.KindCertAuthority, services.KindCertAuthority, services.HostCA, + ) + } + err := client.DeleteCertAuthority(services.CertAuthID{ + Type: services.CertAuthType(rc.ref.SubKind), + DomainName: rc.ref.Name, + }) + if err != nil { + return trace.Wrap(err) + } + fmt.Printf("%s '%s/%s' has been deleted\n", services.KindCertAuthority, rc.ref.SubKind, rc.ref.Name) default: return trace.BadParameter("deleting resources of type %q is not supported", rc.ref.Kind) }