Security: Store datasource passwords encrypted in secureJsonData #16175

aocenas · 2019-03-23T13:29:30Z

This got a bit wide in the sense that it needs to touch much more parts than expected. Because of that I would welcome some early feedback if the approach still makes sense.

For data sources we store passwords either in non encrypted columns or in secureJsonData which is encrypted. MSSql and Postgres already store them encrypted other datasources store them plain text. What this PR is doing. Some caveats:

We store passwords also for browser only datasources which need to be sent back to front end in plaintext.
We have a provisioning format that already uses either secureJsonData or the plain fields depending on the datasource.
Non core datasources can already use secureJsonData for storage.

What this PR does so far:

Adds helper methods on Datasource to return plaintext passwords from secureJsonData if exists with fallback to plaintext db columns.
Uses this to return plaintext passwords to frontend for browser access datasources.
~~Adds migration that will move passwords from plaintext fields to secureJsonData for core datasources.~~
~~When core datasource is provisioned, both password or secureJsonData.password can be used and both will save the password in secureJsonData so the format stays the same but the behaviour changes.~~

After talking to other people it will make more sense to not complicate the code with migration and provisioning changes and instead users with existing datasources can migrate by resaving their datasources or altering their provisioned files. New datasources will be created with encrypted passwords by default.

TODO:

Fix existing tests
Add unitests
Manual testing

pkg/api/frontendsettings.go

pkg/cmd/grafana-cli/commands/migrate_database_command.go

pkg/plugins/plugins.go

pkg/services/provisioning/datasources/config_reader.go

pkg/services/provisioning/datasources/datasources.go

pkg/services/provisioning/datasources/types.go

pkg/services/sqlstore/migrations/encrypt_passwords.go

marefr

Have left a few comments. Think this is a good start. Haven't tested it yet though.

pkg/models/datasource.go

pkg/services/provisioning/datasources/types.go

pkg/services/sqlstore/migrations/encrypt_passwords.go

pkg/services/provisioning/datasources/config_reader.go

pkg/services/provisioning/datasources/datasources.go

pkg/services/sqlstore/migrations/encrypt_passwords.go

pkg/services/provisioning/datasources/types.go

docs/sources/installation/upgrading.md

aocenas · 2019-04-09T09:40:12Z

docs/sources/administration/provisioning.md

@@ -184,8 +188,8 @@ Secure json data is a map of settings that will be encrypted with [secret key](/
 | tlsCACert | string | *All* |CA cert for out going requests |
 | tlsClientCert | string | *All* |TLS Client cert for outgoing requests |
 | tlsClientKey | string | *All* |TLS Client key for outgoing requests |
-| password | string | PostgreSQL | password |
-| user | string | PostgreSQL | user |


Seems like this was an error. User is taken from user field not from secureJsonData

aocenas · 2019-04-09T09:41:14Z

pkg/services/provisioning/datasources/types.go

@@ -135,6 +137,12 @@ func (cfg *DatasourcesAsConfigV1) mapToDatasourceFromConfig(apiVersion int64) *D
 			Editable:          ds.Editable,
 			Version:           ds.Version,
 		})
+		if ds.Password != "" {


Added warnings here, thought not sure if there won't be too much of them if somebody has a lots of datasources.

aocenas · 2019-04-09T09:44:34Z

pkg/services/sqlstore/datasource.go

@@ -178,6 +178,10 @@ func UpdateDataSource(cmd *m.UpdateDataSourceCommand) error {
 		sess.UseBool("basic_auth")
 		sess.UseBool("with_credentials")
 		sess.UseBool("read_only")
+		// Make sure password are zeroed out if empty. We do this as we want to migrate passwords from
+		// plain text fields to SecureJsonData.
+		sess.MustCols("password")


Interestingly at the moment it is not actually possible to remove values from config. I needed to add these to be able to do that for plain text password, but otherwise no other value can be removed. Probably not a big issue but wondered how to properly handle such things. Should there be something like fieldSet bool when serialising the request? Right now we send the whole object to the API so this should not be an issue, but this won't support a partial PUT requests.

aocenas · 2019-04-09T09:46:18Z

public/app/plugins/datasource/influxdb/module.ts

+  onPasswordChange: ReturnType<typeof createChangeHandler>;
+
+  constructor() {
+    this.onPasswordReset = createResetHandler(this, PasswordFieldEnum.Password);


Wanted to reuse the handler logic as it is the same everywhere but my Angular is rusty so this patter was the best I come up with.

pkg/models/datasource.go

pkg/services/provisioning/datasources/config_reader.go

pkg/util/strings.go

bergquist

LGTM!

* grafana/master: Docs: minor docs update for old urls Chore: Add more explicit typing (grafana#16594) Chore: Lowered implicit anys limit to 5977 Chore: Adds typings to lodash (grafana#16590) PanelEditor: Change Queries heading to Query (grafana#16536) Security: Store datasource passwords encrypted in secureJsonData (grafana#16175) More development dashboards (grafana#16550) build: upgrades to golang 1.12.4 (grafana#16545) Use package libfontconfig1, instead of libfontconfig (grafana#16548) Adjust Send on all alerts to default label (grafana#16554) Chore: Lower limit of implicit anys to 6676

Store passwords in secureJsonData

1e7cc43

aocenas changed the title ~~Store datasource passwords ecrypted in secureJsonData~~ [WIP] Store datasource passwords ecrypted in secureJsonData Mar 23, 2019