Merge pull request #50 from keboola/roman-pst-1842

PST-1842: Tolerate empty strings in decryption
keboola · Jun 20, 2024 · fc8555f · fc8555f
2 parents c233142 + b533f9a
commit fc8555f
Show file tree

Hide file tree

Showing 20 changed files with 154 additions and 116 deletions.
diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml
@@ -18,9 +18,9 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Check out the repo
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Google Login
-        uses: 'google-github-actions/auth@v1'
+        uses: 'google-github-actions/auth@v2'
         with:
           credentials_json: ${{ secrets.TEST_GCP_SERVICE_ACCOUNT_KEY }}
           export_environment_variables: true

diff --git a/composer.json b/composer.json
@@ -32,7 +32,7 @@
     },
     "require-dev": {
         "infection/infection": "^0.26",
-        "keboola/coding-standard": "^14.0",
+        "keboola/coding-standard": "^15.0",
         "phpstan/phpstan": "^1.8",
         "phpstan/phpstan-phpunit": "^1.0",
         "phpunit/phpunit": "^9.5",

diff --git a/provisioning/local/.terraform.lock.hcl b/provisioning/local/.terraform.lock.hcl
diff --git a/provisioning/local/env-scripts/extract-variables-common.sh b/provisioning/local/env-scripts/extract-variables-common.sh
@@ -20,7 +20,7 @@ echo ""
 
 output_var 'TEST_GCP_KMS_KEY_ID' "$(terraform_output 'gcp_kms_key_id')"
 PRIVATE_KEY_ENCODED="$(terraform_output 'gcp_private_key')"
-PRIVATE_KEY=$(printf "%s" "$PRIVATE_KEY_ENCODED" | base64 --decode --wrap=0)
+PRIVATE_KEY=$(printf "%s" "$PRIVATE_KEY_ENCODED" | base64 --decode)
 
 output_file 'var/gcp-private-key.json' "$PRIVATE_KEY"
 output_var 'TEST_GOOGLE_APPLICATION_CREDENTIALS' 'var/gcp-private-key.json'

diff --git a/src/EncryptorOptions.php b/src/EncryptorOptions.php
@@ -40,7 +40,7 @@ public function __construct(
         ?string $kmsRole = null,
         ?string $akvUrl = null,
         ?string $gkmsKeyId = null,
-        ?int $backoffMaxTries = null
+        ?int $backoffMaxTries = null,
     ) {
         $this->stackId = $stackId;
         $this->kmsKeyId = $kmsKeyId;

diff --git a/src/ObjectEncryptor.php b/src/ObjectEncryptor.php
@@ -376,7 +376,7 @@ private function findWrapper(string $value, array $wrappers): ?CryptoWrapperInte
 
     private function decryptValue(string $value, array $wrappers): string
     {
-        if (RegexHelper::matchesVariable($value)) {
+        if ($value === '' || RegexHelper::matchesVariable($value)) {
             return $value;
         }
 
@@ -414,7 +414,7 @@ private function encryptItem($key, $value, array $wrappers, CryptoWrapperInterfa
             return $this->encryptObject($value, $wrappers, $wrapper);
         } else {
             throw new ApplicationException(
-                'Invalid item $key - only stdClass, array and scalar can be encrypted.'
+                'Invalid item $key - only stdClass, array and scalar can be encrypted.',
             );
         }
     }
@@ -522,7 +522,7 @@ private function decryptItem($key, $value, array $wrappers)
             return $this->decryptObject($value, $wrappers);
         } else {
             throw new ApplicationException(
-                "Invalid item $key - only stdClass, array and scalar can be decrypted."
+                "Invalid item $key - only stdClass, array and scalar can be decrypted.",
             );
         }
     }
@@ -719,7 +719,7 @@ private function getGMKSWrappers(
                         if ($branchType) {
                             $wrapper = new BranchTypeConfigurationGKMSWrapper(
                                 $this->gkmsClient,
-                                $this->encryptorOptions
+                                $this->encryptorOptions,
                             );
                             $wrapper->setComponentId($componentId);
                             $wrapper->setProjectId($projectId);
@@ -755,21 +755,21 @@ private function getWrappers(
         if ($this->encryptorOptions->getAkvUrl()) {
             $wrappers = array_merge(
                 $wrappers,
-                $this->getAKVWrappers($componentId, $projectId, $configurationId, $branchType)
+                $this->getAKVWrappers($componentId, $projectId, $configurationId, $branchType),
             );
         }
 
         if ($this->encryptorOptions->getGkmsKeyId()) {
             $wrappers = array_merge(
                 $wrappers,
-                $this->getGMKSWrappers($componentId, $projectId, $configurationId, $branchType)
+                $this->getGMKSWrappers($componentId, $projectId, $configurationId, $branchType),
             );
         }
 
         if ($this->encryptorOptions->getKmsKeyRegion() && $this->encryptorOptions->getKmsKeyId()) {
             $wrappers = array_merge(
                 $wrappers,
-                $this->getKMSWrappers($componentId, $projectId, $configurationId, $branchType)
+                $this->getKMSWrappers($componentId, $projectId, $configurationId, $branchType),
             );
         }
         return $wrappers;

diff --git a/src/ObjectEncryptorFactory.php b/src/ObjectEncryptorFactory.php
@@ -17,7 +17,7 @@ public static function getAwsEncryptor(
         string $stackId,
         string $kmsKeyId,
         string $kmsRegion,
-        ?string $kmsRole
+        ?string $kmsRole,
     ): ObjectEncryptor {
         $encryptOptions = new EncryptorOptions($stackId, $kmsKeyId, $kmsRegion, $kmsRole, null);
         return self::getEncryptor($encryptOptions);

diff --git a/src/RegexHelper.php b/src/RegexHelper.php
@@ -20,7 +20,7 @@ public static function matchesVariable(string $value): bool
 
         if ($result === false) {
             throw new ApplicationException(
-                sprintf('Variable regex matching error "%s"', preg_last_error_msg())
+                sprintf('Variable regex matching error "%s"', preg_last_error_msg()),
             );
         }
 

diff --git a/src/Wrapper/GenericAKVWrapper.php b/src/Wrapper/GenericAKVWrapper.php
@@ -52,7 +52,7 @@ public function getClient(): Client
             $this->client = new Client(
                 new GuzzleClientFactory(new NullLogger()),
                 new AuthenticatorFactory(),
-                $this->keyVaultURL
+                $this->keyVaultURL,
             );
         }
         return $this->client;
@@ -116,7 +116,7 @@ public function encrypt(?string $data): string
             $secret = $this->getRetryProxy()->call(function () use ($context) {
                 return $this->getClient()->setSecret(
                     new SetSecretRequest($context, new SecretAttributes()),
-                    uniqid('gen-encryptor')
+                    uniqid('gen-encryptor'),
                 );
             });
             /** @var SecretBundle $secret */
@@ -152,7 +152,7 @@ public function decrypt(string $encryptedData): string
             $decryptedContext = $this->getRetryProxy()->call(function () use ($encrypted) {
                 return $this->getClient()->getSecret(
                     $encrypted[self::SECRET_NAME],
-                    $encrypted[self::SECRET_VERSION]
+                    $encrypted[self::SECRET_VERSION],
                 )->getValue();
             });
             assert(is_string($decryptedContext));

diff --git a/src/Wrapper/GenericGKMSWrapper.php b/src/Wrapper/GenericGKMSWrapper.php
@@ -88,7 +88,7 @@ public function encrypt(?string $data): string
                 $response = $this->client->encrypt(
                     $this->gkmsKeyId,
                     $key->saveToAsciiSafeString(),
-                    ['additionalAuthenticatedData' => $this->encode($this->metadata)]
+                    ['additionalAuthenticatedData' => $this->encode($this->metadata)],
                 );
                 return $response->getCiphertext();
             });
@@ -115,7 +115,7 @@ public function decrypt(string $encryptedData): string
                 $response = $this->client->decrypt(
                     $this->gkmsKeyId,
                     $encrypted[self::KEY_INDEX],
-                    ['additionalAuthenticatedData' => $this->encode($this->metadata)]
+                    ['additionalAuthenticatedData' => $this->encode($this->metadata)],
                 );
                 return $response->getPlaintext();
             });

