Log informative messages for device authn failures (#24849)

lib/client/api.go

@@ -69,6 +69,7 @@ import (
 	wancli "github.com/gravitational/teleport/lib/auth/webauthncli"
 	"github.com/gravitational/teleport/lib/client/terminal"
 	"github.com/gravitational/teleport/lib/defaults"
+	"github.com/gravitational/teleport/lib/devicetrust"
 	dtauthn "github.com/gravitational/teleport/lib/devicetrust/authn"
 	"github.com/gravitational/teleport/lib/events"
 	kubeutils "github.com/gravitational/teleport/lib/kube/utils"
@@ -3279,6 +3280,7 @@ func (tc *TeleportClient) AttemptDeviceLogin(ctx context.Context, key *Key) erro
 	if err != nil {
 		return trace.Wrap(err)
 	}
+
 	if !tc.dtAttemptLoginIgnorePing && pingResp.Auth.DeviceTrustDisabled {
 		log.Debug("Device Trust: skipping device authentication, device trust disabled")
 		return nil
@@ -3289,9 +3291,19 @@ func (tc *TeleportClient) AttemptDeviceLogin(ctx context.Context, key *Key) erro
 		// The TLS certificate is already part of the connection.
 		SshAuthorizedKey: key.Cert,
 	})
-	if err != nil {
+	switch {
+	case errors.Is(err, devicetrust.ErrDeviceKeyNotFound):
+		log.Debug("Device Trust: Skipping device authentication, device key not found")
+		return nil // err swallowed on purpose
+	case errors.Is(err, devicetrust.ErrPlatformNotSupported):
+		log.Debug("Device Trust: Skipping device authentication, platform not supported")
+		return nil // err swallowed on purpose
+	case trace.IsNotImplemented(err):
+		log.Debug("Device Trust: Skipping device authentication, not supported by server")
+		return nil // err swallowed on purpose
+	case err != nil:
 		log.WithError(err).Debug("Device Trust: device authentication failed")
-		return nil // Swallowed on purpose.
+		return nil // err swallowed on purpose
 	}
 
 	log.Debug("Device Trust: acquired augmented user certificates")

lib/devicetrust/authn/authn.go

@@ -20,6 +20,7 @@ import (
 	"github.com/gravitational/trace"
 
 	devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
+	"github.com/gravitational/teleport/lib/devicetrust"
 	"github.com/gravitational/teleport/lib/devicetrust/native"
 )
 
@@ -47,7 +48,7 @@ func RunCeremony(ctx context.Context, devicesClient devicepb.DeviceTrustServiceC
 
 	stream, err := devicesClient.AuthenticateDevice(ctx)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, trace.Wrap(devicetrust.HandleUnimplemented(err))
 	}
 
 	// 1. Init.
@@ -72,12 +73,13 @@ func RunCeremony(ctx context.Context, devicesClient devicepb.DeviceTrustServiceC
 			},
 		},
 	}); err != nil {
-		return nil, trace.Wrap(err)
+		return nil, trace.Wrap(devicetrust.HandleUnimplemented(err))
 	}
 	resp, err := stream.Recv()
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, trace.Wrap(devicetrust.HandleUnimplemented(err))
 	}
+	// Unimplemented errors are not expected to happen after this point.
 
 	// 2. Challenge.
 	chalResp := resp.GetChallenge()

lib/devicetrust/enroll/enroll.go

