From 28c3f1e254f554da970316e7050c510ae483b8f0 Mon Sep 17 00:00:00 2001 From: Renovate Bot Date: Wed, 28 Aug 2024 00:05:00 +0000 Subject: [PATCH 1/2] Update module github.com/go-webauthn/webauthn to v0.11.2 --- go.mod | 10 +++++----- go.sum | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index f2e40bb7a9..fd030bad1a 100644 --- a/go.mod +++ b/go.mod @@ -47,11 +47,11 @@ require ( github.com/go-sql-driver/mysql v1.8.1 github.com/go-swagger/go-swagger v0.30.5 github.com/go-testfixtures/testfixtures/v3 v3.12.0 - github.com/go-webauthn/webauthn v0.10.0 + github.com/go-webauthn/webauthn v0.11.2 github.com/gobwas/glob v0.2.3 github.com/gogs/chardet v0.0.0-20211120154057-b7413eaefb8f github.com/gogs/go-gogs-client v0.0.0-20210131175652-1d7215cd8d85 - github.com/golang-jwt/jwt/v5 v5.2.0 + github.com/golang-jwt/jwt/v5 v5.2.1 github.com/google/go-github/v64 v64.0.0 github.com/google/pprof v0.0.0-20240528025155-186aa0362fba github.com/google/uuid v1.6.0 @@ -171,7 +171,7 @@ require ( github.com/emirpasic/gods v1.18.1 // indirect github.com/fatih/color v1.16.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect - github.com/fxamacker/cbor/v2 v2.5.0 // indirect + github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-ap/errors v0.0.0-20231003111023-183eef4b31b7 // indirect github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect github.com/go-enry/go-oniguruma v1.2.1 // indirect @@ -191,7 +191,7 @@ require ( github.com/go-openapi/strfmt v0.22.0 // indirect github.com/go-openapi/swag v0.22.7 // indirect github.com/go-openapi/validate v0.22.6 // indirect - github.com/go-webauthn/x v0.1.6 // indirect + github.com/go-webauthn/x v0.1.14 // indirect github.com/goccy/go-json v0.10.3 // indirect github.com/golang-jwt/jwt/v4 v4.5.0 // indirect github.com/golang/geo v0.0.0-20230421003525-6adc56603217 // indirect @@ -201,7 +201,7 @@ require ( github.com/google/btree v1.1.2 // indirect github.com/google/go-cmp v0.6.0 // indirect github.com/google/go-querystring v1.1.0 // indirect - github.com/google/go-tpm v0.9.0 // indirect + github.com/google/go-tpm v0.9.1 // indirect github.com/gorilla/css v1.0.1 // indirect github.com/gorilla/handlers v1.5.2 // indirect github.com/gorilla/mux v1.8.1 // indirect diff --git a/go.sum b/go.sum index b980c8b183..3a1e08c6b8 100644 --- a/go.sum +++ b/go.sum @@ -221,8 +221,8 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= -github.com/fxamacker/cbor/v2 v2.5.0 h1:oHsG0V/Q6E/wqTS2O1Cozzsy69nqCiguo5Q1a1ADivE= -github.com/fxamacker/cbor/v2 v2.5.0/go.mod h1:TA1xS00nchWmaBnEIxPSE5oHLuJBAVvqrtAnWBwBCVo= +github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E= +github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= github.com/gliderlabs/ssh v0.3.7 h1:iV3Bqi942d9huXnzEF2Mt+CY9gLu8DNM4Obd+8bODRE= github.com/gliderlabs/ssh v0.3.7/go.mod h1:zpHEXBstFnQYtGnB8k8kQLol82umzn/2/snG7alWVD8= github.com/go-ap/activitypub v0.0.0-20231114162308-e219254dc5c9 h1:j2TrkUG/NATGi/EQS+MvEoF79CxiRUmT16ErFroNcKI= @@ -296,10 +296,10 @@ github.com/go-test/deep v1.1.0 h1:WOcxcdHcvdgThNXjw0t76K42FXTU7HpNQWHpA2HHNlg= github.com/go-test/deep v1.1.0/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/go-testfixtures/testfixtures/v3 v3.12.0 h1:Ew0+c2o1mXSUqMwjuNup3MK/vw1HkLS3ILljX5C6lVE= github.com/go-testfixtures/testfixtures/v3 v3.12.0/go.mod h1:13F0m6/DtqqSDso9IAVuhbZ4I7AiRAHrolmDMu9v5vY= -github.com/go-webauthn/webauthn v0.10.0 h1:yuW2e1tXnRAwAvKrR4q4LQmc6XtCMH639/ypZGhZCwk= -github.com/go-webauthn/webauthn v0.10.0/go.mod h1:l0NiauXhL6usIKqNLCUM3Qir43GK7ORg8ggold0Uv/Y= -github.com/go-webauthn/x v0.1.6 h1:QNAX+AWeqRt9loE8mULeWJCqhVG5D/jvdmJ47fIWCkQ= -github.com/go-webauthn/x v0.1.6/go.mod h1:W8dFVZ79o4f+nY1eOUICy/uq5dhrRl7mxQkYhXTo0FA= +github.com/go-webauthn/webauthn v0.11.2 h1:Fgx0/wlmkClTKlnOsdOQ+K5HcHDsDcYIvtYmfhEOSUc= +github.com/go-webauthn/webauthn v0.11.2/go.mod h1:aOtudaF94pM71g3jRwTYYwQTG1KyTILTcZqN1srkmD0= +github.com/go-webauthn/x v0.1.14 h1:1wrB8jzXAofojJPAaRxnZhRgagvLGnLjhCAwg3kTpT0= +github.com/go-webauthn/x v0.1.14/go.mod h1:UuVvFZ8/NbOnkDz3y1NaxtUN87pmtpC1PQ+/5BBQRdc= github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y= github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8= github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u12GKvMCM= @@ -314,8 +314,8 @@ github.com/gogs/go-gogs-client v0.0.0-20210131175652-1d7215cd8d85 h1:UjoPNDAQ5JP github.com/gogs/go-gogs-client v0.0.0-20210131175652-1d7215cd8d85/go.mod h1:fR6z1Ie6rtF7kl/vBYMfgD5/G5B1blui7z426/sj2DU= github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= -github.com/golang-jwt/jwt/v5 v5.2.0 h1:d/ix8ftRUorsN+5eMIlF4T6J8CAt9rch3My2winC1Jw= -github.com/golang-jwt/jwt/v5 v5.2.0/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= +github.com/golang-jwt/jwt/v5 v5.2.1 h1:OuVbFODueb089Lh128TAcimifWaLhJwVflnrgM17wHk= +github.com/golang-jwt/jwt/v5 v5.2.1/go.mod h1:pqrtFR0X4osieyHYxtmOUWsAWrfe1Q5UVIyoH402zdk= github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9 h1:au07oEsX2xN0ktxqI+Sida1w446QrXBRJ0nee3SNZlA= github.com/golang-sql/civil v0.0.0-20220223132316-b832511892a9/go.mod h1:8vg3r2VgvsThLBIFL93Qb5yWzgyZWhEmBwUJWevAkK0= github.com/golang-sql/sqlexp v0.1.0 h1:ZCD6MBpcuOVfGVqsEmY5/4FtYiKz6tSyUv9LPEDei6A= @@ -352,8 +352,8 @@ github.com/google/go-github/v64 v64.0.0 h1:4G61sozmY3eiPAjjoOHponXDBONm+utovTKby github.com/google/go-github/v64 v64.0.0/go.mod h1:xB3vqMQNdHzilXBiO2I+M7iEFtHf+DP/omBOv6tQzVo= github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= -github.com/google/go-tpm v0.9.0 h1:sQF6YqWMi+SCXpsmS3fd21oPy/vSddwZry4JnmltHVk= -github.com/google/go-tpm v0.9.0/go.mod h1:FkNVkc6C+IsvDI9Jw1OveJmxGZUUaKxtrpOS47QWKfU= +github.com/google/go-tpm v0.9.1 h1:0pGc4X//bAlmZzMKf8iz6IsDo1nYTbYJ6FZN/rg4zdM= +github.com/google/go-tpm v0.9.1/go.mod h1:h9jEsEECg7gtLis0upRBQU+GhYVH6jMjrFxI8u6bVUY= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= From 63736e83011e8d8df681dd73898e4f98045baa10 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 28 Aug 2024 07:40:40 +0200 Subject: [PATCH 2/2] [FEAT] Add support for webauthn credential level 3 - For WebAuthn Credential level 3, the `backup_eligible` and `backup_state` flags are checked if they are consistent with the values given on login. Forgejo never stored this data, so add a database migration that makes all webauthn credentials 'legacy' and on the next first use capture the values of `backup_eligible` and `backup_state`. As suggested in https://github.com/go-webauthn/webauthn/discussions/219#discussioncomment-10429662 - Adds unit tests. - Add E2E test. --- models/auth/webauthn.go | 23 ++++++++-- models/auth/webauthn_test.go | 14 +++++- models/fixtures/webauthn_credential.yml | 1 + models/forgejo_migrations/migrate.go | 2 + models/forgejo_migrations/v22.go | 17 +++++++ routers/web/auth/webauthn.go | 26 ++++++++--- tests/e2e/webauthn.test.e2e.js | 60 +++++++++++++++++++++++++ 7 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 models/forgejo_migrations/v22.go create mode 100644 tests/e2e/webauthn.test.e2e.js diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index a65d2e1e34..aa13cf6cb1 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -40,7 +40,7 @@ func IsErrWebAuthnCredentialNotExist(err error) bool { } // WebAuthnCredential represents the WebAuthn credential data for a public-key -// credential conformant to WebAuthn Level 1 +// credential conformant to WebAuthn Level 3 type WebAuthnCredential struct { ID int64 `xorm:"pk autoincr"` Name string @@ -52,8 +52,12 @@ type WebAuthnCredential struct { AAGUID []byte SignCount uint32 `xorm:"BIGINT"` CloneWarning bool - CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` - UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + BackupEligible bool `XORM:"NOT NULL DEFAULT false"` + BackupState bool `XORM:"NOT NULL DEFAULT false"` + // If legacy is set to true, backup_eligible and backup_state isn't set. + Legacy bool `XORM:"NOT NULL DEFAULT true"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` } func init() { @@ -71,6 +75,12 @@ func (cred *WebAuthnCredential) UpdateSignCount(ctx context.Context) error { return err } +// UpdateFromLegacy update the values that aren't present on legacy credentials. +func (cred *WebAuthnCredential) UpdateFromLegacy(ctx context.Context) error { + _, err := db.GetEngine(ctx).ID(cred.ID).Cols("legacy", "backup_eligible", "backup_state").Update(cred) + return err +} + // BeforeInsert will be invoked by XORM before updating a record func (cred *WebAuthnCredential) BeforeInsert() { cred.LowerName = strings.ToLower(cred.Name) @@ -97,6 +107,10 @@ func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential { ID: cred.CredentialID, PublicKey: cred.PublicKey, AttestationType: cred.AttestationType, + Flags: webauthn.CredentialFlags{ + BackupEligible: cred.BackupEligible, + BackupState: cred.BackupState, + }, Authenticator: webauthn.Authenticator{ AAGUID: cred.AAGUID, SignCount: cred.SignCount, @@ -167,6 +181,9 @@ func CreateCredential(ctx context.Context, userID int64, name string, cred *weba AAGUID: cred.Authenticator.AAGUID, SignCount: cred.Authenticator.SignCount, CloneWarning: false, + BackupEligible: cred.Flags.BackupEligible, + BackupState: cred.Flags.BackupState, + Legacy: false, } if err := db.Insert(ctx, c); err != nil { diff --git a/models/auth/webauthn_test.go b/models/auth/webauthn_test.go index cae590c790..e1cd652009 100644 --- a/models/auth/webauthn_test.go +++ b/models/auth/webauthn_test.go @@ -56,13 +56,23 @@ func TestWebAuthnCredential_UpdateLargeCounter(t *testing.T) { unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, SignCount: 0xffffffff}) } +func TestWebAuthenCredential_UpdateFromLegacy(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + cred := unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1, Legacy: true}) + cred.Legacy = false + cred.BackupEligible = true + cred.BackupState = true + require.NoError(t, cred.UpdateFromLegacy(db.DefaultContext)) + unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, BackupEligible: true, BackupState: true}, "legacy = false") +} + func TestCreateCredential(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) - res, err := auth_model.CreateCredential(db.DefaultContext, 1, "WebAuthn Created Credential", &webauthn.Credential{ID: []byte("Test")}) + res, err := auth_model.CreateCredential(db.DefaultContext, 1, "WebAuthn Created Credential", &webauthn.Credential{ID: []byte("Test"), Flags: webauthn.CredentialFlags{BackupEligible: true, BackupState: true}}) require.NoError(t, err) assert.Equal(t, "WebAuthn Created Credential", res.Name) assert.Equal(t, []byte("Test"), res.CredentialID) - unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1}) + unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1, BackupEligible: true, BackupState: true}, "legacy = false") } diff --git a/models/fixtures/webauthn_credential.yml b/models/fixtures/webauthn_credential.yml index bc43127fcd..edf9935ebf 100644 --- a/models/fixtures/webauthn_credential.yml +++ b/models/fixtures/webauthn_credential.yml @@ -5,5 +5,6 @@ attestation_type: none sign_count: 0 clone_warning: false + legacy: true created_unix: 946684800 updated_unix: 946684800 diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 598ec8bbaa..cca83d6b4d 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -80,6 +80,8 @@ var migrations = []*Migration{ NewMigration("Creating Quota-related tables", CreateQuotaTables), // v21 -> v22 NewMigration("Add SSH keypair to `pull_mirror` table", AddSSHKeypairToPushMirror), + // v22 -> v23 + NewMigration("Add `legacy` to `web_authn_credential` table", AddLegacyToWebAuthnCredential), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v22.go b/models/forgejo_migrations/v22.go new file mode 100644 index 0000000000..eeb738799c --- /dev/null +++ b/models/forgejo_migrations/v22.go @@ -0,0 +1,17 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package forgejo_migrations //nolint:revive + +import "xorm.io/xorm" + +func AddLegacyToWebAuthnCredential(x *xorm.Engine) error { + type WebauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + BackupEligible bool `xorm:"NOT NULL DEFAULT false"` + BackupState bool `xorm:"NOT NULL DEFAULT false"` + Legacy bool `xorm:"NOT NULL DEFAULT true"` + } + + return x.Sync(&WebauthnCredential{}) +} diff --git a/routers/web/auth/webauthn.go b/routers/web/auth/webauthn.go index 1079f44a08..5c93c1410e 100644 --- a/routers/web/auth/webauthn.go +++ b/routers/web/auth/webauthn.go @@ -116,6 +116,25 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) { return } + dbCred, err := auth.GetWebAuthnCredentialByCredID(ctx, user.ID, parsedResponse.RawID) + if err != nil { + ctx.ServerError("GetWebAuthnCredentialByCredID", err) + return + } + + // If the credential is legacy, assume the values are correct. The + // specification mandates these flags don't change. + if dbCred.Legacy { + dbCred.BackupEligible = parsedResponse.Response.AuthenticatorData.Flags.HasBackupEligible() + dbCred.BackupState = parsedResponse.Response.AuthenticatorData.Flags.HasBackupState() + dbCred.Legacy = false + + if err := dbCred.UpdateFromLegacy(ctx); err != nil { + ctx.ServerError("UpdateFromLegacy", err) + return + } + } + // Validate the parsed response. cred, err := wa.WebAuthn.ValidateLogin((*wa.User)(user), *sessionData, parsedResponse) if err != nil { @@ -133,13 +152,6 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) { return } - // Success! Get the credential and update the sign count with the new value we received. - dbCred, err := auth.GetWebAuthnCredentialByCredID(ctx, user.ID, cred.ID) - if err != nil { - ctx.ServerError("GetWebAuthnCredentialByCredID", err) - return - } - dbCred.SignCount = cred.Authenticator.SignCount if err := dbCred.UpdateSignCount(ctx); err != nil { ctx.ServerError("UpdateSignCount", err) diff --git a/tests/e2e/webauthn.test.e2e.js b/tests/e2e/webauthn.test.e2e.js new file mode 100644 index 0000000000..30c92c48f0 --- /dev/null +++ b/tests/e2e/webauthn.test.e2e.js @@ -0,0 +1,60 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT +// @ts-check + +import {expect} from '@playwright/test'; +import {test, login_user, load_logged_in_context} from './utils_e2e.js'; + +test.beforeAll(async ({browser}, workerInfo) => { + await login_user(browser, workerInfo, 'user2'); +}); + +test('WebAuthn register & login flow', async ({browser}, workerInfo) => { + test.skip(workerInfo.project.name !== 'chromium', 'Uses Chrome protocol'); + const context = await load_logged_in_context(browser, workerInfo, 'user2'); + const page = await context.newPage(); + + // Register a security key. + let response = await page.goto('/user/settings/security'); + await expect(response?.status()).toBe(200); + + // https://github.com/microsoft/playwright/issues/7276#issuecomment-1516768428 + const cdpSession = await page.context().newCDPSession(page); + await cdpSession.send('WebAuthn.enable'); + await cdpSession.send('WebAuthn.addVirtualAuthenticator', { + options: { + protocol: 'ctap2', + ctap2Version: 'ctap2_1', + hasUserVerification: true, + transport: 'usb', + automaticPresenceSimulation: true, + isUserVerified: true, + backupEligibility: true, + }, + }); + + await page.locator('input#nickname').fill('Testing Security Key'); + await page.getByText('Add security key').click(); + + // Logout. + await page.locator('div[aria-label="Profile and settings…"]').click(); + await page.getByText('Sign Out').click(); + await page.waitForURL(`${workerInfo.project.use.baseURL}/`); + + // Login. + response = await page.goto('/user/login'); + await expect(response?.status()).toBe(200); + + await page.getByLabel('Username or email address').fill('user2'); + await page.getByLabel('Password').fill('password'); + await page.getByRole('button', {name: 'Sign in'}).click(); + await page.waitForURL(`${workerInfo.project.use.baseURL}/user/webauthn`); + await page.waitForURL(`${workerInfo.project.use.baseURL}/`); + + // Cleanup. + response = await page.goto('/user/settings/security'); + await expect(response?.status()).toBe(200); + await page.getByRole('button', {name: 'Remove'}).click(); + await page.getByRole('button', {name: 'Yes'}).click(); + await page.waitForURL(`${workerInfo.project.use.baseURL}/user/settings/security`); +});