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>

diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index c54c383fb810d21dea5ddd2f3fe9ec5fe0000361..d440722c96f8cd264966bb25d3c48c314d474543 100644 (file)
--- a/models/migrations/migrations.go
+++ b/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

diff --git a/models/migrations/v180.go b/models/migrations/v180.go
new file mode 100644 (file)
index 0000000..c2a3ff9
--- /dev/null
+++ b/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
+}

diff --git a/models/task.go b/models/task.go
index 8d4bfbf0760771109ed31bde6a8fec69d05da35a..a4ab65b5e5e17ed839333fd08b7d430cc00de2f3 100644 (file)
--- a/models/task.go
+++ b/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
        }
 

diff --git a/modules/migrations/base/options.go b/modules/migrations/base/options.go
index f1d9f81e575be8b1f741d83b19cb9f8519393910..b12e1f94aa4a6784ec26ba3aeff2dc1047ec6fb3 100644 (file)
--- a/modules/migrations/base/options.go
+++ b/modules/migrations/base/options.go
@@ -11,10 +11,13 @@ import "code.gitea.io/gitea/modules/structs"
 // this is for internal usage by migrations module and func who interact with it
 type MigrateOptions struct {
        // required: true
-       CloneAddr    string `json:"clone_addr" binding:"Required"`
-       AuthUsername string `json:"auth_username"`
-       AuthPassword string `json:"auth_password"`
-       AuthToken    string `json:"auth_token"`
+       CloneAddr             string `json:"clone_addr" binding:"Required"`
+       CloneAddrEncrypted    string `json:"clone_addr_encrypted,omitempty"`
+       AuthUsername          string `json:"auth_username"`
+       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

diff --git a/modules/task/task.go b/modules/task/task.go
index 0443517c01694eee65d0e36b93bc54b34041bcab..0685aa23d743a1c5c9885838d44a74f9faaa380a 100644 (file)
--- a/modules/task/task.go
+++ b/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 {