[v15] fix: Correctly handle non-registered U2F keys

lib/auth/webauthncli/fido2.go

@@ -234,7 +234,18 @@ func fido2Login(
 			assertions, err = dev.Assertion(actualRPID, ccdHash[:], allowedCreds, pin, opts)
 		}
 		if errors.Is(err, libfido2.ErrNoCredentials) {
-			err = ErrUsingNonRegisteredDevice // "Upgrade" error message.
+			// U2F devices error instantly with ErrNoCredentials.
+			// If that is the case, we mark the error as non-interactive and continue
+			// without this device. This is the only safe option, as it lets the
+			// handleDevice goroutine exit gracefully. Do not attempt to wait for
+			// touch - this causes another slew of problems with abandoned U2F
+			// goroutines during registration.
+			if !info.fido2 {
+				log.Debugf("FIDO2: U2F device %v not registered, ignoring it", info.path)
+				err = &nonInteractiveError{err: err}
+			} else {
+				err = ErrUsingNonRegisteredDevice // "Upgrade" error message.
+			}
 		}
 		if err != nil {
 			return trace.Wrap(err)
@@ -669,8 +680,15 @@ func startDevices(
 
 		dev, err := fidoNewDevice(path)
 		if err != nil {
-			closeAll()
-			return nil, nil, trace.Wrap(err, "device open")
+			// Be resilient to open errors.
+			// This can happen to devices that failed to cancel (and thus are still
+			// asserting) when we run sequential operations. For example: registration
+			// immediately followed by assertion (in a single process).
+			// This is largely safe to ignore, as opening is fairly consistent in
+			// other situations and failures are likely from a non-chosen device in
+			// multi-device scenarios.
+			log.Debugf("FIDO2: Device %v failed to open, skipping: %v", path, err)
+			continue
 		}
 
 		fidoDevs = append(fidoDevs, dev)
@@ -679,6 +697,9 @@ func startDevices(
 			dev:  dev,
 		})
 	}
+	if len(fidoDevs) == 0 {
+		return nil, nil, errors.New("failed to open security keys")
+	}
 
 	// Prompt touch, it's about to begin.
 	ackTouch, err := prompt.PromptTouch()

lib/auth/webauthncli/fido2_test.go

