access_token: encrypt access token with SHA256 (#7008)

* access_token: encrypt access token with SHA256

* revert list access token

* fix lint

* generate schemadoc

* add database migrations

* fix tests

* fix tests

* add test case for access token golden

* fix test in postgres

* `Sha256` -> `SHA256`

* Use GORM for migration

* task generate-schemadoc

* Use unique

* change migration name

* allow read

* task generate-schemadoc

* add changelog

* fix lint error

* update changelog

* remove Debug

* add comments

Co-authored-by: Joe Chen <jc@unknwon.io>

CHANGELOG.md

@@ -19,6 +19,7 @@ All notable changes to Gogs are documented in this file.
 - MSSQL as database backend is deprecated, installation page no longer shows it as an option. Existing installations and manually craft configuration file continue to work. [#6295](https://github.com/gogs/gogs/pull/6295)
 - Use [Task](https://github.com/go-task/task) as the build tool. [#6297](https://github.com/gogs/gogs/pull/6297)
 - The required Go version to compile source code changed to 1.16.
+- Access tokens are now stored using their SHA256 hashes instead of raw values. [#7008](https://github.com/gogs/gogs/pull/7008)
 
 ### Fixed
 

docs/dev/database_schema.md

@@ -14,14 +14,15 @@ Primary keys: id
 # Table "access_token"
 
 ```
-     FIELD    |    COLUMN    |     POSTGRESQL     |         MYSQL         |      SQLITE3        
+     FIELD    |    COLUMN    |         POSTGRESQL          |            MYSQL            |           SQLITE3            
---------------+--------------+--------------------+-----------------------+---------------------
+--------------+--------------+-----------------------------+-----------------------------+------------------------------
-  ID          | id           | BIGSERIAL          | BIGINT AUTO_INCREMENT | INTEGER             
+  ID          | id           | BIGSERIAL                   | BIGINT AUTO_INCREMENT       | INTEGER                      
-  UserID      | uid          | BIGINT             | BIGINT                | INTEGER             
+  UserID      | uid          | BIGINT                      | BIGINT                      | INTEGER                      
-  Name        | name         | TEXT               | LONGTEXT              | TEXT                
+  Name        | name         | TEXT                        | LONGTEXT                    | TEXT                         
-  Sha1        | sha1         | VARCHAR(40) UNIQUE | VARCHAR(40) UNIQUE    | VARCHAR(40) UNIQUE  
+  Sha1        | sha1         | VARCHAR(40) UNIQUE          | VARCHAR(40) UNIQUE          | VARCHAR(40) UNIQUE           
-  CreatedUnix | created_unix | BIGINT             | BIGINT                | INTEGER             
+  SHA256      | sha256       | VARCHAR(64) NOT NULL UNIQUE | VARCHAR(64) NOT NULL UNIQUE | VARCHAR(64) NOT NULL UNIQUE  
-  UpdatedUnix | updated_unix | BIGINT             | BIGINT                | INTEGER             
+  CreatedUnix | created_unix | BIGINT                      | BIGINT                      | INTEGER                      
+  UpdatedUnix | updated_unix | BIGINT                      | BIGINT                      | INTEGER                      
 
 Primary keys: id
 ```

internal/context/auth.go

@@ -130,7 +130,7 @@ func authenticatedUserID(c *macaron.Context, sess session.Store) (_ int64, isTok
 
 		// Let's see if token is valid.
 		if len(tokenSHA) > 0 {
-			t, err := db.AccessTokens.GetBySHA(tokenSHA)
+			t, err := db.AccessTokens.GetBySHA1(tokenSHA)
 			if err != nil {
 				if !db.IsErrAccessTokenNotExist(err) {
 					log.Error("GetAccessTokenBySHA: %v", err)

internal/cryptoutil/sha.go

@@ -6,6 +6,7 @@ package cryptoutil
 
 import (
 	"crypto/sha1"
+	"crypto/sha256"
 	"encoding/hex"
 )
 
@@ -15,3 +16,10 @@ func SHA1(str string) string {
 	_, _ = h.Write([]byte(str))
 	return hex.EncodeToString(h.Sum(nil))
 }
+
+// SHA256 encodes string to hexadecimal of SHA256 checksum.
+func SHA256(str string) string {
+	h := sha256.New()
+	_, _ = h.Write([]byte(str))
+	return hex.EncodeToString(h.Sum(nil))
+}

internal/cryptoutil/sha_test.go

@@ -25,3 +25,19 @@ func TestSHA1(t *testing.T) {
 		})
 	}
 }
+
+func TestSHA256(t *testing.T) {
+	tests := []struct {
+		input  string
+		output string
+	}{
+		{input: "", output: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"},
+		{input: "The quick brown fox jumps over the lazy dog", output: "d7a8fbb307d7809469ca9abcb0082e4f8d5651e46d3cdb762d02d0bf37c9e592"},
+		{input: "The quick brown fox jumps over the lazy dog.", output: "ef537f25c895bfa782526529a9b63d97aa631564d5d789c2b765448c8635fb6c"},
+	}
+	for _, test := range tests {
+		t.Run(test.input, func(t *testing.T) {
+			assert.Equal(t, test.output, SHA256(test.input))
+		})
+	}
+}

internal/db/access_tokens.go

@@ -27,9 +27,9 @@ type AccessTokensStore interface {
 	// 🚨 SECURITY: The "userID" is required to prevent attacker
 	// deletes arbitrary access token that belongs to another user.
 	DeleteByID(userID, id int64) error
-	// GetBySHA returns the access token with given SHA1.
+	// GetBySHA1 returns the access token with given SHA1.
 	// It returns ErrAccessTokenNotExist when not found.
-	GetBySHA(sha string) (*AccessToken, error)
+	GetBySHA1(sha1 string) (*AccessToken, error)
 	// List returns all access tokens belongs to given user.
 	List(userID int64) ([]*AccessToken, error)
 	// Save persists all values of given access token.
@@ -45,6 +45,7 @@ type AccessToken struct {
 	UserID int64 `xorm:"uid INDEX" gorm:"COLUMN:uid;INDEX"`
 	Name   string
 	Sha1   string `xorm:"UNIQUE VARCHAR(40)" gorm:"TYPE:VARCHAR(40);UNIQUE"`
+	SHA256 string `gorm:"type:VARCHAR(64);unique;not null"`
 
 	Created           time.Time `xorm:"-" gorm:"-" json:"-"`
 	CreatedUnix       int64
@@ -104,12 +105,22 @@ func (db *accessTokens) Create(userID int64, name string) (*AccessToken, error)
 		return nil, err
 	}
 
-	token := &AccessToken{
+	token := cryptoutil.SHA1(gouuid.NewV4().String())
+	sha256 := cryptoutil.SHA256(token)
+
+	accessToken := &AccessToken{
 		UserID: userID,
 		Name:   name,
-		Sha1:   cryptoutil.SHA1(gouuid.NewV4().String()),
+		Sha1:   sha256[:40], // To pass the column unique constraint, keep the length of SHA1.
+		SHA256: sha256,
 	}
-	return token, db.DB.Create(token).Error
+	if err = db.DB.Create(accessToken).Error; err != nil {
+		return nil, err
+	}
+
+	// Set back the raw access token value, for the sake of the caller.
+	accessToken.Sha1 = token
+	return accessToken, nil
 }
 
 func (db *accessTokens) DeleteByID(userID, id int64) error {
@@ -135,12 +146,13 @@ func (ErrAccessTokenNotExist) NotFound() bool {
 	return true
 }
 
-func (db *accessTokens) GetBySHA(sha string) (*AccessToken, error) {
+func (db *accessTokens) GetBySHA1(sha1 string) (*AccessToken, error) {
+	sha256 := cryptoutil.SHA256(sha1)
 	token := new(AccessToken)
-	err := db.Where("sha1 = ?", sha).First(token).Error
+	err := db.Where("sha256 = ?", sha256).First(token).Error
 	if err != nil {
 		if err == gorm.ErrRecordNotFound {
-			return nil, ErrAccessTokenNotExist{args: errutil.Args{"sha": sha}}
+			return nil, ErrAccessTokenNotExist{args: errutil.Args{"sha": sha1}}
 		}
 		return nil, err
 	}
@@ -149,7 +161,7 @@ func (db *accessTokens) GetBySHA(sha string) (*AccessToken, error) {
 
 func (db *accessTokens) List(userID int64) ([]*AccessToken, error) {
 	var tokens []*AccessToken
-	return tokens, db.Where("uid = ?", userID).Find(&tokens).Error
+	return tokens, db.Where("uid = ?", userID).Order("id ASC").Find(&tokens).Error
 }
 
 func (db *accessTokens) Save(t *AccessToken) error {

internal/db/access_tokens_test.go

@@ -57,7 +57,7 @@ func TestAccessTokens(t *testing.T) {
 	}{
 		{"Create", accessTokensCreate},
 		{"DeleteByID", accessTokensDeleteByID},
-		{"GetBySHA", accessTokensGetBySHA},
+		{"GetBySHA1", accessTokensGetBySHA},
 		{"List", accessTokensList},
 		{"Save", accessTokensSave},
 	} {
@@ -88,7 +88,7 @@ func accessTokensCreate(t *testing.T, db *accessTokens) {
 	assert.Equal(t, 40, len(token.Sha1), "sha1 length")
 
 	// Get it back and check the Created field
-	token, err = db.GetBySHA(token.Sha1)
+	token, err = db.GetBySHA1(token.Sha1)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -114,11 +114,11 @@ func accessTokensDeleteByID(t *testing.T, db *accessTokens) {
 	}
 
 	// We should be able to get it back
-	_, err = db.GetBySHA(token.Sha1)
+	_, err = db.GetBySHA1(token.Sha1)
 	if err != nil {
 		t.Fatal(err)
 	}
-	_, err = db.GetBySHA(token.Sha1)
+	_, err = db.GetBySHA1(token.Sha1)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -130,7 +130,7 @@ func accessTokensDeleteByID(t *testing.T, db *accessTokens) {
 	}
 
 	// We should get token not found error
-	_, err = db.GetBySHA(token.Sha1)
+	_, err = db.GetBySHA1(token.Sha1)
 	expErr := ErrAccessTokenNotExist{args: errutil.Args{"sha": token.Sha1}}
 	assert.Equal(t, expErr, err)
 }
@@ -143,13 +143,13 @@ func accessTokensGetBySHA(t *testing.T, db *accessTokens) {
 	}
 
 	// We should be able to get it back
-	_, err = db.GetBySHA(token.Sha1)
+	_, err = db.GetBySHA1(token.Sha1)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	// Try to get a non-existent token
-	_, err = db.GetBySHA("bad_sha")
+	_, err = db.GetBySHA1("bad_sha")
 	expErr := ErrAccessTokenNotExist{args: errutil.Args{"sha": "bad_sha"}}
 	assert.Equal(t, expErr, err)
 }
@@ -201,7 +201,7 @@ func accessTokensSave(t *testing.T, db *accessTokens) {
 	}
 
 	// Get back from DB should have Updated set
-	token, err = db.GetBySHA(token.Sha1)
+	token, err = db.GetBySHA1(token.Sha1)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/db/backup_test.go

@@ -63,6 +63,7 @@ func setupDBToDump(t *testing.T, db *gorm.DB) {
 			UserID:      1,
 			Name:        "test1",
 			Sha1:        cryptoutil.SHA1("2910d03d-c0b5-4f71-bad5-c4086e4efae3"),
+			SHA256:      cryptoutil.SHA256(cryptoutil.SHA1("2910d03d-c0b5-4f71-bad5-c4086e4efae3")),
 			CreatedUnix: 1588568886,
 			UpdatedUnix: 1588572486, // 1 hour later
 		},
@@ -70,12 +71,21 @@ func setupDBToDump(t *testing.T, db *gorm.DB) {
 			UserID:      1,
 			Name:        "test2",
 			Sha1:        cryptoutil.SHA1("84117e17-7e67-4024-bd04-1c23e6e809d4"),
+			SHA256:      cryptoutil.SHA256(cryptoutil.SHA1("84117e17-7e67-4024-bd04-1c23e6e809d4")),
 			CreatedUnix: 1588568886,
 		},
 		&AccessToken{
 			UserID:      2,
 			Name:        "test1",
 			Sha1:        cryptoutil.SHA1("da2775ce-73dd-47ba-b9d2-bbcc346585c4"),
+			SHA256:      cryptoutil.SHA256(cryptoutil.SHA1("da2775ce-73dd-47ba-b9d2-bbcc346585c4")),
+			CreatedUnix: 1588568886,
+		},
+		&AccessToken{
+			UserID:      2,
+			Name:        "test2",
+			Sha1:        cryptoutil.SHA256(cryptoutil.SHA1("1b2dccd1-a262-470f-bb8c-7fc73192e9bb"))[:40],
+			SHA256:      cryptoutil.SHA256(cryptoutil.SHA1("1b2dccd1-a262-470f-bb8c-7fc73192e9bb")),
 			CreatedUnix: 1588568886,
 		},
 

internal/db/db.go

@@ -142,6 +142,7 @@ var Tables = []interface{}{
 	new(LFSObject), new(LoginSource),
 }
 
+// Init initializes the database with given logger.
 func Init(w logger.Writer) (*gorm.DB, error) {
 	level := logger.Info
 	if conf.IsProdMode() {

internal/db/migrations/migrations.go

@@ -7,6 +7,7 @@ package migrations
 import (
 	"fmt"
 
+	"gorm.io/gorm"
 	log "unknwon.dev/clog/v2"
 	"xorm.io/xorm"
 )
@@ -15,15 +16,15 @@ const minDBVersion = 19
 
 type Migration interface {
 	Description() string
-	Migrate(*xorm.Engine) error
+	Migrate(*gorm.DB) error
 }
 
 type migration struct {
 	description string
-	migrate     func(*xorm.Engine) error
+	migrate     func(*gorm.DB) error
 }
 
-func NewMigration(desc string, fn func(*xorm.Engine) error) Migration {
+func NewMigration(desc string, fn func(*gorm.DB) error) Migration {
 	return &migration{desc, fn}
 }
 
@@ -31,11 +32,11 @@ func (m *migration) Description() string {
 	return m.description
 }
 
-func (m *migration) Migrate(x *xorm.Engine) error {
+func (m *migration) Migrate(db *gorm.DB) error {
-	return m.migrate(x)
+	return m.migrate(db)
 }
 
-// The version table. Should have only one row with id==1
+// Version represents the version table. It should have only one row with `id == 1`.
 type Version struct {
 	ID      int64
 	Version int64
@@ -52,10 +53,13 @@ var migrations = []Migration{
 	// Add new migration here, example:
 	// v18 -> v19:v0.11.55
 	// NewMigration("clean unlinked webhook and hook_tasks", cleanUnlinkedWebhookAndHookTasks),
+
+	// v19 -> v20:v0.13.0
+	NewMigration("migrate access tokens to store SHA56", migrateAccessTokenToSHA256),
 }
 
 // Migrate database to current version
-func Migrate(x *xorm.Engine) error {
+func Migrate(x *xorm.Engine, db *gorm.DB) error {
 	if err := x.Sync(new(Version)); err != nil {
 		return fmt.Errorf("sync: %v", err)
 	}
@@ -112,7 +116,7 @@ In case you're stilling getting this notice, go through instructions again until
 	}
 	for i, m := range migrations[v-minDBVersion:] {
 		log.Info("Migration: %s", m.Description())
-		if err = m.Migrate(x); err != nil {
+		if err = m.Migrate(db); err != nil {
 			return fmt.Errorf("do migrate: %v", err)
 		}
 		currentVersion.Version = v + int64(i) + 1

internal/db/migrations/v20.go

@@ -0,0 +1,54 @@
+// Copyright 2022 The Gogs 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 (
+	"github.com/pkg/errors"
+	"gorm.io/gorm"
+
+	"gogs.io/gogs/internal/cryptoutil"
+)
+
+func migrateAccessTokenToSHA256(db *gorm.DB) error {
+	return db.Transaction(func(tx *gorm.DB) error {
+		// 1. Add column without constraints because all rows have NULL values for the
+		// "sha256" column.
+		type accessToken struct {
+			ID     int64
+			Sha1   string
+			SHA256 string `gorm:"TYPE:VARCHAR(64)"`
+		}
+		err := tx.Migrator().AddColumn(&accessToken{}, "SHA256")
+		if err != nil {
+			return errors.Wrap(err, "add column")
+		}
+
+		// 2. Generate SHA256 for existing rows from their values in the "sha1" column.
+		var accessTokens []*accessToken
+		err = tx.Where("sha256 IS NULL").Find(&accessTokens).Error
+		if err != nil {
+			return errors.Wrap(err, "list")
+		}
+
+		for _, t := range accessTokens {
+			sha256 := cryptoutil.SHA256(t.Sha1)
+			err = tx.Model(&accessToken{}).Where("id = ?", t.ID).Update("sha256", sha256).Error
+			if err != nil {
+				return errors.Wrap(err, "update")
+			}
+		}
+
+		// 3. We are now safe to apply constraints to the "sha256" column.
+		type accessTokenWithConstraint struct {
+			SHA256 string `gorm:"type:VARCHAR(64);unique;not null"`
+		}
+		err = tx.Table("access_token").AutoMigrate(&accessTokenWithConstraint{})
+		if err != nil {
+			return errors.Wrap(err, "auto migrate")
+		}
+
+		return nil
+	})
+}

internal/db/mocks.go

@@ -17,7 +17,7 @@ var _ AccessTokensStore = (*MockAccessTokensStore)(nil)
 type MockAccessTokensStore struct {
 	MockCreate     func(userID int64, name string) (*AccessToken, error)
 	MockDeleteByID func(userID, id int64) error
-	MockGetBySHA   func(sha string) (*AccessToken, error)
+	MockGetBySHA1  func(sha string) (*AccessToken, error)
 	MockList       func(userID int64) ([]*AccessToken, error)
 	MockSave       func(t *AccessToken) error
 }
@@ -30,8 +30,8 @@ func (m *MockAccessTokensStore) DeleteByID(userID, id int64) error {
 	return m.MockDeleteByID(userID, id)
 }
 
-func (m *MockAccessTokensStore) GetBySHA(sha string) (*AccessToken, error) {
+func (m *MockAccessTokensStore) GetBySHA1(sha string) (*AccessToken, error) {
-	return m.MockGetBySHA(sha)
+	return m.MockGetBySHA1(sha)
 }
 
 func (m *MockAccessTokensStore) List(userID int64) ([]*AccessToken, error) {

internal/db/models.go

@@ -181,16 +181,17 @@ func SetEngine() (*gorm.DB, error) {
 }
 
 func NewEngine() (err error) {
-	if _, err = SetEngine(); err != nil {
+	db, err := SetEngine()
+	if err != nil {
 		return err
 	}
 
-	if err = migrations.Migrate(x); err != nil {
+	if err = migrations.Migrate(x, db); err != nil {
 		return fmt.Errorf("migrate: %v", err)
 	}
 
 	if err = x.StoreEngine("InnoDB").Sync2(legacyTables...); err != nil {
-		return fmt.Errorf("sync structs to database tables: %v\n", err)
+		return errors.Wrap(err, "sync tables")
 	}
 
 	return nil

internal/db/testdata/backup/AccessToken.golden.json

@@ -1,3 +1,4 @@
-{"ID":1,"UserID":1,"Name":"test1","Sha1":"56ed62d55225e9ae1275b1c4aa6e3de62f44e730","CreatedUnix":1588568886,"UpdatedUnix":1588572486}
+{"ID":1,"UserID":1,"Name":"test1","Sha1":"56ed62d55225e9ae1275b1c4aa6e3de62f44e730","SHA256":"d6ba6426326c71d24c0f42a3f266cae492b83fd727b9eb216004489f482fa42b","CreatedUnix":1588568886,"UpdatedUnix":1588572486}
-{"ID":2,"UserID":1,"Name":"test2","Sha1":"16fb74941e834e057d11c59db5d81cdae15be794","CreatedUnix":1588568886,"UpdatedUnix":0}
+{"ID":2,"UserID":1,"Name":"test2","Sha1":"16fb74941e834e057d11c59db5d81cdae15be794","SHA256":"fc9b958d5f2c382302e93d1dd24f296de2d87b0edc38e6e8d424b752ca0bcd99","CreatedUnix":1588568886,"UpdatedUnix":0}
-{"ID":3,"UserID":2,"Name":"test1","Sha1":"09f170f4ee70ba035587f7df8319b2a3a3d2b74a","CreatedUnix":1588568886,"UpdatedUnix":0}
+{"ID":3,"UserID":2,"Name":"test1","Sha1":"09f170f4ee70ba035587f7df8319b2a3a3d2b74a","SHA256":"e9a9cb1fb358ebc8009f4612c10dae7f2bcaa4de2ced2f4f6e4894c8eef31ed3","CreatedUnix":1588568886,"UpdatedUnix":0}
+{"ID":4,"UserID":2,"Name":"test2","Sha1":"97aae28f0aa2cc1b496424cbd2fd9eced51c584c","SHA256":"97aae28f0aa2cc1b496424cbd2fd9eced51c584c3179941efbe4e732a19a1dc8","CreatedUnix":1588568886,"UpdatedUnix":0}

internal/route/lfs/route.go

@@ -72,7 +72,7 @@ func authenticate() macaron.Handler {
 
 		// If username and password authentication failed, try again using username as an access token.
 		if auth.IsErrBadCredentials(err) {
-			token, err := db.AccessTokens.GetBySHA(username)
+			token, err := db.AccessTokens.GetBySHA1(username)
 			if err != nil {
 				if db.IsErrAccessTokenNotExist(err) {
 					askCredentials(c.Resp)

internal/route/lfs/route_test.go

@@ -75,7 +75,7 @@ func Test_authenticate(t *testing.T) {
 				},
 			},
 			mockAccessTokensStore: &db.MockAccessTokensStore{
-				MockGetBySHA: func(sha string) (*db.AccessToken, error) {
+				MockGetBySHA1: func(sha string) (*db.AccessToken, error) {
 					return nil, db.ErrAccessTokenNotExist{}
 				},
 			},
@@ -120,7 +120,7 @@ func Test_authenticate(t *testing.T) {
 				},
 			},
 			mockAccessTokensStore: &db.MockAccessTokensStore{
-				MockGetBySHA: func(sha string) (*db.AccessToken, error) {
+				MockGetBySHA1: func(sha string) (*db.AccessToken, error) {
 					return &db.AccessToken{}, nil
 				},
 				MockSave: func(t *db.AccessToken) error {

internal/route/repo/http.go

@@ -131,7 +131,7 @@ func HTTPContexter() macaron.Handler {
 
 		// If username and password combination failed, try again using username as a token.
 		if authUser == nil {
-			token, err := db.AccessTokens.GetBySHA(authUsername)
+			token, err := db.AccessTokens.GetBySHA1(authUsername)
 			if err != nil {
 				if db.IsErrAccessTokenNotExist(err) {
 					askCredentials(c, http.StatusUnauthorized, "")