From 1ff6fa94248acf3cb16262c8454224ffeee89707 Mon Sep 17 00:00:00 2001 From: Emil Tullstedt Date: Fri, 9 Oct 2020 16:15:24 +0200 Subject: [PATCH] Encryption: Improvements --- pkg/services/secrets/json.go | 6 +-- pkg/services/secrets/secrets.go | 60 ++++++++++++++++++++----- pkg/services/secrets/secrets_test.go | 45 +++++++++++++++++++ pkg/services/sqlstore/data_keys.go | 31 ++++++++++--- pkg/services/sqlstore/data_keys_test.go | 44 ++++++++++++++++++ 5 files changed, 166 insertions(+), 20 deletions(-) create mode 100644 pkg/services/secrets/secrets_test.go create mode 100644 pkg/services/sqlstore/data_keys_test.go diff --git a/pkg/services/secrets/json.go b/pkg/services/secrets/json.go index 9d41110489e..f083c64ba69 100644 --- a/pkg/services/secrets/json.go +++ b/pkg/services/secrets/json.go @@ -11,7 +11,7 @@ type EncryptedJSON struct { // is true if the key exists and false if not. func (j EncryptedJSON) DecryptedValue(key string) (string, bool) { if value, ok := j.Map[key]; ok { - decryptedData, err := j.s.Decrypt(value, "") + decryptedData, err := j.s.Decrypt(value) if err != nil { logger.Error("could not decrypt value", "key", key, "err", err.Error()) return "", false @@ -26,7 +26,7 @@ func (j EncryptedJSON) DecryptedValue(key string) (string, bool) { func (j EncryptedJSON) DecryptJSONMap() (map[string]string, error) { decrypted := make(map[string]string) for key, data := range j.Map { - decryptedData, err := j.s.Decrypt(data, "") + decryptedData, err := j.s.Decrypt(data) if err != nil { logger.Error("could not decrypt map", "err", err.Error()) return nil, err @@ -44,7 +44,7 @@ func (s *Secrets) EncryptJSONMap(m map[string]string) (EncryptedJSON, error) { Map: make(map[string][]byte), } for key, data := range m { - encryptedData, err := s.Encrypt([]byte(data), "") + encryptedData, err := s.Encrypt([]byte(data)) if err != nil { return EncryptedJSON{}, err } diff --git a/pkg/services/secrets/secrets.go b/pkg/services/secrets/secrets.go index f5434815257..d4a9abe3a30 100644 --- a/pkg/services/secrets/secrets.go +++ b/pkg/services/secrets/secrets.go @@ -1,6 +1,8 @@ package secrets import ( + "bytes" + "context" "encoding/base64" "fmt" "time" @@ -16,8 +18,9 @@ var logger = log.New("secrets") type Secrets struct { store *sqlstore.SqlStore `inject:""` - providers map[string]Provider - dataKeyCache map[string]dataKeyCacheItem + defaultEncryptionKey string + providers map[string]Provider + dataKeyCache map[string]dataKeyCacheItem } type dataKeyCacheItem struct { @@ -42,29 +45,60 @@ func (s *Secrets) Init() error { return nil } -func (s *Secrets) Encrypt(payload []byte, key string) ([]byte, error) { +var b64 = base64.RawStdEncoding + +func (s *Secrets) Encrypt(payload []byte) ([]byte, error) { + key := s.defaultEncryptionKey + dataKey, err := s.dataKey(key) if err != nil { return nil, err } - b64 := base64.StdEncoding + encrypted, err := encrypt(payload, dataKey) + if err != nil { + return nil, err + } + prefix := make([]byte, b64.EncodedLen(len(key))+2) b64.Encode(prefix[1:], []byte(key)) prefix[0] = '#' prefix[len(prefix)-1] = '#' - blob := make([]byte, len(prefix)+len(payload)) + blob := make([]byte, len(prefix)+len(encrypted)) copy(blob, prefix) - copy(blob[len(prefix):], payload) + copy(blob[len(prefix):], encrypted) - return encrypt(blob, dataKey) + return blob, nil } -func (s *Secrets) Decrypt(payload []byte, key string) ([]byte, error) { - dataKey, err := s.dataKey(key) - if err != nil { - return nil, err +func (s *Secrets) Decrypt(payload []byte) ([]byte, error) { + if len(payload) == 0 { + return []byte{}, nil + } + + var dataKey []byte + + if payload[0] != '#' { + dataKey = []byte(setting.SecretKey) + } else { + payload = payload[1:] + endOfKey := bytes.Index(payload, []byte{'#'}) + if endOfKey == -1 { + return nil, fmt.Errorf("could not find valid key in encrypted payload") + } + b64Key := payload[:endOfKey] + payload = payload[endOfKey+1:] + key := make([]byte, b64.DecodedLen(len(b64Key))) + _, err := b64.Decode(key, b64Key) + if err != nil { + return nil, err + } + + dataKey, err = s.dataKey(string(key)) + if err != nil { + return nil, err + } } return decrypt(payload, dataKey) @@ -83,8 +117,10 @@ func (s *Secrets) dataKey(key string) ([]byte, error) { } } + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() // 1. get encrypted data key from database - dataKey, err := s.store.GetDataKey(key) + dataKey, err := s.store.GetDataKey(ctx, key) if err != nil { return nil, err } diff --git a/pkg/services/secrets/secrets_test.go b/pkg/services/secrets/secrets_test.go new file mode 100644 index 00000000000..0a9142e5e07 --- /dev/null +++ b/pkg/services/secrets/secrets_test.go @@ -0,0 +1,45 @@ +package secrets + +import ( + "fmt" + "testing" + + "github.com/grafana/grafana/pkg/services/sqlstore" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/setting" +) + +func TestSecrets_Encrypt(t *testing.T) { + s := Secrets{ + store: sqlstore.InitTestDB(t), + } + + require.NoError(t, s.Init()) + + { + old := setting.SecretKey + defer func() { + setting.SecretKey = old + }() + setting.SecretKey = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + } + + plaintexts := [][]byte{ + {}, + []byte("hello, world"), + } + + for _, plaintext := range plaintexts { + t.Run(fmt.Sprintf("encrypting and decrypting %s", string(plaintext)), func(t *testing.T) { + encrypted, err := s.Encrypt(plaintext) + require.NoError(t, err) + decrypted, err := s.Decrypt(encrypted) + require.NoError(t, err) + + assert.Equal(t, plaintext, decrypted) + }) + } +} diff --git a/pkg/services/sqlstore/data_keys.go b/pkg/services/sqlstore/data_keys.go index e4097afcaf6..bd470f2d2f2 100644 --- a/pkg/services/sqlstore/data_keys.go +++ b/pkg/services/sqlstore/data_keys.go @@ -1,19 +1,40 @@ package sqlstore import ( + "context" "fmt" + "time" "github.com/grafana/grafana/pkg/models" "xorm.io/xorm" ) -func (ss *SqlStore) GetDataKey(name string) (*models.DataKey, error) { - return getDataKey(name, ss.engine) +const dataKeysTable = "data_keys" + +func (ss *SqlStore) GetDataKey(ctx context.Context, name string) (*models.DataKey, error) { + return getDataKey(ctx, name, ss.engine) } -func getDataKey(name string, engine *xorm.Engine) (*models.DataKey, error) { - dataKey := &models.DataKey{Name: name} - exists, err := engine.Get(dataKey) +func (ss *SqlStore) CreateDataKey(ctx context.Context, dataKey models.DataKey) error { + dataKey.Created = time.Now() + dataKey.Updated = dataKey.Created + + if !dataKey.Active { + return fmt.Errorf("cannot insert deactivated data keys") + } + + _, err := ss.engine.Context(ctx).Table(dataKeysTable).InsertOne(dataKey) + return err +} + +func (ss *SqlStore) DeleteDataKey(ctx context.Context, name string) error { + _, err := ss.engine.Context(ctx).Table(dataKeysTable).Delete(models.DataKey{Name: name}) + return err +} + +func getDataKey(ctx context.Context, name string, engine *xorm.Engine) (*models.DataKey, error) { + dataKey := &models.DataKey{Name: name, Active: true} + exists, err := engine.Context(ctx).Table(dataKeysTable).Get(dataKey) if err != nil { sqlog.Error("Failed getting data key", "err", err, "name", name) diff --git a/pkg/services/sqlstore/data_keys_test.go b/pkg/services/sqlstore/data_keys_test.go new file mode 100644 index 00000000000..ac4c0aa8fef --- /dev/null +++ b/pkg/services/sqlstore/data_keys_test.go @@ -0,0 +1,44 @@ +package sqlstore + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/models" + "github.com/stretchr/testify/assert" +) + +func TestDataKeys(t *testing.T) { + db := InitTestDB(t) + ctx := context.Background() + + dataKey := models.DataKey{ + Active: true, + Name: "Testing", + Provider: "test", + EncryptedData: []byte{0x62, 0xAF, 0xA1, 0x1A}, + } + + res, err := db.GetDataKey(ctx, dataKey.Name) + assert.Equal(t, models.ErrDataKeyNotFound, err) + assert.Nil(t, res) + + err = db.CreateDataKey(ctx, dataKey) + require.NoError(t, err) + + res, err = db.GetDataKey(ctx, dataKey.Name) + require.NoError(t, err) + assert.Equal(t, dataKey.EncryptedData, res.EncryptedData) + assert.Equal(t, dataKey.Provider, res.Provider) + assert.Equal(t, dataKey.Name, res.Name) + assert.True(t, dataKey.Active) + + err = db.DeleteDataKey(ctx, dataKey.Name) + require.NoError(t, err) + + res, err = db.GetDataKey(ctx, dataKey.Name) + assert.Equal(t, models.ErrDataKeyNotFound, err) + assert.Nil(t, res) +}