Encrypt migration credentials at rest (#15895)

* encrypt migration credentials in task persistence

Not sure this is the best approach, we could encrypt the entire
`PayloadContent` instead. Also instead of clearing individual fields in
payload content, we could just delete the task once it has
(successfully) finished..?

* remove credentials of past migrations

* only run DB migration for completed tasks

* fix binding

* add omitempty

* never serialize unencrypted credentials

* fix import order

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

models/migrations/migrations.go

@@ -309,6 +309,8 @@ var migrations = []Migration{
 	NewMigration("Add LFS columns to Mirror", addLFSMirrorColumns),
 	// v179 -> v180
 	NewMigration("Convert avatar url to text", convertAvatarURLToText),
+	// v180 -> v181
+	NewMigration("Delete credentials from past migrations", deleteMigrationCredentials),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v180.go

@@ -0,0 +1,74 @@
+// Copyright 2021 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 (
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/migrations/base"
+	"code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
+
+	jsoniter "github.com/json-iterator/go"
+	"xorm.io/builder"
+	"xorm.io/xorm"
+)
+
+func deleteMigrationCredentials(x *xorm.Engine) (err error) {
+	const batchSize = 100
+
+	// only match migration tasks, that are not pending or running
+	cond := builder.Eq{
+		"type": structs.TaskTypeMigrateRepo,
+	}.And(builder.Gte{
+		"status": structs.TaskStatusStopped,
+	})
+
+	sess := x.NewSession()
+	defer sess.Close()
+
+	for start := 0; ; start += batchSize {
+		tasks := make([]*models.Task, 0, batchSize)
+		if err = sess.Limit(batchSize, start).Where(cond, 0).Find(&tasks); err != nil {
+			return
+		}
+		if len(tasks) == 0 {
+			break
+		}
+		if err = sess.Begin(); err != nil {
+			return
+		}
+		for _, t := range tasks {
+			if t.PayloadContent, err = removeCredentials(t.PayloadContent); err != nil {
+				return
+			}
+			if _, err = sess.ID(t.ID).Cols("payload_content").Update(t); err != nil {
+				return
+			}
+		}
+		if err = sess.Commit(); err != nil {
+			return
+		}
+	}
+	return
+}
+
+func removeCredentials(payload string) (string, error) {
+	var opts base.MigrateOptions
+	json := jsoniter.ConfigCompatibleWithStandardLibrary
+	err := json.Unmarshal([]byte(payload), &opts)
+	if err != nil {
+		return "", err
+	}
+
+	opts.AuthPassword = ""
+	opts.AuthToken = ""
+	opts.CloneAddr = util.SanitizeURLCredentials(opts.CloneAddr, true)
+
+	confBytes, err := json.Marshal(opts)
+	if err != nil {
+		return "", err
+	}
+	return string(confBytes), nil
+}

models/task.go

@@ -8,8 +8,11 @@ import (
 	"fmt"
 
 	migration "code.gitea.io/gitea/modules/migrations/base"
+	"code.gitea.io/gitea/modules/secret"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 	jsoniter "github.com/json-iterator/go"
 
 	"xorm.io/builder"
@@ -110,6 +113,24 @@ func (task *Task) MigrateConfig() (*migration.MigrateOptions, error) {
 		if err != nil {
 			return nil, err
 		}
+
+		// decrypt credentials
+		if opts.CloneAddrEncrypted != "" {
+			if opts.CloneAddr, err = secret.DecryptSecret(setting.SecretKey, opts.CloneAddrEncrypted); err != nil {
+				return nil, err
+			}
+		}
+		if opts.AuthPasswordEncrypted != "" {
+			if opts.AuthPassword, err = secret.DecryptSecret(setting.SecretKey, opts.AuthPasswordEncrypted); err != nil {
+				return nil, err
+			}
+		}
+		if opts.AuthTokenEncrypted != "" {
+			if opts.AuthToken, err = secret.DecryptSecret(setting.SecretKey, opts.AuthTokenEncrypted); err != nil {
+				return nil, err
+			}
+		}
+
 		return &opts, nil
 	}
 	return nil, fmt.Errorf("Task type is %s, not Migrate Repo", task.Type.Name())
@@ -205,12 +226,31 @@ func createTask(e Engine, task *Task) error {
 func FinishMigrateTask(task *Task) error {
 	task.Status = structs.TaskStatusFinished
 	task.EndTime = timeutil.TimeStampNow()
+
+	// delete credentials when we're done, they're a liability.
+	conf, err := task.MigrateConfig()
+	if err != nil {
+		return err
+	}
+	conf.AuthPassword = ""
+	conf.AuthToken = ""
+	conf.CloneAddr = util.SanitizeURLCredentials(conf.CloneAddr, true)
+	conf.AuthPasswordEncrypted = ""
+	conf.AuthTokenEncrypted = ""
+	conf.CloneAddrEncrypted = ""
+	json := jsoniter.ConfigCompatibleWithStandardLibrary
+	confBytes, err := json.Marshal(conf)
+	if err != nil {
+		return err
+	}
+	task.PayloadContent = string(confBytes)
+
 	sess := x.NewSession()
 	defer sess.Close()
 	if err := sess.Begin(); err != nil {
 		return err
 	}
-	if _, err := sess.ID(task.ID).Cols("status", "end_time").Update(task); err != nil {
+	if _, err := sess.ID(task.ID).Cols("status", "end_time", "payload_content").Update(task); err != nil {
 		return err
 	}
 

modules/migrations/base/options.go

@@ -12,9 +12,12 @@ import "code.gitea.io/gitea/modules/structs"
 type MigrateOptions struct {
 	// required: true
 	CloneAddr             string `json:"clone_addr" binding:"Required"`
+	CloneAddrEncrypted    string `json:"clone_addr_encrypted,omitempty"`
 	AuthUsername          string `json:"auth_username"`
-	AuthPassword string `json:"auth_password"`
-	AuthToken    string `json:"auth_token"`
+	AuthPassword          string `json:"-"`
+	AuthPasswordEncrypted string `json:"auth_password_encrypted,omitempty"`
+	AuthToken             string `json:"-"`
+	AuthTokenEncrypted    string `json:"auth_token_encrypted,omitempty"`
 	// required: true
 	UID int `json:"uid" binding:"Required"`
 	// required: true

modules/task/task.go

@@ -13,8 +13,11 @@ import (
 	"code.gitea.io/gitea/modules/migrations/base"
 	"code.gitea.io/gitea/modules/queue"
 	repo_module "code.gitea.io/gitea/modules/repository"
+	"code.gitea.io/gitea/modules/secret"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
+	"code.gitea.io/gitea/modules/util"
 	jsoniter "github.com/json-iterator/go"
 )
 
@@ -65,6 +68,24 @@ func MigrateRepository(doer, u *models.User, opts base.MigrateOptions) error {
 
 // CreateMigrateTask creates a migrate task
 func CreateMigrateTask(doer, u *models.User, opts base.MigrateOptions) (*models.Task, error) {
+	// encrypt credentials for persistence
+	var err error
+	opts.CloneAddrEncrypted, err = secret.EncryptSecret(setting.SecretKey, opts.CloneAddr)
+	if err != nil {
+		return nil, err
+	}
+	opts.CloneAddr = util.SanitizeURLCredentials(opts.CloneAddr, true)
+	opts.AuthPasswordEncrypted, err = secret.EncryptSecret(setting.SecretKey, opts.AuthPassword)
+	if err != nil {
+		return nil, err
+	}
+	opts.AuthPassword = ""
+	opts.AuthTokenEncrypted, err = secret.EncryptSecret(setting.SecretKey, opts.AuthToken)
+	if err != nil {
+		return nil, err
+	}
+	opts.AuthToken = ""
+
 	json := jsoniter.ConfigCompatibleWithStandardLibrary
 	bs, err := json.Marshal(&opts)
 	if err != nil {