34 changes: 34 additions & 0 deletions routers/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package install

import (
goctx "context"
"encoding/base64"
"fmt"
"net/http"
"os"
Expand Down Expand Up @@ -33,6 +34,7 @@ import (
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/modules/web/middleware"
"code.gitea.io/gitea/services/forms"
"code.gitea.io/gitea/services/secrets"

"gitea.com/go-chi/session"
"gopkg.in/ini.v1"
Expand Down Expand Up @@ -162,6 +164,7 @@ func Install(ctx *context.Context) {
form.DefaultEnableTimetracking = setting.Service.DefaultEnableTimetracking
form.NoReplyAddress = setting.Service.NoReplyAddress
form.PasswordAlgorithm = setting.PasswordHashAlgo
form.MasterKeyProvider = secrets.MasterKeyProviderTypePlain

middleware.AssignForm(form, ctx.Data)
ctx.HTML(http.StatusOK, tplInstall)
Expand Down Expand Up @@ -387,10 +390,40 @@ func SubmitInstall(ctx *context.Context) {
log.Error("Failed to load custom conf '%s': %v", setting.CustomConf, err)
}
}

// Setup master key provider
cfg.Section("security").Key("MASTER_KEY_PROVIDER").SetValue(string(form.MasterKeyProvider))
var provider secrets.MasterKeyProvider
switch form.MasterKeyProvider {
case secrets.MasterKeyProviderTypePlain:
provider = secrets.NewPlainMasterKeyProvider()
}
var masterKey []byte
if provider != nil {
if err = provider.Init(); err != nil {
ctx.RenderWithErr(ctx.Tr("install.master_key_failed", err), tplInstall, &form)
return
}
// Generate master key
if _, err = provider.GenerateMasterKey(); err != nil {
ctx.RenderWithErr(ctx.Tr("install.master_key_failed", err), tplInstall, &form)
return
}
masterKey, err = provider.GetMasterKey()
if err != nil {
ctx.RenderWithErr(ctx.Tr("install.master_key_failed", err), tplInstall, &form)
return
}
if form.MasterKeyProvider == secrets.MasterKeyProviderTypePlain {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if form.MasterKeyProvider == secrets.MasterKeyProviderTypePlain {
if form.MasterKeyProvider != secrets.MasterKeyProviderTypeNone {

?

cfg.Section("security").Key("MASTER_KEY").SetValue(base64.StdEncoding.EncodeToString(masterKey))
}
}

cfg.Section("database").Key("DB_TYPE").SetValue(setting.Database.Type)
cfg.Section("database").Key("HOST").SetValue(setting.Database.Host)
cfg.Section("database").Key("NAME").SetValue(setting.Database.Name)
cfg.Section("database").Key("USER").SetValue(setting.Database.User)
// TODO: Encrypt secret
cfg.Section("database").Key("PASSWD").SetValue(setting.Database.Passwd)
cfg.Section("database").Key("SCHEMA").SetValue(setting.Database.Schema)
cfg.Section("database").Key("SSL_MODE").SetValue(setting.Database.SSLMode)
Expand Down Expand Up @@ -432,6 +465,7 @@ func SubmitInstall(ctx *context.Context) {
cfg.Section("mailer").Key("SMTP_PORT").SetValue(form.SMTPPort)
cfg.Section("mailer").Key("FROM").SetValue(form.SMTPFrom)
cfg.Section("mailer").Key("USER").SetValue(form.SMTPUser)
// TODO: Encrypt secret
cfg.Section("mailer").Key("PASSWD").SetValue(form.SMTPPasswd)
} else {
cfg.Section("mailer").Key("ENABLED").SetValue("false")
Expand Down
47 changes: 47 additions & 0 deletions routers/web/org/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"code.gitea.io/gitea/services/org"
container_service "code.gitea.io/gitea/services/packages/container"
repo_service "code.gitea.io/gitea/services/repository"
secret_service "code.gitea.io/gitea/services/secrets"
user_service "code.gitea.io/gitea/services/user"
)

Expand All @@ -37,6 +38,8 @@ const (
tplSettingsHooks base.TplName = "org/settings/hooks"
// tplSettingsLabels template path for render labels settings
tplSettingsLabels base.TplName = "org/settings/labels"
// tplSettingsSecrets template path for render secrets settings
tplSettingsSecrets base.TplName = "org/settings/secrets"
)

// Settings render the main settings page
Expand Down Expand Up @@ -246,3 +249,47 @@ func Labels(ctx *context.Context) {
ctx.Data["LabelTemplates"] = repo_module.LabelTemplates
ctx.HTML(http.StatusOK, tplSettingsLabels)
}

// Secrets render organization secrets page
func Secrets(ctx *context.Context) {
ctx.Data["Title"] = ctx.Tr("repo.secrets")
ctx.Data["PageIsOrgSettings"] = true
ctx.Data["PageIsOrgSettingsSecrets"] = true
ctx.Data["RequireTribute"] = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we remove that?


secrets, err := secret_service.FindOwnerSecrets(ctx, ctx.Org.Organization.ID)
if err != nil {
ctx.ServerError("FindOwnerSecrets", err)
return
}
ctx.Data["Secrets"] = secrets

ctx.HTML(http.StatusOK, tplSettingsSecrets)
}

// SecretsPost add secrets
func SecretsPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.AddSecretForm)
if err := secret_service.InsertOwnerSecret(ctx, ctx.Org.Organization.ID, form.Title, form.Content); err != nil {
ctx.ServerError("InsertOwnerSecret", err)
return
}

log.Trace("Org %d: secret added", ctx.Org.Organization.ID)
ctx.Flash.Success(ctx.Tr("repo.settings.add_secret_success", form.Title))
ctx.Redirect(ctx.Org.OrgLink + "/settings/secrets")
}

// SecretsDelete delete secrets
func SecretsDelete(ctx *context.Context) {
if err := secret_service.DeleteSecretByID(ctx, ctx.FormInt64("id")); err != nil {
ctx.Flash.Error(ctx.Tr("repo.settings.secret_deletion_failed"))
log.Error("delete secret %d: %v", ctx.FormInt64("id"), err)
} else {
ctx.Flash.Success(ctx.Tr("repo.settings.secret_deletion_success"))
}

ctx.JSON(http.StatusOK, map[string]interface{}{
"redirect": ctx.Org.OrgLink + "/settings/secrets",
})
}
45 changes: 45 additions & 0 deletions routers/web/repo/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
mirror_service "code.gitea.io/gitea/services/mirror"
org_service "code.gitea.io/gitea/services/org"
repo_service "code.gitea.io/gitea/services/repository"
secret_service "code.gitea.io/gitea/services/secrets"
wiki_service "code.gitea.io/gitea/services/wiki"
)

Expand Down Expand Up @@ -1113,12 +1114,38 @@ func DeployKeys(ctx *context.Context) {
}
ctx.Data["Deploykeys"] = keys

secrets, err := secret_service.FindRepoSecrets(ctx, ctx.Repo.Repository.ID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, that's only half of the answer.
The imported org secrets are currently not shown, which is not a good idea for consistency.
GitHub does this nicely, they show what org secrets are available as well (read-only) and simply redirect to the org secrets settings page where you can do whatever you want with them once you click on an org secret.
That also has the benefit that we don't care about permissions or similar, since a user that has owner access in a repo must also have owner access in the org to access that page.

if err != nil {
ctx.ServerError("FindRepoSecrets", err)
return
}
ctx.Data["Secrets"] = secrets

ctx.HTML(http.StatusOK, tplDeployKeys)
}

// SecretsPost response for creating a new secret
func SecretsPost(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.AddKeyForm)
if err := secret_service.InsertRepoSecret(ctx, ctx.Repo.Repository.ID, form.Title, form.Content); err != nil {
ctx.ServerError("InsertRepoSecret", err)
return
}

log.Trace("Secret added: %d", ctx.Repo.Repository.ID)
ctx.Flash.Success(ctx.Tr("repo.settings.add_secret_success", form.Title))
ctx.Redirect(ctx.Repo.RepoLink + "/settings/keys")
}

