Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix : NPE if reading record without a signature fields #152

Merged
merged 14 commits into from
Aug 20, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,16 @@ public Map<String, AttributeValue> decryptRecord(
if (attributeFlags.isEmpty()) {
return itemAttributes;
}
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved

if (!itemAttributes.containsKey(signatureFieldName)
&& !itemAttributes.containsKey(materialDescriptionFieldName)) {
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved
// Check if any key from the received raw document is marked with EncryptionFlags in model
if (isAnyKeyMarkedWithEncryptionFlags(itemAttributes.keySet(), attributeFlags)) {
throw new IllegalArgumentException("Bad data, missing " + signatureFieldName);
} else {
return itemAttributes;
}
}
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved
// Copy to avoid changing anyone elses objects
itemAttributes = new HashMap<String, AttributeValue>(itemAttributes);

Expand Down Expand Up @@ -291,6 +301,11 @@ public Map<String, AttributeValue> decryptRecord(
return itemAttributes;
}

private boolean isAnyKeyMarkedWithEncryptionFlags(
Set<String> keysToCheck, Map<String, Set<EncryptionFlags>> attributeFlags) {
return keysToCheck.stream().anyMatch(key -> !attributeFlags.get(key).isEmpty());
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Returns the encrypted (and signed) record, which is a map of item attributes. There is no side
* effect on the input parameters upon calling this method.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.amazonaws.services.dynamodbv2.datamodeling.encryption;

import static com.amazonaws.services.dynamodbv2.datamodeling.encryption.utils.EncryptionContextOperators.overrideEncryptionContextTableName;
import static java.util.stream.Collectors.toMap;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.not;
Expand All @@ -23,6 +24,7 @@
import static org.testng.AssertJUnit.assertNotNull;
import static org.testng.AssertJUnit.assertNull;
import static org.testng.AssertJUnit.assertTrue;
import static org.testng.collections.Sets.newHashSet;

import com.amazonaws.services.dynamodbv2.datamodeling.encryption.materials.DecryptionMaterials;
import com.amazonaws.services.dynamodbv2.datamodeling.encryption.materials.EncryptionMaterials;
Expand Down Expand Up @@ -498,6 +500,17 @@ public void toByteArray() throws ReflectiveOperationException {
assertToByteArray("Direct", expected, buff);
}

@Test
public void testDecryptWithMissingSignatureFields() throws GeneralSecurityException {
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved
Map<String, Set<EncryptionFlags>> attributeWithEmptyEncryptionFlags =
attribs.keySet().stream().collect(toMap(k -> k, k -> newHashSet()));

Map<String, AttributeValue> decryptedAttributes =
encryptor.decryptRecord(attribs, attributeWithEmptyEncryptionFlags, context);

assertThat(decryptedAttributes, AttrMatcher.match(attribs));
}

private void assertToByteArray(
final String msg, final byte[] expected, final ByteBuffer testValue)
throws ReflectiveOperationException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/*
* Copyright 2015 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except
* in compliance with the License. A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*/
package com.amazonaws.services.dynamodbv2.mapper.integration;

import static org.testng.Assert.assertEquals;

import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBAttribute;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBHashKey;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMappingException;
import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBTable;
import com.amazonaws.services.dynamodbv2.datamodeling.encryption.DoNotTouch;
import com.amazonaws.services.dynamodbv2.mapper.encryption.StringAttributeTestClass;
import com.amazonaws.services.dynamodbv2.mapper.encryption.TestDynamoDBMapperFactory;
import com.amazonaws.services.dynamodbv2.model.AttributeValue;
import com.amazonaws.services.dynamodbv2.model.PutItemRequest;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

/** Test reading of document which does not contain signature field. */
public class MissingSignatureFieldsITCase extends DynamoDBMapperCryptoIntegrationTestBase {
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved

private static final String ORIGINAL_NAME_ATTRIBUTE = "originalName";
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved
private static final String STRING_ATTRIBUTE = "stringAttribute";
private static final List<Map<String, AttributeValue>> attrs = new LinkedList<>();

// Test data
static {
for (int i = 0; i < 5; i++) {
Map<String, AttributeValue> attr = new HashMap<String, AttributeValue>();
attr.put(KEY_NAME, new AttributeValue().withS("" + startKey++));
attr.put(STRING_ATTRIBUTE, new AttributeValue().withS("" + startKey++));
attr.put(ORIGINAL_NAME_ATTRIBUTE, new AttributeValue().withS("" + startKey++));
attrs.add(attr);
}
}

@BeforeClass
public static void setUp() throws Exception {
DynamoDBMapperCryptoIntegrationTestBase.setUp();
for (Map<String, AttributeValue> attr : attrs) {
dynamo.putItem(new PutItemRequest(TABLE_NAME, attr));
}
}

@Test
public void testLoadWithMissingSignatureFields() {
DynamoDBMapper util = TestDynamoDBMapperFactory.createDynamoDBMapper(dynamo);

for (Map<String, AttributeValue> attr : attrs) {
AllDoNotTouchTable load = util.load(AllDoNotTouchTable.class, attr.get(KEY_NAME).getS());
assertEquals(load.getKey(), attr.get(KEY_NAME).getS());
assertEquals(load.getStringAttribute(), attr.get(STRING_ATTRIBUTE).getS());
}
}

@Test(expectedExceptions = DynamoDBMappingException.class)
public void testLoadWithBadMissingSignatureFields() {
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved
TestDynamoDBMapperFactory.createDynamoDBMapper(dynamo)
.load(StringAttributeTestClass.class, attrs.get(0).get(KEY_NAME).getS());
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved
}

@DynamoDBTable(tableName = "aws-java-sdk-util-crypto")
public static final class AllDoNotTouchTable {
bhuvangu marked this conversation as resolved.
Show resolved Hide resolved

private String key;

private String stringAttribute;

@DynamoDBHashKey
@DoNotTouch
public String getKey() {
return key;
}

public void setKey(String key) {
this.key = key;
}

@DynamoDBAttribute
@DoNotTouch
public String getStringAttribute() {
return stringAttribute;
}

public void setStringAttribute(String stringAttribute) {
this.stringAttribute = stringAttribute;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
AllDoNotTouchTable that = (AllDoNotTouchTable) o;
return key.equals(that.key) && stringAttribute.equals(that.stringAttribute);
}
}
}