Add WebAuthn credential registration

[DJAndries]

#102

[github-actions]

The security team is monitoring all repositories for certain keywords. This PR includes the word(s) "authn" and so security team members have been added as reviewers to take a look.

No need to request a full security review at this stage, the security team will take a look shortly and either clear the label or request more information/changes.

Notifications have already been sent, but if this is blocking your merge feel free to reach out directly to the security team on Slack so that we can expedite this check.

[github-actions]

[puLL-Merge] - brave/accounts@128

Description

This PR adds WebAuthn (Web Authentication) support to the Brave accounts service, enabling users to configure hardware security keys and biometric authentication as a second factor alongside TOTP. The implementation uses the go-webauthn library and includes credential management (registration, storage, and deletion), integration with the existing 2FA flow, and proper database migrations.

Key changes:

• Adds WebAuthn credential registration and management endpoints
• Stores WebAuthn credentials and interim registration states in PostgreSQL
• Updates the 2FA configuration model to support multiple authentication methods
• Refactors TOTP-specific naming to be 2FA-method-agnostic
• Adds environment variables for WebAuthn configuration (RP ID and allowed origins)
• Limits accounts to 10 WebAuthn credentials maximum

Security Hotspots

1. WebAuthn Origin Validation (Medium Risk)

   • Location: services/twofa.go:89-93, .env.example:14
   • The WEBAUTHN_ORIGINS environment variable allows configuring trusted origins but defaults to BASE_URL if not set. In development, this defaults to http://localhost:8081 (in .env.example) but http://localhost:8080 elsewhere. Ensure production deployments use HTTPS origins only and that the origin list is properly restricted.
   • The origins are used by the webauthn library for response validation, so misconfiguration could allow credential replay attacks from unauthorized origins.

2. WebAuthn ID Generation (Low Risk)

   • Location: datastore/accounts.go:413-426
   • The 64-byte WebAuthn user handle is generated using crypto/rand.Read() without checking for collisions. While UUID collision is astronomically unlikely with 64 bytes, the code lacks defensive collision detection.
   • The ID is stored unencrypted, which is acceptable per WebAuthn spec as it's not meant to be secret.

3. Session Data Storage (Low Risk)

   • Location: datastore/webauthn.go:38-40, datastore/password_states.go:37
   • WebAuthn challenge session data is stored in JSON format in the database. The serialization/deserialization is handled by GORM but ensure the JSON serializer doesn't introduce vulnerabilities with untrusted data.

4. Credential Limit Enforcement (Low Risk)

   • Location: datastore/webauthn.go:44-50
   • The 10-credential limit check uses a WHERE clause excluding the credential being saved (id != ?). This prevents the upsert operation from being blocked by itself, but the race condition window between count and insert could theoretically allow exceeding the limit in concurrent operations (mitigated by transaction).

5. Recovery Key Generation Timing (Low Risk)

   • Location: controllers/accounts.go:413-434
   • Recovery keys are only generated once when enabling the first 2FA method. Verify that this behavior is acceptable and that users understand they need to save it on first 2FA setup.

Privacy Hotspots

1. WebAuthn Credential Metadata (Medium Risk)

   • Location: datastore/webauthn.go:19-25, controllers/accounts.go:658-680
   • Credential names (user-provided strings like "YubiKey 5C") are stored and returned in API responses. These could be personally identifying. Consider:
     • Limiting name length (currently max 100 chars via validation)
     • Sanitizing names for XSS if displayed in web interfaces
     • Adding audit logging when credentials are viewed/modified

2. Email Address in WebAuthn Display Name (Low Risk)

   • Location: services/twofa.go:397-399
   • The user's email is set as both WebAuthnName() and WebAuthnDisplayName(). Per WebAuthn spec, displayName may be shown by authenticators. For privacy-conscious users, consider allowing a pseudonymous display name separate from the email.

3. Credential Metadata Exposure (Low Risk)

   • Location: datastore/accounts.go:343-351
   • The GetTwoFAConfiguration endpoint returns all WebAuthn credentials including their IDs and names. Ensure this is only accessible to authenticated users (it is, via authMiddleware).

Changes

Changes

.air.toml and .github/workflows/checks.yaml

• Added --parseDependency flag to swag init commands to properly parse WebAuthn types from dependencies

.env.example

• Added WEBAUTHN_ORIGINS environment variable for configuring allowed WebAuthn origins

README.md

• Documented new environment variables: TWOFA_ISSUER, TOTP_QR_SIZE, WEBAUTHN_RP_ID, WEBAUTHN_ORIGINS

controllers/accounts.go

• Renamed TwoFAInitRequest/Response to TOTPInitRequest/Response for clarity
• Added WebAuthnRegistrationInitResponse and WebAuthnRegistrationFinalizeRequest types
• Added three new endpoints:
  • POST /v2/accounts/2fa/webauthn/init - Initialize WebAuthn registration
  • POST /v2/accounts/2fa/webauthn/finalize - Complete WebAuthn registration
  • DELETE /v2/accounts/2fa/webauthn/{credentialID} - Delete a credential
• Added maybeGenerateRecoveryKey() helper to DRY recovery key generation logic
• Updated 2FA status checks to use IsTwoFAEnabled() method that checks both TOTP and WebAuthn

datastore/accounts.go

