-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
WebAuthn CredentialID field needs to be increased in size (#20530)
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>
- Loading branch information
Showing
10 changed files
with
363 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
...igrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/expected_webauthn_credential.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| - | ||
| id: 1 | ||
| credential_id: "TVHE44TOH7DF7V48SEAIT3EMMJ7TGBOQ289E5AQB34S98LFCUFJ7U2NAVI8RJG6K2F4TC8AQ8KBNO7AGEOQOL9NE43GR63HTEHJSLOG=" | ||
| - | ||
| id: 2 | ||
| credential_id: "051CLMMKB62S6M9M2A4H54K7MMCQALFJ36G4TGB2S9A47APLTILU6C6744CEBG4EKCGV357N21BSLH8JD33GQMFAR6DQ70S76P34J6FR=" | ||
| - | ||
| id: 4 | ||
| credential_id: "APU4B1NDTEVTEM60V4T0FRL7SRJMO9KIE2AKFQ8JDGTQ7VHFI41FDEFTDLBVQEAE4ER49QV2GTGVFDNBO31BPOA3OQN6879OT6MTU3G=" |
31 changes: 31 additions & 0 deletions
31
models/migrations/fixtures/Test_storeWebauthnCredentialIDAsBytes/webauthn_credential.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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() | ||
| } |
Oops, something went wrong.