diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index 2dc3043780101..d3062342f545b 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -6,7 +6,6 @@ package auth import ( "context" - "encoding/base32" "fmt" "strings" @@ -20,14 +19,14 @@ import ( // ErrWebAuthnCredentialNotExist represents a "ErrWebAuthnCRedentialNotExist" kind of error. type ErrWebAuthnCredentialNotExist struct { ID int64 - CredentialID string + CredentialID []byte } func (err ErrWebAuthnCredentialNotExist) Error() string { - if err.CredentialID == "" { + if len(err.CredentialID) == 0 { return fmt.Sprintf("WebAuthn credential does not exist [id: %d]", err.ID) } - return fmt.Sprintf("WebAuthn credential does not exist [credential_id: %s]", err.CredentialID) + return fmt.Sprintf("WebAuthn credential does not exist [credential_id: %x]", err.CredentialID) } // IsErrWebAuthnCredentialNotExist checks if an error is a ErrWebAuthnCredentialNotExist. @@ -43,7 +42,7 @@ type WebAuthnCredential struct { Name string LowerName string `xorm:"unique(s)"` UserID int64 `xorm:"INDEX unique(s)"` - CredentialID string `xorm:"INDEX VARCHAR(410)"` + CredentialID []byte `xorm:"INDEX VARBINARY(1024)"` PublicKey []byte AttestationType string AAGUID []byte @@ -94,9 +93,8 @@ type WebAuthnCredentialList []*WebAuthnCredential func (list WebAuthnCredentialList) ToCredentials() []webauthn.Credential { creds := make([]webauthn.Credential, 0, len(list)) for _, cred := range list { - credID, _ := base32.HexEncoding.DecodeString(cred.CredentialID) creds = append(creds, webauthn.Credential{ - ID: credID, + ID: cred.CredentialID, PublicKey: cred.PublicKey, AttestationType: cred.AttestationType, Authenticator: webauthn.Authenticator{ @@ -164,11 +162,11 @@ func HasWebAuthnRegistrationsByUID(uid int64) (bool, error) { } // GetWebAuthnCredentialByCredID returns WebAuthn credential by credential ID -func GetWebAuthnCredentialByCredID(userID int64, credID string) (*WebAuthnCredential, error) { +func GetWebAuthnCredentialByCredID(userID int64, credID []byte) (*WebAuthnCredential, error) { return getWebAuthnCredentialByCredID(db.DefaultContext, userID, credID) } -func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID string) (*WebAuthnCredential, error) { +func getWebAuthnCredentialByCredID(ctx context.Context, userID int64, credID []byte) (*WebAuthnCredential, error) { cred := new(WebAuthnCredential) if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id = ?", userID, credID).Get(cred); err != nil { return nil, err @@ -187,7 +185,7 @@ func createCredential(ctx context.Context, userID int64, name string, cred *weba c := &WebAuthnCredential{ UserID: userID, Name: name, - CredentialID: base32.HexEncoding.EncodeToString(cred.ID), + CredentialID: cred.ID, PublicKey: cred.PublicKey, AttestationType: cred.AttestationType, AAGUID: cred.Authenticator.AAGUID, diff --git a/models/auth/webauthn_test.go b/models/auth/webauthn_test.go index 216bf110806eb..cc39691ce2d36 100644 --- a/models/auth/webauthn_test.go +++ b/models/auth/webauthn_test.go @@ -5,7 +5,6 @@ package auth import ( - "encoding/base32" "testing" "code.gitea.io/gitea/models/unittest" @@ -61,9 +60,7 @@ func TestCreateCredential(t *testing.T) { res, err := CreateCredential(1, "WebAuthn Created Credential", &webauthn.Credential{ID: []byte("Test")}) assert.NoError(t, err) assert.Equal(t, "WebAuthn Created Credential", res.Name) - bs, err := base32.HexEncoding.DecodeString(res.CredentialID) - assert.NoError(t, err) - assert.Equal(t, []byte("Test"), bs) + assert.Equal(t, []byte("Test"), res.CredentialID) unittest.AssertExistsIf(t, true, &WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1}) } diff --git a/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml new file mode 100644 index 0000000000000..55a237a0d633d --- /dev/null +++ b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml @@ -0,0 +1,9 @@ +- + id: 1 + credential_id: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG=" +- + id: 2 + credential_id: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR=" +- + id: 4 + credential_id: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G=" diff --git a/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml new file mode 100644 index 0000000000000..c02a76e3742a9 --- /dev/null +++ b/models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml @@ -0,0 +1,31 @@ +- + id: 1 + lower_name: "u2fkey-correctly-migrated" + name: "u2fkey-correctly-migrated" + user_id: 1 + credential_id: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG=" + public_key: 0x040d0967a2cad045011631187576492a0beb5b377954b4f694c5afc8bdf25270f87f09a9ab6ce9c282f447ba71b2f2bae2105b32b847e0704f310f48644e3eddf2 + attestation_type: 'fido-u2f' + sign_count: 1 + clone_warning: false +- + id: 2 + lower_name: "non-u2f-key" + name: "non-u2f-key" + user_id: 1 + credential_id: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR" + public_key: 0x040d0967a2cad045011631187576492a0beb5b377954b4f694c5afc8bdf25270f87f09a9ab6ce9c282f447ba71b2f2bae2105b32b847e0704f310f48644e3eddf2 + attestation_type: 'none' + sign_count: 1 + clone_warning: false +- + id: 4 + lower_name: "packed-key" + name: "packed-key" + user_id: 1 + credential_id: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G=" + public_key: 0x040d0967a2cad045011631187576492a0beb5b377954b4f694c5afc8bdf25270f87f09a9ab6ce9c282f447ba71b2f2bae2105b32b847e0704f310f48644e3eddf2 + attestation_type: 'fido-u2f' + sign_count: 1 + clone_warning: false + diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index beeba866dc70f..2719f45efbbbc 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -400,6 +400,12 @@ var migrations = []Migration{ NewMigration("Add sync_on_commit column to push_mirror table", addSyncOnCommitColForPushMirror), // v220 -> v221 NewMigration("Add container repository property", addContainerRepositoryProperty), + // v221 -> v222 + NewMigration("Store WebAuthentication CredentialID as bytes and increase size to at least 1024", storeWebauthnCredentialIDAsBytes), + // v222 -> v223 + NewMigration("Drop old CredentialID column", dropOldCredentialIDColumn), + // v223 -> v224 + NewMigration("Rename CredentialIDBytes column to CredentialID", renameCredentialIDBytes), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v221.go b/models/migrations/v221.go new file mode 100644 index 0000000000000..f3bcfcdf1de20 --- /dev/null +++ b/models/migrations/v221.go @@ -0,0 +1,75 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "encoding/base32" + "fmt" + + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func storeWebauthnCredentialIDAsBytes(x *xorm.Engine) error { + // Create webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(410)"` + // Note the lack of INDEX here - these will be created once the column is renamed in v223.go + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + if err := x.Sync2(&webauthnCredential{}); err != nil { + return err + } + + var start int + creds := make([]*webauthnCredential, 0, 50) + for { + err := x.Select("id, credential_id").OrderBy("id").Limit(50, start).Find(&creds) + if err != nil { + return err + } + + err = func() error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return fmt.Errorf("unable to allow start session. Error: %w", err) + } + for _, cred := range creds { + cred.CredentialIDBytes, err = base32.HexEncoding.DecodeString(cred.CredentialID) + if err != nil { + return fmt.Errorf("unable to parse credential id %s for credential[%d]: %w", cred.CredentialID, cred.ID, err) + } + count, err := sess.ID(cred.ID).Cols("credential_id_bytes").Update(cred) + if count != 1 || err != nil { + return fmt.Errorf("unable to update credential id bytes for credential[%d]: %d,%w", cred.ID, count, err) + } + } + return sess.Commit() + }() + if err != nil { + return err + } + + if len(creds) < 50 { + break + } + start += 50 + creds = creds[:0] + } + return nil +} diff --git a/models/migrations/v221_test.go b/models/migrations/v221_test.go new file mode 100644 index 0000000000000..c50ca5c873291 --- /dev/null +++ b/models/migrations/v221_test.go @@ -0,0 +1,65 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "encoding/base32" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_storeWebauthnCredentialIDAsBytes(t *testing.T) { + // Create webauthnCredential table + type WebauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(410)"` + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + } + + type ExpectedWebauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + CredentialID string // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + } + + type ConvertedWebauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + } + + // Prepare and load the testing database + x, deferable := prepareTestEnv(t, 0, new(WebauthnCredential), new(ExpectedWebauthnCredential)) + defer deferable() + if x == nil || t.Failed() { + return + } + + if err := storeWebauthnCredentialIDAsBytes(x); err != nil { + assert.NoError(t, err) + return + } + + expected := []ExpectedWebauthnCredential{} + if err := x.Table("expected_webauthn_credential").Asc("id").Find(&expected); !assert.NoError(t, err) { + return + } + + got := []ConvertedWebauthnCredential{} + if err := x.Table("webauthn_credential").Select("id, credential_id_bytes").Asc("id").Find(&got); !assert.NoError(t, err) { + return + } + + for i, e := range expected { + credIDBytes, _ := base32.HexEncoding.DecodeString(e.CredentialID) + assert.Equal(t, credIDBytes, got[i].CredentialIDBytes) + } +} diff --git a/models/migrations/v222.go b/models/migrations/v222.go new file mode 100644 index 0000000000000..99acdfd20608a --- /dev/null +++ b/models/migrations/v222.go @@ -0,0 +1,64 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func dropOldCredentialIDColumn(x *xorm.Engine) error { + // This migration maybe rerun so that we should check if it has been run + credentialIDExist, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id") + if err != nil { + return err + } + if !credentialIDExist { + // Column is already non-extant + return nil + } + credentialIDBytesExists, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id_bytes") + if err != nil { + return err + } + if !credentialIDBytesExists { + // looks like 221 hasn't properly run + return fmt.Errorf("webauthn_credential does not have a credential_id_bytes column... it is not safe to run this migration") + } + + // Create webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID string `xorm:"INDEX VARCHAR(410)"` + // Note the lack of the INDEX on CredentialIDBytes - we will add this in v223.go + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + if err := x.Sync2(&webauthnCredential{}); err != nil { + return err + } + + // Drop the old credential ID + sess := x.NewSession() + defer sess.Close() + + if err := dropTableColumns(sess, "webauthn_credential", "credential_id"); err != nil { + return fmt.Errorf("unable to drop old credentialID column: %w", err) + } + return sess.Commit() +} diff --git a/models/migrations/v223.go b/models/migrations/v223.go new file mode 100644 index 0000000000000..d7ee4812b8264 --- /dev/null +++ b/models/migrations/v223.go @@ -0,0 +1,103 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func renameCredentialIDBytes(x *xorm.Engine) error { + // This migration maybe rerun so that we should check if it has been run + credentialIDExist, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id") + if err != nil { + return err + } + if credentialIDExist { + credentialIDBytesExists, err := x.Dialect().IsColumnExist(x.DB(), context.Background(), "webauthn_credential", "credential_id_bytes") + if err != nil { + return err + } + if !credentialIDBytesExists { + return nil + } + } + + err = func() error { + // webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + // Note the lack of INDEX here + CredentialIDBytes []byte `xorm:"VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if err := sess.Sync2(new(webauthnCredential)); err != nil { + return fmt.Errorf("error on Sync2: %v", err) + } + + if credentialIDExist { + // if both errors and message exist, drop message at first + if err := dropTableColumns(sess, "webauthn_credential", "credential_id"); err != nil { + return err + } + } + + switch { + case setting.Database.UseMySQL: + if _, err := sess.Exec("ALTER TABLE `webauthn_credential` CHANGE credential_id_bytes credential_id VARBINARY(1024)"); err != nil { + return err + } + case setting.Database.UseMSSQL: + if _, err := sess.Exec("sp_rename 'webauthn_credential.credential_id_bytes', 'credential_id', 'COLUMN'"); err != nil { + return err + } + default: + if _, err := sess.Exec("ALTER TABLE `webauthn_credential` RENAME COLUMN credential_id_bytes TO credential_id"); err != nil { + return err + } + } + return sess.Commit() + }() + if err != nil { + return err + } + + // Create webauthnCredential table + type webauthnCredential struct { + ID int64 `xorm:"pk autoincr"` + Name string + LowerName string `xorm:"unique(s)"` + UserID int64 `xorm:"INDEX unique(s)"` + CredentialID []byte `xorm:"INDEX VARBINARY(1024)"` // CredentialID is at most 1023 bytes as per spec released 20 July 2022 + PublicKey []byte + AttestationType string + AAGUID []byte + SignCount uint32 `xorm:"BIGINT"` + CloneWarning bool + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + return x.Sync2(&webauthnCredential{}) +} diff --git a/routers/web/auth/webauthn.go b/routers/web/auth/webauthn.go index 4778c9a9a3690..917cbdd57bf67 100644 --- a/routers/web/auth/webauthn.go +++ b/routers/web/auth/webauthn.go @@ -5,7 +5,6 @@ package auth import ( - "encoding/base32" "errors" "net/http" @@ -129,7 +128,7 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) { } // Success! Get the credential and update the sign count with the new value we received. - dbCred, err := auth.GetWebAuthnCredentialByCredID(user.ID, base32.HexEncoding.EncodeToString(cred.ID)) + dbCred, err := auth.GetWebAuthnCredentialByCredID(user.ID, cred.ID) if err != nil { ctx.ServerError("GetWebAuthnCredentialByCredID", err) return