@@ -22,6 +22,7 @@
 package webauthncli_test
 
 import (
+	"bytes"
 	"context"
 	"crypto/rand"
 	"errors"
@@ -1016,6 +1017,124 @@ func TestFIDO2Login_u2fDevice(t *testing.T) {
 	assert.NoError(t, err, "FIDO2Login errored")
 }
 
+// TestFIDO2Login_u2fDeviceNotRegistered tests assertions with a non-registered
+// U2F device plugged.
+//
+// U2F devices error immediately when not registered, which makes their behavior
+// distinct from FIDO2 and requires additional logic to be correctly handled.
+//
+// This test captures an U2F assertion regression.
+func TestFIDO2Login_u2fDeviceNotRegistered(t *testing.T) {
+	resetFIDO2AfterTests(t)
+
+	u2fDev := mustNewFIDO2Device("/u2f", "" /* pin */, nil /* info */)
+	u2fDev.u2fOnly = true
+
+	registeredDev := mustNewFIDO2Device("/dev2", "" /* pin */, &libfido2.DeviceInfo{
+		Options: bioOpts,
+	})
+
+	f2 := newFakeFIDO2(u2fDev, registeredDev)
+	f2.setCallbacks()
+
+	const rpID = "example.com"
+	const origin = "https://example.com"
+
+	// Set a ctx timeout in case something goes wrong.
+	// Under normal circumstances the test gets nowhere near this timeout.
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	// Register our "registeredDev".
+	cc := &wantypes.CredentialCreation{
+		Response: wantypes.PublicKeyCredentialCreationOptions{
+			Challenge: []byte{1, 2, 3, 4, 5}, // arbitrary
+			RelyingParty: wantypes.RelyingPartyEntity{
+				ID: rpID,
+				CredentialEntity: wantypes.CredentialEntity{
+					Name: "rp name",
+				},
+			},
+			Parameters: []wantypes.CredentialParameter{
+				{
+					Type:      protocol.PublicKeyCredentialType,
+					Algorithm: webauthncose.AlgES256,
+				},
+			},
+			User: wantypes.UserEntity{
+				ID: []byte{1, 2, 3, 4, 1}, // arbitrary,
+				CredentialEntity: wantypes.CredentialEntity{
+					Name: "user name",
+				},
+				DisplayName: "user display name",
+			},
+			AuthenticatorSelection: wantypes.AuthenticatorSelection{
+				UserVerification: protocol.VerificationDiscouraged,
+			},
+			Attestation: protocol.PreferNoAttestation,
+		},
+	}
+	registeredDev.setUP() // simulate touch
+	ccr, err := wancli.FIDO2Register(ctx, origin, cc, registeredDev /* prompt */)
+	require.NoError(t, err, "FIDO2Register errored")
+
+	assertion := &wantypes.CredentialAssertion{
+		Response: wantypes.PublicKeyCredentialRequestOptions{
+			Challenge:      []byte{1, 2, 3, 4, 5}, // arbitrary
+			RelyingPartyID: rpID,
+			AllowedCredentials: []wantypes.CredentialDescriptor{
+				{
+					Type:         protocol.PublicKeyCredentialType,
+					CredentialID: ccr.GetWebauthn().GetRawId(),
+				},
+			},
+			UserVerification: protocol.VerificationDiscouraged,
+		},
+	}
+
+	tests := []struct {
+		name    string
+		prompt  wancli.LoginPrompt
+		timeout time.Duration
+		wantErr error
+	}{
+		{
+			name:   "registered device touched",
+			prompt: &delayedPrompt{registeredDev}, // Give the U2F device time to fail.
+		},
+		{
+			name:    "no devices touched",
+			prompt:  noopPrompt{}, // `registered` not touched, U2F won't blink.
+			timeout: 10 * time.Millisecond,
+			wantErr: context.DeadlineExceeded,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			// Apply custom timeout.
+			ctx := ctx
+			if test.timeout > 0 {
+				var cancel context.CancelFunc
+				ctx, cancel = context.WithTimeout(context.Background(), test.timeout)
+				defer cancel()
+			}
+
+			_, _, err := wancli.FIDO2Login(ctx, origin, assertion, test.prompt, nil /* opts */)
+			assert.ErrorIs(t, err, test.wantErr, "FIDO2Login error mismatch")
+		})
+	}
+}
+
+type delayedPrompt struct {
+	wancli.LoginPrompt
+}
+
+func (p *delayedPrompt) PromptTouch() (wancli.TouchAcknowledger, error) {
+	const delay = 100 * time.Millisecond
+	time.Sleep(delay)
+	return p.LoginPrompt.PromptTouch()
+}
+
 func TestFIDO2Login_bioErrorHandling(t *testing.T) {
 	resetFIDO2AfterTests(t)
 
@@ -2203,6 +2322,24 @@ func (f *fakeFIDO2Device) Assertion(
 		privilegedAccess = true
 	}
 
+	// U2F only: exit without user interaction if there are no credentials.
+	if f.u2fOnly {
+		found := false
+		for _, cid := range credentialIDs {
+			if bytes.Equal(cid, f.key.KeyHandle) {
+				found = true
+				break
+			}
+		}
+		if !found {
+			return nil, libfido2.ErrNoCredentials
+		}
+
+		// TODO(codingllama): Verify f.wantRPID in here as well?
+		//  We don't exercise this particular scenario presently, so it's not coded
+		//  either.
+	}
+
 	// Block for user presence before accessing any credential data.
 	if err := f.maybeLockUntilInteraction(opts.UP == libfido2.True); err != nil {
 		return nil, err