Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Commit

Permalink
Return 404 instead of 500 for mismatched paths
Browse files Browse the repository at this point in the history
  • Loading branch information
jharen committed May 15, 2023
1 parent e51a24e commit 86a5c1c
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,17 @@ class CerberusCompositeApiActions {
"create, read, update then delete a secret node"(cerberusAuthToken, ROOT_INTEGRATION_TEST_SDB_PATH)
}

static void "verify 404 with mismatched case in path"(String cerberusAuthToken, String sdbPath){
def path = sdbPath.toUpperCase()

try {
readSecretNode(path, cerberusAuthToken)
}
catch(java.lang.AssertionError e){
assertTrue(e.getMessage().contains("Expected status code <200> but was <404>"))
}
}

static void "create, read, update then delete a secret node"(String cerberusAuthToken, String sdbPath) {
def path = "${sdbPath}/${UUID.randomUUID().toString()}"
String value1 = 'value1'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ class CerberusIamApiV2Tests {
testPartition = PropUtils.getPropWithDefaultValue("TEST_PARTITION", "aws")
}

@Test
void "test that a 404 is returned with mismatched case in path"() {
"verify 404 with mismatched case in path"(cerberusAuthToken, ROOT_INTEGRATION_TEST_SDB_PATH)
}

@Test
void "test that an authenticated IAM role can create, read, update then delete a secret node"() {
"create, read, update then delete a secret node"(cerberusAuthToken)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.nike.cerberus.error;

import com.nike.backstopper.apierror.ApiError;
import java.util.Map;
import javax.servlet.http.HttpServletResponse;

public class InvalidEncryptionContextApiError implements ApiError {

private final String message;

public InvalidEncryptionContextApiError(String message) {
this.message = message;
}

@Override
public String getName() {
return "InvalidEncryptionContextException";
}

@Override
public String getErrorCode() {
return DefaultApiError.ENTITY_NOT_FOUND.getErrorCode();
}

@Override
public String getMessage() {
return this.message;
}

@Override
public Map<String, Object> getMetadata() {
return null;
}

@Override
public int getHttpStatusCode() {
return HttpServletResponse.SC_NOT_FOUND;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import com.amazonaws.encryptionsdk.multi.MultipleProviderFactory;
import com.amazonaws.regions.Region;
import com.google.common.collect.Lists;
import com.nike.backstopper.exception.ApiException;
import com.nike.cerberus.error.InvalidEncryptionContextApiError;
import com.nike.cerberus.util.CiphertextUtils;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
Expand Down Expand Up @@ -232,7 +234,10 @@ public static String decrypt(
*
* <p>This step validates that the encrypted payload was created for the SDB that is currently
* being decrypted. It is an integrity check. If this validation fails then the encrypted payload
* may have been tampered with, e.g. copying the encrypted payload between two SDBs.
* may have been tampered with, e.g. copying the encrypted payload between two SDBs. However,
* another possibility exists: there are times wherein the case of the path differs. In either
* case we still consider the validation to have failed, but we want to log the different case
* scenario specifically.
*
* @param parsedCiphertext the ciphertext to read the encryptionContext from
* @param sdbPath the path expected in the encryptionContext
Expand All @@ -241,9 +246,15 @@ private void validateEncryptionContext(ParsedCiphertext parsedCiphertext, String
Map<String, String> encryptionContext = parsedCiphertext.getEncryptionContextMap();
String pathFromEncryptionContext = encryptionContext.getOrDefault(SDB_PATH_PROPERTY_NAME, null);
if (!StringUtils.equals(pathFromEncryptionContext, sdbPath)) {
log.error("EncryptionContext did not have expected path, possible tampering: " + sdbPath);
throw new IllegalArgumentException(
"EncryptionContext did not have expected path, possible tampering: " + sdbPath);
String msg = "EncryptionContext did not have expected path: " + sdbPath;
if (StringUtils.equalsIgnoreCase(pathFromEncryptionContext, sdbPath)) {
msg += " (case path change detected)";
} else {
msg += " (possible tampering)";
}
log.error(msg);
InvalidEncryptionContextApiError iece = new InvalidEncryptionContextApiError(msg);
throw ApiException.newBuilder().withApiErrors(iece).build();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.amazonaws.regions.Region;
import com.amazonaws.regions.Regions;
import com.google.common.collect.Lists;
import com.nike.backstopper.exception.ApiException;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -118,11 +119,32 @@ public void testDecryptWhenEncryptionContentDidNotHaveExpectedPath() {
String exceptionMessage = "";
try {
encryptionService.decrypt("encryptedPayload", "sdbPath");
} catch (IllegalArgumentException illegalArgumentException) {
exceptionMessage = illegalArgumentException.getMessage();
} catch (ApiException apie) {
exceptionMessage = apie.getMessage();
}
assertEquals(
"EncryptionContext did not have expected path: sdbPath (possible tampering)",
exceptionMessage);
}

@Test
public void testDecryptWhenEncryptionContentDidNotHaveExpectedPathCase() {
EncryptionService encryptionService = Mockito.spy(getEncryptionService());
ParsedCiphertext parsedCiphertext = getParsedCipherText();
Mockito.doReturn(parsedCiphertext)
.when(encryptionService)
.getParsedCipherText("encryptedPayload");
Map<String, String> contextMap = new HashMap<>();
contextMap.put(SDB_PATH_PROPERTY_NAME, "sdbpath");
Mockito.when(parsedCiphertext.getEncryptionContextMap()).thenReturn(contextMap);
String exceptionMessage = "";
try {
encryptionService.decrypt("encryptedPayload", "SDBPath");
} catch (ApiException apie) {
exceptionMessage = apie.getMessage();
}
assertEquals(
"EncryptionContext did not have expected path, possible tampering: sdbPath",
"EncryptionContext did not have expected path: SDBPath (case path change detected)",
exceptionMessage);
}

Expand Down Expand Up @@ -157,11 +179,32 @@ public void testDecryptBytesWhenEncryptionContentDidNotHaveExpectedPath() {
String exceptionMessage = "";
try {
encryptionService.decrypt("encryptedPayload".getBytes(StandardCharsets.UTF_8), "sdbPath");
} catch (IllegalArgumentException illegalArgumentException) {
exceptionMessage = illegalArgumentException.getMessage();
} catch (ApiException apie) {
exceptionMessage = apie.getMessage();
}
assertEquals(
"EncryptionContext did not have expected path, possible tampering: sdbPath",
"EncryptionContext did not have expected path: sdbPath (possible tampering)",
exceptionMessage);
}

@Test
public void testDecryptBytesWhenEncryptionContentDidNotHaveExpectedPathCase() {
EncryptionService encryptionService = Mockito.spy(getEncryptionService());
ParsedCiphertext parsedCiphertext = getParsedCipherText();
Mockito.doReturn(parsedCiphertext)
.when(encryptionService)
.getParsedCipherText("encryptedPayload".getBytes(StandardCharsets.UTF_8));
Map<String, String> contextMap = new HashMap<>();
contextMap.put(SDB_PATH_PROPERTY_NAME, "SdBpAtH");
Mockito.when(parsedCiphertext.getEncryptionContextMap()).thenReturn(contextMap);
String exceptionMessage = "";
try {
encryptionService.decrypt("encryptedPayload".getBytes(StandardCharsets.UTF_8), "sdbPath");
} catch (ApiException apie) {
exceptionMessage = apie.getMessage();
}
assertEquals(
"EncryptionContext did not have expected path: sdbPath (case path change detected)",
exceptionMessage);
}

Expand Down Expand Up @@ -197,11 +240,11 @@ public void testReEncryptWhenEncryptionContentDidNotHaveExpectedPath() {
String exceptionMessage = "";
try {
encryptionService.reencrypt("encryptedPayload", "sdbPath");
} catch (IllegalArgumentException illegalArgumentException) {
exceptionMessage = illegalArgumentException.getMessage();
} catch (ApiException apie) {
exceptionMessage = apie.getMessage();
}
assertEquals(
"EncryptionContext did not have expected path, possible tampering: sdbPath",
"EncryptionContext did not have expected path: sdbPath (possible tampering)",
exceptionMessage);
}

Expand Down Expand Up @@ -270,11 +313,11 @@ public void testReEncryptBytesWhenEncryptionContentDidNotHaveExpectedPath() {
String exceptionMessage = "";
try {
encryptionService.reencrypt("encryptedPayload".getBytes(StandardCharsets.UTF_8), "sdbPath");
} catch (IllegalArgumentException illegalArgumentException) {
exceptionMessage = illegalArgumentException.getMessage();
} catch (ApiException apie) {
exceptionMessage = apie.getMessage();
}
assertEquals(
"EncryptionContext did not have expected path, possible tampering: sdbPath",
"EncryptionContext did not have expected path: sdbPath (possible tampering)",
exceptionMessage);
}

Expand Down

0 comments on commit 86a5c1c

Please sign in to comment.