// DeployKeysPost response for adding a deploy key of a repository
func DeployKeysPost(ctx *context.Context) {
if ctx.FormString("act") == "secret" {
SecretsPost(ctx)
return
}
Comment on lines +1142 to +1145
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this check? I do not think mixing handlers is correct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put the secrets in the same page as deploy keys. So I reused the Post method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we should use a different route?
Because I'm not a big fan of requiring query parameters to be present so that it works as intended…


form := web.GetForm(ctx).(*forms.AddKeyForm)

ctx.Data["Title"] = ctx.Tr("repo.settings.deploy_keys")
ctx.Data["PageIsSettingsKeys"] = true
ctx.Data["DisableSSH"] = setting.SSH.Disabled
Expand Down Expand Up @@ -1177,8 +1204,26 @@ func DeployKeysPost(ctx *context.Context) {
ctx.Redirect(ctx.Repo.RepoLink + "/settings/keys")
}

func DeleteSecret(ctx *context.Context) {
if err := secret_service.DeleteSecretByID(ctx, ctx.FormInt64("id")); err != nil {
ctx.Flash.Error(ctx.Tr("repo.settings.secret_deletion_failed"))
log.Error("delete secret %d: %v", ctx.FormInt64("id"), err)
} else {
ctx.Flash.Success(ctx.Tr("repo.settings.secret_deletion_success"))
}

ctx.JSON(http.StatusOK, map[string]interface{}{
"redirect": ctx.Repo.RepoLink + "/settings/keys",
})
}

// DeleteDeployKey response for deleting a deploy key
func DeleteDeployKey(ctx *context.Context) {
if ctx.FormString("act") == "secret" {
DeleteSecret(ctx)
return
}

if err := asymkey_service.DeleteDeployKey(ctx.Doer, ctx.FormInt64("id")); err != nil {
ctx.Flash.Error("DeleteDeployKey: " + err.Error())
} else {
Expand Down
6 changes: 6 additions & 0 deletions routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,12 @@ func RegisterRoutes(m *web.Route) {
m.Post("/initialize", web.Bind(forms.InitializeLabelsForm{}), org.InitializeLabels)
})

m.Group("/secrets", func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm almost in favor of setting context.OrgAssignment(true, true) here as a duplicate middleware simply to ensure that there is no chance ever that someone changes the permissions below and doesn't notice that the secrets are now publicly available.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's unnecessary. Even if it does happen, the secret value is still not visible.

image

m.Get("", org.Secrets)
m.Post("", web.Bind(forms.AddSecretForm{}), org.SecretsPost)
m.Post("/delete", org.SecretsDelete)
})

m.Route("/delete", "GET,POST", org.SettingsDelete)

m.Group("/packages", func() {
Expand Down
14 changes: 14 additions & 0 deletions services/forms/user_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/web/middleware"
"code.gitea.io/gitea/services/secrets"

"gitea.com/go-chi/binding"
)
Expand Down Expand Up @@ -63,6 +64,7 @@ type InstallForm struct {
NoReplyAddress string

PasswordAlgorithm string
MasterKeyProvider secrets.MasterKeyProviderType `binding:"Required;In(none,plain)"`

AdminName string `binding:"OmitEmpty;Username;MaxSize(30)" locale:"install.admin_name"`
AdminPasswd string `binding:"OmitEmpty;MaxSize(255)" locale:"install.admin_password"`
Expand Down Expand Up @@ -363,6 +365,18 @@ func (f *AddKeyForm) Validate(req *http.Request, errs binding.Errors) binding.Er
return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
}

// AddSecretForm for adding secrets
type AddSecretForm struct {
Title string `binding:"Required;MaxSize(50)"`
Content string `binding:"Required"`
}

// Validate validates the fields
func (f *AddSecretForm) Validate(req *http.Request, errs binding.Errors) binding.Errors {
ctx := context.GetContext(req)
return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
}

// NewAccessTokenForm form for creating access token
type NewAccessTokenForm struct {
Name string `binding:"Required;MaxSize(255)"`
Expand Down
5 changes: 5 additions & 0 deletions services/org/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/util"
secret_service "code.gitea.io/gitea/services/secrets"
)

// DeleteOrganization completely and permanently deletes everything of organization.
Expand All @@ -39,6 +40,10 @@ func DeleteOrganization(org *organization.Organization) error {
return models.ErrUserOwnPackages{UID: org.ID}
}

if err := secret_service.DeleteSecretsByOwnerID(ctx, org.ID); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you would simply use models/DeleteUser#72-91.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved in the new PR.

return err
}

if err := organization.DeleteOrganization(ctx, org); err != nil {
return fmt.Errorf("DeleteOrganization: %w", err)
}
Expand Down
5 changes: 5 additions & 0 deletions services/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting"
pull_service "code.gitea.io/gitea/services/pull"
secret_service "code.gitea.io/gitea/services/secrets"
)