diff --git a/src/Wrapper/GenericKMSWrapper.php b/src/Wrapper/GenericKMSWrapper.php
@@ -145,7 +145,7 @@ public function decrypt(string $encryptedData): string
             assert(is_string($decryptedKey));
             $safeKey = Encoding::saveBytesToChecksummedAsciiSafeString(
                 Key::KEY_CURRENT_VERSION,
-                $decryptedKey
+                $decryptedKey,
             );
             $key = Key::loadFromAsciiSafeString($safeKey);
             return Crypto::decrypt($encrypted[0], $key, true);

diff --git a/src/Wrapper/GkmsClientFactory.php b/src/Wrapper/GkmsClientFactory.php
@@ -37,7 +37,7 @@ public function createClient(EncryptorOptions $encryptorOptions): KeyManagementS
                             'httpHandler' => [$handler, 'async'],
                         ],
                     ],
-                ]
+                ],
             );
         } catch (Throwable $e) {
             throw new ApplicationException('Cipher key settings are invalid: ' . $e->getMessage(), 0, $e);

diff --git a/tests/BranchTypeProjectWideWrapperTest.php b/tests/BranchTypeProjectWideWrapperTest.php
@@ -123,7 +123,7 @@ public function testInvalidSetupGKMS(): void
     {
         $options = new EncryptorOptions(
             stackId: 'some-stack',
-            akvUrl: 'some-url'
+            akvUrl: 'some-url',
         );
 
         self::expectException(ApplicationException::class);
@@ -138,7 +138,7 @@ public function testInvalidSetupKMS(): void
     {
         $options = new EncryptorOptions(
             stackId: 'some-stack',
-            akvUrl: 'some-url'
+            akvUrl: 'some-url',
         );
 
         self::expectException(ApplicationException::class);

diff --git a/tests/EncryptorOptionsTest.php b/tests/EncryptorOptionsTest.php
@@ -19,7 +19,7 @@ public function testAccessors(): void
             kmsRole: 'role',
             akvUrl: 'akv-url',
             gkmsKeyId: 'gkms-key-id',
-            backoffMaxTries: 1
+            backoffMaxTries: 1,
         );
         self::assertSame('my-stack', $options->getStackId());
         self::assertSame('my-kms-id', $options->getKmsKeyId());

diff --git a/tests/GenericAKVWrapperTest.php b/tests/GenericAKVWrapperTest.php
@@ -29,7 +29,7 @@ public function setUp(): void
         foreach ($envs as $env) {
             if (!getenv($env)) {
                 throw new RuntimeException(
-                    sprintf('At least one of %s environment variables is empty.', implode(', ', $envs))
+                    sprintf('At least one of %s environment variables is empty.', implode(', ', $envs)),
                 );
             }
         }
@@ -44,7 +44,7 @@ private function clearSecrets(): void
         $client = new Client(
             new GuzzleClientFactory(new NullLogger()),
             new AuthenticatorFactory(),
-            self::getAkvUrl()
+            self::getAkvUrl(),
         );
         foreach ($client->getAllSecrets() as $secret) {
             $client->deleteSecret($secret->getName());
@@ -144,10 +144,10 @@ public function testRetryEncryptDecrypt(): void
         $secretInternal = '';
         $mockClient->expects(self::exactly(3))->method('setSecret')
             ->willReturnCallback(function (
-                SetSecretRequest $setSecretRequest
+                SetSecretRequest $setSecretRequest,
             ) use (
                 &$callNoSet,
-                &$secretInternal
+                &$secretInternal,
             ) {
                 $callNoSet++;
                 $secretInternal = $setSecretRequest->getArray()['value'];
@@ -165,10 +165,10 @@ public function testRetryEncryptDecrypt(): void
         $mockClient->expects(self::exactly(3))->method('getSecret')
             ->willReturnCallback(function (
                 $secretName,
-                $secretVersion
+                $secretVersion,
             ) use (
                 &$callNoGet,
-                &$secretInternal
+                &$secretInternal,
             ) {
                 $callNoGet++;
                 if ($callNoGet < 3) {
@@ -200,7 +200,7 @@ public function testRetryEncryptFail(): void
             ->getMock();
         $mockClient->method('setSecret')
             ->willThrowException(
-                new ConnectException('mock failed to connect', new Request('GET', 'some-uri'))
+                new ConnectException('mock failed to connect', new Request('GET', 'some-uri')),
             );
 
         $secret = 'secret';
@@ -222,13 +222,13 @@ public function testRetryDecryptFail(): void
             ->getMock();
         $mockClient->method('getSecret')
             ->willThrowException(
-                new ConnectException('mock failed to connect', new Request('GET', 'some-uri'))
+                new ConnectException('mock failed to connect', new Request('GET', 'some-uri')),
             );
 
         $secret = 'secret';
         $wrapper = new GenericAKVWrapper(new EncryptorOptions(
             stackId: 'some-stack',
-            akvUrl: self::getAkvUrl()
+            akvUrl: self::getAkvUrl(),
         ));
         $encrypted = $wrapper->encrypt($secret);
         self::assertNotEquals($secret, $encrypted);
@@ -288,9 +288,9 @@ public function testInvalidSecretCipher(): void
         $mockClient->method('setSecret')
             ->willReturnCallback(function (
                 SetSecretRequest $setSecretRequest,
-                $secretName
+                $secretName,
             ) use (
-                &$secretInternal
+                &$secretInternal,
             ) {
                 $secretInternal = $setSecretRequest->getArray()['value'];
                 return new SecretBundle([
@@ -368,7 +368,7 @@ public function testInvalidSetupInvalidCredentials(): void
         putenv('AZURE_CLIENT_ID=invalid');
         $wrapper = new GenericAKVWrapper(new EncryptorOptions(
             stackId: 'some-stack',
-            akvUrl: self::getAkvUrl()
+            akvUrl: self::getAkvUrl(),
         ));
         self::expectException(ApplicationException::class);
         self::expectExceptionMessage('Ciphering failed: Failed to get authentication token');

diff --git a/tests/GenericGKMWrapperTest.php b/tests/GenericGKMWrapperTest.php
@@ -150,7 +150,7 @@ public function testRetryEncryptFail(): void
         $mockClient = $this->createMock(KeyManagementServiceClient::class);
         $mockClient->expects(self::exactly(1))->method('encrypt')
             ->willThrowException(
-                new ConnectException('mock failed to connect', new Request('GET', 'some-uri'))
+                new ConnectException('mock failed to connect', new Request('GET', 'some-uri')),
             );
 
         $options = new EncryptorOptions(
@@ -169,7 +169,7 @@ public function testRetryDecryptFail(): void
         $mockClient = $this->createMock(KeyManagementServiceClient::class);
         $mockClient->expects(self::exactly(1))->method('decrypt')
             ->willThrowException(
-                new ConnectException('mock failed to connect', new Request('GET', 'some-uri'))
+                new ConnectException('mock failed to connect', new Request('GET', 'some-uri')),
             );
 
         $options = new EncryptorOptions(
@@ -231,7 +231,7 @@ public function testInvalidSetupMissingKeyId(): void
                 stackId: 'some-stack',
                 kmsKeyId: 'test-key',
                 kmsRegion: 'test-region',
-            )
+            ),
         );
     }
 
@@ -242,7 +242,7 @@ public function testInvalidSetupInvalidKeyId(): void
             new EncryptorOptions(
                 stackId: 'some-stack',
                 gkmsKeyId: 'test-key',
-            )
+            ),
         );
         self::expectException(ApplicationException::class);
         self::expectExceptionMessage('Ciphering failed: Could not map bindings for');
@@ -256,7 +256,7 @@ public function testInvalidSetupInvalidUrlDecrypt(): void
             new EncryptorOptions(
                 stackId: 'some-stack',
                 gkmsKeyId: 'test-key',
-            )
+            ),
         );
         self::expectException(ApplicationException::class);
         self::expectExceptionMessage('Deciphering failed.');
@@ -272,7 +272,7 @@ public function testInvalidSetupInvalidCredentialsAfterConstruct(): void
             new EncryptorOptions(
                 stackId: 'some-stack',
                 gkmsKeyId: self::getGkmsKeyId(),
-            )
+            ),
         );
         $encrypted = $wrapper->encrypt('test');
         self::assertNotEquals('test', $encrypted);

diff --git a/tests/GenericKMSWrapperTest.php b/tests/GenericKMSWrapperTest.php
@@ -140,7 +140,7 @@ public function testRetryEncryptFail(): void
             ->getMock();
         $mockKmsClient->method('execute')
             ->willThrowException(
-                new ConnectException('mock failed to connect', new Request('GET', 'some-uri'))
+                new ConnectException('mock failed to connect', new Request('GET', 'some-uri')),
             );
 
         $mockWrapper = new GenericKMSWrapper(
@@ -206,7 +206,7 @@ public function testRetryDecryptFail(): void
             ->getMock();
         $mockKmsClient->method('execute')
             ->willThrowException(
-                new ConnectException('mock failed to connect', new Request('GET', 'some-uri'))
+                new ConnectException('mock failed to connect', new Request('GET', 'some-uri')),
             );
 
         $mockWrapper = new GenericKMSWrapper(
@@ -385,7 +385,7 @@ public function testInvalidNonExistentRegion(): void
             stackId: 'some-stack',
             kmsKeyId: self::getKmsKeyId(),
             kmsRegion: 'non-existent',
-            backoffMaxTries: 1
+            backoffMaxTries: 1,
         );
 
         $wrapper = new GenericKMSWrapper(
-Original file line number
+Diff line change
@@ Expand Up @@
                                 'httpHandler' => [$handler, 'async'],
                             ],
                         ],
-                    ]
+                    ],
                 );
             } catch (Throwable $e) {
                 throw new ApplicationException('Cipher key settings are invalid: ' . $e->getMessage(), 0, $e);
@@ Expand Down @@