diff --git a/lib/auth/grpcserver_test.go b/lib/auth/grpcserver_test.go index 30ee3fca6aa48..fd9f828522fb2 100644 --- a/lib/auth/grpcserver_test.go +++ b/lib/auth/grpcserver_test.go @@ -54,7 +54,6 @@ import ( apidefaults "github.com/gravitational/teleport/api/defaults" clusterconfigpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/clusterconfig/v1" "github.com/gravitational/teleport/api/internalutils/stream" - "github.com/gravitational/teleport/api/metadata" "github.com/gravitational/teleport/api/mfa" "github.com/gravitational/teleport/api/observability/tracing" "github.com/gravitational/teleport/api/types" @@ -4198,74 +4197,6 @@ func TestGRPCServer_GetInstallers(t *testing.T) { } } -// DELETE IN 16.0 -// TestPing_VersionCheck_AccessMonitoringFlag tests that client versions <=14.2.0 -// sets the IGS flag. Old clients will expect IGS == access monitoring. -func TestPing_VersionCheck_AccessMonitoringFlag(t *testing.T) { - modules.SetTestModules(t, &modules.TestModules{ - TestFeatures: modules.Features{ - AccessMonitoring: modules.AccessMonitoringFeature{ - Enabled: true, - }, - }, - }) - - srv := newTestTLSServer(t) - srv.Auth().accessMonitoringEnabled = true - - user, _, err := CreateUserAndRoleWithoutRoles(srv.Auth(), "user", nil) - require.NoError(t, err) - - client, err := srv.NewClient(TestUser(user.GetName())) - require.NoError(t, err) - - // Test with latest major version that supports access monitoring field. - // Should NOT enable IGS. - ctx := context.Background() - ctx = metadata.AddMetadataToContext(ctx, map[string]string{ - metadata.VersionKey: "15.0.0", - }) - ping, err := client.Ping(ctx) - require.NoError(t, err) - require.False(t, ping.ServerFeatures.IdentityGovernance, "expected field IdentityGovernance to be false") - require.True(t, ping.ServerFeatures.AccessMonitoring.Enabled, "expected field AccessMonitoring.Enabled to be true") - - // Test with minimum major/minor/patch version that supports access monitoring field. - // Should NOT enable IGS. - ctx2 := context.Background() - ctx2 = metadata.AddMetadataToContext(ctx2, map[string]string{ - metadata.VersionKey: "14.2.1", - }) - ping, err = client.Ping(ctx2) - require.NoError(t, err) - require.False(t, ping.ServerFeatures.IdentityGovernance, "expected field IdentityGovernance to be false") - require.True(t, ping.ServerFeatures.AccessMonitoring.Enabled, "expected field AccessMonitoring.Enabled to be true") - - // Test with version that does NOT support access monitoring field. - // Should return with IGS enabled to mean access monitoring was enabled - // (because that's what older clients expect) - ctx3 := context.Background() - ctx3 = metadata.AddMetadataToContext(ctx3, map[string]string{ - metadata.VersionKey: "14.2.0", - }) - ping, err = client.Ping(ctx3) - require.NoError(t, err) - require.True(t, ping.ServerFeatures.IdentityGovernance, "expected field IdentityGovernance to be true") - require.True(t, ping.ServerFeatures.AccessMonitoring.Enabled, "expected field AccessMonitoring.Enabled to be true") - - // Test with an older version. - // Should return with IGS enabled to mean access monitoring was enabled - // (because that's what older clients expect) - ctx4 := context.Background() - ctx4 = metadata.AddMetadataToContext(ctx4, map[string]string{ - metadata.VersionKey: "13.3.0", - }) - ping, err = client.Ping(ctx4) - require.NoError(t, err) - require.True(t, ping.ServerFeatures.IdentityGovernance, "expected field IdentityGovernance to be true") - require.True(t, ping.ServerFeatures.AccessMonitoring.Enabled, "expected field AccessMonitoring.Enabled to be true") -} - func TestUpsertApplicationServerOrigin(t *testing.T) { t.Parallel() diff --git a/lib/auth/middleware.go b/lib/auth/middleware.go index 0d0e99e980140..d7638196a9095 100644 --- a/lib/auth/middleware.go +++ b/lib/auth/middleware.go @@ -23,10 +23,12 @@ import ( "crypto/tls" "crypto/x509" "encoding/json" + "fmt" "net" "net/http" "os" "slices" + "sync/atomic" "time" "github.com/coreos/go-semver/semver" @@ -167,9 +169,9 @@ func NewTLSServer(ctx context.Context, cfg TLSServerConfig) (*TLSServer, error) return nil, trace.Wrap(err) } - var oldestSupportedVersion *semver.Version - if os.Getenv("TELEPORT_UNSTABLE_REJECT_OLD_CLIENTS") == "yes" { - oldestSupportedVersion = &teleport.MinClientSemVersion + oldestSupportedVersion := &teleport.MinClientSemVersion + if os.Getenv("TELEPORT_UNSTABLE_ALLOW_OLD_CLIENTS") == "yes" { + oldestSupportedVersion = nil } // authMiddleware authenticates request assuming TLS client authentication @@ -181,6 +183,9 @@ func NewTLSServer(ctx context.Context, cfg TLSServerConfig) (*TLSServer, error) Limiter: limiter, GRPCMetrics: grpcMetrics, OldestSupportedVersion: oldestSupportedVersion, + AlertCreator: func(ctx context.Context, a types.ClusterAlert) error { + return trace.Wrap(cfg.AuthServer.UpsertClusterAlert(ctx, a)) + }, } apiServer, err := NewAPIServer(&cfg.APIConfig) @@ -336,6 +341,13 @@ type Middleware struct { // originated from a client that is using an unsupported version. If not set, then no // rejection occurs. OldestSupportedVersion *semver.Version + // AlertCreator if provided is used to generate a cluster alert when any + // unsupported connections are rejected. + AlertCreator func(ctx context.Context, a types.ClusterAlert) error + + // lastRejectedAlertTime is the timestamp at which the last alert + // was created in response to rejecting unsupported clients. + lastRejectedAlertTime atomic.Int64 } // Wrap sets next handler in chain @@ -391,23 +403,67 @@ func (a *Middleware) ValidateClientVersion(ctx context.Context, info IdentityInf clientVersion, err := semver.NewVersion(clientVersionString) if err != nil { logger.WithError(err).Warn("Failed to determine client version") + a.displayRejectedClientAlert(ctx) if err := info.Conn.Close(); err != nil { logger.WithError(err).Warn("Failed to close client connection") } + return trace.AccessDenied("client version is unsupported") } if clientVersion.LessThan(*a.OldestSupportedVersion) { logger.Info("Terminating connection of client using unsupported version") + a.displayRejectedClientAlert(ctx) + if err := info.Conn.Close(); err != nil { logger.WithError(err).Warn("Failed to close client connection") } + return trace.AccessDenied("client version is unsupported") } return nil } +// displayRejectedClientAlert creates an alert to notify admins that +// unsupported Teleport versions exist in the cluster and are explicitly +// being denied to prevent causing issues. Alerts are limited to being +// created once per day to reduce backend writes if there are a large +// number of unsupported clients constantly being rejected. +func (a *Middleware) displayRejectedClientAlert(ctx context.Context) { + if a.AlertCreator == nil { + return + } + + now := time.Now() + lastAlert := a.lastRejectedAlertTime.Load() + then := time.Unix(0, lastAlert) + if lastAlert != 0 && now.Before(then.Add(24*time.Hour)) { + return + } + + if !a.lastRejectedAlertTime.CompareAndSwap(lastAlert, now.UnixNano()) { + return + } + + alert, err := types.NewClusterAlert( + "rejected-unsupported-connection", + fmt.Sprintf("Connections were rejected from agents running unsupported Teleport versions(<%s), they will be inaccessible until upgraded.", a.OldestSupportedVersion), + types.WithAlertSeverity(types.AlertSeverity_MEDIUM), + types.WithAlertLabel(types.AlertOnLogin, "yes"), + types.WithAlertLabel(types.AlertVerbPermit, fmt.Sprintf("%s:%s", types.KindToken, types.VerbCreate)), + ) + if err != nil { + log.WithError(err).Warn("failed to create rejected-unsupported-connection alert") + return + } + + if err := a.AlertCreator(ctx, alert); err != nil { + log.WithError(err).Warn("failed to persist rejected-unsupported-connection alert") + return + } +} + // withAuthenticatedUser returns a new context with the ContextUser field set to // the caller's user identity as authenticated by their client TLS certificate. func (a *Middleware) withAuthenticatedUser(ctx context.Context) (context.Context, error) { diff --git a/lib/auth/middleware_test.go b/lib/auth/middleware_test.go index f0f56fbae1f94..3e080ca2c1e78 100644 --- a/lib/auth/middleware_test.go +++ b/lib/auth/middleware_test.go @@ -28,6 +28,7 @@ import ( "net" "net/http" "net/http/httptest" + "sync" "testing" "time" @@ -35,6 +36,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/gravitational/trace" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/metadata" @@ -671,26 +673,27 @@ func (f fakeConn) Close() error { func TestValidateClientVersion(t *testing.T) { cases := []struct { name string - middleware Middleware + middleware *Middleware clientVersion string errAssertion func(t *testing.T, err error) }{ { - name: "rejection disabled", + name: "rejection disabled", + middleware: &Middleware{}, errAssertion: func(t *testing.T, err error) { require.NoError(t, err) }, }, { name: "rejection enabled and client version not specified", - middleware: Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, + middleware: &Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, errAssertion: func(t *testing.T, err error) { require.NoError(t, err) }, }, { name: "client rejected", - middleware: Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, + middleware: &Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, clientVersion: semver.Version{Major: teleport.SemVersion.Major - 2}.String(), errAssertion: func(t *testing.T, err error) { require.True(t, trace.IsAccessDenied(err), "got %T, expected access denied error", err) @@ -698,7 +701,7 @@ func TestValidateClientVersion(t *testing.T) { }, { name: "valid client v-1", - middleware: Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, + middleware: &Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, clientVersion: semver.Version{Major: teleport.SemVersion.Major - 1}.String(), errAssertion: func(t *testing.T, err error) { require.NoError(t, err) @@ -706,7 +709,7 @@ func TestValidateClientVersion(t *testing.T) { }, { name: "valid client v-0", - middleware: Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, + middleware: &Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, clientVersion: semver.Version{Major: teleport.SemVersion.Major}.String(), errAssertion: func(t *testing.T, err error) { require.NoError(t, err) @@ -714,7 +717,7 @@ func TestValidateClientVersion(t *testing.T) { }, { name: "invalid client version", - middleware: Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, + middleware: &Middleware{OldestSupportedVersion: &teleport.MinClientSemVersion}, clientVersion: "abc123", errAssertion: func(t *testing.T, err error) { require.True(t, trace.IsAccessDenied(err), "got %T, expected access denied error", err) @@ -733,3 +736,57 @@ func TestValidateClientVersion(t *testing.T) { }) } } + +func TestRejectedClientClusterAlert(t *testing.T) { + var alerts []types.ClusterAlert + mw := Middleware{ + OldestSupportedVersion: &teleport.MinClientSemVersion, + AlertCreator: func(ctx context.Context, a types.ClusterAlert) error { + alerts = append(alerts, a) + return nil + }, + } + + // Validate an unsupported client, which should trigger an alert + ctx := metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{ + "version": semver.Version{Major: teleport.SemVersion.Major - 20}.String(), + })) + err := mw.ValidateClientVersion(ctx, IdentityInfo{Conn: fakeConn{}, IdentityGetter: TestBuiltin(types.RoleNode).I}) + assert.Error(t, err) + + // Validate a client with an unknown version, which should trigger an alert, however, + // due to rate limiting of 1 alert per 24h no alert should be created. + ctx = metadata.NewIncomingContext(context.Background(), metadata.New(map[string]string{ + "version": "abcd", + })) + err = mw.ValidateClientVersion(ctx, IdentityInfo{Conn: fakeConn{}, IdentityGetter: TestBuiltin(types.RoleNode).I}) + assert.Error(t, err) + + // Assert that only a single alert was created based on the above rejections. + require.Len(t, alerts, 1) + require.Equal(t, "rejected-unsupported-connection", alerts[0].GetName()) + + // Reset the last alert time to a time beyond the rate limit, allowing the next + // rejection to trigger another alert. + mw.lastRejectedAlertTime.Store(time.Now().Add(-25 * time.Hour).UnixNano()) + + // Validate two unsupported clients in parallel to verify that concurrent attempts + // to create an alert are prevented. + var wg sync.WaitGroup + for i := 0; i < 2; i++ { + wg.Add(1) + go func() { + defer wg.Done() + err := mw.ValidateClientVersion(ctx, IdentityInfo{Conn: fakeConn{}, IdentityGetter: TestBuiltin(types.RoleNode).I}) + assert.Error(t, err) + }() + } + + wg.Wait() + + // Assert that only a single additional alert was created. + require.Len(t, alerts, 2) + for _, alert := range alerts { + assert.Equal(t, "rejected-unsupported-connection", alert.GetName()) + } +}