From cb940c4312981893fdb54cbd0e07520546776b34 Mon Sep 17 00:00:00 2001 From: Norwin Date: Mon, 31 May 2021 08:25:47 +0000 Subject: [PATCH] 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 Co-authored-by: zeripath Co-authored-by: Lunny Xiao --- models/migrations/migrations.go | 2 + models/migrations/v180.go | 74 ++++++++++++++++++++++++++++++ models/task.go | 42 ++++++++++++++++- modules/migrations/base/options.go | 11 +++-- modules/task/task.go | 21 +++++++++ 5 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 models/migrations/v180.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index c54c383fb8..d440722c96 100644 --- 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 index 0000000000..c2a3ff961a --- /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 8d4bfbf076..a4ab65b5e5 100644 --- 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 f1d9f81e57..b12e1f94aa 100644 --- 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 0443517c01..0685aa23d7 100644 --- 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 {