[SEC] Add keying
module
The keying modules tries to solve two problems, the lack of key separation and the lack of AEAD being used for encryption. The currently used `secrets` doesn't provide this and is hard to adjust to provide this functionality. For encryption, the additional data is now a parameter that can be used, as the underlying primitive is an AEAD constructions. This allows for context binding to happen and can be seen as defense-in-depth; it ensures that if a value X is encrypted for context Y (e.g. ID=3, Column="private_key") it will only decrypt if that context Y is also given in the Decrypt function. This makes confused deputy attack harder to exploit.[^1] For key separation, HKDF is used to derives subkeys from some IKM, which is the value of the `[service].SECRET_KEY` config setting. The context for subkeys are hardcoded, any variable should be shuffled into the the additional data parameter when encrypting. [^1]: This is still possible, because the used AEAD construction is not key-comitting. For Forgejo's current use-case this risk is negligible, because the subkeys aren't known to a malicious user (which is required for such attack), unless they also have access to the IKM (at which point you can assume the whole system is compromised). See https://scottarc.blog/2022/10/17/lucid-multi-key-deputies-require-commitment/
This commit is contained in:
parent
5b81cab0ed
commit
12f97ef51f
4 changed files with 214 additions and 0 deletions
|
@ -170,6 +170,11 @@ code.gitea.io/gitea/modules/json
|
||||||
StdJSON.NewDecoder
|
StdJSON.NewDecoder
|
||||||
StdJSON.Indent
|
StdJSON.Indent
|
||||||
|
|
||||||
|
code.gitea.io/gitea/modules/keying
|
||||||
|
DeriveKey
|
||||||
|
Key.Encrypt
|
||||||
|
Key.Decrypt
|
||||||
|
|
||||||
code.gitea.io/gitea/modules/markup
|
code.gitea.io/gitea/modules/markup
|
||||||
GetRendererByType
|
GetRendererByType
|
||||||
RenderString
|
RenderString
|
||||||
|
|
111
modules/keying/keying.go
Normal file
111
modules/keying/keying.go
Normal file
|
@ -0,0 +1,111 @@
|
||||||
|
// Copyright 2024 The Forgejo Authors. All rights reserved.
|
||||||
|
// SPDX-License-Identifier: MIT
|
||||||
|
|
||||||
|
// Keying is a module that allows for subkeys to be determistically generated
|
||||||
|
// from the same master key. It allows for domain seperation to take place by
|
||||||
|
// using new keys for new subsystems/domains. These subkeys are provided with
|
||||||
|
// an API to encrypt and decrypt data. The module panics if a bad interaction
|
||||||
|
// happened, the panic should be seen as an non-recoverable error.
|
||||||
|
//
|
||||||
|
// HKDF (per RFC 5869) is used to derive new subkeys in a safe manner. It
|
||||||
|
// provides a KDF security property, which is required for Forgejo, as the
|
||||||
|
// secret key would be an ASCII string and isn't a random uniform bit string.
|
||||||
|
// XChaCha-Poly1305 (per draft-irtf-cfrg-xchacha-01) is used as AEAD to encrypt
|
||||||
|
// and decrypt messages. A new fresh random nonce is generated for every
|
||||||
|
// encryption. The nonce gets prepended to the ciphertext.
|
||||||
|
package keying
|
||||||
|
|
||||||
|
import (
|
||||||
|
"crypto/rand"
|
||||||
|
"crypto/sha256"
|
||||||
|
|
||||||
|
"golang.org/x/crypto/chacha20poly1305"
|
||||||
|
"golang.org/x/crypto/hkdf"
|
||||||
|
)
|
||||||
|
|
||||||
|
var (
|
||||||
|
// The hash used for HKDF.
|
||||||
|
hash = sha256.New
|
||||||
|
// The AEAD used for encryption/decryption.
|
||||||
|
aead = chacha20poly1305.NewX
|
||||||
|
aeadKeySize = chacha20poly1305.KeySize
|
||||||
|
aeadNonceSize = chacha20poly1305.NonceSizeX
|
||||||
|
// The pseudorandom key generated by HKDF-Extract.
|
||||||
|
prk []byte
|
||||||
|
)
|
||||||
|
|
||||||
|
// Set the main IKM for this module.
|
||||||
|
func Init(ikm []byte) {
|
||||||
|
// Salt is intentionally left empty, it's not useful to Forgejo's use case.
|
||||||
|
prk = hkdf.Extract(hash, ikm, nil)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Specifies the context for which a subkey should be derived for.
|
||||||
|
// This must be a hardcoded string and must not be arbitrarily constructed.
|
||||||
|
type Context string
|
||||||
|
|
||||||
|
// Derive *the* key for a given context, this is a determistic function. The
|
||||||
|
// same key will be provided for the same context.
|
||||||
|
func DeriveKey(context Context) *Key {
|
||||||
|
if len(prk) == 0 {
|
||||||
|
panic("keying: not initialized")
|
||||||
|
}
|
||||||
|
|
||||||
|
r := hkdf.Expand(hash, prk, []byte(context))
|
||||||
|
|
||||||
|
key := make([]byte, aeadKeySize)
|
||||||
|
// This should never return an error, but if it does, panic.
|
||||||
|
if _, err := r.Read(key); err != nil {
|
||||||
|
panic(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return &Key{key}
|
||||||
|
}
|
||||||
|
|
||||||
|
type Key struct {
|
||||||
|
key []byte
|
||||||
|
}
|
||||||
|
|
||||||
|
// Encrypts the specified plaintext with some additional data that is tied to
|
||||||
|
// this plaintext. The additional data can be seen as the context in which the
|
||||||
|
// data is being encrypted for, this is different than the context for which the
|
||||||
|
// key was derrived this allows for more granuality without deriving new keys.
|
||||||
|
// Avoid any user-generated data to be passed into the additional data. The most
|
||||||
|
// common usage of this would be to encrypt a database field, in that case use
|
||||||
|
// the ID and database column name as additional data. The additional data isn't
|
||||||
|
// appended to the ciphertext and may be publicly known, it must be available
|
||||||
|
// when decryping the ciphertext.
|
||||||
|
func (k *Key) Encrypt(plaintext, additionalData []byte) []byte {
|
||||||
|
// Construct a new AEAD with the key.
|
||||||
|
e, err := aead(k.key)
|
||||||
|
if err != nil {
|
||||||
|
panic(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Generate a random nonce.
|
||||||
|
nonce := make([]byte, aeadNonceSize)
|
||||||
|
if _, err := rand.Read(nonce); err != nil {
|
||||||
|
panic(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Returns the ciphertext of this plaintext.
|
||||||
|
return e.Seal(nonce, nonce, plaintext, additionalData)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Decrypts the ciphertext and authenticates it against the given additional
|
||||||
|
// data that was given when it was encrypted. It returns an error if the
|
||||||
|
// authentication failed.
|
||||||
|
func (k *Key) Decrypt(ciphertext, additionalData []byte) ([]byte, error) {
|
||||||
|
if len(ciphertext) <= aeadNonceSize {
|
||||||
|
panic("keying: ciphertext is too short")
|
||||||
|
}
|
||||||
|
|
||||||
|
e, err := aead(k.key)
|
||||||
|
if err != nil {
|
||||||
|
panic(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
nonce, ciphertext := ciphertext[:aeadNonceSize], ciphertext[aeadNonceSize:]
|
||||||
|
|
||||||
|
return e.Open(nil, nonce, ciphertext, additionalData)
|
||||||
|
}
|
96
modules/keying/keying_test.go
Normal file
96
modules/keying/keying_test.go
Normal file
|
@ -0,0 +1,96 @@
|
||||||
|
// Copyright 2024 The Forgejo Authors. All rights reserved.
|
||||||
|
// SPDX-License-Identifier: MIT
|
||||||
|
|
||||||
|
package keying_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/modules/keying"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"golang.org/x/crypto/chacha20poly1305"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestKeying(t *testing.T) {
|
||||||
|
t.Run("Not initalized", func(t *testing.T) {
|
||||||
|
assert.Panics(t, func() {
|
||||||
|
keying.DeriveKey(keying.Context("TESTING"))
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Initialization", func(t *testing.T) {
|
||||||
|
keying.Init([]byte{0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07})
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Context seperation", func(t *testing.T) {
|
||||||
|
key1 := keying.DeriveKey(keying.Context("TESTING"))
|
||||||
|
key2 := keying.DeriveKey(keying.Context("TESTING2"))
|
||||||
|
|
||||||
|
ciphertext := key1.Encrypt([]byte("This is for context TESTING"), nil)
|
||||||
|
|
||||||
|
plaintext, err := key2.Decrypt(ciphertext, nil)
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Empty(t, plaintext)
|
||||||
|
|
||||||
|
plaintext, err = key1.Decrypt(ciphertext, nil)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.EqualValues(t, "This is for context TESTING", plaintext)
|
||||||
|
})
|
||||||
|
|
||||||
|
context := keying.Context("TESTING PURPOSES")
|
||||||
|
plainText := []byte("Forgejo is run by [Redacted]")
|
||||||
|
var cipherText []byte
|
||||||
|
t.Run("Encrypt", func(t *testing.T) {
|
||||||
|
key := keying.DeriveKey(context)
|
||||||
|
|
||||||
|
cipherText = key.Encrypt(plainText, []byte{0x05, 0x06})
|
||||||
|
cipherText2 := key.Encrypt(plainText, []byte{0x05, 0x06})
|
||||||
|
|
||||||
|
// Ensure ciphertexts don't have an determistic output.
|
||||||
|
assert.NotEqualValues(t, cipherText, cipherText2)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Decrypt", func(t *testing.T) {
|
||||||
|
key := keying.DeriveKey(context)
|
||||||
|
|
||||||
|
t.Run("Succesful", func(t *testing.T) {
|
||||||
|
convertedPlainText, err := key.Decrypt(cipherText, []byte{0x05, 0x06})
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.EqualValues(t, plainText, convertedPlainText)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Not enougn additional data", func(t *testing.T) {
|
||||||
|
plainText, err := key.Decrypt(cipherText, []byte{0x05})
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Empty(t, plainText)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Too much additional data", func(t *testing.T) {
|
||||||
|
plainText, err := key.Decrypt(cipherText, []byte{0x05, 0x06, 0x07})
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Empty(t, plainText)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Incorrect nonce", func(t *testing.T) {
|
||||||
|
// Flip the first byte of the nonce.
|
||||||
|
cipherText[0] = ^cipherText[0]
|
||||||
|
|
||||||
|
plainText, err := key.Decrypt(cipherText, []byte{0x05, 0x06})
|
||||||
|
require.Error(t, err)
|
||||||
|
assert.Empty(t, plainText)
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("Incorrect ciphertext", func(t *testing.T) {
|
||||||
|
assert.Panics(t, func() {
|
||||||
|
key.Decrypt(nil, nil)
|
||||||
|
})
|
||||||
|
|
||||||
|
assert.Panics(t, func() {
|
||||||
|
cipherText := make([]byte, chacha20poly1305.NonceSizeX)
|
||||||
|
key.Decrypt(cipherText, nil)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
|
@ -10,6 +10,7 @@ import (
|
||||||
|
|
||||||
"code.gitea.io/gitea/modules/auth/password/hash"
|
"code.gitea.io/gitea/modules/auth/password/hash"
|
||||||
"code.gitea.io/gitea/modules/generate"
|
"code.gitea.io/gitea/modules/generate"
|
||||||
|
"code.gitea.io/gitea/modules/keying"
|
||||||
"code.gitea.io/gitea/modules/log"
|
"code.gitea.io/gitea/modules/log"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -110,6 +111,7 @@ func loadSecurityFrom(rootCfg ConfigProvider) {
|
||||||
// Until it supports rotating an existing secret key, we shouldn't move users off of the widely used default value
|
// Until it supports rotating an existing secret key, we shouldn't move users off of the widely used default value
|
||||||
SecretKey = "!#@FDEWREWR&*(" //nolint:gosec
|
SecretKey = "!#@FDEWREWR&*(" //nolint:gosec
|
||||||
}
|
}
|
||||||
|
keying.Init([]byte(SecretKey))
|
||||||
|
|
||||||
CookieRememberName = sec.Key("COOKIE_REMEMBER_NAME").MustString("gitea_incredible")
|
CookieRememberName = sec.Key("COOKIE_REMEMBER_NAME").MustString("gitea_incredible")
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue