From e81af03d6378bab474735db1bc98c5408a8057f3 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Tue, 5 Nov 2024 10:58:48 -0300 Subject: [PATCH] fix: Assert credentials individually on U2F devices (#45289) (#48402) * Simulate "internal error" on multiple credentials * fix: Assert credentials individually on U2F devices * Use bytes.Repeat * Comment on U2F and libfido2.ErrUserPresenceRequired * Move errorOnUnknownCredential failure after the "tap" --- lib/auth/webauthncli/fido2.go | 71 +++++++++++++++++-- lib/auth/webauthncli/fido2_test.go | 110 +++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 4 deletions(-) diff --git a/lib/auth/webauthncli/fido2.go b/lib/auth/webauthncli/fido2.go index d3030ca211529..a20b714fc2fcb 100644 --- a/lib/auth/webauthncli/fido2.go +++ b/lib/auth/webauthncli/fido2.go @@ -25,6 +25,7 @@ import ( "context" "crypto/sha256" "encoding/base64" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -220,7 +221,7 @@ func fido2Login( if uv { opts.UV = libfido2.True } - assertions, err := dev.Assertion(actualRPID, ccdHash[:], allowedCreds, pin, opts) + assertions, err := devAssertion(dev, info, actualRPID, ccdHash[:], allowedCreds, pin, opts) if errors.Is(err, libfido2.ErrUnsupportedOption) && uv && pin != "" { // Try again if we are getting "unsupported option" and the PIN is set. // Happens inconsistently in some authenticator series (YubiKey 5). @@ -228,7 +229,7 @@ func fido2Login( // authenticator will set the UV bit regardless of it being requested. log.Debugf("FIDO2: Device %v: retrying assertion without UV", info.path) opts.UV = libfido2.Default - assertions, err = dev.Assertion(actualRPID, ccdHash[:], allowedCreds, pin, opts) + assertions, err = devAssertion(dev, info, actualRPID, ccdHash[:], allowedCreds, pin, opts) } if errors.Is(err, libfido2.ErrNoCredentials) { // U2F devices error instantly with ErrNoCredentials. @@ -312,13 +313,75 @@ func usesAppID(dev FIDODevice, info *deviceInfo, ccdHash []byte, allowedCreds [] isRegistered := func(id string) bool { const pin = "" // Not necessary here. - _, err := dev.Assertion(id, ccdHash, allowedCreds, pin, opts) + _, err := devAssertion(dev, info, id, ccdHash, allowedCreds, pin, opts) return err == nil || (!info.fido2 && errors.Is(err, libfido2.ErrUserPresenceRequired)) } return isRegistered(appID) && !isRegistered(rpID) } +func devAssertion( + dev FIDODevice, + info *deviceInfo, + rpID string, + ccdHash []byte, + allowedCreds [][]byte, + pin string, + opts *libfido2.AssertionOpts, +) ([]*libfido2.Assertion, error) { + // Handle U2F devices separately when there is more than one allowed + // credential. + // This avoids "internal errors" on older Yubikey models (eg, FIDO U2F + // Security Key firmware 4.1.8). + if !info.fido2 && len(allowedCreds) > 1 { + cred, ok := findFirstKnownCredential(dev, info, rpID, ccdHash, allowedCreds) + if ok { + isCredentialCheck := pin == "" && opts != nil && opts.UP == libfido2.False + if isCredentialCheck { + // No need to assert again, reply as the U2F authenticator would. + return nil, trace.Wrap(libfido2.ErrUserPresenceRequired) + } + + if log.IsLevelEnabled(log.DebugLevel) { + credPrefix := hex.EncodeToString(cred) + const prefixLen = 10 + if len(credPrefix) > prefixLen { + credPrefix = credPrefix[:prefixLen] + } + log.Debugf("FIDO2: Device %v: Using credential %v...", info.path, credPrefix) + } + + allowedCreds = [][]byte{cred} + } + } + + assertion, err := dev.Assertion(rpID, ccdHash, allowedCreds, pin, opts) + return assertion, trace.Wrap(err) +} + +func findFirstKnownCredential( + dev FIDODevice, + info *deviceInfo, + rpID string, + ccdHash []byte, + allowedCreds [][]byte, +) ([]byte, bool) { + const pin = "" + opts := &libfido2.AssertionOpts{ + UP: libfido2.False, + } + for _, cred := range allowedCreds { + _, err := dev.Assertion(rpID, ccdHash, [][]byte{cred}, pin, opts) + // FIDO2 devices return err=nil on up=false queries; U2F devices return + // libfido2.ErrUserPresenceRequired. + // https://github.com/Yubico/libfido2/blob/03c18d396eb209a42bbf62f5f4415203cba2fc50/src/u2f.c#L787-L791. + if err == nil || (!info.fido2 && errors.Is(err, libfido2.ErrUserPresenceRequired)) { + return cred, true + } + } + return nil, false +} + func pickAssertion( assertions []*libfido2.Assertion, prompt LoginPrompt, user string, passwordless bool, ) (*libfido2.Assertion, error) { @@ -452,7 +515,7 @@ func fido2Register( // Does the device hold an excluded credential? const pin = "" // not required to filter - switch _, err := dev.Assertion(rp.ID, ccdHash[:], excludeList, pin, &libfido2.AssertionOpts{ + switch _, err := devAssertion(dev, info, rp.ID, ccdHash[:], excludeList, pin, &libfido2.AssertionOpts{ UP: libfido2.False, }); { case errors.Is(err, libfido2.ErrNoCredentials): diff --git a/lib/auth/webauthncli/fido2_test.go b/lib/auth/webauthncli/fido2_test.go index 0e4486ab85db3..e1fc0890981d5 100644 --- a/lib/auth/webauthncli/fido2_test.go +++ b/lib/auth/webauthncli/fido2_test.go @@ -1954,6 +1954,102 @@ func TestFIDO2Register_u2fExcludedCredentials(t *testing.T) { require.NoError(t, err, "FIDO2Register errored, expected a successful registration") } +// TestFIDO2Login_u2fInternalError tests the scenario described by issue +// https://github.com/gravitational/teleport/issues/44912. +func TestFIDO2Login_u2fInternalError(t *testing.T) { + resetFIDO2AfterTests(t) + + dev1 := mustNewFIDO2Device("/dev1", "" /* pin */, &libfido2.DeviceInfo{ + Options: authOpts, + }) + dev2 := mustNewFIDO2Device("/dev2", "" /* pin */, &libfido2.DeviceInfo{ + Options: authOpts, + }) + u2fDev := mustNewFIDO2Device("/u2f", "" /* pin */, nil /* info */) + u2fDev.u2fOnly = true + u2fDev.errorOnUnknownCredential = true + + f2 := newFakeFIDO2(dev1, dev2, u2fDev) + f2.setCallbacks() + + const origin = "https://example.com" + ctx := context.Background() + + // Register all authenticators. + cc := &wantypes.CredentialCreation{ + Response: wantypes.PublicKeyCredentialCreationOptions{ + Challenge: make([]byte, 32), + RelyingParty: wantypes.RelyingPartyEntity{ + CredentialEntity: protocol.CredentialEntity{ + Name: "example.com", + }, + ID: "example.com", + }, + User: wantypes.UserEntity{ + CredentialEntity: protocol.CredentialEntity{ + Name: "alpaca", + }, + DisplayName: "Alpaca", + ID: []byte{1, 2, 3, 4, 5}, // arbitrary + }, + Parameters: []wantypes.CredentialParameter{ + {Type: protocol.PublicKeyCredentialType, Algorithm: webauthncose.AlgES256}, + }, + AuthenticatorSelection: wantypes.AuthenticatorSelection{ + RequireResidentKey: protocol.ResidentKeyNotRequired(), + ResidentKey: protocol.ResidentKeyRequirementDiscouraged, + UserVerification: protocol.VerificationDiscouraged, + }, + Attestation: protocol.PreferNoAttestation, + }, + } + allowedCreds := make([]wantypes.CredentialDescriptor, 0, len(f2.devices)) + for _, dev := range f2.devices { + ctx, cancel := context.WithTimeout(ctx, 1*time.Second) + mfaResp, err := wancli.FIDO2Register(ctx, origin, cc, dev) + cancel() + require.NoError(t, err, "FIDO2Register failed") + + allowedCreds = append(allowedCreds, wantypes.CredentialDescriptor{ + Type: protocol.PublicKeyCredentialType, + CredentialID: mfaResp.GetWebauthn().RawId, + }) + } + + // Sanity check: authenticator errors in the presence of unknown credentials. + u2fDev.open() + _, err := u2fDev.Assertion( + "example.com", + []byte(`55cde2973243a946b85a477d2e164a35d2e4f3daaeb11ac5e9a1c4cf3297033e`), // clientDataHash + [][]byte{ + u2fDev.credentialID(), + bytes.Repeat([]byte("A"), 96), + }, + "", // pin + &libfido2.AssertionOpts{UP: libfido2.False}, + ) + require.ErrorIs(t, err, libfido2.ErrInternal, "u2fDev.Assert error mismatch") + u2fDev.Close() + + t.Run("login with multiple credentials", func(t *testing.T) { + assertion := &wantypes.CredentialAssertion{ + Response: wantypes.PublicKeyCredentialRequestOptions{ + Challenge: make([]byte, 32), + RelyingPartyID: "example.com", + AllowedCredentials: allowedCreds, + UserVerification: protocol.VerificationDiscouraged, + }, + } + + ctx, cancel := context.WithTimeout(ctx, 1*time.Second) + defer cancel() + _, _, err := wancli.FIDO2Login(ctx, origin, assertion, u2fDev, &wancli.LoginOpts{ + User: "alpaca", + }) + require.NoError(t, err, "FIDO2Login failed") + }) +} + func resetFIDO2AfterTests(t *testing.T) { pollInterval := wancli.FIDO2PollInterval devLocations := wancli.FIDODeviceLocations @@ -2015,6 +2111,10 @@ type fakeFIDO2Device struct { // Causes libfido2.ErrNotFIDO2 on Info. u2fOnly bool + // errorOnUnknownCredential makes the device fail assertions if an unknown + // credential is present. + errorOnUnknownCredential bool + // assertionErrors is a chain of errors to return from Assertion. // Errors are returned from start to end and removed, one-by-one, on each // invocation of the Assertion method. @@ -2291,6 +2391,9 @@ func (f *fakeFIDO2Device) Assertion( found = true break } + if f.errorOnUnknownCredential { + return nil, fmt.Errorf("failed to get assertion: %w", libfido2.ErrInternal) + } } if !found { return nil, libfido2.ErrNoCredentials @@ -2316,6 +2419,13 @@ func (f *fakeFIDO2Device) Assertion( credIDs := make(map[string]struct{}) for _, cred := range credentialIDs { credIDs[string(cred)] = struct{}{} + + // Simulate "internal error" on unknown credential handles. + // Sometimes happens with Yubikeys firmware 4.1.8. + // Requires a tap to happen. + if f.errorOnUnknownCredential && !bytes.Equal(cred, f.key.KeyHandle) { + return nil, fmt.Errorf("failed to get assertion: %w", libfido2.ErrInternal) + } } // Assemble one assertion for each allowed credential we hold.