AES secrets encryption #2300

zc-devs · 2023-08-20T20:39:57Z

PR enables AES secrets encryption.

It consist of #1475, #1544. Part of #1541 and #1814.

Made decorator over SecretService instead of SecretStore
Temporary removed Tink encryption
Removed unnecessary code
Fixed documentation

# Conflicts: # cmd/server/flags.go # cmd/server/server.go # server/store/datastore/secret.go # server/store/encryped/secret.go

# Conflicts: # server/store/encryped/secret.go

https://stackoverflow.com/questions/47382035/unable-to-decrypt-after-aes-gcm-base64-in-go

zc-devs · 2023-08-20T20:46:06Z

Config:

  WOODPECKER_ENCRYPTION_KEY: GjVHT007c4x3N+YPbsZld+hifba1enXkOzIb/0h6oW8=

Test pipeline:

steps:
  test:
    image: alpine
    commands:
      - echo "$SOME_SECRET" | sed -e 's/\(.\)/\1\ /g'
    secrets:
      - some_secret

New secret creation

Logs:

{"level":"debug","id":0,"name":"some_secret","time":"2023-08-20T19:37:42Z","caller":"/woodpecker/server/plugins/secrets/encrypted.go:219","message":"encryption"}
{"level":"debug","id":9,"name":"some_secret","time":"2023-08-20T19:37:42Z","caller":"/woodpecker/server/plugins/secrets/encrypted.go:230","message":"decryption"}

DB:

woodpecker=# select secret_id, secret_name, secret_value from secrets;
 secret_id | secret_name |                          secret_value
-----------+-------------+----------------------------------------------------------------
         9 | some_secret | PUattjAz6EOP28sbJOEaDSZyXRDrPxGQv9EyQHQPimrWLQELr59WYp83DUNQ6w
(1 row)

Test run:

+ echo "$SOME_SECRET" | sed -e 's/\(.\)/\1\n/g'
s u p e r - s e c r e t - v a l u e

Changed secret value

DB:

woodpecker=# select secret_id, secret_name, secret_value from secrets;
 secret_id | secret_name |                           secret_value
-----------+-------------+------------------------------------------------------------------
         9 | some_secret | ttXvtoOY3ixRMeU/niFNG/O6jcO2VCuCK1MZjFr68OzlytBD5d6q8ylUsvRfXs/M
(1 row)

Test run:

+ echo "$SOME_SECRET" | sed -e 's/\(.\)/\1\ /g'
c h a n g e d - s e c r e t - v a l u e

cmd/server/server.go

docs/docs/30-administration/40-encryption.md

anbraten · 2023-08-20T20:55:15Z

server/plugins/secrets/encrypted.go

