Skip to content
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.

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

[v15] fix: Prevent passkey downgrades #40409

Merged
merged 2 commits into from Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 10 additions & 5 deletions lib/auth/webauthn/register.go
Expand Up @@ -147,11 +147,16 @@ func (f *RegistrationFlow) Begin(ctx context.Context, user string, passwordless
if dev.GetU2F() != nil {
continue
}
// Skip resident/non-resident keys depending on whether it's a passwordless
// registration.
// Letting users have both allows them to "swap" between key types in the
// same device.
if webDev := dev.GetWebauthn(); webDev != nil && webDev.ResidentKey != passwordless {

// Let authenticator "upgrades" from non-resident (MFA) to resident
// (passwordless) happen, but prevent "downgrades" from resident to
// non-resident.
//
// Modern passkey implementations will "disobey" our MFA registrations and
// actually create passkeys, silently replacing the old passkey with the new
// "MFA" key, which can make Teleport confused (for example, by letting the
// "MFA" key be deleted because Teleport thinks the passkey still exists).
if webDev := dev.GetWebauthn(); webDev != nil && !webDev.ResidentKey && passwordless {
continue
}

Expand Down
35 changes: 20 additions & 15 deletions lib/auth/webauthn/register_test.go
Expand Up @@ -143,32 +143,32 @@ func TestRegistrationFlow_Begin_excludeList(t *testing.T) {
const user = "llama"
const rpID = "localhost"

dev1ID := []byte{1, 1, 1} // U2F
web1ID := []byte{1, 1, 2} // WebAuthn / MFA
rk1ID := []byte{1, 1, 3} // WebAuthn / passwordless
dev1 := &types.MFADevice{
u2fID := []byte{1, 1, 1} // U2F
mfaID := []byte{1, 1, 2} // WebAuthn / MFA
passkeyID := []byte{1, 1, 3} // WebAuthn / passwordless
u2fDev := &types.MFADevice{
Device: &types.MFADevice_U2F{
U2F: &types.U2FDevice{
KeyHandle: dev1ID,
KeyHandle: u2fID,
},
},
}
web1 := &types.MFADevice{
mfaDev := &types.MFADevice{
Device: &types.MFADevice_Webauthn{
Webauthn: &types.WebauthnDevice{
CredentialId: web1ID,
CredentialId: mfaID,
},
},
}
rk1 := &types.MFADevice{
passkeyDev := &types.MFADevice{
Device: &types.MFADevice_Webauthn{
Webauthn: &types.WebauthnDevice{
CredentialId: rk1ID,
CredentialId: passkeyID,
ResidentKey: true,
},
},
}
identity := newFakeIdentity(user, dev1, web1, rk1)
identity := newFakeIdentity(user, u2fDev, mfaDev, passkeyDev)

rf := wanlib.RegistrationFlow{
Webauthn: &types.Webauthn{
Expand All @@ -184,13 +184,18 @@ func TestRegistrationFlow_Begin_excludeList(t *testing.T) {
wantExcludeList [][]byte
}{
{
name: "MFA",
wantExcludeList: [][]byte{web1ID}, // U2F and resident excluded
name: "MFA",
wantExcludeList: [][]byte{
mfaID,
passkeyID, // Prevents "downgrades"
},
},
{
name: "passwordless",
passwordless: true,
wantExcludeList: [][]byte{rk1ID}, // U2F and MFA excluded
name: "passwordless",
passwordless: true,
wantExcludeList: [][]byte{
passkeyID,
},
},
}
for _, test := range tests {
Expand Down