Unify device verification logic under lib/devicetrust/authz

lib/devicetrust/authz/authz.go

@@ -47,18 +47,24 @@ func IsTLSDeviceVerified(ext *tlsca.DeviceExtensions) bool {
 
 // VerifyTLSUser verifies if the TLS identity has the required extensions to
 // fulfill the device trust configuration.
-func VerifyTLSUser(ctx context.Context, dt *types.DeviceTrust, identity tlsca.Identity) error {
-	return verifyDeviceExtensions(ctx, dt, identity.Username, identity.IsBot(), IsTLSDeviceVerified(&identity.DeviceExtensions))
+func VerifyTLSUser(ctx context.Context, dt *types.DeviceTrust, id tlsca.Identity) error {
+	return verifyDeviceExtensions(ctx,
+		dt,
+		id.Username,
+		VerifyTrustedDeviceModeParams{
+			IsTrustedDevice: IsTLSDeviceVerified(&id.DeviceExtensions),
+			IsBot:           id.IsBot(),
+		})
 }
 
 // IsSSHDeviceVerified returns true if cert contains all required device
 // extensions.
-func IsSSHDeviceVerified(ident *sshca.Identity) bool {
+func IsSSHDeviceVerified(id *sshca.Identity) bool {
 	// Expect all device extensions to be present.
-	return ident != nil &&
-		ident.DeviceID != "" &&
-		ident.DeviceAssetTag != "" &&
-		ident.DeviceCredentialID != ""
+	return id != nil &&
+		id.DeviceID != "" &&
+		id.DeviceAssetTag != "" &&
+		id.DeviceCredentialID != ""
 }
 
 // HasDeviceTrustExtensions returns true if the certificate's extension names
@@ -85,31 +91,92 @@ func HasDeviceTrustExtensions(extensions []string) bool {
 
 // VerifySSHUser verifies if the SSH certificate has the required extensions to
 // fulfill the device trust configuration.
-func VerifySSHUser(ctx context.Context, dt *types.DeviceTrust, ident *sshca.Identity) error {
-	if ident == nil {
+func VerifySSHUser(ctx context.Context, dt *types.DeviceTrust, id *sshca.Identity) error {
+	if id == nil {
 		return trace.BadParameter("ssh identity required")
 	}
-	return verifyDeviceExtensions(ctx, dt, ident.Username, ident.IsBot(), IsSSHDeviceVerified(ident))
+	return verifyDeviceExtensions(ctx,
+		dt,
+		id.Username,
+		VerifyTrustedDeviceModeParams{
+			IsTrustedDevice: IsSSHDeviceVerified(id),
+			IsBot:           id.IsBot(),
+		})
 }
 
-func verifyDeviceExtensions(ctx context.Context, dt *types.DeviceTrust, username string, isBot bool, verified bool) error {
-	mode := dtconfig.GetEnforcementMode(dt)
+func verifyDeviceExtensions(
+	ctx context.Context,
+	dt *types.DeviceTrust,
+	username string,
+	params VerifyTrustedDeviceModeParams,
+) error {
+	enforcementMode := dtconfig.GetEnforcementMode(dt)
 
-	var pass bool
-	switch mode {
+	if err := VerifyTrustedDeviceMode(enforcementMode, params); err != nil {
+		slog.DebugContext(ctx, "Device Trust: denied access for unidentified device", "user", username)
+		return trace.Wrap(err)
+	}
+
+	return nil
+}
+
+// VerifyTrustedDeviceModeParams holds additional parameters for
+// [VerifyTrustedDeviceMode].
+type VerifyTrustedDeviceModeParams struct {
+	// IsTrustedDevice informs if the device in use is trusted.
+	IsTrustedDevice bool
+	// IsBot informs if the user is a bot.
+	IsBot bool
+	// AllowEmptyMode allows an empty "enforcementMode", treating it similarly to
+	// DeviceTrustModeOff.
+	AllowEmptyMode bool
+}
+
+// VerifyTrustedDeviceMode runs the fundamental device trust authorization
+// logic, checking an effective device trust mode against a set of access
+// params.
+//
+// Most callers should use a higher level function, such as [VerifyTLSUser] or
+// [VerifySSHUser].
+//
+// If enforcementMode comes from the global config it must be resolved via
+// [dtconfig.GetEnforcementMode] prior to calling the method.
+//
+// Returns an error, typically ErrTrustedDeviceRequired, if the checked device
+// is not allowed.
+func VerifyTrustedDeviceMode(
+	enforcementMode constants.DeviceTrustMode,
+	params VerifyTrustedDeviceModeParams,
+) error {
+	if enforcementMode == "" && params.AllowEmptyMode {
+		return nil // Equivalent to mode=off.
+	}
+
+	// Assume required so it denies by default.
+	required := true
+
+	// Switch on mode before any exemptions so we catch unknown modes.
+	switch enforcementMode {
 	case constants.DeviceTrustModeOff, constants.DeviceTrustModeOptional:
 		// OK, extensions not enforced.
-		pass = true
+		required = false
+
 	case constants.DeviceTrustModeRequiredForHumans:
 		// Humans must use trusted devices, bots can use untrusted devices.
-		pass = verified || isBot
+		required = !params.IsBot
+
 	case constants.DeviceTrustModeRequired:
 		// Only trusted devices allowed for bot human and bot users.
-		pass = verified
+
+	default:
+		slog.WarnContext(context.Background(),
+			"Unknown device trust mode, treating device as untrusted",
+			"mode", enforcementMode,
+		)
+		return trace.Wrap(ErrTrustedDeviceRequired)
 	}
 
-	if !pass {
-		slog.DebugContext(ctx, "Device Trust: denied access for unidentified device", "user", username)
+	if required && !params.IsTrustedDevice {
 		return trace.Wrap(ErrTrustedDeviceRequired)
 	}
 

lib/devicetrust/authz/authz_test.go

@@ -20,6 +20,7 @@ package authz_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/gravitational/trace"
@@ -277,3 +278,145 @@ func runVerifyUserTest(t *testing.T, method string, verify func(dt *types.Device
 		})
 	}
 }
+
+func TestVerifyTrustedDeviceMode(t *testing.T) {
+	t.Parallel()
+
+	type testCase struct {
+		name    string
+		mode    constants.DeviceTrustMode
+		params  authz.VerifyTrustedDeviceModeParams
+		wantErr bool
+	}
+
+	var tests []testCase
+
+	// Mode "off"/"optional" matrix. Always passes.
+	for _, mode := range []constants.DeviceTrustMode{
+		constants.DeviceTrustModeOff,
+		constants.DeviceTrustModeOptional,
+	} {
+		for _, trusted := range []bool{false, true} {
+			for _, bot := range []bool{false, true} {
+				tests = append(tests, testCase{
+					name: fmt.Sprintf("%s: trusted=%v bot=%v", mode, trusted, bot),
+					mode: mode,
+					params: authz.VerifyTrustedDeviceModeParams{
+						IsTrustedDevice: trusted,
+						IsBot:           bot,
+					},
+					wantErr: false, // Allowed.
+				})
+			}
+		}
+	}
+
+	// Mode "required" matrix.
+	tests = append(tests,
+		testCase{
+			name:    "required: untrusted human",
+			mode:    constants.DeviceTrustModeRequired,
+			wantErr: true,
+		},
+		testCase{
+			name: "required: untrusted bot",
+			mode: constants.DeviceTrustModeRequired,
+			params: authz.VerifyTrustedDeviceModeParams{
+				IsBot: true,
+			},
+			wantErr: true,
+		},
+		testCase{
+			name: "required: trusted human",
+			mode: constants.DeviceTrustModeRequired,
+			params: authz.VerifyTrustedDeviceModeParams{
+				IsTrustedDevice: true,
+			},
+		},
+		testCase{
+			name: "required: trusted bot",
+			mode: constants.DeviceTrustModeRequired,
+			params: authz.VerifyTrustedDeviceModeParams{
+				IsTrustedDevice: true,
+				IsBot:           true,
+			},
+		},
+	)
+
+	// Mode "required-for-humans" matrix.
+	tests = append(tests,
+		testCase{
+			name:    "required-for-humans: untrusted human",
+			mode:    constants.DeviceTrustModeRequiredForHumans,
+			wantErr: true,
+		},
+		testCase{
+			name: "required-for-humans: untrusted bot",
+			mode: constants.DeviceTrustModeRequiredForHumans,
+			params: authz.VerifyTrustedDeviceModeParams{
+				IsBot: true,
+			},
+			wantErr: false, // Allowed because bot.
+		},
+		testCase{
+			name: "required-for-humans: trusted human",
+			mode: constants.DeviceTrustModeRequiredForHumans,
+			params: authz.VerifyTrustedDeviceModeParams{
+				IsTrustedDevice: true,
+			},
+		},
+		testCase{
+			name: "required-for-humans: trusted bot",
+			mode: constants.DeviceTrustModeRequiredForHumans,
+			params: authz.VerifyTrustedDeviceModeParams{
+				IsTrustedDevice: true,
+				IsBot:           true,
+			},
+		},
+	)
+
+	// mode="".
+	tests = append(tests,
+		testCase{
+			name:    "empty mode: AllowEmptyMode=false",
+			wantErr: true, // Unknown mode always errors.
+		},
+		testCase{
+			name: "empty mode: AllowEmptyMode=true",
+			params: authz.VerifyTrustedDeviceModeParams{
+				AllowEmptyMode: true,
+			},
+			wantErr: false, // Treated as mode="off".
+		},
+	)
+
+	// Unknown modes.
+	tests = append(tests,
+		testCase{
+			name:    "unknown mode: untrusted human",
+			mode:    "llama",
+			wantErr: true, // Unknown mode always errors.
+		},
+		testCase{
+			name: "unknown mode: trusted human",
+			mode: "llama",
+			params: authz.VerifyTrustedDeviceModeParams{
+				IsTrustedDevice: true,
+			},
+			wantErr: true, // Unknown mode always errors.
+		},
+	)
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := authz.VerifyTrustedDeviceMode(test.mode, test.params)
+			if test.wantErr {
+				assert.ErrorIs(t, got, authz.ErrTrustedDeviceRequired)
+				return
+			}
+			assert.NoError(t, got)
+		})
+	}
+}

lib/services/role.go

@@ -2770,23 +2770,18 @@ func (set RoleSet) checkAccess(r AccessCheckable, traits wrappers.Traits, state
 		}
 
 		// Device verification.
-		var deviceVerificationPassed bool
-		switch role.GetOptions().DeviceTrustMode {
-		case constants.DeviceTrustModeOff, constants.DeviceTrustModeOptional, "":
-			// OK, extensions not enforced.
-			deviceVerificationPassed = true
-		case constants.DeviceTrustModeRequiredForHumans:
-			// Humans must use trusted devices, bots can use untrusted devices.
-			deviceVerificationPassed = deviceTrusted || state.IsBot
-		case constants.DeviceTrustModeRequired:
-			// Only trusted devices allowed for bot human and bot users.
-			deviceVerificationPassed = deviceTrusted
-		}
-		if !deviceVerificationPassed {
+		if err := dtauthz.VerifyTrustedDeviceMode(
+			role.GetOptions().DeviceTrustMode,
+			dtauthz.VerifyTrustedDeviceModeParams{
+				IsTrustedDevice: deviceTrusted,
+				IsBot:           state.IsBot,
+				AllowEmptyMode:  true, // Empty mode on roles is equivalent to "off".
+			},
+		); err != nil {
 			logger.LogAttrs(ctx, logutils.TraceLevel, "Access to resource denied, role requires a trusted device",
 				slog.String("role", role.GetName()),
 			)
-			return ErrTrustedDeviceRequired
+			return trace.Wrap(err)
 		}
 
 		// Current role allows access, but keep looking for a more restrictive