[v12] Record and verify WebAuthn RPIDs

api/proto/teleport/legacy/types/types.proto

@@ -2963,6 +2963,13 @@ message WebauthnDevice {
   // view of the registration process; the authenticator alone can determine
   // if a key is truly resident.)
   bool resident_key = 7;
+  // Relying Party ID used by the credential.
+  // Recorded on registration for new credentials, or on first successful
+  // authentication for "old" credentials (created before the field existed).
+  // Ideally, this is always the same as the configured RPID.
+  // If an RPID change does happen, this helps Teleport detect it and react
+  // accordingly.
+  string credential_rp_id = 8;
 }
 
 // WebauthnLocalAuth holds settings necessary for local webauthn use.

lib/auth/auth_login_test.go

@@ -37,12 +37,6 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
 
-	svr := newTestTLSServer(t)
-	authServer := svr.Auth()
-	mfa := configureForMFA(t, svr)
-	username := mfa.User
-	password := mfa.Password
-
 	tests := []struct {
 		name            string
 		spec            *types.AuthPreferenceSpecV2
@@ -107,13 +101,13 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
 					AppID: "https://myoldappid.com",
 				},
 				Webauthn: &types.Webauthn{
-					RPID: "myexplicitid",
+					RPID: "localhost",
 				},
 			},
 			assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
 				require.Empty(t, challenge.GetTOTP())
 				require.NotEmpty(t, challenge.GetWebauthnChallenge())
