diff --git a/api/utils/keys/policy.go b/api/utils/keys/policy.go index b109e4f85a166..70585666700a1 100644 --- a/api/utils/keys/policy.go +++ b/api/utils/keys/policy.go @@ -116,6 +116,10 @@ func (p PrivateKeyPolicy) IsHardwareKeyPolicy() bool { // MFAVerified checks that private keys with this key policy count as MFA verified. // Both Hardware key touch and pin are count as MFA verification. +// +// Note: MFA checks with private key policies are only performed during the establishment +// of the connection, during the TLS/SSH handshake. For long term connections, MFA should +// be re-verified through other methods (e.g. webauthn). func (p PrivateKeyPolicy) MFAVerified() bool { return p.isHardwareKeyTouchVerified() || p.isHardwareKeyPINVerified() } diff --git a/lib/authz/permissions.go b/lib/authz/permissions.go index e5c86a6b712c7..6957442ac5ceb 100644 --- a/lib/authz/permissions.go +++ b/lib/authz/permissions.go @@ -490,12 +490,6 @@ func (a *authorizer) isAdminActionAuthorizationRequired(ctx context.Context, aut } func (a *authorizer) authorizeAdminAction(ctx context.Context, authContext *Context) error { - // Certain hardware-key based private key policies require MFA for each request. - if authContext.Identity.GetIdentity().PrivateKeyPolicy.MFAVerified() { - authContext.AdminActionAuthState = AdminActionAuthMFAVerified - return nil - } - // MFA is required to be passed through the request context. mfaResp, err := mfa.CredentialsFromContext(ctx) if err != nil { diff --git a/lib/authz/permissions_test.go b/lib/authz/permissions_test.go index 890ce20489101..37a796507ec24 100644 --- a/lib/authz/permissions_test.go +++ b/lib/authz/permissions_test.go @@ -561,6 +561,16 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { }, }, wantAdminActionAuthorized: false, + }, { + name: "NOK local user mfa verified private key policy", + user: LocalUser{ + Username: localUser.GetName(), + Identity: tlsca.Identity{ + Username: localUser.GetName(), + PrivateKeyPolicy: keys.PrivateKeyPolicyHardwareKeyTouch, + }, + }, + wantAdminActionAuthorized: false, }, { // edge case for the admin role check. name: "NOK local user with host-like username", @@ -613,16 +623,6 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) { withMFA: validMFAWithReuse, allowedReusedMFA: true, wantAdminActionAuthorized: true, - }, { - name: "OK local user mfa verified private key policy", - user: LocalUser{ - Username: localUser.GetName(), - Identity: tlsca.Identity{ - Username: localUser.GetName(), - PrivateKeyPolicy: keys.PrivateKeyPolicyHardwareKeyTouch, - }, - }, - wantAdminActionAuthorized: true, }, { name: "OK admin", user: BuiltinRole{ diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index a0a1acb0f2712..6f0b730780e2d 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -1119,7 +1119,10 @@ func (id Identity) GetSessionMetadata(sid string) events.SessionMetadata { } } -// IsMFAVerified returns whether this identity is MFA verified. +// IsMFAVerified returns whether this identity is MFA verified. This MFA +// verification may or may not have taken place recently, so it should not +// be treated as blanket MFA verification uncritically. For example, MFA +// should be re-verified for login procedures or admin actions. func (id *Identity) IsMFAVerified() bool { return id.MFAVerified != "" || id.PrivateKeyPolicy.MFAVerified() }