WebAuthn CredentialID field needs to be increased in size

models/auth/webauthn.go

@@ -20,14 +20,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: %s]", base32.HexEncoding.EncodeToString(err.CredentialID))
 }
 
 // IsErrWebAuthnCredentialNotExist checks if an error is a ErrWebAuthnCredentialNotExist.
@@ -39,18 +39,18 @@ func IsErrWebAuthnCredentialNotExist(err error) bool {
 // WebAuthnCredential represents the WebAuthn credential data for a public-key
 // credential conformant to WebAuthn Level 1
 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
-	CreatedUnix     timeutil.TimeStamp `xorm:"INDEX created"`
-	UpdatedUnix     timeutil.TimeStamp `xorm:"INDEX updated"`
+	ID                int64 `xorm:"pk autoincr"`
+	Name              string
+	LowerName         string `xorm:"unique(s)"`
+	UserID            int64  `xorm:"INDEX unique(s)"`
+	CredentialIDBytes []byte `xorm:"INDEX VARBINARY(1024)"`
+	PublicKey         []byte
+	AttestationType   string
+	AAGUID            []byte
+	SignCount         uint32 `xorm:"BIGINT"`
+	CloneWarning      bool
+	CreatedUnix       timeutil.TimeStamp `xorm:"INDEX created"`
+	UpdatedUnix       timeutil.TimeStamp `xorm:"INDEX updated"`
 }
 
 func init() {
@@ -94,9 +94,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.CredentialIDBytes,
 			PublicKey:       cred.PublicKey,
 			AttestationType: cred.AttestationType,
 			Authenticator: webauthn.Authenticator{
@@ -164,13 +163,13 @@ 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 {
+	if found, err := db.GetEngine(ctx).Where("user_id = ? AND credential_id_bytes = ?", userID, credID).Get(cred); err != nil {
 		return nil, err
 	} else if !found {
 		return nil, ErrWebAuthnCredentialNotExist{CredentialID: credID}
@@ -185,14 +184,14 @@ func CreateCredential(userID int64, name string, cred *webauthn.Credential) (*We
 
 func createCredential(ctx context.Context, userID int64, name string, cred *webauthn.Credential) (*WebAuthnCredential, error) {
 	c := &WebAuthnCredential{
-		UserID:          userID,
-		Name:            name,
-		CredentialID:    base32.HexEncoding.EncodeToString(cred.ID),
-		PublicKey:       cred.PublicKey,
-		AttestationType: cred.AttestationType,
-		AAGUID:          cred.Authenticator.AAGUID,
-		SignCount:       cred.Authenticator.SignCount,
-		CloneWarning:    false,
+		UserID:            userID,
+		Name:              name,
+		CredentialIDBytes: cred.ID,
+		PublicKey:         cred.PublicKey,
+		AttestationType:   cred.AttestationType,
+		AAGUID:            cred.Authenticator.AAGUID,
+		SignCount:         cred.Authenticator.SignCount,
+		CloneWarning:      false,
 	}
 
 	if err := db.Insert(ctx, c); err != nil {

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.CredentialIDBytes)
 
 	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,8 @@ 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),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v221.go

@@ -0,0 +1,82 @@
+// 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)"`
+		CredentialIDBytes []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"`
+	}
+	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]
+	}
+
+	// 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()
+}

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:"INDEX 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)
+	}
+}

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