Skip to content

Commit

Permalink
feat: obfuscate secrets to avoid tampering (#1916)
Browse files Browse the repository at this point in the history
closes #1889
closes #1772

- Secrets manager has an obfuscator, config manager does not (`Secrets`
conforms to `ObfuscatorProvider`)
- `configuration.Obfuscator` allows obfuscating/revealing of values
- These changes apply to all secrets providers
- Providers can also save a comment warning about tampering
- ASM does this by prefixing the secret with comments (lines starting
with `#`)
    - 1Password will support comments in another PR
  • Loading branch information
matt2e authored Jul 4, 2024
1 parent bc7868a commit 0e17f9f
Show file tree
Hide file tree
Showing 6 changed files with 241 additions and 6 deletions.
7 changes: 4 additions & 3 deletions common/configuration/asm_leader.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (l *asmLeader) sync(ctx context.Context, secrets *xsync.MapOf[Ref, cachedSe
if s.SecretBinary != nil {
return fmt.Errorf("secret for %s in ASM is not a string", ref)
}
data := []byte(*s.SecretString)
data := unwrapComments([]byte(*s.SecretString))
secrets.Store(ref, cachedSecret{
value: data,
versionToken: optional.Some[any](refsToLoad[ref]),
Expand Down Expand Up @@ -155,9 +155,10 @@ func (l *asmLeader) load(ctx context.Context, ref Ref, key *url.URL) ([]byte, er

// store and if the secret already exists, update it.
func (l *asmLeader) store(ctx context.Context, ref Ref, value []byte) (*url.URL, error) {
valueWithComments := aws.String(string(wrapWithComments(value, defaultSecretModificationWarning)))
_, err := l.client.CreateSecret(ctx, &secretsmanager.CreateSecretInput{
Name: aws.String(ref.String()),
SecretString: aws.String(string(value)),
SecretString: valueWithComments,
Tags: []types.Tag{
{Key: aws.String(asmTagKey), Value: aws.String(ref.Module.Default(""))},
},
Expand All @@ -168,7 +169,7 @@ func (l *asmLeader) store(ctx context.Context, ref Ref, value []byte) (*url.URL,
if errors.As(err, &apiErr) && apiErr.ErrorCode() == "ResourceExistsException" {
_, err = l.client.UpdateSecret(ctx, &secretsmanager.UpdateSecretInput{
SecretId: aws.String(ref.String()),
SecretString: aws.String(string(value)),
SecretString: valueWithComments,
})
if err != nil {
return nil, fmt.Errorf("unable to update secret in ASM: %w", err)
Expand Down
32 changes: 32 additions & 0 deletions common/configuration/comments.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package configuration

import (
"strings"

"github.com/TBD54566975/ftl/internal/slices"
)

const defaultSecretModificationWarning = `This secret is managed by "ftl secret set", DO NOT MODIFY`

// wrapWithComments wraps the secret with a comment to indicate that it is managed by FTL.
//
// This is used by providers that want to include a warning to avoid manual modification.
// The provider must support multiline secrets.
// Comment lines are prefixed with '# ' in the result.
func wrapWithComments(secret []byte, comments string) []byte {
lines := []string{}
for _, line := range strings.Split(comments, "\n") {
lines = append(lines, "# "+line)
}
lines = append(lines, string(secret))
return []byte(strings.Join(lines, "\n"))
}

// unwrapComments removes comments if they exist by looking for the lines starting with '#'
func unwrapComments(secret []byte) []byte {
lines := strings.Split(string(secret), "\n")
lines = slices.Filter(lines, func(line string) bool {
return !strings.HasPrefix(line, "#")
})
return []byte(strings.Join(lines, "\n"))
}
34 changes: 34 additions & 0 deletions common/configuration/comments_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package configuration

import (
"testing"

"github.com/alecthomas/assert/v2"
)

func TestComments(t *testing.T) {
for _, tt := range []struct {
input string
comment string
output string
}{
{
input: "test input can be anything",
comment: "This is a test",
output: "# This is a test\ntest input can be anything",
},
{
input: "{\n \"key\": \"value\"\n}",
comment: "This is a multi\nline\ncomment",
output: "# This is a multi\n# line\n# comment\n{\n \"key\": \"value\"\n}",
},
} {
t.Run(tt.input, func(t *testing.T) {
wrapped := wrapWithComments([]byte(tt.input), tt.comment)
assert.Equal(t, tt.output, string(wrapped))

unwrapped := unwrapComments(wrapped)
assert.Equal(t, tt.input, string(unwrapped))
})
}
}
33 changes: 30 additions & 3 deletions common/configuration/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,22 @@ type Secrets struct{}

func (Secrets) String() string { return "secrets" }

func (Secrets) obfuscator() Obfuscator {
return Obfuscator{
key: []byte("obfuscatesecrets"), // 16 characters (AES-128), not meant to provide security
}
}

type Configuration struct{}

func (Configuration) String() string { return "configuration" }

// Manager is a high-level configuration manager that abstracts the details of
// the Router and Provider interfaces.
type Manager[R Role] struct {
providers map[string]Provider[R]
router Router[R]
providers map[string]Provider[R]
router Router[R]
obfuscator optional.Option[Obfuscator]
}

func ConfigFromEnvironment() []string {
Expand Down Expand Up @@ -61,6 +68,9 @@ func New[R Role](ctx context.Context, router Router[R], providers []Provider[R])
for _, p := range providers {
m.providers[p.Key()] = p
}
if provider, ok := any(new(R)).(ObfuscatorProvider); ok {
m.obfuscator = optional.Some(provider.obfuscator())
}
m.router = router
return m, nil
}
Expand All @@ -87,6 +97,12 @@ func (m *Manager[R]) getData(ctx context.Context, ref Ref) ([]byte, error) {
if err != nil {
return nil, fmt.Errorf("%s: %w", ref, err)
}
if obfuscator, ok := m.obfuscator.Get(); ok {
data, err = obfuscator.Reveal(data)
if err != nil {
return nil, fmt.Errorf("could not reveal obfuscated value: %w", err)
}
}
return data, nil
}

Expand Down Expand Up @@ -127,12 +143,23 @@ func (m *Manager[R]) SetJSON(ctx context.Context, pkey string, ref Ref, value js
if err := checkJSON(value); err != nil {
return fmt.Errorf("invalid value for %s, must be JSON: %w", m.router.Role(), err)
}
var bytes []byte
if obfuscator, ok := m.obfuscator.Get(); ok {
var err error
bytes, err = obfuscator.Obfuscate(value)
if err != nil {
return fmt.Errorf("could not obfuscate: %w", err)
}
} else {
bytes = value
}

provider, ok := m.providers[pkey]
if !ok {
pkeys := strings.Join(m.availableProviderKeys(), ", ")
return fmt.Errorf("no provider for key %q, specify one of: %s", pkey, pkeys)
}
key, err := provider.Store(ctx, ref, value)
key, err := provider.Store(ctx, ref, bytes)
if err != nil {
return err
}
Expand Down
69 changes: 69 additions & 0 deletions common/configuration/obfuscator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package configuration

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

type ObfuscatorProvider interface {
obfuscator() Obfuscator
}

// Obfuscator hides and reveals a value, but does not provide real security
// instead the aim of this Obfuscator is to make values not easily human readable
//
// Obfuscation is done by XOR-ing the input with the AES key. Length of key must be 16, 24 or 32 bytes (corresponding to AES-128, AES-192 or AES-256 keys).
type Obfuscator struct {
key []byte
}

// Obfuscate takes a value and returns an obfuscated value (encoded in base64)
func (o Obfuscator) Obfuscate(input []byte) ([]byte, error) {
block, err := aes.NewCipher(o.key)
if err != nil {
return nil, fmt.Errorf("could not create cypher for obfuscation: %w", err)
}
ciphertext := make([]byte, aes.BlockSize+len(input))
iv := ciphertext[:aes.BlockSize]
if _, err := io.ReadFull(rand.Reader, iv); err != nil {
return nil, fmt.Errorf("could not generate IV for obfuscation: %w", err)
}
cfb := cipher.NewCFBEncrypter(block, iv)
cfb.XORKeyStream(ciphertext[aes.BlockSize:], input)
return []byte(base64.StdEncoding.EncodeToString(ciphertext)), nil
}

// Reveal takes an obfuscated value and de-obfuscates the base64 encoded value
func (o Obfuscator) Reveal(input []byte) ([]byte, error) {
// check if the input looks like it was obfuscated
if !strings.ContainsRune("0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ+/=", rune(input[0])) {
// known issue: an unobfuscated value which is just a number will be considered obfuscated
return input, nil
}

obfuscated, err := base64.StdEncoding.DecodeString(string(input))
if err != nil {
return nil, fmt.Errorf("expected hexadecimal string: %w", err)
}
block, err := aes.NewCipher(o.key)
if err != nil {
return nil, fmt.Errorf("could not create cypher for decoding obfuscation: %w", err)
}
if len(obfuscated) < aes.BlockSize {
return nil, errors.New("obfuscated value too short to decode")
}
iv := obfuscated[:aes.BlockSize]
obfuscated = obfuscated[aes.BlockSize:]
cfb := cipher.NewCFBDecrypter(block, iv)

var output = make([]byte, len(obfuscated))
cfb.XORKeyStream(output, obfuscated)

return output, nil
}
72 changes: 72 additions & 0 deletions common/configuration/obfuscator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package configuration

import (
"testing"

"github.com/alecthomas/assert/v2"
"github.com/alecthomas/types/optional"
)

func TestObfuscator(t *testing.T) {
defaultKey := []byte("1234567890123456") // 32 characters
for _, tt := range []struct {
input string
key []byte
expectedError optional.Option[string]
backwardsCompatible bool
}{
{
input: "test input can be anything",
key: defaultKey,
backwardsCompatible: false,
},
{
input: `"test input can be anything"`,
key: defaultKey,
backwardsCompatible: true,
},
{
input: `"{\n "key": "value"\n}`,
key: defaultKey,
backwardsCompatible: true,
},
{
input: `1.2345`,
key: defaultKey,
backwardsCompatible: false,
},
{
input: "key is too short",
key: []byte("too-short"),
expectedError: optional.Some("could not create cypher for obfuscation: crypto/aes: invalid key size 9"),
},
} {
t.Run(tt.input, func(t *testing.T) {
o := Obfuscator{
key: tt.key,
}
// obfuscate
obfuscated, err := o.Obfuscate([]byte(tt.input))
if expectedError, ok := tt.expectedError.Get(); ok {
assert.EqualError(t, err, expectedError)
return
}
assert.NoError(t, err)

// reveal obfuscated value
revealed, err := o.Reveal(obfuscated)
assert.NoError(t, err)
assert.Equal(t, tt.input, string(revealed))

// obfuscated value should not include the input we are trying to obfuscate
assert.NotContains(t, string(obfuscated), tt.input)

// reveal unobfuscated value to check backwards compatibility
if tt.backwardsCompatible {
revealed, err = o.Reveal([]byte(tt.input))
assert.NoError(t, err)
assert.Equal(t, tt.input, string(revealed))
}
})
}
}

0 comments on commit 0e17f9f

Please sign in to comment.