## Part 1: Improve `Webauthn::AuthenticateService` and `Passkey::AuthenticateService` code paths `Webauthn::AuthenticateService` and `Passkey::AuthenticateService` are essential code paths for GitLab application, especially from security perspective. It is very important to keep and make those code paths clear, maintainable, and secure. Currenly, `Webauthn::AuthenticateService` and `Passkey::AuthenticateService` - partially use [webauthn-ruby](https://github.com/cedarcode/webauthn-ruby)'s credential verification logic - partially duplicate implementation from webauthn-ruby's credential verification logic - contain implementation to support migrated U2F credentials(https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42159+). This mix of credential verification implementations makes those code paths unclear and it has already led to a security vulnerability introduced in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/213428, where we assumed that credential verification should always raise error for invalid credential, as per webauthn-ruby, but we overlooked that our duplicated implementation in the GitLab application didn't raise error, but returns `false`. That vulnerability was reported and fixed in https://gitlab.com/gitlab-org/gitlab/-/work_items/585333+. webauthn-ruby has [a setting that enables support for migrated U2F credentials](https://github.com/cedarcode/webauthn-ruby/blob/v3.0.0/docs/u2f_migration.md#authenticate-migrated-u2f-credentials). We can leverage that setting - this will allow us refactor verification implementation in `Webauthn::AuthenticateService` and `Passkey::AuthenticateService` by removing that mix of credential verification implementations in favor of only webauthn-ruby's credential verification logic. See the following MR for more details: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/222461+. I think that refactoring is essential and should make those code paths more clear, maintainable, and secure. ## Part 2: Enhance security for passkey authentication & passkey creation ### Passkey authentication (Sign in with a passkey) #### User presence verification According to W3C WebAuthn specification(https://www.w3.org/TR/webauthn/#sctn-verifying-assertion), verifying user presence is **required** for passkey authentication. This is core security requirement to ensure the user is physically interacting with the authenticator. User presence protection prevents silent, passive authentication without the user's knowledge. It guards against several attack vectors: - Malware/Software-Based Attacks - Malware running on the device cannot trigger authentication without user interaction - Prevents background processes from silently authenticating as the user - Remote Attacks - An attacker who compromises the device remotely still needs the user to physically interact with the authenticator - Blocks automated exploitation attempts - Unauthorized Access by Privileged Users - System administrators or device owners cannot silently authenticate as other users - Requires explicit user interaction each time In https://gitlab.com/gitlab-org/gitlab/-/merge_requests/224111, we want to add a spec to confirm that user presence verification is enabled for passkey authentication. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/224111/diffs#493b06d78846513e501c797687432b3b2e059d73_118_120. The existing implementation for passkey authentication does not explicitly state that user presence verification is enabled. Thanks to webauthn-ruby default configurations, user presence verification is enabled by default, as per `WebAuthn.configuration.silent_authentication` that is set to `false` by default. See https://github.com/cedarcode/webauthn-ruby/blob/v3.4.3/lib/webauthn/authenticator_response.rb#L43-L46. The main goal of this issue was to confirm that user presence verification is enabled for passkey authentication, as it is required from security perspective. :relieved: #### User verification According to W3C WebAuthn specification(https://www.w3.org/TR/webauthn/#sctn-verifying-assertion), user verification is **optional** for passkey authentication. While user verification is discouraged for 2FA, it is strongly recommended as mandatory for passkey authentication for security-sensitive applications. For instance, Microsoft and GitHub require user verification for passkey authentication on their websites. User verification protects by confirming the user's identity through additional authentication factors; for example, through a touch plus pin code, password entry, or biometric recognition. It guards against unauthorized use or stolen device attack. Even if someone has the device, they cannot authenticate without the user's biometric/PIN. The passkey alone is insufficient; the user must prove their identity. :warning: We already inform the client/frontend that user verification is required for passkey authentication, see https://gitlab.com/gitlab-org/gitlab/-/blob/b265cf5c3e9e61cd39300b9151aa002375982b63/app/controllers/concerns/authenticates_with_two_factor.rb#L182. However, I noticed that we don't check authentication assertion, on the backend side, to verify if user verification was performed. Meaning that despite we inform the client/frontend that user verification is required for passkey authentication, it could potentially be bypassed. For example, on the client side, [webauthnParams](https://gitlab.com/gitlab-org/gitlab/-/blob/1e9ebc3ef8f491332628228a3c8cbca6446fed19/app/assets/javascripts/authentication/webauthn/components/passkey_authentication.vue#L60-L62) that inform the client that user verification is required could be intercepted and changed and the client could submit authentication assertion without user verification for passkey authentication. To prevent that, https://gitlab.com/gitlab-org/gitlab/-/merge_requests/224111#note_3138255233 adds user verification check for authentication assertion on the backend side. ### Passkey creation #### User presence verification In https://gitlab.com/gitlab-org/gitlab/-/merge_requests/224111, we want to add a spec to confirm that user presence verification is enabled for passkey registration. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/224111/diffs#66f7bb8a6d70183ce673eecffd1174f3c242b36d_96_101. #### User verification Same here, we already inform the client/frontend that user verification is required for passkey registration, see https://gitlab.com/gitlab-org/gitlab/-/blob/e986659a85b3feb69be0fa02ee137bef1fbcf559/app/controllers/profiles/passkeys_controller.rb#L134. However, we don't check credential attestation data, on the backend side, to verify if user verification was performed. No security concerns here, but technically, users could register credentials that don't support user verification. To prevent that, https://gitlab.com/gitlab-org/gitlab/-/merge_requests/224111#note_3138283949 adds user verification check for credential attestation data on the backend side. #### Passkey eligibility validation We already inform the client/frontend that for registering new passkey, the credential should be `resident_key` (aka passkey), see https://gitlab.com/gitlab-org/gitlab/-/blob/e986659a85b3feb69be0fa02ee137bef1fbcf559/app/controllers/profiles/passkeys_controller.rb#L135. https://gitlab.com/gitlab-org/gitlab/-/merge_requests/224111#note_3138310064 adds validation for credential attestation data on the backend side to confirm that submitted credential is passkey eligible. ## Resources - https://www.w3.org/TR/webauthn/ - https://developers.yubico.com/WebAuthn/WebAuthn_Developer_Guide/User_Presence_vs_User_Verification.html