[v7] backport #10741 (leaf cluster CA sanitizing)

api/types/authority.go

@@ -723,3 +723,46 @@ func (k *JWTKeyPair) CheckAndSetDefaults() error {
 	}
 	return nil
 }
+
+type CertAuthorityFilter map[CertAuthType]string
+
+func (f CertAuthorityFilter) IsEmpty() bool {
+	return len(f) == 0
+}
+
+// Match checks if a given CA matches this filter.
+func (f CertAuthorityFilter) Match(ca CertAuthority) bool {
+	if len(f) == 0 {
+		return true
+	}
+
+	return f[ca.GetType()] == Wildcard || f[ca.GetType()] == ca.GetClusterName()
+}
+
+// IntoMap makes this filter into a map for use as the Filter in a WatchKind.
+func (f CertAuthorityFilter) IntoMap() map[string]string {
+	if len(f) == 0 {
+		return nil
+	}
+
+	m := make(map[string]string, len(f))
+	for caType, name := range f {
+		m[string(caType)] = name
+	}
+	return m
+}
+
+// FromMap converts the provided map into this filter.
+func (f *CertAuthorityFilter) FromMap(m map[string]string) {
+	if len(m) == 0 {
+		*f = nil
+		return
+	}
+
+	*f = make(CertAuthorityFilter, len(m))
+	// there's not a lot of value in rejecting unknown values from the filter
+	for key, val := range m {
+		(*f)[CertAuthType(key)] = val
+	}
+
+}

api/types/events.go

@@ -150,13 +150,22 @@ func (kind WatchKind) Matches(e Event) (bool, error) {
 				return false, trace.Wrap(err)
 			}
 			return target.Match(res), nil
+		case CertAuthority:
+			var filter CertAuthorityFilter
+			filter.FromMap(kind.Filter)
+			return filter.Match(res), nil
 		default:
-			return false, trace.BadParameter("unfilterable resource type %T", e.Resource)
+			// we don't know about this filter, let the event through
 		}
 	}
 	return true, nil
 }
 
+// IsTrivial returns true iff the WatchKind only specifies a Kind but no other field.
+func (kind WatchKind) IsTrivial() bool {
+	return kind.SubKind == "" && kind.Name == "" && !kind.LoadSecrets && len(kind.Filter) == 0
+}
+
 // Events returns new events interface
 type Events interface {
 	// NewWatcher returns a new event watcher

api/types/events/oneof.go

@@ -351,6 +351,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")

lib/auth/grpcserver.go

@@ -304,6 +304,12 @@ func (g *GRPCServer) WatchEvents(watch *proto.Watch, stream proto.AuthService_Wa
 	for _, kind := range watch.Kinds {
 		servicesWatch.Kinds = append(servicesWatch.Kinds, proto.ToWatchKind(kind))
 	}
+
+	if clusterName, err := auth.GetClusterName(); err == nil {
+		// we might want to enforce a filter for older clients in certain conditions
+		maybeFilterCertAuthorityWatches(stream.Context(), clusterName.GetClusterName(), auth.Checker.RoleNames(), &servicesWatch)
+	}
+
 	watcher, err := auth.NewWatcher(stream.Context(), servicesWatch)
 	if err != nil {
 		return trace.Wrap(err)
@@ -332,6 +338,60 @@ func (g *GRPCServer) WatchEvents(watch *proto.Watch, stream proto.AuthService_Wa
 	}
 }
 
+// maybeFilterCertAuthorityWatches will add filters to the CertAuthority
+// WatchKinds in the watch if the client is authenticated as just a `Node` with
+// no other roles and if the client is older than the cutoff version, and if the
+// WatchKind for KindCertAuthority is trivial, i.e. it's a WatchKind{Kind:
+// KindCertAuthority} with no other fields set. In any other case we will assume
+// that the client knows what it's doing and the cache watcher will still send
+// everything.
+//
+// DELETE IN 10.0, no supported clients should require this at that point
+func maybeFilterCertAuthorityWatches(ctx context.Context, clusterName string, roleNames []string, watch *types.Watch) {
+	if len(roleNames) != 1 || roleNames[0] != string(types.RoleNode) {
+		return
+	}
+
+	clientVersionString, ok := metadata.ClientVersionFromContext(ctx)
+	if !ok {
+		log.Debug("no client version found in grpc context")
+		return
+	}
+
+	clientVersion, err := semver.NewVersion(clientVersionString)
+	if err != nil {
+		log.WithError(err).Debugf("couldn't parse client version %q", clientVersionString)
+		return
+	}
+
+	// we treat the entire previous major version as "old" for this version
+	// check, even if there might have been backports; compliant clients will
+	// supply their own filter anyway
+	if !clientVersion.LessThan(certAuthorityFilterVersionCutoff) {
+		return
+	}
+
+	for i, k := range watch.Kinds {
+		if k.Kind != types.KindCertAuthority || !k.IsTrivial() {
+			continue
+		}
+
+		log.Debugf("Injecting filter for CertAuthority watch for Node-only watcher with version %v", clientVersion)
+		watch.Kinds[i].Filter = NodeCertAuthorityFilter(clusterName).IntoMap()
+	}
+}
+
+func NodeCertAuthorityFilter(clusterName string) types.CertAuthorityFilter {
+	return types.CertAuthorityFilter{
+		types.HostCA: clusterName,
+		types.UserCA: types.Wildcard,
+	}
+}
+
+// certAuthorityFilterVersionCutoff is the version starting from which we stop
+// injecting filters for CertAuthority watches in maybeFilterCertAuthorityWatches.
+var certAuthorityFilterVersionCutoff = *semver.New("9.0.0")
+
 // resourceLabel returns the label for the provided types.Event
 func resourceLabel(event types.Event) string {
 	if event.Resource == nil {

lib/auth/rotate.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/defaults"
+	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/sshutils"
 	"github.com/gravitational/teleport/lib/tlsca"
 	"github.com/gravitational/teleport/lib/utils"
@@ -279,7 +280,22 @@ func (a *Server) RotateExternalCertAuthority(ca types.CertAuthority) error {
 	if err := updated.SetAdditionalTrustedKeys(ca.GetAdditionalTrustedKeys().Clone()); err != nil {
 		return trace.Wrap(err)
 	}
+
+	// a rotation state of "" gets stored as "standby" after
+	// CheckAndSetDefaults, so if `ca` came in with a zeroed rotation we must do
+	// this before checking if `updated` is the same as `existing` or the check
+	// will fail for no reason (CheckAndSetDefaults is idempotent so it's fine
+	// to call it both here and in CompareAndSwapCertAuthority)
 	updated.SetRotation(ca.GetRotation())
+	if err := updated.CheckAndSetDefaults(); err != nil {
+		return trace.Wrap(err)
+	}
+
+	// CASing `updated` over `existing` if they're equivalent will only cause
+	// backend and watcher spam for no gain, so we exit early if that's the case
+	if services.CertAuthoritiesEquivalent(existing, updated) {
+		return nil
+	}
 
 	// use compare and swap to protect from concurrent updates
 	// by trusted cluster API

lib/auth/trustedcluster.go

@@ -472,15 +472,34 @@ 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.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)
 	}
+	_, 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 := types.NewRemoteCluster(remoteClusterName)
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -498,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

lib/auth/trustedcluster_test.go

@@ -10,6 +10,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"
@@ -85,6 +86,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(ctx context.Context, t *testing.T, name ...string) *Server {
 	bk, err := memory.New(memory.Config{})
 	require.NoError(t, err)