Repository: kestra-io/kestra
Issue: Prevent sensitive outputs such as AWS ECR token or GCP Auth token from being stored in plain text
Labels: enhancement

## Use cases
- fetching container registry token
- fetching sensitive value from a Secrets manager or parameter store

## Examples

ECR token
![image](https://github.com/kestra-io/kestra/assets/86264395/50ac29e6-230f-4753-a010-269a2aa3a7bf)

GCP auth token
![image](https://github.com/kestra-io/kestra/assets/86264395/fb613640-a990-4937-87d7-7ed646aca173)

## Possible solution
- Add Pebble expression(s) to encrypt and another one to decrypt sensitive outputs
- Apply those to the two tasks: AWS ECR fetch token + GCP Auth fetch token so that the output of those tasks are no longer stored in plain text 
- Analyze other plugins that need to be adjusted to avoid sensitive information in outputs in plain text

## Acceptance criteria

Whatever solution is used (pebble encrypt/decrypt) etc, there are **no sensitive outputs stored in plain text** in Kestra's plugins.

Issue comments:
Done thanks to:
- Automatic encryption and decryption of task output: https://github.com/kestra-io/kestra/pull/3005
- Encryption of GCP Auht Token: https://github.com/kestra-io/plugin-gcp/pull/324
- Encrypton of ECR token: https://github.com/kestra-io/plugin-aws/pull/373 (will be merged soon)

Pull request review comments:
## Encryption algorithms should be used with secure mode and padding scheme

<!--SONAR_ISSUE_KEY:AY0hk3_ygJ56SSpZTR-1-->Use another cipher mode or disable padding. <p>See more on <a href="https://sonarcloud.io/project/issues?id=kestra-io_kestra&issues=AY0hk3_ygJ56SSpZTR-1&open=AY0hk3_ygJ56SSpZTR-1&pullRequest=2892">SonarCloud</a></p>

[Show more details](https://github.com/kestra-io/kestra/security/code-scanning/35)
## Counter Mode initialization vectors should not be reused

<!--SONAR_ISSUE_KEY:AY0hk3_ygJ56SSpZTR-2-->Use a dynamically-generated initialization vector (IV) to avoid IV-key pair reuse. <p>See more on <a href="https://sonarcloud.io/project/issues?id=kestra-io_kestra&issues=AY0hk3_ygJ56SSpZTR-2&open=AY0hk3_ygJ56SSpZTR-2&pullRequest=2892">SonarCloud</a></p>

[Show more details](https://github.com/kestra-io/kestra/security/code-scanning/36)
## Encryption algorithms should be used with secure mode and padding scheme

<!--SONAR_ISSUE_KEY:AY0hk3_ygJ56SSpZTR-3-->Use another cipher mode or disable padding. <p>See more on <a href="https://sonarcloud.io/project/issues?id=kestra-io_kestra&issues=AY0hk3_ygJ56SSpZTR-3&open=AY0hk3_ygJ56SSpZTR-3&pullRequest=2892">SonarCloud</a></p>

[Show more details](https://github.com/kestra-io/kestra/security/code-scanning/37)
It's fixed, I have no idea why the Sonar action didn't close this comment as 'Fixed'.
For `AES/GCM/NoPadding` IV length must be `12`. Others lengths will require more computation and can be less secure. The remaning 4 bytes are used internally for the counter. Furthermore, as the general recommendation is to have an IV of 12 bytes in length, this will make our encryption compatible with most libraries.
Done

Patch that resolved the issue:
diff --git a/core/src/main/java/io/kestra/core/crypto/CryptoService.java b/core/src/main/java/io/kestra/core/crypto/CryptoService.java
new file mode 100644
index 0000000000..c9393def6e
--- /dev/null
+++ b/core/src/main/java/io/kestra/core/crypto/CryptoService.java
@@ -0,0 +1,86 @@
+package io.kestra.core.crypto;
+
+import com.google.common.primitives.Bytes;
+import io.micronaut.context.annotation.Value;
+import jakarta.annotation.PostConstruct;
+import jakarta.inject.Singleton;
+
+import javax.crypto.*;
+import javax.crypto.spec.GCMParameterSpec;
+import javax.crypto.spec.SecretKeySpec;
+import java.security.GeneralSecurityException;
+import java.security.SecureRandom;
+import java.util.Arrays;
+import java.util.Base64;
+import java.util.Optional;
+
+/**
+ * Service for encryption and decryption of secrets.
+ * It will not work if the configuration 'kestra.crypto.secret-key' is not set.
+ */
+@Singleton
+public class CryptoService {
+    private static final String CIPHER_ALGORITHM = "AES/GCM/NoPadding";
+    private static final int IV_LENGTH = 12;
+    private static final int AUTH_TAG_LENGTH = 128;
+
+    @Value("${kestra.crypto.secret-key}")
+    private Optional<String> secretKey;
+
+    private SecretKey key;
+    private SecureRandom secureRandom;
+
+    @PostConstruct
+    void loadPublicKey() {
+        secretKey.ifPresent(s -> {
+            this.key = new SecretKeySpec(s.getBytes(), "AES");
+            this.secureRandom = new SecureRandom();
+        });
+    }
+
+    /**
+     * Encrypt a String using the AES/GCM/NoPadding algorithm and the ${kestra.crypto.secret-key} key.
+     * The IV is concatenated at the beginning of the string.
+     */
+    public String encrypt(String plainText) throws GeneralSecurityException {
+        ensureConfiguration();
+
+        Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM);
+        byte[] iv = generateIv();
+        GCMParameterSpec ivParameter= new GCMParameterSpec(AUTH_TAG_LENGTH, iv);
+        cipher.init(Cipher.ENCRYPT_MODE, key, ivParameter);
+        byte[] encrypted = cipher.doFinal(plainText.getBytes());
+        byte[] output = Bytes.concat(iv, encrypted);
+        return Base64.getEncoder().encodeToString(output);
+    }
+
+    /**
+     * Decrypt a String using the AES/GCM/NoPadding algorithm and the ${kestra.crypto.secret-key} key.
+     * The IV is recovered from the beginning of the string.
+     */
+    public String decrypt(String cipherText) throws GeneralSecurityException {
+        ensureConfiguration();
+
+        byte[] input = Base64.getDecoder().decode(cipherText);
+        byte[] iv = Arrays.copyOf(input, IV_LENGTH);
+        byte[] encrypted =Arrays.copyOfRange(input, IV_LENGTH, input.length);
+        Cipher cipher = Cipher.getInstance(CIPHER_ALGORITHM);
+        GCMParameterSpec ivParameter= new GCMParameterSpec(AUTH_TAG_LENGTH, iv);
+        cipher.init(Cipher.DECRYPT_MODE, key, ivParameter);
+        byte[] plainText = cipher.doFinal(encrypted);
+        return new String(plainText);
+    }
+
+    private void ensureConfiguration() {
+        if (secretKey.isEmpty()) {
+            throw new IllegalArgumentException("You must configure a base64 encoded AES 256 bit secret key in the 'kestra.crypto.secret-key' " +
+                "configuration property to be able to use encryption and decryption facilities" );
+        }
+    }
+
+    private byte[] generateIv() {
+        byte[] iv = new byte[IV_LENGTH];
+        this.secureRandom.nextBytes(iv);
+        return iv;
+    }
+}
diff --git a/core/src/main/java/io/kestra/core/runners/pebble/Extension.java b/core/src/main/java/io/kestra/core/runners/pebble/Extension.java
index e72a79d55d..dbb5237817 100644
--- a/core/src/main/java/io/kestra/core/runners/pebble/Extension.java
+++ b/core/src/main/java/io/kestra/core/runners/pebble/Extension.java
@@ -1,6 +1,5 @@
 package io.kestra.core.runners.pebble;
 
-import io.kestra.core.runners.VariableRenderer;
 import io.kestra.core.runners.pebble.functions.*;
 import io.micronaut.core.annotation.Nullable;
 import io.pebbletemplates.pebble.extension.*;
@@ -35,6 +34,12 @@ public class Extension extends AbstractExtension {
     @Nullable
     private RenderFunction renderFunction;
 
+    @Inject
+    private EncryptFunction encryptFunction;
+
+    @Inject
+    private DecryptFunction decryptFunction;
+
     @Override
     public List<TokenParser> getTokenParsers() {
         return null;
@@ -100,6 +105,8 @@ public Map<String, Function> getFunctions() {
         if (this.renderFunction != null) {
             functions.put("render", renderFunction);
         }
+        functions.put("encrypt", encryptFunction);
+        functions.put("decrypt", decryptFunction);
 
         return functions;
     }
diff --git a/core/src/main/java/io/kestra/core/runners/pebble/functions/DecryptFunction.java b/core/src/main/java/io/kestra/core/runners/pebble/functions/DecryptFunction.java
new file mode 100644
index 0000000000..9f588ab62e
--- /dev/null
+++ b/core/src/main/java/io/kestra/core/runners/pebble/functions/DecryptFunction.java
@@ -0,0 +1,38 @@
+package io.kestra.core.runners.pebble.functions;
+
+import io.kestra.core.crypto.CryptoService;
+import io.pebbletemplates.pebble.error.PebbleException;
+import io.pebbletemplates.pebble.extension.Function;
+import io.pebbletemplates.pebble.template.EvaluationContext;
+import io.pebbletemplates.pebble.template.PebbleTemplate;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+
+import java.security.GeneralSecurityException;
+import java.util.List;
+import java.util.Map;
+
+@Singleton
+public class DecryptFunction implements Function {
+    @Inject
+    private CryptoService cryptoService;
+
+    @Override
+    public List<String> getArgumentNames() {
+        return List.of("string");
+    }
+    @Override
+    public Object execute(Map<String, Object> args, PebbleTemplate self, EvaluationContext context, int lineNumber) {
+        if (!args.containsKey("string")) {
+            throw new PebbleException(null, "The 'decrypt' function expects an argument 'string'.", lineNumber, self.getName());
+        }
+
+        String s = (String) args.get("string");
+        try {
+            return cryptoService.decrypt(s);
+        }
+        catch (GeneralSecurityException e) {
+            throw new PebbleException(e, e.getMessage(), lineNumber, self.getName());
+        }
+    }
+}
diff --git a/core/src/main/java/io/kestra/core/runners/pebble/functions/EncryptFunction.java b/core/src/main/java/io/kestra/core/runners/pebble/functions/EncryptFunction.java
new file mode 100644
index 0000000000..4e6a08e4a5
--- /dev/null
+++ b/core/src/main/java/io/kestra/core/runners/pebble/functions/EncryptFunction.java
@@ -0,0 +1,39 @@
+package io.kestra.core.runners.pebble.functions;
+
+import io.kestra.core.crypto.CryptoService;
+import io.pebbletemplates.pebble.error.PebbleException;
+import io.pebbletemplates.pebble.extension.Function;
+import io.pebbletemplates.pebble.template.EvaluationContext;
+import io.pebbletemplates.pebble.template.PebbleTemplate;
+import jakarta.inject.Inject;
+import jakarta.inject.Singleton;
+
+import java.security.GeneralSecurityException;
+import java.util.List;
+import java.util.Map;
+
+@Singleton
+public class EncryptFunction implements Function {
+    @Inject
+    private CryptoService cryptoService;
+
+    @Override
+    public List<String> getArgumentNames() {
+        return List.of("string");
+    }
+
+    @Override
+    public Object execute(Map<String, Object> args, PebbleTemplate self, EvaluationContext context, int lineNumber) {
+        if (!args.containsKey("string")) {
+            throw new PebbleException(null, "The 'encrypt' function expects an argument 'string'.", lineNumber, self.getName());
+        }
+
+        String s = (String) args.get("string");
+        try {
+            return cryptoService.encrypt(s);
+        }
+        catch (GeneralSecurityException e) {
+            throw new PebbleException(e, e.getMessage(), lineNumber, self.getName());
+        }
+    }
+}


Test patch:
diff --git a/core/src/test/java/io/kestra/core/runners/pebble/functions/EncryptDecryptFunctionTest.java b/core/src/test/java/io/kestra/core/runners/pebble/functions/EncryptDecryptFunctionTest.java
new file mode 100644
index 0000000000..04fc7e00de
--- /dev/null
+++ b/core/src/test/java/io/kestra/core/runners/pebble/functions/EncryptDecryptFunctionTest.java
@@ -0,0 +1,44 @@
+package io.kestra.core.runners.pebble.functions;
+
+import io.kestra.core.exceptions.IllegalVariableEvaluationException;
+import io.kestra.core.runners.VariableRenderer;
+import io.micronaut.context.annotation.Property;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import org.junit.jupiter.api.Test;
+
+import java.util.Collections;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+@MicronautTest(rebuildContext = true)
+class EncryptDecryptFunctionTest {
+    @Inject
+    private VariableRenderer variableRenderer;
+
+    @Test
+    void notConfigured() {
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> variableRenderer.render("{{encrypt('toto')}}", Collections.emptyMap())
+        );
+
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> variableRenderer.render("{{decrypt('toto')}}", Collections.emptyMap())
+        );
+    }
+
+    @Test
+    @Property(name = "kestra.crypto.secret-key", value = "I6EGNzRESu3X3pKZidrqCGOHQFUFC0yK")
+    void encryptDecrypt() throws IllegalVariableEvaluationException {
+        String encrypted = variableRenderer.render("{{encrypt('toto')}}", Collections.emptyMap());
+        assertThat(encrypted, notNullValue());
+
+        String decrypted = variableRenderer.render("{{decrypt('" + encrypted + "')}}", Collections.emptyMap());
+        assertThat(decrypted, is("toto"));
+    }
+}
\ No newline at end of file