+func (ess *encryptedSecretService) decrypt(secret *model.Secret) error {
+	log.Debug().Int64("id", secret.ID).Str("name", secret.Name).Msg("decryption")
+
+	decryptedValue, err := ess.encryptionSvc.Decrypt(secret.Value, secret.Name)


Could we add some logic here that detects the encryption method. So it would know if a secret was encrypted using aes or maybe tink, ... And if the secret wasn't encrypted at all (before enabling the encryption) it will simply skip the decryption

Could we add some logic here that detects the encryption method. So it would know if a secret was encrypted using aes or maybe tink

It's overcomplicated, I think. Admin should know, what encryption he enables and set up WP accordingly. So, we know encryption method at runtime.

And if the secret wasn't encrypted at all (before enabling the encryption) it will simply skip the decryption

Then what's the point of encrypted decorator? In that case plain SecretService should be used.

I would just get back migration logic. From plain to encrypted according to settings (AES, Tink). I'll do it in this PR.

As for more complicated migrations like AES -> Tink, Encrypted -> Plain, I would use CLI or maybe some button in admin panel. But this is out of scope of this PR.

I had sth simple like this common-helper in mind which would just support decrypting plain secrets as well. It would also allow us to not do an active migration and instead do a migration on demand each time a user adds a new secrets or updates an existing one.

func decryptValue(secret *Secret) (string, string) { if strings.HasPrefix(secret.Value, "_aes_") { return "aes", strings.TrimPrefix(secret.Value, "_aes_") } return "plain", secret.Value } func decrypt(secret *Secret) error { method, value := decryptValue(secret) if method == "plain" { // do nothing } else if method == "aes" { // do aes stuff value, err := aesMagic(value) secret.Value = value return err } return fmt.Errorf("unknown secret encryption method") }

I've added encoding/decoding as you sad. At least it looks more readable in DB.

not do an active migration and instead do a migration on demand each time a user adds a new secrets or updates an existing one

However, as an administrator I prefer active migration. I schedule time to evaluate new feature and then to roll it out. And I would do it carefully under my control. I would not want to have issues month later after enabling the feature.

woodpecker-bot · 2023-08-20T21:00:51Z

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-2300.surge.sh

6543 · 2023-08-23T02:59:18Z

well before we merge we have to make sure migration of non encrypted and back and mix in-between works properly ... we had to deactivate it by code last time :/

6543 · 2023-08-23T03:00:58Z

cmd/server/flags.go

 		FilePath: os.Getenv("WOODPECKER_ENCRYPTION_KEY_FILE"),
 	},
 	&cli.StringFlag{
 		EnvVars: []string{"WOODPECKER_ENCRYPTION_TINK_KEYSET_FILE"},
 		Name:    "encryption-tink-keyset",
 		Usage:   "Google tink AEAD-compatible keyset file to encrypt secrets in DB",
 	},
-	&cli.BoolFlag{


there should be an option to revert encryption back to unencrypted ... we could add a warning if that's really the intended case ... but we should have it

make sure migration of non encrypted and back

Done

mix in-between

This can be achieved via disabling encryption. For example, Plain -> AES -> Plain -> Tink. I would leave it like that, for this PR at least.

- secret's value encoding/decoding - public encryption service - encryption modes - migrations

zc-devs · 2023-09-24T11:11:11Z

Sorry for pretty long delay. Have been some traveling.

There are tests of encryption / decryption below:

 secret_id | secret_name | secret_value
-----------+-------------+--------------
         1 | s-name      | s-value

WOODPECKER_ENCRYPTION_KEY: GjVHT007c4x3N+YPbsZld+hifba1enXkOzIb/0h6oW8=
WOODPECKER_SECRETS_ENCRYPTION_MODE: EnabledAndEncrypt

{"time":"2023-09-24T10:47:26Z","caller":"/woodpecker/cmd/common/logger.go:99","message":"LogLevel = debug"}
{"level":"debug","mode":"EnabledAndEncrypt","time":"2023-09-24T10:47:28Z","caller":"/woodpecker/server/plugins/secrets/setup.go:39","message":"setting up secrets service"}
{"level":"fatal","error":"secret and encryption services cannot be nil","time":"2023-09-24T10:47:28Z","caller":"/woodpecker/cmd/server/setup.go:127","message":"failed to set up secrets service"}

WOODPECKER_ENCRYPTION_AES_KEY: GjVHT007c4x3N+YPbsZld+hifba1enXkOzIb/0h6oW8=
WOODPECKER_SECRETS_ENCRYPTION_MODE: EnabledAndEncrypt

{"time":"2023-09-24T10:49:20Z","caller":"/woodpecker/cmd/common/logger.go:99","message":"LogLevel = debug"}
{"level":"debug","time":"2023-09-24T10:49:21Z","caller":"/woodpecker/server/plugins/encryption/aes.go:41","message":"initializing AES encryption service"}
{"level":"debug","time":"2023-09-24T10:49:21Z","caller":"/woodpecker/server/plugins/encryption/aes.go:68","message":"AES encryption service has been initialized"}
{"level":"debug","mode":"EnabledAndEncrypt","time":"2023-09-24T10:49:21Z","caller":"/woodpecker/server/plugins/secrets/setup.go:39","message":"setting up secrets service"}
{"level":"debug","time":"2023-09-24T10:49:21Z","caller":"/woodpecker/server/plugins/secrets/setup.go:74","message":"encrypting secrets"}
{"level":"debug","id":1,"name":"s-name","time":"2023-09-24T10:49:21Z","caller":"/woodpecker/server/plugins/secrets/encrypted.go:48","message":"encryption"}
[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.

 secret_id | secret_name |                     secret_value
-----------+-------------+------------------------------------------------------
         1 | s-name      | _aes_D9Nimg9+pyyQQaaLcuGOm2GbkBMQcIvp0qAY7NE9oQcctGM

 secret_id | secret_name |                     secret_value
-----------+-------------+-------------------------------------------------------
         1 | s-name      | _aes_D9Nimg9+pyyQQaaLcuGOm2GbkBMQcIvp0qAY7NE9oQcctGM
         2 | second      | _aes_gD0ybRavPstWzDUH0guQz3myx53Z0r6dVBZgGmPs5W36fT7T

WOODPECKER_ENCRYPTION_AES_KEY: GjVHT007c4x3N+YPbsZld+hifba1enXkOzIb/0h6oW8=
WOODPECKER_SECRETS_ENCRYPTION_MODE: DisabledAndDecrypt

{"time":"2023-09-24T10:52:07Z","caller":"/woodpecker/cmd/common/logger.go:99","message":"LogLevel = debug"}
{"level":"debug","time":"2023-09-24T10:52:08Z","caller":"/woodpecker/server/plugins/encryption/aes.go:41","message":"initializing AES encryption service"}
{"level":"debug","time":"2023-09-24T10:52:09Z","caller":"/woodpecker/server/plugins/encryption/aes.go:68","message":"AES encryption service has been initialized"}
{"level":"debug","mode":"DisabledAndDecrypt","time":"2023-09-24T10:52:09Z","caller":"/woodpecker/server/plugins/secrets/setup.go:39","message":"setting up secrets service"}
{"level":"debug","time":"2023-09-24T10:52:09Z","caller":"/woodpecker/server/plugins/secrets/setup.go:94","message":"decrypting secrets"}
{"level":"debug","id":1,"name":"s-name","time":"2023-09-24T10:52:09Z","caller":"/woodpecker/server/plugins/secrets/encrypted.go:64","message":"decryption"}
{"level":"debug","id":2,"name":"second","time":"2023-09-24T10:52:09Z","caller":"/woodpecker/server/plugins/secrets/encrypted.go:64","message":"decryption"}
[GIN-debug] [WARNING] Running in "debug" mode. Switch to "release" mode in production.

 secret_id | secret_name | secret_value
-----------+-------------+--------------
         1 | s-name      | s-value
         2 | second      | second-v

securitym0nkey · 2024-03-08T17:16:19Z

Considering that the Encryption Documentation was complete remove do we have to assume that this feature is killed? We really like to see encryption. Is there something we can do to can help?

zc-devs · 2024-03-08T21:01:00Z

In regard to this PR, I would rework "migration" from env vars hack to CLI command and/or UI button <- requires API.

For plain->encrypted direction the flow can be

Load application with plain secrets service.
Issue encrypt command or click button.
Lock secrets functionality.
Load encrypted service/decorator.
Encrypt all secrets.
Unlock secrets service.

But there were thoughts about external secrets providers and addons story => this might not suit the "core".

zc-devs · 2024-03-09T18:06:17Z

Didn't know, that deleting a branch closes PR... Anyway, code is here, someone can continue the work.

anbraten and others added 10 commits August 20, 2023 23:25

cleanup encryption code

a6f397a

# Conflicts: # cmd/server/flags.go # cmd/server/server.go # server/store/datastore/secret.go # server/store/encryped/secret.go

simplify EncryptedStore

57586e4

# Conflicts: # server/store/encryped/secret.go

next ...

32d7786

Added encrypted secret service

24e37f5

Changed AES service

72341b6

Moved encryption to plugins

02c04ca

Cleaning up

ee068e9

Changed base64 and added logs

325850b

https://stackoverflow.com/questions/47382035/unable-to-decrypt-after-aes-gcm-base64-in-go

Fixed token creation (using name instead of id)

9a444a3

Fixed documentation

ae2ccd3

anbraten reviewed Aug 20, 2023

View reviewed changes

6543 added feature add new functionality security labels Aug 23, 2023

6543 added this to the 1.1.0 milestone Aug 23, 2023

6543 reviewed Aug 23, 2023

View reviewed changes

Update docs/docs/30-administration/40-encryption.md

f4ef1e8

zc-devs force-pushed the 1541-secret-encryption-2 branch from f555fa1 to f4ef1e8 Compare September 24, 2023 11:00

zc-devs added 2 commits September 24, 2023 14:05

Made encryption service system-wide

09002eb

- secret's value encoding/decoding - public encryption service - encryption modes - migrations

Fixed NPE

7b853e5

Fixed docs

111e99c

pat-s modified the milestones: 2.0.0, 2.x.x Oct 13, 2023

qwerty287 mentioned this pull request Nov 21, 2023

Fix and reenable Secrets-Encryption #1541

Open

anbraten removed this from the 3.x.x milestone Jan 30, 2024

zc-devs mentioned this pull request Mar 7, 2024

Documentation improvements #3376

Merged

zc-devs closed this Mar 9, 2024

zc-devs deleted the 1541-secret-encryption-2 branch March 9, 2024 17:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AES secrets encryption #2300

AES secrets encryption #2300

zc-devs commented Aug 20, 2023 •

edited

Loading

zc-devs commented Aug 20, 2023

anbraten Aug 20, 2023

zc-devs Aug 22, 2023

anbraten Aug 23, 2023

zc-devs Sep 24, 2023

woodpecker-bot commented Aug 20, 2023 •

edited

Loading

6543 commented Aug 23, 2023

6543 Aug 23, 2023

zc-devs Sep 24, 2023

zc-devs commented Sep 24, 2023 •

edited

Loading

securitym0nkey commented Mar 8, 2024

zc-devs commented Mar 8, 2024 •

edited

Loading

zc-devs commented Mar 9, 2024

AES secrets encryption #2300

AES secrets encryption #2300

Conversation

zc-devs commented Aug 20, 2023 • edited Loading

zc-devs commented Aug 20, 2023

anbraten Aug 20, 2023

Choose a reason for hiding this comment

zc-devs Aug 22, 2023

Choose a reason for hiding this comment

anbraten Aug 23, 2023

Choose a reason for hiding this comment

zc-devs Sep 24, 2023

Choose a reason for hiding this comment

woodpecker-bot commented Aug 20, 2023 • edited Loading

6543 commented Aug 23, 2023

6543 Aug 23, 2023

Choose a reason for hiding this comment

zc-devs Sep 24, 2023

Choose a reason for hiding this comment

zc-devs commented Sep 24, 2023 • edited Loading

securitym0nkey commented Mar 8, 2024

zc-devs commented Mar 8, 2024 • edited Loading

zc-devs commented Mar 9, 2024

zc-devs commented Aug 20, 2023 •

edited

Loading

woodpecker-bot commented Aug 20, 2023 •

edited

Loading

zc-devs commented Sep 24, 2023 •

edited

Loading

zc-devs commented Mar 8, 2024 •

edited

Loading