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"
gravitational · Nov 5, 2024 · e81af03 · e81af03
1 parent 04c3b97
commit e81af03
Show file tree

Hide file tree

Showing 2 changed files with 177 additions and 4 deletions.
diff --git 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,15 +221,15 @@ 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).
 			// We are relying on the fact that, because the PIN is set, the
 			// 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
@@ -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.