Skip to content

Commit

Permalink
[v15] fix: Prevent passkey downgrades (#40409)
Browse files Browse the repository at this point in the history
* fix: Prevent passkey downgrades

* Clarify comments
  • Loading branch information
codingllama committed Apr 10, 2024
1 parent 3b827b5 commit c21ca6d
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 20 deletions.
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

0 comments on commit c21ca6d

Please sign in to comment.