// CreateRepository creates a repository for the user/organization.
Expand Down Expand Up @@ -51,6 +52,10 @@ func DeleteRepository(ctx context.Context, doer *user_model.User, repo *repo_mod
return err
}

if err := secret_service.DeleteSecretsByRepoID(ctx, repo.ID); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you would simply use models.DeleteRepository#130-152 for it…

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved in the new PR.

return err
}

return packages_model.UnlinkRepositoryFromAllPackages(ctx, repo.ID)
}

Expand Down
15 changes: 15 additions & 0 deletions services/secrets/encryption.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

// EncryptionProvider encrypts and decrypts secrets
type EncryptionProvider interface {
Encrypt(secret, key []byte) ([]byte, error)

EncryptString(secret string, key []byte) (string, error)

Decrypt(enc, key []byte) ([]byte, error)

DecryptString(enc string, key []byte) (string, error)
}
87 changes: 87 additions & 0 deletions services/secrets/encryption_aes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

import (
"crypto/aes"
"crypto/cipher"
"crypto/rand"
"encoding/base64"
"fmt"
"io"
)

type aesEncryptionProvider struct{}

func NewAesEncryptionProvider() EncryptionProvider {
return &aesEncryptionProvider{}
}

func (e *aesEncryptionProvider) Encrypt(secret, key []byte) ([]byte, error) {
block, err := aes.NewCipher(key)
if err != nil {
return nil, err
}

c, err := cipher.NewGCM(block)
if err != nil {
return nil, err
}

nonce := make([]byte, c.NonceSize(), c.NonceSize()+c.Overhead()+len(secret))
if _, err = io.ReadFull(rand.Reader, nonce); err != nil {
return nil, err
}
out := c.Seal(nil, nonce, secret, nil)

return append(nonce, out...), nil
}

