Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: bug with subkeys, verify test fields on startup #2444

Merged
merged 10 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions backend/controller/cronjobs/sql/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions backend/controller/dal/dal.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,9 @@ func New(ctx context.Context, conn *stdsql.DB, encryptionBuilder encryption.Buil
return nil, fmt.Errorf("failed to setup encryptor: %w", err)
}
d.encryptor = encryptor
if err = d.verifyEncryptor(ctx); err != nil {
return nil, fmt.Errorf("failed to verify encryption: %w", err)
}

return d, nil
}
Expand Down
78 changes: 78 additions & 0 deletions backend/controller/dal/dal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -542,3 +542,81 @@ func TestDeleteOldEvents(t *testing.T) {
assert.Equal(t, int64(0), count)
})
}

func TestVerifyEncryption(t *testing.T) {
ctx := log.ContextWithNewDefaultLogger(context.Background())
conn := sqltest.OpenForTesting(ctx, t)
uri := "fake-kms://CK6YwYkBElQKSAowdHlwZS5nb29nbGVhcGlzLmNvbS9nb29nbGUuY3J5cHRvLnRpbmsuQWVzR2NtS2V5EhIaEJy4TIQgfCuwxA3ZZgChp_wYARABGK6YwYkBIAE"

t.Run("DeleteVerificationColumns", func(t *testing.T) {
dal, err := New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.NoError(t, err)

// check that there are columns set in encryption_keys
row, err := dal.db.GetOnlyEncryptionKey(ctx)
assert.NoError(t, err)
assert.NotZero(t, row.VerifyTimeline.Ok())
assert.NotZero(t, row.VerifyAsync.Ok())

// delete the columns to see if they are recreated
err = dal.db.UpdateEncryptionVerification(ctx, optional.None[encryption.EncryptedTimelineColumn](), optional.None[encryption.EncryptedAsyncColumn]())
assert.NoError(t, err)

dal, err = New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.NoError(t, err)

row, err = dal.db.GetOnlyEncryptionKey(ctx)
assert.NoError(t, err)
assert.NotZero(t, row.VerifyTimeline.Ok())
assert.NotZero(t, row.VerifyAsync.Ok())
})

t.Run("DifferentKey", func(t *testing.T) {
_, err := New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.NoError(t, err)

differentKey := "fake-kms://CJP7ksIKElQKSAowdHlwZS5nb29nbGVhcGlzLmNvbS9nb29nbGUuY3J5cHRvLnRpbmsuQWVzR2NtS2V5EhIaEJWT3z-xdW23HO7hc9vF3YoYARABGJP7ksIKIAE"
_, err = New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(differentKey)))
assert.Error(t, err)
assert.Contains(t, err.Error(), "decryption failed")
})

t.Run("SameKeyButWrongTimelineVerification", func(t *testing.T) {
dal, err := New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.NoError(t, err)

err = dal.db.UpdateEncryptionVerification(ctx, optional.Some[encryption.EncryptedTimelineColumn]([]byte("123")), optional.None[encryption.EncryptedAsyncColumn]())
assert.NoError(t, err)
_, err = New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.Error(t, err)
assert.Contains(t, err.Error(), "verification sanity")
assert.Contains(t, err.Error(), "verify timeline")

err = dal.db.UpdateEncryptionVerification(ctx, optional.None[encryption.EncryptedTimelineColumn](), optional.Some[encryption.EncryptedAsyncColumn]([]byte("123")))
assert.NoError(t, err)
_, err = New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.Error(t, err)
assert.Contains(t, err.Error(), "verification sanity")
assert.Contains(t, err.Error(), "verify async")
})

t.Run("SameKeyButEncryptWrongPlainText", func(t *testing.T) {
result, err := conn.Exec("DELETE FROM encryption_keys")
assert.NoError(t, err)
affected, err := result.RowsAffected()
assert.NoError(t, err)
assert.Equal(t, int64(1), affected)
dal, err := New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.NoError(t, err)

encrypted := encryption.EncryptedColumn[encryption.TimelineSubKey]{}
err = dal.encrypt([]byte("123"), &encrypted)
assert.NoError(t, err)

err = dal.db.UpdateEncryptionVerification(ctx, optional.Some(encrypted), optional.None[encryption.EncryptedAsyncColumn]())
assert.NoError(t, err)
_, err = New(ctx, conn, encryption.NewBuilder().WithKMSURI(optional.Some(uri)))
assert.Error(t, err)
assert.Contains(t, err.Error(), "string does not match")
})
}
85 changes: 82 additions & 3 deletions backend/controller/dal/encryption.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"encoding/json"
"fmt"

"github.com/alecthomas/types/optional"

"github.com/TBD54566975/ftl/backend/controller/sql"
"github.com/TBD54566975/ftl/backend/dal"
"github.com/TBD54566975/ftl/internal/encryption"
"github.com/TBD54566975/ftl/internal/log"
Expand Down Expand Up @@ -66,10 +69,11 @@ func (d *DAL) EnsureKey(ctx context.Context, generateKey func() ([]byte, error))
}
defer tx.CommitOrRollback(ctx, &err)