• Removed IsTwoFAEnabled() method from Account (moved to InterimPasswordState)
• Renamed TwoFADetails to TwoFAConfiguration with WebAuthn fields
• Added SetWebAuthnSetting() for enabling/disabling WebAuthn
• Updated GetTwoFAConfiguration() to include WebAuthn credentials
• Added GetOrCreateWebAuthnID() to generate unique user handles for WebAuthn

datastore/password_states.go

• Split RequiresTwoFA field into separate TOTPEnabled and WebAuthnEnabled fields
• Added WebAuthnChallenge field for storing login challenge data
• Added IsTwoFAEnabled() method to check if any 2FA method is enabled
• Updated state creation functions to accept both TOTP and WebAuthn flags

datastore/twofa.go

• Changed TOTP key retrieval to return ErrBadTOTPCode instead of ErrKeyNotFound for better error messages

datastore/webauthn.go (new file)

• Added DBWebAuthnCredential model for storing credentials
• Added InterimWebAuthnRegistrationState for temporary registration data
• Implemented credential CRUD operations with 10-credential limit
• Added registration state management with 5-minute expiry

services/opaque.go

• Updated login/registration flows to handle separate TOTP and WebAuthn enabled flags
• Modified state creation to pass both 2FA method flags

services/ses.go

• Refactored base URL retrieval to use new util.GetBaseURL() helper

services/twofa.go

• Renamed totpIssuerEnv to twoFAIssuerEnv for consistency
• Added WebAuthn configuration with RP ID and origin validation
• Implemented webAuthnUser type satisfying the webauthn.User interface
• Added CreateWebAuthnRegistrationChallenge() and FinalizeWebAuthnCredentialRegistration() methods
• Updated DisableTwoFA() to handle both TOTP and WebAuthn cleanup

util/error.go

• Added WebAuthn-specific error codes (11006-11009) for credential operations

util/util.go

• Added GetBaseURL() helper function for consistent base URL retrieval

migrations/

• Migration 20251028053111_webauthn.up.sql adds:
  • webauthn_id, webauthn_enabled, webauthn_enabled_at columns to accounts
  • webauthn_credentials table
  • interim_webauthn_registration_states table
  • Renames requires_twofa to totp_enabled in interim_password_states
• Updated cron job to clean expired WebAuthn registration states

go.mod / go.sum

• Added github.com/go-webauthn/webauthn v0.14.0 for WebAuthn protocol handling
• Added github.com/descope/virtualwebauthn v1.0.3 for testing
• Updated various dependencies including golang-jwt, stretchr/testify, and Go toolchain packages

Test files

• Added comprehensive test coverage for WebAuthn registration, finalization, credential deletion
• Added tests for credential limits, expired states, and error handling
• Added helper functions for creating virtual WebAuthn authenticators and credentials in tests

sequenceDiagram
    participant Client
    participant AccountsController
    participant TwoFAService
    participant Datastore
    participant WebAuthn

    %% Registration Flow
    Client->>AccountsController: POST /2fa/webauthn/init
    AccountsController->>TwoFAService: CreateWebAuthnRegistrationChallenge
    TwoFAService->>Datastore: GetOrCreateWebAuthnID
    Datastore-->>TwoFAService: webauthnID
    TwoFAService->>Datastore: GetWebAuthnCredentials
    Datastore-->>TwoFAService: existing credentials
    TwoFAService->>WebAuthn: BeginMediatedRegistration
    WebAuthn-->>TwoFAService: CredentialCreation + SessionData
    TwoFAService->>Datastore: CreateInterimWebAuthnState
    Datastore-->>TwoFAService: registrationID
    TwoFAService-->>AccountsController: CredentialCreation + registrationID
    AccountsController-->>Client: Challenge + registrationID

    Client->>Client: User interacts with authenticator
    Client->>AccountsController: POST /2fa/webauthn/finalize<br/>(registrationID, name, response)
    AccountsController->>TwoFAService: FinalizeWebAuthnCredentialRegistration
    TwoFAService->>Datastore: GetAndDeleteInterimWebAuthnState
    Datastore-->>TwoFAService: SessionData
    TwoFAService->>Datastore: GetWebAuthnCredentials (for user object)
    Datastore-->>TwoFAService: credentials
    TwoFAService->>WebAuthn: CreateCredential
    WebAuthn-->>TwoFAService: Credential (validated)
    TwoFAService->>Datastore: SaveWebAuthnCredential
    Datastore-->>TwoFAService: success
    TwoFAService-->>AccountsController: credential
    AccountsController->>AccountsController: maybeGenerateRecoveryKey
    AccountsController->>Datastore: SetWebAuthnSetting(true)
    Datastore-->>AccountsController: success
    AccountsController-->>Client: TwoFAFinalizeResponse (with recovery key if new)

    %% Credential Deletion Flow
    Client->>AccountsController: DELETE /2fa/webauthn/{credentialID}
    AccountsController->>Datastore: GetWebAuthnCredentials
    Datastore-->>AccountsController: all credentials
    alt Last credential
        AccountsController->>Datastore: SetWebAuthnSetting(false)
    end
    AccountsController->>Datastore: DeleteWebAuthnCredential
    Datastore-->>AccountsController: success
    AccountsController-->>Client: 204 No Content

Loading

[fmarier]

Maybe we should put this in a transaction to eliminate any possible races?