WebAuthn CredentialID field needs to be increased in size

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,

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})
 }

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="

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
+

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

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
+}

models/migrations/v221_test.go

@@ -0,0 +1,66 @@
+// 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))
+if x == nil || t.Failed() {
+defer deferable()
+return
+}
+defer deferable()
+
+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)
+}
+}

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()
+}