encryptedKey, err = tx.db.GetOnlyEncryptionKey(ctx)
var key []byte
row, err := tx.db.GetOnlyEncryptionKey(ctx)
if err != nil && dal.IsNotFound(err) {
logger.Debugf("No encryption key found, generating a new one")
key, err := generateKey()
key, err = generateKey()
if err != nil {
return nil, fmt.Errorf("failed to generate key: %w", err)
}
Expand All @@ -84,5 +88,80 @@ func (d *DAL) EnsureKey(ctx context.Context, generateKey func() ([]byte, error))
}

logger.Debugf("Encryption key found, using it")
return encryptedKey, nil

return row.Key, nil
}

const verification = "FTL - Towards a 𝝺-calculus for large-scale systems"

func (d *DAL) verifyEncryptor(ctx context.Context) (err error) {
var tx *Tx
tx, err = d.Begin(ctx)
if err != nil {
return fmt.Errorf("failed to begin transaction: %w", err)
}
defer tx.CommitOrRollback(ctx, &err)

var row sql.GetOnlyEncryptionKeyRow
gak marked this conversation as resolved.
Show resolved Hide resolved
row, err = tx.db.GetOnlyEncryptionKey(ctx)
if err != nil {
if dal.IsNotFound(err) {
// No encryption key found, probably using noop.
return nil
}
return fmt.Errorf("failed to get encryption row from the db: %w", err)
}

up := false
up, err = verifySubkey(d.encryptor, &row.VerifyTimeline)
if err != nil {
return fmt.Errorf("failed to verify timeline subkey: %w", err)
}
needsUpdate := up

up, err = verifySubkey(d.encryptor, &row.VerifyAsync)
if err != nil {
return fmt.Errorf("failed to verify async subkey: %w", err)
}
needsUpdate = needsUpdate || up

if !needsUpdate {
return nil
}

if !row.VerifyTimeline.Ok() || !row.VerifyAsync.Ok() {
panic("should be unreachable. verifySubkey should have set the subkey")
}

err = tx.db.UpdateEncryptionVerification(ctx, row.VerifyTimeline, row.VerifyAsync)
if err != nil {
return fmt.Errorf("failed to update encryption verification: %w", err)
}

return nil
}

// verifySubkey checks if the subkey is set and if not, sets it to a verification string.
// It returns true if the subkey was set.
func verifySubkey[SK encryption.SubKey](encryptor encryption.DataEncryptor, encrypted *optional.Option[encryption.EncryptedColumn[SK]]) (bool, error) {
gak marked this conversation as resolved.
Show resolved Hide resolved
verifyField, ok := encrypted.Get()
if !ok {
err := encryptor.Encrypt([]byte(verification), &verifyField)
if err != nil {
return false, fmt.Errorf("failed to encrypt verification sanity string: %w", err)
}
*encrypted = optional.Some(verifyField)
return true, nil
}

decrypted, err := encryptor.Decrypt(&verifyField)
if err != nil {
return false, fmt.Errorf("failed to decrypt verification sanity string: %w", err)
}

if string(decrypted) != verification {
return false, fmt.Errorf("decrypted verification string does not match expected value")
}

return false, nil
}
8 changes: 5 additions & 3 deletions backend/controller/sql/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion backend/controller/sql/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion backend/controller/sql/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -901,10 +901,16 @@ FROM topic_events
WHERE id = $1::BIGINT;

-- name: GetOnlyEncryptionKey :one
SELECT key
SELECT key, verify_timeline, verify_async
FROM encryption_keys
WHERE id = 1;

-- name: CreateOnlyEncryptionKey :exec
INSERT INTO encryption_keys (id, key)
VALUES (1, $1);

-- name: UpdateEncryptionVerification :exec
UPDATE encryption_keys
SET verify_timeline = $1,
verify_async = $2
WHERE id = 1;
28 changes: 23 additions & 5 deletions backend/controller/sql/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-- migrate:up

ALTER TABLE encryption_keys
ADD COLUMN verify_timeline encrypted_timeline,
ADD COLUMN verify_async encrypted_async;

-- migrate:down

ALTER TABLE encryption_keys
gak marked this conversation as resolved.
Show resolved Hide resolved
DROP COLUMN verify_timeline,
DROP COLUMN verify_async;
8 changes: 5 additions & 3 deletions internal/configuration/sql/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/encryption/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type SubKey interface{ SubKey() string }
// TimelineSubKey is a type that represents the subkey for logs.
type TimelineSubKey struct{}

func (TimelineSubKey) SubKey() string { return "logs" }
func (TimelineSubKey) SubKey() string { return "timeline" }

// AsyncSubKey is a type that represents the subkey for async.
type AsyncSubKey struct{}
Expand Down
Loading