func (e *aesEncryptionProvider) EncryptString(secret string, key []byte) (string, error) {
out, err := e.Encrypt([]byte(secret), key)
if err != nil {
return "", err
}
return base64.StdEncoding.EncodeToString(out), nil
}

func (e *aesEncryptionProvider) Decrypt(enc, key []byte) ([]byte, error) {
block, err := aes.NewCipher(key)
if err != nil {
return nil, err
}

c, err := cipher.NewGCM(block)
if err != nil {
return nil, err
}

if len(enc) < c.NonceSize() {
return nil, fmt.Errorf("encrypted value has length %d, which is too short for expected %d", len(enc), c.NonceSize())
}

nonce := enc[:c.NonceSize()]
ciphertext := enc[c.NonceSize():]

out, err := c.Open(nil, nonce, ciphertext, nil)
if err != nil {
return nil, err
}

return out, nil
}

func (e *aesEncryptionProvider) DecryptString(enc string, key []byte) (string, error) {
encb, err := base64.StdEncoding.DecodeString(enc)
if err != nil {
return "", err
}

out, err := e.Decrypt(encb, key)
if err != nil {
return "", err
}

return string(out), nil
}
21 changes: 21 additions & 0 deletions services/secrets/encryption_aes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestEncryptDecrypt(t *testing.T) {
provider := NewAesEncryptionProvider()
key := []byte("1111111111111111")
pri := "vvvvvvv"
enc, err := provider.EncryptString(pri, key)
assert.NoError(t, err)
v, err := provider.DecryptString(enc, key)
assert.NoError(t, err)
assert.EqualValues(t, pri, v)
}
26 changes: 26 additions & 0 deletions services/secrets/masterkey.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

import (
"fmt"
)

// ErrMasterKeySealed is returned when trying to use master key that is sealed
var ErrMasterKeySealed = fmt.Errorf("master key sealed")

