Nulo/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

WebAuthn CredentialID field needs to be increased in size (#20530)

Browse source 
WebAuthn have updated their specification to set the maximum size of the
CredentialID to 1023 bytes. This is somewhat larger than our current
size and therefore we need to migrate.

The PR changes the struct to add CredentialIDBytes and migrates the CredentialID string 
to the bytes field before another migration drops the old CredentialID field. Another migration
renames this field back.

Fix #20457

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2022-07-30 14:25:26 +01:00 committed by GitHub
parent 692707f145
commit e819da0837
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 363 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
models/auth/webauthn.go
View file

@ -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,

5
models/auth/webauthn_test.go
View file

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

9
models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml Normal file
View file

@ -0,0 +1,9 @@
-
id: 1
credential_id: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG="
-
id: 2
credential_id: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR="
-
id: 4
credential_id: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G="

31
models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml Normal file
View file

@ -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

6
models/migrations/migrations.go
View file

@ -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

75
models/migrations/v221.go Normal file
View file

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

65
models/migrations/v221_test.go Normal file
View file

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

64
models/migrations/v222.go Normal file
View file

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

103
models/migrations/v223.go Normal file
View file

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

3
routers/web/auth/webauthn.go
View file

@ -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