From f9840a04fcaaa036d631cb7a8b76a3b9cbc78009 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Mon, 20 May 2024 15:12:28 +0200 Subject: [PATCH 1/3] OSIS-147: handle decryption failure for secret key Adds the capability of handling decryption faulire for secret key data stored in redis sentinel by the method `retrieveSecretKey` used by get and get credentials APIs. - Does not throw any error for any decryption failure - Logs all decryption failures - Removes the keys from Redis automatically for any decryption failure, the keys remain in vault and can be used the user if secret key is accessible (cherry picked from commit a76452efefee8a9cdfd28781cbdff84c2abb760a) --- .../service/impl/ScalityOsisServiceImpl.java | 19 +++++++++++------- .../ScalityOsisServiceCredentialsTests.java | 20 +++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java index 47b580a..e735e71 100644 --- a/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java +++ b/osis-core/src/main/java/com/scality/osis/service/impl/ScalityOsisServiceImpl.java @@ -1391,14 +1391,19 @@ private String retrieveSecretKey(String repoKey) throws Exception { String secretKey = null; if (repoVal != null) { + try { + // Using `repoKey` for Associated Data during decryption + secretKey = cipherFactory.getCipherByID(repoVal.getKeyID()) + .decrypt(repoVal, + cipherFactory.getSecretCipherKeyByID(repoVal.getKeyID()), + repoKey); - // Using `repoKey` for Associated Data during encryption - secretKey = cipherFactory.getCipherByID(repoVal.getKeyID()) - .decrypt(repoVal, - cipherFactory.getSecretCipherKeyByID(repoVal.getKeyID()), - repoKey); - - logger.debug("[Cache] Retrieve Secret Key successful"); + logger.debug("[Cache] Retrieve Secret Key successful"); + } catch (Exception e) { + logger.error("Error: Unable to decrypt secret key data for Redis key: {}. Error details: {}", repoKey, e.getMessage()); + logger.debug("Full stack trace:", e); + deleteSecretKey(repoKey); + } } return secretKey; } diff --git a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceCredentialsTests.java b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceCredentialsTests.java index 4bedd47..d0e4fa6 100644 --- a/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceCredentialsTests.java +++ b/osis-core/src/test/java/com/scality/osis/service/impl/ScalityOsisServiceCredentialsTests.java @@ -11,8 +11,10 @@ import org.mockito.stubbing.Answer; import org.springframework.http.HttpStatus; +import javax.crypto.AEADBadTagException; import java.util.Collections; import java.util.Date; +import java.util.List; import static com.scality.osis.utils.ScalityConstants.*; import static com.scality.osis.utils.ScalityTestUtils.*; @@ -451,6 +453,15 @@ void testGetS3CredentialWithNullTenantIdAndUserId() { assertTrue(result.getActive()); } + @Test + void testGetS3CredentialsKeyPresentInRedisUnableToDecrypt() throws Exception { + when(baseCipherMock.decrypt(any(), any(), any())).thenThrow(new AEADBadTagException("Decryption failed")); + final OsisS3Credential result = scalityOsisServiceUnderTest.getS3Credential(SAMPLE_TENANT_ID, TEST_USER_ID, TEST_ACCESS_KEY); + // When decryption fails, the API call should succeed, and we should return the result with secret key listed as + // "Not Available" + assertEquals("Not Available", result.getSecretKey()); + } + @Test void testListS3Credentials() { // Setup @@ -526,6 +537,15 @@ void testListS3CredentialsWithNoKeyOnRedis() { } + @Test + void testListS3CredentialsKeyPresentInRedisUnableToDecrypt() throws Exception { + when(baseCipherMock.decrypt(any(), any(), any())).thenThrow(new AEADBadTagException("Decryption failed")); + final List result = scalityOsisServiceUnderTest.listS3Credentials(TEST_TENANT_ID, + TEST_USER_ID, 0L, 1000L).getItems(); + // When decryption fails, the API call should succeed, and we should get a new access key in the result + assertEquals(2, result.size()); + } + @Test void testListS3CredentialsErr() { // Setup From d88c6cedae23a821d87d7f40ba473ea145aa8d1a Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Wed, 22 May 2024 11:11:37 +0200 Subject: [PATCH 2/3] OSIS-147: bump-OSIS-to-v2.1.3 --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index ce0460e..0a58046 100644 --- a/build.gradle +++ b/build.gradle @@ -1,6 +1,6 @@ buildscript { ext { - osisVersion = '2.1.2' + osisVersion = '2.1.3' vaultclientVersion = '1.1.2' springBootVersion = '2.7.6' } @@ -120,4 +120,4 @@ task app { } } } -compileJava.dependsOn app \ No newline at end of file +compileJava.dependsOn app From e6d600998561e58154e0e004b07945b347882302 Mon Sep 17 00:00:00 2001 From: Anurag Mittal Date: Wed, 22 May 2024 11:16:29 +0200 Subject: [PATCH 3/3] OSIS-147: Updated code owner to match main --- .github/CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a4cc01a..c5fa6a3 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* @scality/object-lead @XinLiScality +* @scality/object