Encrypt the storing of API secrets

[harmeet-status]

API secrets are stored in Spiff DB, unencrypted. The host is encrypted though.

[harmeet-status]

Comment from @burnettk:

There is code in backend to encrypt them, so we could turn that on via configuration and re-install secrets on the four relevant envs. https://github.com/sartography/spiff-arena/blob/10eb5865a7d4b6a286c1435197f290e357846f80/spiffworkflow-backend/src/spiffworkflow_backend/config/default.py#L159.

[burnettk]

@jakubgs would you possibly be able to update the FLASK_SESSION_SECRET_KEY on each env to make it 32 characters long. spiff_backend_session_secret_key: '{{lookup("passwordstore", "services/spiff-workflow/"+stage+"/session-key")}}'.

uuidgen | sed 's/-//g' sort of thing.

we are currently using FLASK_SESSION_SECRET_KEY to encrypt the secrets, and if we use the cryptography lib, it wants a decryption key of length 32. we could truncate FLASK_SESSION_SECRET_KEY for this purpose, but that feels confusing. we could also use the simple crypt lib, which doesn't appear to have requirement. lots of options, but if it's easy/reasonable to change the FLASK_SESSION_SECRET_KEY values (i don't think we're really using them for anything at the moment, so changing them should have no effect), that would make this simpler.

[jakubgs]

Our secrets currently have 64 characters:

 > pass services/spiff-workflow/app-test/session-key | tr -d '[:space:]' | wc -c
64

I don't know if I buy making them shorter. What kind of library are you trying to use exactly?

[jakubgs]

If these secrets will be used for encryption and current ones are unused then the variable names should be changed to reflect their actual function.

Also, if they are not used, then how exactly are we handling cookies?

[jasquat]

We updated the configs needed to turn on encryption in PR #673. Set the configs mentioned there to turn it on.

[burnettk]

oh yeah, you're right, we're using the FLASK_SESSION_SECRET_KEY for cookies. it's the cryptography lib. we're adding a new environment variable to specify the encryption key for this new "secrets encryption" purpose: SPIFFWORKFLOW_BACKEND_ENCRYPTION_KEY_FOR_SECRETS

[jakubgs]

You could avoid 4 characters in that variable name by calling it SPIFFWORKFLOW_BACKEND_SECRETS_ENCRYPTION_KEY.

But ah well.

[jakubgs]

Done:

• infra-spiff-workflow#ac8d5987 - spiff-workflow: env variable for secrets encryption

All environments have it.

[jasquat]

Reopening since we still need to merge PR #673 and release it.

[jakubgs]

The PR has been merged.

[burnettk]

I think we're releasing the code that makes use of this new env var to status prod on monday, and then we'll need to have a little party where we actually enable encryption and re-encrypt the values on each env (it will be a party because we don't have access to merge to infra--to change the env var to enable encryption--or to edit secrets on prod).

[harmeet-status]

@madhurrya moving this back to Ready for Release, as we need to enable this on our infrastructure. Once that is done, we can close this ticket.

[jakubgs]

This has been deployed since 2 weeks ago.

[harmeet-status]

Changing to Resolved.

[burnettk]

this ticket has multiple parts. the code changes to support it are live, but the env var to actually enable encryption (SPIFFWORKFLOW_BACKEND_ENCRYPTION_LIB) has not been set, and we have not manually re-encrypted secrets on all envs. @harmeet-status , do you want to set up a meeting with us and @jakubgs or a delegate who can update infra and we can knock out these two pieces?

[jakubgs]

I don't know if we need a meeting for this. If you need me to run some kind of script/commands to encrypt stuff in the database I'm happy to do that manually if it's just a on-off(per stage) operation.

[burnettk]

the environment variable to enable encryption is SPIFFWORKFLOW_BACKEND_ENCRYPTION_LIB=cryptography, but we have to delete all secrets before changing that environment variable.

[jakubgs]

Approved and merged the PR:

• https://github.com/status-im/infra-spiff-workflow/pull/34

Backed up secrets from the secret table and then re-added the secrets on app-dev:

admin@node-01.sm-eu-zur1.spiff.app-dev:~ % /docker/spiff-workflow/backend-db-admin.sh -D spiff-workflow-backend -e 'SELECT count(*) FROM secret;'
+----------+
| count(*) |
+----------+
|        7 |
+----------+

[jakubgs]

Is there anything left to do here?

[jakubgs]

I see on production the secrets are still not encrypted. https://github.com/status-im/infra-spiff-workflow/blob/955e605502ac5318bb2077c2719cdf30944db375/ansible/group_vars/spiff.yml#L30-L31

[jakubgs]

BTW, this issue should have been created in infra-spiff, not here. This should be closed favor of:

• https://github.com/status-im/infra-spiff-workflow/issues/40

[burnettk]

closing in favor of the linked infra issue.

[harmeet-status]

@burnettk I'm thinking that we dont let the user retrieve a secret, only save it. This is just an additional level of security.

What do you think?

[burnettk]

turning retrieval off seems like a reasonable configuration option. we stole the existing UX from the way aws handles secrets, so I suspect some people will want one way and others will want the other way. it is undeniably handy to be able to fetch them from the UI. i would personally still fetch them, but i would need to go onto the server and run a script to be able to do so. we could remove the feature from the UI, kill the API, or both, potentially based on configuration settings. maybe we should just use permissions to implement this on status envs, since that would be simpler and already mostly built?

[harmeet-status]

Opening this ticket to allow us to action my last comment @burnettk

Comment:

Secrets should not be able to be retrieved, they can only be saved

[burnettk]

Opened https://github.com/sartography/spiff-arena/issues/1348 for this.

[calexh-sar]

@harmeet-status closing this ticket and will accomplish this additional work in #1348.