Skip to content

Commit bb01dff

Browse files
codingllamacodingllama
authored
Unify device verification logic under lib/devicetrust/authz (#62523) (#63155)
* Unify device verification logic under lib/devicetrust/authz * Refactor verifyDeviceExtensions, nits * Disallow unknown modes * Reword comment
1 parent 5c1b9c0 commit bb01dff

File tree

3 files changed

+238
-33
lines changed

3 files changed

+238
-33
lines changed

‎lib/devicetrust/authz/authz.go‎

Lines changed: 86 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,24 @@ func IsTLSDeviceVerified(ext *tlsca.DeviceExtensions) bool {
4747

4848
// VerifyTLSUser verifies if the TLS identity has the required extensions to
4949
// fulfill the device trust configuration.
50-
func VerifyTLSUser(ctx context.Context, dt *types.DeviceTrust, identity tlsca.Identity) error {
51-
return verifyDeviceExtensions(ctx, dt, identity.Username, identity.IsBot(), IsTLSDeviceVerified(&identity.DeviceExtensions))
50+
func VerifyTLSUser(ctx context.Context, dt *types.DeviceTrust, id tlsca.Identity) error {
51+
return verifyDeviceExtensions(ctx,
52+
dt,
53+
id.Username,
54+
VerifyTrustedDeviceModeParams{
55+
IsTrustedDevice: IsTLSDeviceVerified(&id.DeviceExtensions),
56+
IsBot: id.IsBot(),
57+
})
5258
}
5359

5460
// IsSSHDeviceVerified returns true if cert contains all required device
5561
// extensions.
56-
func IsSSHDeviceVerified(ident *sshca.Identity) bool {
62+
func IsSSHDeviceVerified(id *sshca.Identity) bool {
5763
// Expect all device extensions to be present.
58-
return ident != nil &&
59-
ident.DeviceID != "" &&
60-
ident.DeviceAssetTag != "" &&
61-
ident.DeviceCredentialID != ""
64+
return id != nil &&
65+
id.DeviceID != "" &&
66+
id.DeviceAssetTag != "" &&
67+
id.DeviceCredentialID != ""
6268
}
6369

6470
// HasDeviceTrustExtensions returns true if the certificate's extension names
@@ -85,31 +91,92 @@ func HasDeviceTrustExtensions(extensions []string) bool {
8591

8692
// VerifySSHUser verifies if the SSH certificate has the required extensions to
8793
// fulfill the device trust configuration.
88-
func VerifySSHUser(ctx context.Context, dt *types.DeviceTrust, ident *sshca.Identity) error {
89-
if ident == nil {
94+
func VerifySSHUser(ctx context.Context, dt *types.DeviceTrust, id *sshca.Identity) error {
95+
if id == nil {
9096
return trace.BadParameter("ssh identity required")
9197
}
92-
return verifyDeviceExtensions(ctx, dt, ident.Username, ident.IsBot(), IsSSHDeviceVerified(ident))
98+
return verifyDeviceExtensions(ctx,
99+
dt,
100+
id.Username,
101+
VerifyTrustedDeviceModeParams{
102+
IsTrustedDevice: IsSSHDeviceVerified(id),
103+
IsBot: id.IsBot(),
104+
})
93105
}
94106

95-
func verifyDeviceExtensions(ctx context.Context, dt *types.DeviceTrust, username string, isBot bool, verified bool) error {
96-
mode := dtconfig.GetEnforcementMode(dt)
107+
func verifyDeviceExtensions(
108+
ctx context.Context,
109+
dt *types.DeviceTrust,
110+
username string,
111+
params VerifyTrustedDeviceModeParams,
112+
) error {
113+
enforcementMode := dtconfig.GetEnforcementMode(dt)
97114

98-
var pass bool
99-
switch mode {
115+
if err := VerifyTrustedDeviceMode(enforcementMode, params); err != nil {
116+
slog.DebugContext(ctx, "Device Trust: denied access for unidentified device", "user", username)
117+
return trace.Wrap(err)
118+
}
119+
120+
return nil
121+
}
122+
123+
// VerifyTrustedDeviceModeParams holds additional parameters for
124+
// [VerifyTrustedDeviceMode].
125+
type VerifyTrustedDeviceModeParams struct {
126+
// IsTrustedDevice informs if the device in use is trusted.
127+
IsTrustedDevice bool
128+
// IsBot informs if the user is a bot.
129+
IsBot bool
130+
// AllowEmptyMode allows an empty "enforcementMode", treating it similarly to
131+
// DeviceTrustModeOff.
132+
AllowEmptyMode bool
133+
}
134+
135+
// VerifyTrustedDeviceMode runs the fundamental device trust authorization
136+
// logic, checking an effective device trust mode against a set of access
137+
// params.
138+
//
139+
// Most callers should use a higher level function, such as [VerifyTLSUser] or
140+
// [VerifySSHUser].
141+
//
142+
// If enforcementMode comes from the global config it must be resolved via
143+
// [dtconfig.GetEnforcementMode] prior to calling the method.
144+
//
145+
// Returns an error, typically ErrTrustedDeviceRequired, if the checked device
146+
// is not allowed.
147+
func VerifyTrustedDeviceMode(
148+
enforcementMode constants.DeviceTrustMode,
149+
params VerifyTrustedDeviceModeParams,
150+
) error {
151+
if enforcementMode == "" && params.AllowEmptyMode {
152+
return nil // Equivalent to mode=off.
153+
}
154+
155+
// Assume required so it denies by default.
156+
required := true
157+
158+
// Switch on mode before any exemptions so we catch unknown modes.
159+
switch enforcementMode {
100160
case constants.DeviceTrustModeOff, constants.DeviceTrustModeOptional:
101161
// OK, extensions not enforced.
102-
pass = true
162+
required = false
163+
103164
case constants.DeviceTrustModeRequiredForHumans:
104165
// Humans must use trusted devices, bots can use untrusted devices.
105-
pass = verified || isBot
166+
required = !params.IsBot
167+
106168
case constants.DeviceTrustModeRequired:
107169
// Only trusted devices allowed for bot human and bot users.
108-
pass = verified
170+
171+
default:
172+
slog.WarnContext(context.Background(),
173+
"Unknown device trust mode, treating device as untrusted",
174+
"mode", enforcementMode,
175+
)
176+
return trace.Wrap(ErrTrustedDeviceRequired)
109177
}
110178

111-
if !pass {
112-
slog.DebugContext(ctx, "Device Trust: denied access for unidentified device", "user", username)
179+
if required && !params.IsTrustedDevice {
113180
return trace.Wrap(ErrTrustedDeviceRequired)
114181
}
115182

‎lib/devicetrust/authz/authz_test.go‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package authz_test
2020

2121
import (
2222
"context"
23+
"fmt"
2324
"testing"
2425

2526
"github.com/gravitational/trace"
@@ -278,3 +279,145 @@ func runVerifyUserTest(t *testing.T, method string, verify func(dt *types.Device
278279
})
279280
}
280281
}
282+
283+
func TestVerifyTrustedDeviceMode(t *testing.T) {
284+
t.Parallel()
285+
286+
type testCase struct {
287+
name string
288+
mode constants.DeviceTrustMode
289+
params authz.VerifyTrustedDeviceModeParams
290+
wantErr bool
291+
}
292+
293+
var tests []testCase
294+
295+
// Mode "off"/"optional" matrix. Always passes.
296+
for _, mode := range []constants.DeviceTrustMode{
297+
constants.DeviceTrustModeOff,
298+
constants.DeviceTrustModeOptional,
299+
} {
300+
for _, trusted := range []bool{false, true} {
301+
for _, bot := range []bool{false, true} {
302+
tests = append(tests, testCase{
303+
name: fmt.Sprintf("%s: trusted=%v bot=%v", mode, trusted, bot),
304+
mode: mode,
305+
params: authz.VerifyTrustedDeviceModeParams{
306+
IsTrustedDevice: trusted,
307+
IsBot: bot,
308+
},
309+
wantErr: false, // Allowed.
310+
})
311+
}
312+
}
313+
}
314+
315+
// Mode "required" matrix.
316+
tests = append(tests,
317+
testCase{
318+
name: "required: untrusted human",
319+
mode: constants.DeviceTrustModeRequired,
320+
wantErr: true,
321+
},
322+
testCase{
323+
name: "required: untrusted bot",
324+
mode: constants.DeviceTrustModeRequired,
325+
params: authz.VerifyTrustedDeviceModeParams{
326+
IsBot: true,
327+
},
328+
wantErr: true,
329+
},
330+
testCase{
331+
name: "required: trusted human",
332+
mode: constants.DeviceTrustModeRequired,
333+
params: authz.VerifyTrustedDeviceModeParams{
334+
IsTrustedDevice: true,
335+
},
336+
},
337+
testCase{
338+
name: "required: trusted bot",
339+
mode: constants.DeviceTrustModeRequired,
340+
params: authz.VerifyTrustedDeviceModeParams{
341+
IsTrustedDevice: true,
342+
IsBot: true,
343+
},
344+
},
345+
)
346+
347+
// Mode "required-for-humans" matrix.
348+
tests = append(tests,
349+
testCase{
350+
name: "required-for-humans: untrusted human",
351+
mode: constants.DeviceTrustModeRequiredForHumans,
352+
wantErr: true,
353+
},
354+
testCase{
355+
name: "required-for-humans: untrusted bot",
356+
mode: constants.DeviceTrustModeRequiredForHumans,
357+
params: authz.VerifyTrustedDeviceModeParams{
358+
IsBot: true,
359+
},
360+
wantErr: false, // Allowed because bot.
361+
},
362+
testCase{
363+
name: "required-for-humans: trusted human",
364+
mode: constants.DeviceTrustModeRequiredForHumans,
365+
params: authz.VerifyTrustedDeviceModeParams{
366+
IsTrustedDevice: true,
367+
},
368+
},
369+
testCase{
370+
name: "required-for-humans: trusted bot",
371+
mode: constants.DeviceTrustModeRequiredForHumans,
372+
params: authz.VerifyTrustedDeviceModeParams{
373+
IsTrustedDevice: true,
374+
IsBot: true,
375+
},
376+
},
377+
)
378+
379+
// mode="".
380+
tests = append(tests,
381+
testCase{
382+
name: "empty mode: AllowEmptyMode=false",
383+
wantErr: true, // Unknown mode always errors.
384+
},
385+
testCase{
386+
name: "empty mode: AllowEmptyMode=true",
387+
params: authz.VerifyTrustedDeviceModeParams{
388+
AllowEmptyMode: true,
389+
},
390+
wantErr: false, // Treated as mode="off".
391+
},
392+
)
393+
394+
// Unknown modes.
395+
tests = append(tests,
396+
testCase{
397+
name: "unknown mode: untrusted human",
398+
mode: "llama",
399+
wantErr: true, // Unknown mode always errors.
400+
},
401+
testCase{
402+
name: "unknown mode: trusted human",
403+
mode: "llama",
404+
params: authz.VerifyTrustedDeviceModeParams{
405+
IsTrustedDevice: true,
406+
},
407+
wantErr: true, // Unknown mode always errors.
408+
},
409+
)
410+
411+
for _, test := range tests {
412+
t.Run(test.name, func(t *testing.T) {
413+
t.Parallel()
414+
415+
got := authz.VerifyTrustedDeviceMode(test.mode, test.params)
416+
if test.wantErr {
417+
assert.ErrorIs(t, got, authz.ErrTrustedDeviceRequired)
418+
return
419+
}
420+
assert.NoError(t, got)
421+
})
422+
}
423+
}

‎lib/services/role.go‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2777,23 +2777,18 @@ func (set RoleSet) checkAccess(r AccessCheckable, traits wrappers.Traits, state
27772777
}
27782778

27792779
// Device verification.
2780-
var deviceVerificationPassed bool
2781-
switch role.GetOptions().DeviceTrustMode {
2782-
case constants.DeviceTrustModeOff, constants.DeviceTrustModeOptional, "":
2783-
// OK, extensions not enforced.
2784-
deviceVerificationPassed = true
2785-
case constants.DeviceTrustModeRequiredForHumans:
2786-
// Humans must use trusted devices, bots can use untrusted devices.
2787-
deviceVerificationPassed = deviceTrusted || state.IsBot
2788-
case constants.DeviceTrustModeRequired:
2789-
// Only trusted devices allowed for bot human and bot users.
2790-
deviceVerificationPassed = deviceTrusted
2791-
}
2792-
if !deviceVerificationPassed {
2780+
if err := dtauthz.VerifyTrustedDeviceMode(
2781+
role.GetOptions().DeviceTrustMode,
2782+
dtauthz.VerifyTrustedDeviceModeParams{
2783+
IsTrustedDevice: deviceTrusted,
2784+
IsBot: state.IsBot,
2785+
AllowEmptyMode: true, // Empty mode on roles is equivalent to "off".
2786+
},
2787+
); err != nil {
27932788
logger.LogAttrs(ctx, logutils.TraceLevel, "Access to resource denied, role requires a trusted device",
27942789
slog.String("role", role.GetName()),
27952790
)
2796-
return ErrTrustedDeviceRequired
2791+
return trace.Wrap(err)
27972792
}
27982793

27992794
// Current role allows access, but keep looking for a more restrictive

0 commit comments

Comments
 (0)