@@ -34,14 +34,6 @@ var (
 
 // RunCeremony performs the client-side device enrollment ceremony.
 func RunCeremony(ctx context.Context, devicesClient devicepb.DeviceTrustServiceClient, enrollToken string) (*devicepb.Device, error) {
-	dev, err := runCeremony(ctx, devicesClient, enrollToken)
-	if err != nil {
-		return nil, trace.Wrap(devicetrust.HandleUnimplemented(err))
-	}
-	return dev, err
-}
-
-func runCeremony(ctx context.Context, devicesClient devicepb.DeviceTrustServiceClient, enrollToken string) (*devicepb.Device, error) {
 	// Start by checking the OSType, this lets us exit early with a nicer message
 	// for non-supported OSes.
 	if getOSType() != devicepb.OSType_OS_TYPE_MACOS {
@@ -57,19 +49,20 @@ func runCeremony(ctx context.Context, devicesClient devicepb.DeviceTrustServiceC
 	// 1. Init.
 	stream, err := devicesClient.EnrollDevice(ctx)
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, trace.Wrap(devicetrust.HandleUnimplemented(err))
 	}
 	if err := stream.Send(&devicepb.EnrollDeviceRequest{
 		Payload: &devicepb.EnrollDeviceRequest_Init{
 			Init: init,
 		},
 	}); err != nil {
-		return nil, trace.Wrap(err)
+		return nil, trace.Wrap(devicetrust.HandleUnimplemented(err))
 	}
 	resp, err := stream.Recv()
 	if err != nil {
-		return nil, trace.Wrap(err)
+		return nil, trace.Wrap(devicetrust.HandleUnimplemented(err))
 	}
+	// Unimplemented errors are not expected to happen after this point.
 
 	// 2. Challenge.
 	// Only macOS is supported, see the guard at the beginning of the method.

lib/devicetrust/errors.go

@@ -16,20 +16,43 @@ package devicetrust
 
 import (
 	"errors"
+	"io"
 
+	"github.com/gravitational/trace"
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
+// ErrDeviceKeyNotFound is raised for missing device key during device
+// authentication.
+// May be raised in situations where the binary is missing entitlements, as
+// Sec/Keychain queries return empty in both cases.
+// If checking for equality always use [errors.Is], as other errors may
+// "impersonate" this error.
+var ErrDeviceKeyNotFound = errors.New("device key not found")
+
+// ErrPlatformNotSupported is raised for device operations attempted on
+// non-supported platforms.
+// trace.NotImplemented is purposefully avoided, as NotImplemented errors are
+// used to detect the lack of server-side device trust support.
+var ErrPlatformNotSupported = errors.New("platform not supported")
+
 // HandleUnimplemented turns remote unimplemented errors to a more user-friendly
 // error.
 func HandleUnimplemented(err error) error {
+	const notSupportedMsg = "device trust not supported by remote cluster"
+
+	if errors.Is(err, io.EOF) {
+		log.Debug("Device Trust: interpreting EOF as an older Teleport cluster")
+		return trace.NotImplemented(notSupportedMsg)
+	}
+
 	for e := err; e != nil; {
 		switch s, ok := status.FromError(e); {
 		case ok && s.Code() == codes.Unimplemented:
-			log.WithError(err).Debug("Device Trust: interpreting error as OSS or older Enterprise cluster")
-			return errors.New("device trust not supported by remote cluster")
+			log.WithError(err).Debug("Device Trust: interpreting gRPC Unimplemented as OSS or older Enterprise cluster")
+			return trace.NotImplemented(notSupportedMsg)
 		case ok:
 			return err // Unexpected status error.
 		default:

lib/devicetrust/native/others.go

@@ -17,27 +17,22 @@
 package native
 
 import (
-	"errors"
-
 	devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1"
+	"github.com/gravitational/teleport/lib/devicetrust"
 )
 
-// trace.NotImplemented avoided on purpose: we use NotImplemented errors to
-// detect the lack of a server-side Device Trust implementation.
-var errPlatformNotSupported = errors.New("platform not supported")
-
 func enrollDeviceInit() (*devicepb.EnrollDeviceInit, error) {
-	return nil, errPlatformNotSupported
+	return nil, devicetrust.ErrPlatformNotSupported
 }
 
 func collectDeviceData() (*devicepb.DeviceCollectedData, error) {
-	return nil, errPlatformNotSupported
+	return nil, devicetrust.ErrPlatformNotSupported
 }
 
 func signChallenge(chal []byte) (sig []byte, err error) {
-	return nil, errPlatformNotSupported
+	return nil, devicetrust.ErrPlatformNotSupported
 }
 
 func getDeviceCredential() (*devicepb.DeviceCredential, error) {
-	return nil, errPlatformNotSupported
+	return nil, devicetrust.ErrPlatformNotSupported
 }

lib/devicetrust/native/status_error.go

@@ -1,5 +1,3 @@
-//go:build darwin
-
 // Copyright 2022 Gravitational, Inc
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -16,7 +14,11 @@
 
 package native
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/gravitational/teleport/lib/devicetrust"
+)
 
 const (
 	// https://www.osstatus.com/search/results?framework=Security&search=-25300
@@ -45,3 +47,12 @@ func (e *statusError) Error() string {
 		return fmt.Sprintf("status %d", e.status)
 	}
 }
+
+func (e *statusError) Is(target error) bool {
+	if target == devicetrust.ErrDeviceKeyNotFound && e.status == errSecItemNotFound {
+		return true
+	}
+
+	other, ok := target.(*statusError)
+	return ok && other.status == e.status
+}

lib/devicetrust/native/status_error_test.go

@@ -0,0 +1,62 @@
+// Copyright 2023 Gravitational, Inc
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package native
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/gravitational/teleport/lib/devicetrust"
+)
+
+func TestStatusError_Is(t *testing.T) {
+	errNotFound := &statusError{status: errSecItemNotFound}
+	errMissingEntitlement := &statusError{status: errSecMissingEntitlement}
+	errOtherStatus := &statusError{status: -12345}
+
+	tests := []struct {
+		name   string
+		err    *statusError
+		target error
+		want   bool
+	}{
+		{
+			name:   "same statuses are equal",
+			err:    errOtherStatus,
+			target: &statusError{status: errOtherStatus.status}, // distinct instance
+			want:   true,
+		},
+		{
+			name:   "distinct statuses are not equal",
+			err:    errNotFound,
+			target: errMissingEntitlement,
+			want:   false,
+		},
+		{
+			name:   "errSecItemNotFound is the same as ErrDeviceKeyNotFound",
+			err:    errNotFound,
+			target: devicetrust.ErrDeviceKeyNotFound,
+			want:   true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := errors.Is(test.err, test.target)
+			if got != test.want {
+				t.Errorf("errors.Is() = %v, want %v", got, test.want)
+			}
+		})
+	}
+}