From 0730ff7ab987546c2049c18f432bbd7695a7719e Mon Sep 17 00:00:00 2001 From: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com> Date: Thu, 19 Jan 2023 15:59:33 +0200 Subject: [PATCH 1/3] feat: config encryption Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com> --- cmd/migrate.go | 14 ++- cmd/root.go | 2 + cmd/server.go | 8 +- internal/app/integration/manager.go | 4 +- internal/app/integration/store.go | 2 +- internal/app/integration/storememory.go | 4 +- .../app/migrations/006_conifg_encryption.go | 95 +++++++++++++++++++ internal/app/models/connector.go | 7 +- internal/app/storage/connectors.go | 33 +++---- internal/app/storage/module.go | 4 +- internal/app/storage/repository.go | 9 +- 11 files changed, 148 insertions(+), 34 deletions(-) create mode 100644 internal/app/migrations/006_conifg_encryption.go diff --git a/cmd/migrate.go b/cmd/migrate.go index 0d12d6b6..01a97e54 100644 --- a/cmd/migrate.go +++ b/cmd/migrate.go @@ -4,10 +4,11 @@ import ( "fmt" "log" + "github.com/formancehq/payments/internal/app/migrations" + "github.com/spf13/viper" - // allow blank import to initiate migrations. - _ "github.com/formancehq/payments/internal/app/migrations" + // Import the postgres driver. _ "github.com/lib/pq" "github.com/pressly/goose/v3" @@ -48,6 +49,15 @@ func runMigrate(cmd *cobra.Command, args []string) error { return fmt.Errorf("postgres uri is not set") } + cfgEncryptionKey := viper.GetString(configEncryptionKeyFlag) + if cfgEncryptionKey == "" { + cfgEncryptionKey = cmd.Flag(configEncryptionKeyFlag).Value.String() + } + + if cfgEncryptionKey != "" { + migrations.EncryptionKey = cfgEncryptionKey + } + database, err := goose.OpenDBWithDriver("postgres", postgresURI) if err != nil { return fmt.Errorf("failed to open database: %w", err) diff --git a/cmd/root.go b/cmd/root.go index 4f3facb8..f60b270a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -43,9 +43,11 @@ func rootCommand() *cobra.Command { root.PersistentFlags().Bool(debugFlag, false, "Debug mode") migrate.Flags().String(postgresURIFlag, "postgres://localhost/payments", "PostgreSQL DB address") + migrate.Flags().String(configEncryptionKeyFlag, "", "Config encryption key") server.Flags().BoolP("toggle", "t", false, "Help message for toggle") server.Flags().String(postgresURIFlag, "postgres://localhost/payments", "PostgreSQL DB address") + server.Flags().String(configEncryptionKeyFlag, "", "Config encryption key") server.Flags().String(envFlag, "local", "Environment") server.Flags().Bool(publisherKafkaEnabledFlag, false, "Publish write events to kafka") server.Flags().StringSlice(publisherKafkaBrokerFlag, []string{}, "Kafka address is kafka enabled") diff --git a/cmd/server.go b/cmd/server.go index b872227f..53759785 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -30,6 +30,7 @@ import ( //nolint:gosec // false positive const ( postgresURIFlag = "postgres-uri" + configEncryptionKeyFlag = "config-encryption-key" otelTracesFlag = "otel-traces" envFlag = "env" publisherKafkaEnabledFlag = "publisher-kafka-enabled" @@ -157,7 +158,12 @@ func prepareDatabaseOptions() (fx.Option, error) { return nil, errors.New("missing postgres uri") } - return storage.Module(postgresURI), nil + configEncryptionKey := viper.GetString(configEncryptionKeyFlag) + if configEncryptionKey == "" { + return nil, errors.New("missing config encryption key") + } + + return storage.Module(postgresURI, configEncryptionKey), nil } func topicsMapping() map[string]string { diff --git a/internal/app/integration/manager.go b/internal/app/integration/manager.go index b3c106fb..ad20fc33 100644 --- a/internal/app/integration/manager.go +++ b/internal/app/integration/manager.go @@ -213,8 +213,8 @@ func (l *ConnectorManager[ConnectorConfig]) IsEnabled(ctx context.Context) (bool return l.store.IsEnabled(ctx, l.loader.Name()) } -func (l *ConnectorManager[ConnectorConfig]) FindAll(ctx context.Context) ([]models.Connector, error) { - return l.store.FindAll(ctx) +func (l *ConnectorManager[ConnectorConfig]) FindAll(ctx context.Context) ([]*models.Connector, error) { + return l.store.ListConnectors(ctx) } func (l *ConnectorManager[ConnectorConfig]) IsInstalled(ctx context.Context) (bool, error) { diff --git a/internal/app/integration/store.go b/internal/app/integration/store.go index 8fe0ea8d..fa124c36 100644 --- a/internal/app/integration/store.go +++ b/internal/app/integration/store.go @@ -8,7 +8,7 @@ import ( ) type Repository interface { - FindAll(ctx context.Context) ([]models.Connector, error) + ListConnectors(ctx context.Context) ([]*models.Connector, error) IsInstalled(ctx context.Context, name models.ConnectorProvider) (bool, error) Install(ctx context.Context, name models.ConnectorProvider, config json.RawMessage) error Uninstall(ctx context.Context, name models.ConnectorProvider) error diff --git a/internal/app/integration/storememory.go b/internal/app/integration/storememory.go index 3208b602..93ba4f3f 100644 --- a/internal/app/integration/storememory.go +++ b/internal/app/integration/storememory.go @@ -21,8 +21,8 @@ func (i *InMemoryConnectorStore) Uninstall(ctx context.Context, name models.Conn return nil } -func (i *InMemoryConnectorStore) FindAll(_ context.Context) ([]models.Connector, error) { - return []models.Connector{}, nil +func (i *InMemoryConnectorStore) ListConnectors(_ context.Context) ([]*models.Connector, error) { + return []*models.Connector{}, nil } func (i *InMemoryConnectorStore) IsInstalled(ctx context.Context, name models.ConnectorProvider) (bool, error) { diff --git a/internal/app/migrations/006_conifg_encryption.go b/internal/app/migrations/006_conifg_encryption.go new file mode 100644 index 00000000..b33e15ee --- /dev/null +++ b/internal/app/migrations/006_conifg_encryption.go @@ -0,0 +1,95 @@ +package migrations + +import ( + "database/sql" + "fmt" + + "github.com/pkg/errors" + + "github.com/pressly/goose/v3" +) + +// EncryptionKey is set from the migration utility to specify default encryption key to migrate to. +// This can remain empty. Then the config will be removed. +// +//nolint:gochecknoglobals // This is a global variable by design. +var EncryptionKey string + +func init() { + up := func(tx *sql.Tx) error { + var exists bool + + err := tx.QueryRow("SELECT EXISTS(SELECT 1 FROM connectors.connector)").Scan(&exists) + if err != nil { + return fmt.Errorf("failed to check if connectors table exists: %w", err) + } + + if exists && EncryptionKey == "" { + return errors.New("encryption key is not set") + } + + _, err = tx.Exec(` + CREATE EXTENSION IF NOT EXISTS pgcrypto; + ALTER TABLE connectors.connector RENAME COLUMN config TO config_unencrypted; + ALTER TABLE connectors.connector ADD COLUMN config bytea NULL; + `) + if err != nil { + return fmt.Errorf("failed to create config column: %w", err) + } + + _, err = tx.Exec(` + UPDATE connectors.connector SET config = pgp_sym_encrypt(config_unencrypted::TEXT, $1, 'compress-algo=1, cipher-algo=aes256'); + `, EncryptionKey) + if err != nil { + return fmt.Errorf("failed to encrypt config: %w", err) + } + + _, err = tx.Exec(` + ALTER TABLE connectors.connector DROP COLUMN config_unencrypted; + `) + if err != nil { + return fmt.Errorf("failed to drop config_unencrypted column: %w", err) + } + + return nil + } + + down := func(tx *sql.Tx) error { + var exists bool + + err := tx.QueryRow("SELECT EXISTS(SELECT 1 FROM connectors.connector)").Scan(&exists) + if err != nil { + return fmt.Errorf("failed to check if connectors table exists: %w", err) + } + + if exists && EncryptionKey == "" { + return errors.New("encryption key is not set") + } + + _, err = tx.Exec(` + ALTER TABLE connectors.connector RENAME COLUMN config TO config_encrypted; + ALTER TABLE connectors.connector ADD COLUMN config JSON NULL; + `) + if err != nil { + return fmt.Errorf("failed to create config column: %w", err) + } + + _, err = tx.Exec(` + UPDATE connectors.connector SET config = pgp_sym_decrypt(config_encrypted, $1, 'compress-algo=1, cipher-algo=aes256')::JSON; + `, EncryptionKey) + if err != nil { + return fmt.Errorf("failed to decrypt config: %w", err) + } + + _, err = tx.Exec(` + ALTER TABLE connectors.connector DROP COLUMN config_encrypted; + `) + if err != nil { + return fmt.Errorf("failed to drop config_encrypted column: %w", err) + } + + return nil + } + + goose.AddMigration(up, down) +} diff --git a/internal/app/models/connector.go b/internal/app/models/connector.go index 892dd61e..0a39c2a2 100644 --- a/internal/app/models/connector.go +++ b/internal/app/models/connector.go @@ -19,8 +19,11 @@ type Connector struct { Provider ConnectorProvider Enabled bool - // TODO: Enable DB-level encryption - Config json.RawMessage + // EncryptedConfig is a PGP-encrypted JSON string. + EncryptedConfig string `bun:"config"` + + // Config is a decrypted config. It is not stored in the database. + Config json.RawMessage `bun:"decrypted_config,scanonly"` Tasks []*Task `bun:"rel:has-many,join:id=connector_id"` Payments []*Payment `bun:"rel:has-many,join:id=connector_id"` diff --git a/internal/app/storage/connectors.go b/internal/app/storage/connectors.go index 8ba0c154..1ce82a94 100644 --- a/internal/app/storage/connectors.go +++ b/internal/app/storage/connectors.go @@ -9,20 +9,25 @@ import ( ) func (s *Storage) ListConnectors(ctx context.Context) ([]*models.Connector, error) { - var res []*models.Connector - err := s.db.NewSelect().Model(&res).Scan(ctx) + var connectors []*models.Connector + + err := s.db.NewSelect(). + Model(&connectors). + ColumnExpr("*, pgp_sym_decrypt(config, ?, ?) AS decrypted_config", s.configEncryptionKey, encryptionOptions). + Scan(ctx) if err != nil { return nil, e("list connectors", err) } - return res, nil + return connectors, nil } func (s *Storage) GetConfig(ctx context.Context, connectorProvider models.ConnectorProvider, destination any) error { var connector models.Connector - err := s.db.NewSelect().Model(&connector). - Column("config"). + err := s.db.NewSelect(). + Model(&connector). + ColumnExpr("pgp_sym_decrypt(config, ?, ?) AS decrypted_config", s.configEncryptionKey, encryptionOptions). Where("provider = ?", connectorProvider). Scan(ctx) if err != nil { @@ -37,17 +42,6 @@ func (s *Storage) GetConfig(ctx context.Context, connectorProvider models.Connec return nil } -func (s *Storage) FindAll(ctx context.Context) ([]models.Connector, error) { - var connectors []models.Connector - - err := s.db.NewSelect().Model(&connectors).Scan(ctx) - if err != nil { - return nil, e("find all connectors", err) - } - - return connectors, err -} - func (s *Storage) IsInstalled(ctx context.Context, provider models.ConnectorProvider) (bool, error) { exists, err := s.db.NewSelect(). Model(&models.Connector{}). @@ -64,7 +58,6 @@ func (s *Storage) Install(ctx context.Context, provider models.ConnectorProvider connector := models.Connector{ Provider: provider, Enabled: true, - Config: config, } _, err := s.db.NewInsert().Model(&connector).Exec(ctx) @@ -72,7 +65,7 @@ func (s *Storage) Install(ctx context.Context, provider models.ConnectorProvider return e("install connector", err) } - return nil + return s.UpdateConfig(ctx, provider, config) } func (s *Storage) Uninstall(ctx context.Context, provider models.ConnectorProvider) error { @@ -90,7 +83,7 @@ func (s *Storage) Uninstall(ctx context.Context, provider models.ConnectorProvid func (s *Storage) UpdateConfig(ctx context.Context, provider models.ConnectorProvider, config json.RawMessage) error { _, err := s.db.NewUpdate(). Model(&models.Connector{}). - Set("config = ?", config). + Set("config = pgp_sym_encrypt(?::TEXT, ?, ?)", config, s.configEncryptionKey, encryptionOptions). Where("provider = ?", provider). Exec(ctx) if err != nil { @@ -131,6 +124,7 @@ func (s *Storage) IsEnabled(ctx context.Context, provider models.ConnectorProvid err := s.db.NewSelect(). Model(&connector). + Column("enabled"). Where("provider = ?", provider). Scan(ctx) if err != nil { @@ -145,6 +139,7 @@ func (s *Storage) GetConnector(ctx context.Context, provider models.ConnectorPro err := s.db.NewSelect(). Model(&connector). + ColumnExpr("*, pgp_sym_decrypt(config, ?, ?) AS decrypted_config", s.configEncryptionKey, encryptionOptions). Where("provider = ?", provider). Scan(ctx) if err != nil { diff --git a/internal/app/storage/module.go b/internal/app/storage/module.go index eb82307a..0ba80d52 100644 --- a/internal/app/storage/module.go +++ b/internal/app/storage/module.go @@ -20,7 +20,7 @@ import ( const dbName = "paymentsDB" -func Module(uri string) fx.Option { +func Module(uri, configEncryptionKey string) fx.Option { return fx.Options( fx.Provide(func() (*pgx.ConnConfig, error) { config, err := pgx.ParseConfig(uri) @@ -40,7 +40,7 @@ func Module(uri string) fx.Option { db.AddQueryHook(bunotel.NewQueryHook(bunotel.WithDBName(dbName))) - return newStorage(db) + return newStorage(db, configEncryptionKey) }), fx.Invoke(func(lc fx.Lifecycle, repo *Storage) { diff --git a/internal/app/storage/repository.go b/internal/app/storage/repository.go index 1804800e..6abecc59 100644 --- a/internal/app/storage/repository.go +++ b/internal/app/storage/repository.go @@ -6,11 +6,14 @@ import ( ) type Storage struct { - db *bun.DB + db *bun.DB + configEncryptionKey string } -func newStorage(db *bun.DB) *Storage { - return &Storage{db: db} +const encryptionOptions = "compress-algo=1, cipher-algo=aes256" + +func newStorage(db *bun.DB, configEncryptionKey string) *Storage { + return &Storage{db: db, configEncryptionKey: configEncryptionKey} } // nolint:unused // used for SQL debugging purposes From 3b62b93b657635fcdfa2f3ea4b02e625a3fa2593 Mon Sep 17 00:00:00 2001 From: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com> Date: Thu, 19 Jan 2023 16:16:46 +0200 Subject: [PATCH 2/3] feat: obfuscate sensitive fields Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com> --- internal/app/connectors/bankingcircle/config.go | 7 +++++++ internal/app/connectors/currencycloud/config.go | 7 +++++++ internal/app/connectors/dummypay/config.go | 2 +- internal/app/connectors/modulr/config.go | 7 +++++++ internal/app/connectors/stripe/config.go | 4 +++- internal/app/connectors/wise/config.go | 6 ++++++ internal/app/models/connector.go | 9 +++++++++ 7 files changed, 40 insertions(+), 2 deletions(-) diff --git a/internal/app/connectors/bankingcircle/config.go b/internal/app/connectors/bankingcircle/config.go index 233e09c6..cc257a3d 100644 --- a/internal/app/connectors/bankingcircle/config.go +++ b/internal/app/connectors/bankingcircle/config.go @@ -2,6 +2,7 @@ package bankingcircle import ( "encoding/json" + "fmt" "github.com/formancehq/payments/internal/app/connectors/configtemplate" ) @@ -13,6 +14,12 @@ type Config struct { AuthorizationEndpoint string `json:"authorizationEndpoint" yaml:"authorizationEndpoint" bson:"authorizationEndpoint"` } +// String obfuscates sensitive fields and returns a string representation of the config. +// This is used for logging. +func (c Config) String() string { + return fmt.Sprintf("username=%s, password=****, endpoint=%s, authorizationEndpoint=%s", c.Username, c.Endpoint, c.AuthorizationEndpoint) +} + func (c Config) Validate() error { if c.Username == "" { return ErrMissingUsername diff --git a/internal/app/connectors/currencycloud/config.go b/internal/app/connectors/currencycloud/config.go index ea9c3673..3015566c 100644 --- a/internal/app/connectors/currencycloud/config.go +++ b/internal/app/connectors/currencycloud/config.go @@ -2,6 +2,7 @@ package currencycloud import ( "encoding/json" + "fmt" "time" "github.com/formancehq/payments/internal/app/connectors/configtemplate" @@ -14,6 +15,12 @@ type Config struct { PollingPeriod Duration `json:"pollingPeriod" bson:"pollingPeriod"` } +// String obfuscates sensitive fields and returns a string representation of the config. +// This is used for logging. +func (c Config) String() string { + return fmt.Sprintf("loginID=%s, endpoint=%s, pollingPeriod=%s, apiKey=****", c.LoginID, c.Endpoint, c.PollingPeriod.String()) +} + func (c Config) Validate() error { if c.APIKey == "" { return ErrMissingAPIKey diff --git a/internal/app/connectors/dummypay/config.go b/internal/app/connectors/dummypay/config.go index f45ffe50..40cde2a2 100644 --- a/internal/app/connectors/dummypay/config.go +++ b/internal/app/connectors/dummypay/config.go @@ -23,7 +23,7 @@ type Config struct { // String returns a string representation of the configuration. func (c Config) String() string { - return fmt.Sprintf("directory: %s, filePollingPeriod: %s, fileGenerationPeriod: %s", + return fmt.Sprintf("directory=%s, filePollingPeriod=%s, fileGenerationPeriod=%s", c.Directory, c.FilePollingPeriod.String(), c.FileGenerationPeriod.String()) } diff --git a/internal/app/connectors/modulr/config.go b/internal/app/connectors/modulr/config.go index da0ba040..b2ca2636 100644 --- a/internal/app/connectors/modulr/config.go +++ b/internal/app/connectors/modulr/config.go @@ -2,6 +2,7 @@ package modulr import ( "encoding/json" + "fmt" "github.com/formancehq/payments/internal/app/connectors/configtemplate" ) @@ -12,6 +13,12 @@ type Config struct { Endpoint string `json:"endpoint" bson:"endpoint"` } +// String obfuscates sensitive fields and returns a string representation of the config. +// This is used for logging. +func (c Config) String() string { + return fmt.Sprintf("endpoint=%s, apiSecret=***, apiKey=****", c.Endpoint) +} + func (c Config) Validate() error { if c.APIKey == "" { return ErrMissingAPIKey diff --git a/internal/app/connectors/stripe/config.go b/internal/app/connectors/stripe/config.go index 2edb0634..ec61d930 100644 --- a/internal/app/connectors/stripe/config.go +++ b/internal/app/connectors/stripe/config.go @@ -16,8 +16,10 @@ type Config struct { TimelineConfig `bson:",inline"` } +// String obfuscates sensitive fields and returns a string representation of the config. +// This is used for logging. func (c Config) String() string { - return fmt.Sprintf("pollingPeriod=%d, pageSize=%d, apiKey=%s", c.PollingPeriod, c.PageSize, c.APIKey) + return fmt.Sprintf("pollingPeriod=%d, pageSize=%d, apiKey=****", c.PollingPeriod, c.PageSize) } func (c Config) Validate() error { diff --git a/internal/app/connectors/wise/config.go b/internal/app/connectors/wise/config.go index b1c01d51..31552dd6 100644 --- a/internal/app/connectors/wise/config.go +++ b/internal/app/connectors/wise/config.go @@ -10,6 +10,12 @@ type Config struct { APIKey string `json:"apiKey" yaml:"apiKey" bson:"apiKey"` } +// String obfuscates sensitive fields and returns a string representation of the config. +// This is used for logging. +func (c Config) String() string { + return "apiKey=***" +} + func (c Config) Validate() error { if c.APIKey == "" { return ErrMissingAPIKey diff --git a/internal/app/models/connector.go b/internal/app/models/connector.go index 0a39c2a2..51046398 100644 --- a/internal/app/models/connector.go +++ b/internal/app/models/connector.go @@ -29,6 +29,15 @@ type Connector struct { Payments []*Payment `bun:"rel:has-many,join:id=connector_id"` } +func (c Connector) String() string { + c.EncryptedConfig = "****" + c.Config = nil + + var t any = c + + return fmt.Sprintf("%+v", t) +} + type ConnectorProvider string const ( From 53ca70fd418658dae3af07a2719b64dcb361952a Mon Sep 17 00:00:00 2001 From: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com> Date: Thu, 19 Jan 2023 16:19:56 +0200 Subject: [PATCH 3/3] fix: config test case Signed-off-by: Lawrence Zawila <113581282+darkmatterpool@users.noreply.github.com> --- internal/app/connectors/dummypay/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/app/connectors/dummypay/config_test.go b/internal/app/connectors/dummypay/config_test.go index 3375c6da..8eba6239 100644 --- a/internal/app/connectors/dummypay/config_test.go +++ b/internal/app/connectors/dummypay/config_test.go @@ -19,7 +19,7 @@ func TestConfigString(t *testing.T) { FileGenerationPeriod: connectors.Duration{Duration: time.Minute}, } - assert.Equal(t, "directory: test, filePollingPeriod: 1s, fileGenerationPeriod: 1m0s", config.String()) + assert.Equal(t, "directory=test, filePollingPeriod=1s, fileGenerationPeriod=1m0s", config.String()) } // TestConfigValidate tests the validation of the config.