Skip to content

fix: Correctly handle passwordless in MFA audit events #40607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
codingllama merged 3 commits into from
Apr 17, 2024
Merged

fix: Correctly handle passwordless in MFA audit events #40607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7df35fe
Record passwordless username in the mfa_auth_challenge.validate event
codingllama Apr 16, 2024
83bbcca
Correctly render mfa_auth_challenge.create events without an user
codingllama Apr 16, 2024
3ada1a8
Update snapshots
codingllama Apr 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 60 additions & 33 deletions lib/auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6299,7 +6299,66 @@ func (a *Server) validateMFAAuthResponseForRegister(ctx context.Context, resp *p
// required challenge extensions will be checked against the stored challenge when
// applicable (webauthn only). Returns the authentication data derived from the solved
// challenge.
func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAuthenticateResponse, user string, requiredExtensions *mfav1.ChallengeExtensions) (mfaAuthData *authz.MFAAuthData, err error) {
func (a *Server) ValidateMFAAuthResponse(
ctx context.Context,
resp *proto.MFAAuthenticateResponse,
user string,
requiredExtensions *mfav1.ChallengeExtensions,
) (*authz.MFAAuthData, error) {

authData, validateErr := a.validateMFAAuthResponseInternal(ctx, resp, user, requiredExtensions)
// validateErr handled after audit.

// Read ClusterName for audit.
var clusterName string
if cn, err := a.GetClusterName(); err != nil {
log.WithError(err).Warn("Failed to read cluster name")
// err swallowed on purpose.
} else {
clusterName = cn.GetClusterName()
}

// Take the user from the authData if the user param is empty.
Copy link
Copy Markdown
Contributor Author

@codingllama codingllama Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code here is mainly a move from below, minus the user == "" logic you see here.

Copy link
Copy Markdown
Contributor Author

@codingllama codingllama Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted for an "internal" func so we avoid shenanigans with defers reading named return variables, which can be harder to maintain over time.

// This happens for passwordless.
if user == "" && authData != nil {
user = authData.User
}

// Emit audit event.
auditEvent := &apievents.ValidateMFAAuthResponse{
Metadata: apievents.Metadata{
Type: events.ValidateMFAAuthResponseEvent,
ClusterName: clusterName,
},
UserMetadata: authz.ClientUserMetadataWithUser(ctx, user),
ChallengeScope: requiredExtensions.Scope.String(),
}
if validateErr != nil {
auditEvent.Code = events.ValidateMFAAuthResponseFailureCode
auditEvent.Success = false
auditEvent.UserMessage = validateErr.Error()
auditEvent.Error = validateErr.Error()
} else {
auditEvent.Code = events.ValidateMFAAuthResponseCode
auditEvent.Success = true
deviceMetadata := mfaDeviceEventMetadata(authData.Device)
auditEvent.MFADevice = &deviceMetadata
auditEvent.ChallengeAllowReuse = authData.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES
}
if err := a.emitter.EmitAuditEvent(ctx, auditEvent); err != nil {
log.WithError(err).Warn("Failed to emit ValidateMFAAuthResponse event")
// err swallowed on purpose.
}

return authData, trace.Wrap(validateErr)
}

func (a *Server) validateMFAAuthResponseInternal(
ctx context.Context,
resp *proto.MFAAuthenticateResponse,
user string,
requiredExtensions *mfav1.ChallengeExtensions,
) (*authz.MFAAuthData, error) {
if requiredExtensions == nil {
return nil, trace.BadParameter("required challenge extensions parameter required")
}
Expand All @@ -6311,38 +6370,6 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut
return nil, trace.BadParameter("user required")
}

clusterName, err := a.GetClusterName()
if err != nil {
return nil, trace.Wrap(err)
}

defer func() {
auditEvent := &apievents.ValidateMFAAuthResponse{
Metadata: apievents.Metadata{
Type: events.ValidateMFAAuthResponseEvent,
ClusterName: clusterName.GetClusterName(),
},
UserMetadata: authz.ClientUserMetadataWithUser(ctx, user),
ChallengeScope: requiredExtensions.Scope.String(),
}
if err != nil {
auditEvent.Code = events.ValidateMFAAuthResponseFailureCode
auditEvent.Success = false
auditEvent.UserMessage = err.Error()
auditEvent.Error = err.Error()
} else {
auditEvent.Code = events.ValidateMFAAuthResponseCode
auditEvent.Status.Success = true
deviceMetadata := mfaDeviceEventMetadata(mfaAuthData.Device)
auditEvent.MFADevice = &deviceMetadata
auditEvent.ChallengeAllowReuse = mfaAuthData.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES
}

if err := a.emitter.EmitAuditEvent(ctx, auditEvent); err != nil {
log.WithError(err).Warn("Failed to emit ValidateMFAAuthResponse event.")
}
}()

switch res := resp.Response.(type) {
// cases in order of preference
case *proto.MFAAuthenticateResponse_Webauthn:
Expand Down
112 changes: 110 additions & 2 deletions web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,12 +406,12 @@ exports[`list of all events 1`] = `
</strong>
-
<strong>
237
239
</strong>
of

<strong>
237
239
</strong>
</div>
<button
Expand Down Expand Up @@ -537,6 +537,114 @@ exports[`list of all events 1`] = `
</tr>
</thead>
<tbody>
<tr>
<td
style="vertical-align: inherit;"
>
<div
class="c18"
>
<span
class="c19 icon icon-info"
>
<svg
fill="currentColor"
height="20"
viewBox="0 0 24 24"
width="20"
>
<path
d="M11.625 8.8125C12.1428 8.8125 12.5625 8.39277 12.5625 7.875C12.5625 7.35723 12.1428 6.9375 11.625 6.9375C11.1072 6.9375 10.6875 7.35723 10.6875 7.875C10.6875 8.39277 11.1072 8.8125 11.625 8.8125Z"
/>
<path
d="M10.5 11.25C10.5 10.8358 10.8358 10.5 11.25 10.5C11.6478 10.5 12.0294 10.658 12.3107 10.9393C12.592 11.2206 12.75 11.6022 12.75 12V15.75C13.1642 15.75 13.5 16.0858 13.5 16.5C13.5 16.9142 13.1642 17.25 12.75 17.25C12.3522 17.25 11.9706 17.092 11.6893 16.8107C11.408 16.5294 11.25 16.1478 11.25 15.75V12C10.8358 12 10.5 11.6642 10.5 11.25Z"
/>
<path
clip-rule="evenodd"
d="M12 2.25C6.61522 2.25 2.25 6.61522 2.25 12C2.25 17.3848 6.61522 21.75 12 21.75C17.3848 21.75 21.75 17.3848 21.75 12C21.75 6.61522 17.3848 2.25 12 2.25ZM3.75 12C3.75 7.44365 7.44365 3.75 12 3.75C16.5563 3.75 20.25 7.44365 20.25 12C20.25 16.5563 16.5563 20.25 12 20.25C7.44365 20.25 3.75 16.5563 3.75 12Z"
fill-rule="evenodd"
/>
</svg>
</span>
MFA Authentication Attempt
</div>
</td>
<td
style="word-break: break-word;"
>
Passwordless user requested an MFA authentication challenge
</td>
<td
style="min-width: 120px;"
>
2024-04-16T21:47:20.69Z
</td>
<td
align="right"
>
<button
class="c20"
kind="border"
width="87px"
>
Details
</button>
</td>
</tr>
<tr>
<td
style="vertical-align: inherit;"
>
<div
class="c18"
>
<span
class="c19 icon icon-info"
>
<svg
fill="currentColor"
height="20"
viewBox="0 0 24 24"
width="20"
>
<path
d="M11.625 8.8125C12.1428 8.8125 12.5625 8.39277 12.5625 7.875C12.5625 7.35723 12.1428 6.9375 11.625 6.9375C11.1072 6.9375 10.6875 7.35723 10.6875 7.875C10.6875 8.39277 11.1072 8.8125 11.625 8.8125Z"
/>
<path
d="M10.5 11.25C10.5 10.8358 10.8358 10.5 11.25 10.5C11.6478 10.5 12.0294 10.658 12.3107 10.9393C12.592 11.2206 12.75 11.6022 12.75 12V15.75C13.1642 15.75 13.5 16.0858 13.5 16.5C13.5 16.9142 13.1642 17.25 12.75 17.25C12.3522 17.25 11.9706 17.092 11.6893 16.8107C11.408 16.5294 11.25 16.1478 11.25 15.75V12C10.8358 12 10.5 11.6642 10.5 11.25Z"
/>
<path
clip-rule="evenodd"
d="M12 2.25C6.61522 2.25 2.25 6.61522 2.25 12C2.25 17.3848 6.61522 21.75 12 21.75C17.3848 21.75 21.75 17.3848 21.75 12C21.75 6.61522 17.3848 2.25 12 2.25ZM3.75 12C3.75 7.44365 7.44365 3.75 12 3.75C16.5563 3.75 20.25 7.44365 20.25 12C20.25 16.5563 16.5563 20.25 12 20.25C7.44365 20.25 3.75 16.5563 3.75 12Z"
fill-rule="evenodd"
/>
</svg>
</span>
MFA Authentication Attempt
</div>
</td>
<td
style="word-break: break-word;"
>
User [llama] requested an MFA authentication challenge
</td>
<td
style="min-width: 120px;"
>
2024-04-16T21:46:59.317Z
</td>
<td
align="right"
>
<button
class="c20"
kind="border"
width="87px"
>
Details
</button>
</td>
</tr>
<tr>
<td
style="vertical-align: inherit;"
Expand Down
24 changes: 24 additions & 0 deletions web/packages/teleport/src/Audit/fixtures/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3459,6 +3459,30 @@ export const events = [
user: 'bot-test12',
user_kind: 2,
},
{
challenge_allow_reuse: false,
challenge_scope: 'CHALLENGE_SCOPE_LOGIN',
cluster_name: 'zarq',
code: 'T1015I',
ei: 0,
event: 'mfa_auth_challenge.create',
time: '2024-04-16T21:46:59.317Z',
uid: '815bbcf4-fb05-4e08-917c-7259e9332d69',
user: 'llama',
user_kind: 1,
},
{
challenge_allow_reuse: false,
challenge_scope: 'CHALLENGE_SCOPE_PASSWORDLESS_LOGIN',
cluster_name: 'zarq',
code: 'T1015I',
ei: 0,
event: 'mfa_auth_challenge.create',
time: '2024-04-16T21:47:20.69Z',
uid: 'e4946d15-3a6f-4f0a-b456-1704a7586572',
// user is missing for passwordless challenges.
user_kind: 1,
},
].map(makeEvent);

// Do not add new events to this array, add it to `events` list.
Expand Down
8 changes: 6 additions & 2 deletions web/packages/teleport/src/services/audit/makeEvent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,12 @@ export const formatters: Formatters = {
[eventCodes.CREATE_MFA_AUTH_CHALLENGE]: {
type: 'mfa_auth_challenge.create',
desc: 'MFA Authentication Attempt',
format: ({ user }) =>
`User [${user}] requested an MFA authentication challenge`,
format: ({ user }) => {
if (user) {
return `User [${user}] requested an MFA authentication challenge`;
}
return `Passwordless user requested an MFA authentication challenge`;
},
},
[eventCodes.VALIDATE_MFA_AUTH_RESPONSE]: {
type: 'mfa_auth_challenge.validate',
Expand Down