// MasterKeyProvider provides master key used for encryption
type MasterKeyProvider interface {
Init() error

GenerateMasterKey() ([][]byte, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will there ever be providers which return multiple secrets? If yes, why does the Unseal method only take one secret?

Copy link
Member Author

@lafriks lafriks Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes there could be for example using Shamir's Secret Sharing (https://github.com/lafriks/go-shamir) where master key would be split to multiple parts. I initially wanted to add this also but that would require too many changes so I left it out. It would require gitea to start in unsealed state (similar to install lock = false state) and would switch to normal state only after X parts of Y part secrets are provided to it.
So process would be:

  • Configure Gitea to have master key split into 5 parts where 3 parts would be required to unseal it.
  • Generate 5 secrets and give it to 5 persons
  • Gitea starts in unsealed state
  • 3 of 5 persons would have to connect to gitea server and provide their part individually using Unseal method (for example through cli).
  • Gitea changes into normal state and start accepting requests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this system replace the SECRET_KEY ini setting? Because I do not understand why these secrets should be handled differently than the current secrets (mirror credentials, webhooks).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SECRET_KEY could be stored encrypted by MASTER_KEY in the future when we add server-wide secret storage we could than use "internal" secrets that would be protected by master key

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only secrets we can not protect even in future by master key are INTERNAL_TOKEN and database PASSWD

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes there could be for example using Shamir's Secret Sharing (https://github.com/lafriks/go-shamir) where master key would be split to multiple parts.

Perhaps you should document that inside the function signature to not confuse future developers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still applies.


Unseal(secret []byte) error

Seal() error
Comment on lines +19 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write a comment on what these methods are supposed to do…
I don't really know what I have to think of a sealed or unsealed provider. Is it unable to generate further secrets, or what is its purpose?


IsSealed() bool

GetMasterKey() ([]byte, error)
}
41 changes: 41 additions & 0 deletions services/secrets/masterkey_nop.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

type nopMasterKeyProvider struct{}

// NewNopMasterKeyProvider returns master key provider that holds no master key and is always unsealed
func NewNopMasterKeyProvider() MasterKeyProvider {
return &nopMasterKeyProvider{}
}

// Init initializes master key provider
func (k *nopMasterKeyProvider) Init() error {
return nil
}

// GenerateMasterKey always returns empty master key
func (k *nopMasterKeyProvider) GenerateMasterKey() ([][]byte, error) {
return nil, nil
}

// Unseal master key by providing unsealing secret
func (k *nopMasterKeyProvider) Unseal(secret []byte) error {
return nil
}

// Seal master key
func (k *nopMasterKeyProvider) Seal() error {
return nil
}

// IsSealed always returns false
func (k *nopMasterKeyProvider) IsSealed() bool {
return false
}

// GetMasterKey returns empty master key
func (k *nopMasterKeyProvider) GetMasterKey() ([]byte, error) {
return nil, nil
}
15 changes: 15 additions & 0 deletions services/secrets/masterkey_nop_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestNopMasterKey_IsSealed(t *testing.T) {
k := NewNopMasterKeyProvider()
assert.False(t, k.IsSealed())
}
58 changes: 58 additions & 0 deletions services/secrets/masterkey_plain.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package secrets

import (
"code.gitea.io/gitea/modules/generate"
"code.gitea.io/gitea/modules/setting"
)

type plainMasterKeyProvider struct {
key []byte
}

// NewPlainMasterKeyProvider returns unsecured static master key provider
func NewPlainMasterKeyProvider() MasterKeyProvider {
return &plainMasterKeyProvider{}
}

// Init initializes master key provider
func (k *plainMasterKeyProvider) Init() error {
return k.Unseal(nil)
}

// GenerateMasterKey generates a new master key and returns secret or secrets for unsealing
func (k *plainMasterKeyProvider) GenerateMasterKey() ([][]byte, error) {
key, err := generate.NewMasterKey()
if err != nil {
return nil, err
}
k.key = key
return [][]byte{key}, nil
}

// Unseal master key by providing unsealing secret
func (k *plainMasterKeyProvider) Unseal(secret []byte) error {
k.key = setting.MasterKey
return nil
}

// Seal master key
func (k *plainMasterKeyProvider) Seal() error {
k.key = nil
return nil
}

// IsSealed returns if master key is sealed
func (k *plainMasterKeyProvider) IsSealed() bool {
return len(k.key) == 0
}

// GetMasterKey returns master key
func (k *plainMasterKeyProvider) GetMasterKey() ([]byte, error) {
if k.IsSealed() {
return nil, ErrMasterKeySealed
}
return k.key, nil
}
155 changes: 155 additions & 0 deletions services/secrets/secrets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we copyright the year the code was written, or the year it is merged?
I would have thought it is the year it is merged…

// SPDX-License-Identifier: MIT

package secrets

import (
"context"
"fmt"

auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/setting"

"xorm.io/builder"
)

// MasterKeyProviderType is the type of master key provider
type MasterKeyProviderType string

// Types of master key providers
const (
MasterKeyProviderTypeNone MasterKeyProviderType = "none"
MasterKeyProviderTypePlain MasterKeyProviderType = "plain"
)

var (
masterKey MasterKeyProvider
encProvider EncryptionProvider
)

// Init initializes master key provider based on settings
func Init() error {
switch MasterKeyProviderType(setting.MasterKeyProvider) {
case MasterKeyProviderTypeNone:
masterKey = NewNopMasterKeyProvider()
case MasterKeyProviderTypePlain:
masterKey = NewPlainMasterKeyProvider()
default:
return fmt.Errorf("invalid master key provider %v", setting.MasterKeyProvider)
}

if err := masterKey.Init(); err != nil {
return err
}

encProvider = NewAesEncryptionProvider()

return nil
}

// GenerateMasterKey generates a new master key and returns secret or secrets for unsealing
func GenerateMasterKey() ([][]byte, error) {
return masterKey.GenerateMasterKey()
}

func Encrypt(secret []byte) ([]byte, error) {
key, err := masterKey.GetMasterKey()
if err != nil {
return nil, err
}

if len(key) == 0 {
return secret, nil
}

return encProvider.Encrypt(secret, key)
}

func EncryptString(secret string) (string, error) {
key, err := masterKey.GetMasterKey()
if err != nil {
return "", err
}

if len(key) == 0 {
return secret, nil
}

return encProvider.EncryptString(secret, key)
}

func Decrypt(enc []byte) ([]byte, error) {
key, err := masterKey.GetMasterKey()
if err != nil {
return nil, err
}

if len(key) == 0 {
return enc, nil
}

return encProvider.Decrypt(enc, key)
}

func DecryptString(enc string) (string, error) {
key, err := masterKey.GetMasterKey()
if err != nil {
return "", err
}

if len(key) == 0 {
return enc, nil
}

return encProvider.DecryptString(enc, key)
}

func InsertRepoSecret(ctx context.Context, repoID int64, key, data string) error {
v, err := EncryptString(data)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've also mentioned it above, but here it is again:
I think it would be a good idea to strip leading and trailing whitespace from secrets.
There is only a marginal benefit in not doing it (who has a secret that starts or ends with whitespace?).
However, there is a huge benefit for usability, since many copying programs add for example whitespace at the end, meaning you would get an invalid secret that you first have to debug why it isn't working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solved in the new PR.

if err != nil {
return err
}
return db.Insert(ctx, &auth_model.Secret{
RepoID: repoID,
Name: key,
Data: v,
})
}

func InsertOwnerSecret(ctx context.Context, ownerID int64, key, data string) error {
v, err := EncryptString(data)
if err != nil {
return err
}
return db.Insert(ctx, &auth_model.Secret{
OwnerID: ownerID,
Name: key,
Data: v,
})
}

func DeleteSecretByID(ctx context.Context, id int64) error {
_, err := db.DeleteByBean(ctx, &auth_model.Secret{ID: id})
return err
}

func DeleteSecretsByRepoID(ctx context.Context, repoID int64) error {
_, err := db.DeleteByBean(ctx, &auth_model.Secret{RepoID: repoID})
return err
}

func DeleteSecretsByOwnerID(ctx context.Context, ownerID int64) error {
_, err := db.DeleteByBean(ctx, &auth_model.Secret{ID: ownerID})
return err
}

func FindRepoSecrets(ctx context.Context, repoID int64) ([]*auth_model.Secret, error) {
var res []*auth_model.Secret
return res, db.GetEngine(ctx).Where(builder.Eq{"repo_id": repoID}).Find(&res)
}

func FindOwnerSecrets(ctx context.Context, ownerID int64) ([]*auth_model.Secret, error) {
var res []*auth_model.Secret
return res, db.GetEngine(ctx).Where(builder.Eq{"owner_id": ownerID}).Find(&res)
}
16 changes: 16 additions & 0 deletions templates/install.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,22 @@
<span class="help">{{.locale.Tr "install.enable_update_checker_helper"}}</span>
</div>

<!-- Security Settings, especially MASTER_KEY -->
<h4 class="ui dividing header">{{.locale.Tr "install.security_title"}}</h4>

<div class="inline required field">
<label>{{.locale.Tr "install.master_key_provider"}}</label>
<div class="ui selection master-key-provider dropdown">
<input type="hidden" name="master_key_provider" value="{{if .master_key_provider}}{{.master_key_provider}}{{else}}plain{{end}}">
<div class="text">{{.locale.Tr "install.master_key_provider_plain"}}</div>
{{svg "octicon-triangle-down" 14 "dropdown icon"}}
<div class="menu">
<div class="item" data-value="none">{{.locale.Tr "install.master_key_provider_none"}}</div>
<div class="item" data-value="plain">{{.locale.Tr "install.master_key_provider_plain"}}</div>
</div>
</div>
<span class="help">{{.locale.Tr "install.master_key_provider_helper"}}</span>
</div>

<!-- Optional Settings -->
<h4 class="ui dividing header">{{.locale.Tr "install.optional_title"}}</h4>
Expand Down
3 changes: 3 additions & 0 deletions templates/org/settings/navbar.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
<a class="{{if .PageIsOrgSettingsLabels}}active {{end}}item" href="{{.OrgLink}}/settings/labels">
{{.locale.Tr "repo.labels"}}
</a>
<a class="{{if .PageIsOrgSettingsSecrets}}active {{end}}item" href="{{.OrgLink}}/settings/secrets">
{{.locale.Tr "org.settings.secrets"}}
</a>
{{if .EnableOAuth2}}
<a class="{{if .PageIsSettingsApplications}}active {{end}}item" href="{{.OrgLink}}/settings/applications">
{{.locale.Tr "settings.applications"}}
Expand Down
85 changes: 85 additions & 0 deletions templates/org/settings/secrets.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
{{template "base/head" .}}
<div class="page-content organization settings webhooks">
{{template "org/header" .}}
<div class="ui container">
<div class="ui grid">
{{template "org/settings/navbar" .}}
<div class="ui twelve wide column content">
{{template "base/alert" .}}
<h4 class="ui top attached header">
{{.locale.Tr "repo.settings.secrets"}}
<div class="ui right">
<div class="ui primary tiny show-panel button" data-panel="#add-secret-panel">{{.locale.Tr "repo.settings.add_secret"}}</div>
</div>
</h4>
<div class="ui attached segment">
<div class="{{if not .HasError}}hide {{end}}mb-4" id="add-secret-panel">
<form class="ui form" action="{{.Link}}?act=secret" method="post">
{{.CsrfTokenHtml}}
<div class="field">
{{.locale.Tr "repo.settings.secret_desc"}}
</div>
<div class="field {{if .Err_Title}}error{{end}}">
<label for="title">{{.locale.Tr "repo.settings.secret_name"}}</label>
<input id="key-title" name="title" value="{{.title}}" autofocus required>
</div>
<div class="field {{if .Err_Content}}error{{end}}">
<label for="content">{{.locale.Tr "repo.settings.secret_content"}}</label>
<textarea id="key-content" name="content" placeholder="{{.locale.Tr "repo.settings.secret_value_content_placeholder"}}" required>{{.content}}</textarea>
</div>
<button class="ui green button">
{{.locale.Tr "repo.settings.add_secret"}}
</button>
<button class="ui hide-panel button" data-panel="#add-secret-panel">
{{.locale.Tr "cancel"}}
</button>
</form>
</div>
{{if .Secrets}}
<div class="ui key list">
{{range .Secrets}}
<div class="item">
<div class="right floated content">
<button class="ui red tiny button delete-button" data-url="{{$.Link}}/delete" data-id="{{.ID}}">
{{$.locale.Tr "settings.delete_key"}}
</button>
</div>
<div class="left floated content">
<i class="tooltip">{{svg "octicon-key" 32}}</i>
</div>
<div class="content">
<strong>{{.Name}}</strong>
<div class="print meta">
{{Shadow .Data}}
</div>
<div class="activity meta">
<i>
{{$.locale.Tr "settings.add_on"}}
<span>{{.CreatedUnix.FormatShort}}</span>
</i>
</div>
</div>
</div>
{{end}}
</div>
{{else}}
{{.locale.Tr "repo.settings.no_secret"}}
{{end}}
</div>
</div>
</div>
</div>
</div>

<div class="ui small basic delete modal">
<div class="ui header">
{{svg "octicon-trash" 16 "mr-2"}}
{{.locale.Tr "repo.settings.secret_deletion"}}
</div>
Comment on lines +75 to +78
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that can be deleted as it doesn't really look good.

<div class="content">
<p>{{.locale.Tr "repo.settings.secret_deletion_desc"}}</p>
</div>
{{template "base/delete_modal_actions" .}}
</div>

{{template "base/footer" .}}
2 changes: 2 additions & 0 deletions templates/repo/settings/deploy_keys.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
{{end}}
</div>
</div>
<br/>
{{template "repo/settings/secrets" .}}
</div>

<div class="ui small basic delete modal">
Expand Down
2 changes: 1 addition & 1 deletion templates/repo/settings/nav.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
{{if or .SignedUser.AllowGitHook .SignedUser.IsAdmin}}
<li {{if .PageIsSettingsGitHooks}}class="current"{{end}}><a href="{{.RepoLink}}/settings/hooks/git">{{.locale.Tr "repo.settings.githooks"}}</a></li>
{{end}}
<li {{if .PageIsSettingsKeys}}class="current"{{end}}><a href="{{.RepoLink}}/settings/keys">{{.locale.Tr "repo.settings.deploy_keys"}}</a></li>
<li {{if .PageIsSettingsKeys}}class="current"{{end}}><a href="{{.RepoLink}}/settings/keys">{{.locale.Tr "repo.settings.secrets"}}</a></li>
</ul>
</div>
</div>
2 changes: 1 addition & 1 deletion templates/repo/settings/navbar.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
</a>
{{end}}
<a class="{{if .PageIsSettingsKeys}}active {{end}}item" href="{{.RepoLink}}/settings/keys">
{{.locale.Tr "repo.settings.deploy_keys"}}
{{.locale.Tr "repo.settings.secrets"}}
</a>
{{if .LFSStartServer}}
<a class="{{if .PageIsSettingsLFS}}active {{end}}item" href="{{.RepoLink}}/settings/lfs">
Expand Down
62 changes: 62 additions & 0 deletions templates/repo/settings/secrets.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<div class="ui container">
<h4 class="ui top attached header">
{{.locale.Tr "repo.settings.secrets"}}
<div class="ui right">
<div class="ui primary tiny show-panel button" data-panel="#add-secret-panel">{{.locale.Tr "repo.settings.add_secret"}}</div>
</div>
</h4>
<div class="ui attached segment">
<div class="{{if not .HasError}}hide {{end}}mb-4" id="add-secret-panel">
<form class="ui form" action="{{.Link}}?act=secret" method="post">
{{.CsrfTokenHtml}}
<div class="field">
{{.locale.Tr "repo.settings.secret_desc"}}
</div>
<div class="field {{if .Err_Title}}error{{end}}">
<label for="title">{{.locale.Tr "repo.settings.secret_name"}}</label>
<input id="ssh-key-title" name="title" value="{{.title}}" autofocus required>
</div>
<div class="field {{if .Err_Content}}error{{end}}">
<label for="content">{{.locale.Tr "repo.settings.secret_content"}}</label>
<textarea id="ssh-key-content" name="content" placeholder="{{.locale.Tr "repo.settings.secret_value_content_placeholder"}}" required>{{.content}}</textarea>
lunny marked this conversation as resolved.
Show resolved Hide resolved
</div>
<button class="ui green button">
{{.locale.Tr "repo.settings.add_secret"}}
</button>
<button class="ui hide-panel button" data-panel="#add-secret-panel">
{{.locale.Tr "cancel"}}
</button>
</form>
</div>
{{if .Secrets}}
<div class="ui key list">
{{range .Secrets}}
<div class="item">
<div class="right floated content">
<button class="ui red tiny button delete-button" data-url="{{$.Link}}/delete?act=secret" data-id="{{.ID}}">
{{$.locale.Tr "settings.delete_key"}}
</button>
</div>
<div class="left floated content">
<i class="tooltip">{{svg "octicon-key" 32}}</i>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tooltip? And why italic? Wouldn't the svg suffice?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remvoed in the new PR.

</div>
<div class="content">
<strong>{{.Name}}</strong>
<div class="print meta">
{{Shadow .Data}}
</div>
<div class="activity meta">
<i>
{{$.locale.Tr "settings.add_on"}}
<span>{{.CreatedUnix.FormatShort}}</span>
</i>
</div>
</div>
</div>
{{end}}
</div>
{{else}}
{{.locale.Tr "repo.settings.no_secret"}}
{{end}}
</div>
</div>