-				require.Equal(t, "myexplicitid", challenge.GetWebauthnChallenge().GetPublicKey().GetRpId())
+				require.Equal(t, "localhost", challenge.GetWebauthnChallenge().GetPublicKey().GetRpId())
 			},
 		},
 		{
@@ -146,7 +140,16 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
 		},
 	}
 	for _, test := range tests {
+		test := test
 		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			svr := newTestTLSServer(t)
+			authServer := svr.Auth()
+			mfa := configureForMFA(t, svr)
+			username := mfa.User
+			password := mfa.Password
+
 			authPreference, err := types.NewAuthPreference(*test.spec)
 			require.NoError(t, err)
 			require.NoError(t, authServer.SetAuthPreference(ctx, authPreference))

lib/auth/webauthn/login.go

@@ -28,6 +28,7 @@ import (
 	wan "github.com/go-webauthn/webauthn/webauthn"
 	"github.com/gravitational/trace"
 	log "github.com/sirupsen/logrus"
+	"golang.org/x/exp/slices"
 
 	"github.com/gravitational/teleport/api/types"
 	wantypes "github.com/gravitational/teleport/api/types/webauthn"
@@ -83,6 +84,29 @@ func (f *loginFlow) begin(ctx context.Context, user string, passwordless bool) (
 			return nil, trace.Wrap(err)
 		}
 
+		// Filter devices with the wrong RPID and log an error.
+		foundInvalid := false
+		for i := 0; i < len(devices); i++ {
+			webDev := devices[i].GetWebauthn()
+			if webDev == nil || webDev.CredentialRpId == "" || webDev.CredentialRpId == f.Webauthn.RPID {
+				continue
+			}
+
+			log.Errorf(""+
+				"User device %q/%q has unexpected RPID=%q, excluding from allowed credentials. "+
+				"RPID changes are not supported by WebAuthn, this is likely to cause permanent authentication problems for your users. "+
+				"Consider reverting the change or reset your users so they may register their devices again.",
+				user,
+				devices[i].GetName(),
+				webDev.CredentialRpId)
+
+			// "Cut" device from slice.
+			devices = slices.Delete(devices, i, i+1)
+			i--
+
+			foundInvalid = true
+		}
+
 		// Sort non-resident keys first, which may cause clients to favor them for
 		// MFA in some scenarios (eg, tsh).
 		sort.Slice(devices, func(i, j int) bool {
@@ -98,6 +122,9 @@ func (f *loginFlow) begin(ctx context.Context, user string, passwordless bool) (
 		// Let's make sure we have at least one registered credential here, since we
 		// have to allow zero credentials for passwordless below.
 		if len(u.credentials) == 0 {
+			if foundInvalid {
+				return nil, trace.Wrap(ErrInvalidCredentials)
+			}
 			return nil, trace.NotFound("found no credentials for user %q", user)
 		}
 	}
@@ -272,6 +299,12 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *CredentialAss
 	if err := setCounterAndTimestamps(dev, credential); err != nil {
 		return nil, "", trace.Wrap(err)
 	}
+	// Retroactively write the credential RPID, now that it cleared authn.
+	if webDev := dev.GetWebauthn(); webDev != nil && webDev.CredentialRpId == "" {
+		log.Debugf("WebAuthn: Recording RPID=%q in device %q/%q", rpID, user, dev.GetName())
+		webDev.CredentialRpId = rpID
+	}
+
 	if err := f.identity.UpsertMFADevice(ctx, user, dev); err != nil {
 		return nil, "", trace.Wrap(err)
 	}

lib/auth/webauthn/login_mfa.go

@@ -24,6 +24,13 @@ import (
 	wantypes "github.com/gravitational/teleport/api/types/webauthn"
 )
 
+// ErrInvalidCredentials is a special kind of credential "NotFound" error, where
+// the user has only devices registered to other RPIDs.
+// Possible fixes include reseting the affected users (likely the entire
+// cluster), or rolling back to a good WebAuthn configuration (if still
+// possible).
+var ErrInvalidCredentials = errors.New("user has only invalid WebAuthn registrations, consider a user reset")
+
 // LoginIdentity represents the subset of Identity methods used by LoginFlow.
 // It exists to better scope LoginFlow's use of Identity and to facilitate
 // testing.

lib/auth/webauthn/login_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/gogo/protobuf/proto"
 	"github.com/google/go-cmp/cmp"
 	"github.com/gravitational/trace"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/api/types"
@@ -504,6 +505,122 @@ func TestPasswordlessFlow_Finish_errors(t *testing.T) {
 	}
 }
 
+// TestCredentialRPID tests the recording of CredentialRpId and scenarios
+// related to RPID mismatch.
+func TestCredentialRPID(t *testing.T) {
+	const origin = "https://example.com"
+	const originOther = "https://notexample.com"
+	const rpID = "example.com"
+	const user = "llama"
+
+	ctx := context.Background()
+	identity := newFakeIdentity(user)
+	webConfig := &types.Webauthn{RPID: rpID}
+	webOtherRP := &types.Webauthn{RPID: "notexample.com"}
+
+	dev1Key, err := mocku2f.Create()
+	require.NoError(t, err)
+
+	register := func(config *types.Webauthn, user, origin, deviceName string, key *mocku2f.Key) (*types.MFADevice, error) {
+		webRegistration := &wanlib.RegistrationFlow{
+			Webauthn: config,
+			Identity: identity,
+		}
+
+		const passwordless = false
+		cc, err := webRegistration.Begin(ctx, user, passwordless)
+		if err != nil {
+			return nil, err
+		}
+
+		ccr, err := key.SignCredentialCreation(origin, cc)
+		if err != nil {
+			return nil, err
+		}
+
+		return webRegistration.Finish(ctx, wanlib.RegisterResponse{
+			User:             user,
+			DeviceName:       deviceName,
+			CreationResponse: ccr,
+			Passwordless:     passwordless,
+		})
+	}
+
+	t.Run("register writes credential RPID", func(t *testing.T) {
+		mfaDev, err := register(webConfig, user, origin, "dev1" /* deviceName */, dev1Key)
+		require.NoError(t, err, "Registration failed")
+		assert.Equal(t, rpID, mfaDev.GetWebauthn().CredentialRpId, "CredentialRpId mismatch")
+	})
+
+	// "Reset" all stored CredentialRpIds to simulate devices created before the
+	// field existed.
+	assert.Len(t, identity.User.GetLocalAuth().MFA, 1, "MFA device count mismatch")
+	for _, dev := range identity.User.GetLocalAuth().MFA {
+		dev.GetWebauthn().CredentialRpId = ""
+	}
+
+	t.Run("login issues challenges for unknown credential RPID", func(t *testing.T) {
+		webLogin := &wanlib.LoginFlow{
+			Webauthn: webOtherRP, // Wrong RPID!
+			Identity: identity,
+		}
+
+		_, err := webLogin.Begin(ctx, user)
+		assert.NoError(t, err, "Begin failed, expected assertion for `dev1`")
+	})
+
+	t.Run("login writes credential RPID", func(t *testing.T) {
+		webLogin := &wanlib.LoginFlow{
+			Webauthn: webConfig,
+			Identity: identity,
+		}
+
+		assertion, err := webLogin.Begin(ctx, user)
+		require.NoError(t, err, "Begin failed")
+
+		car, err := dev1Key.SignAssertion(origin, assertion)
+		require.NoError(t, err, "SignAssertion failed")
+
+		mfaDev, err := webLogin.Finish(ctx, user, car)
+		require.NoError(t, err, "Finish failed")
+		assert.Equal(t, rpID, mfaDev.GetWebauthn().CredentialRpId, "CredentialRpId mismatch")
+	})
+
+	t.Run("login doesn't issue challenges for the wrong RPIDs", func(t *testing.T) {
+		webLogin := &wanlib.LoginFlow{
+			Webauthn: webOtherRP, // Wrong RPID!
+			Identity: identity,
+		}
+
+		_, err := webLogin.Begin(ctx, user)
+		assert.ErrorIs(t, err, wanlib.ErrInvalidCredentials, "Begin error mismatch")
+	})
+
+	t.Run("login issues challenges if at least one device matches", func(t *testing.T) {
+		other1Key, err := mocku2f.Create()
+		require.NoError(t, err)
+
+		// Register a device for the wrong/new RPID.
+		// Storage is now a mix of devices for both RPs.
+		_, err = register(webOtherRP, user, originOther, "other1" /* deviceName */, other1Key)
+		require.NoError(t, err, "Registration failed")
+
+		webLogin := &wanlib.LoginFlow{
+			Webauthn: webOtherRP,
+			Identity: identity,
+		}
+		assertion, err := webLogin.Begin(ctx, user)
+		require.NoError(t, err, "Begin failed, expected assertion for device `other1`")
+
+		// Verify that we got the correct device.
+		assert.Len(t, assertion.Response.AllowedCredentials, 1, "AllowedCredentials")
+		assert.Equal(t,
+			other1Key.KeyHandle,
+			assertion.Response.AllowedCredentials[0].CredentialID,
+			"Expected key handle for device `other1`")
+	})
+}
+
 type fakeIdentity struct {
 	User *types.UserV2
 	// MappedUser is used as the reply to GetTeleportUserByWebauthnID.

lib/auth/webauthn/register.go

@@ -224,7 +224,8 @@ func upsertOrGetWebID(ctx context.Context, user string, identity RegistrationIde
 	return wla.UserID, nil
 }
 
-// RegisterResponse represents fields needed to finish registering a new webautn device.
+// RegisterResponse represents fields needed to finish registering a new
+// WebAuthn device.
 type RegisterResponse struct {
 	// User is the device owner.
 	User string
@@ -318,6 +319,7 @@ func (f *RegistrationFlow) Finish(ctx context.Context, req RegisterResponse) (*t
 			SignatureCounter:  credential.Authenticator.SignCount,
 			AttestationObject: req.CreationResponse.AttestationResponse.AttestationObject,
 			ResidentKey:       req.Passwordless,
+			CredentialRpId:    f.Webauthn.RPID,
 		},
 	}
 	// We delegate a few checks to identity, including:

lib/auth/webauthn/register_test.go

@@ -119,6 +119,7 @@ func TestRegistrationFlow_BeginFinish(t *testing.T) {
 				SignatureCounter:  0,
 				AttestationObject: ccr.AttestationResponse.AttestationObject,
 				ResidentKey:       test.wantResidentKey,
+				CredentialRpId:    rpID,
 			}
 			if diff := cmp.Diff(wantDevice, gotDevice); diff != "" {
 				t.Errorf("Finish() mismatch (-want +got):\n%s", diff)

lib/auth/webauthncli/fido2.go

@@ -116,7 +116,7 @@ func fido2Login(
 	// Presence of any allowed credential is interpreted as the user identity
 	// being partially established, aka non-passwordless.
 	passwordless := len(allowedCreds) == 0
-	log.Debugf("FIDO2: assertion: passwordless=%v, uv=%v", passwordless, uv)
+	log.Debugf("FIDO2: assertion: passwordless=%v, uv=%v, %v allowed credentials", passwordless, uv, len(allowedCreds))
 
 	// Prepare challenge data for the device.
 	ccdJSON, err := json.Marshal(&CollectedClientData{