From b988c8e7b6920caf575f69c6700119ad43c5cf77 Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Fri, 3 Aug 2018 11:22:09 -0700 Subject: [PATCH 1/6] ISSUE #42. Update documentation and warnings related to SaveBehavior. --- README.md | 1 + .../examples/AwsKmsEncryptedObject.java | 1 + .../datamodeling/AttributeEncryptor.java | 25 ++++++++++++++----- .../TransformerHolisticTests.java | 13 ++++++---- .../dynamodbv2/testing/FakeParameters.java | 19 +++++++++++--- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index b7800e8a..018fd157 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ The **[Amazon DynamoDB][ddb] Client-side Encryption in Java** supports encryption and signing of your data when stored in Amazon DynamoDB. A typical use of this library is when you are using [DynamoDBMapper][ddbmapper], where transparent protection of all objects serialized through the mapper can be enabled via configuring an [AttributeEncryptor][attrencryptor]. +**Please note that it is critically important that you use `SaveBehavior.CLOBBER` when using AttributeEncryptor.** For more advanced use cases where tighter control over the encryption and signing process is necessary, the low-level [DynamoDBEncryptor][ddbencryptor] can be used directly. diff --git a/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java b/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java index 0b3417e2..46987bf0 100644 --- a/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java +++ b/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java @@ -52,6 +52,7 @@ public static void encryptRecord(final String cmkArn, final String region) { // Encryptor creation final DynamoDBEncryptor encryptor = DynamoDBEncryptor.getInstance(cmp); // Mapper Creation + // Please note the use of SaveBehavior.CLOBBER. Omitting this may result in data-corruption. DynamoDBMapperConfig mapperConfig = DynamoDBMapperConfig.builder().withSaveBehavior(SaveBehavior.CLOBBER).build(); DynamoDBMapper mapper = new DynamoDBMapper(ddb, mapperConfig, new AttributeEncryptor(encryptor)); diff --git a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java index 363885c8..92f22647 100644 --- a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java +++ b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java @@ -14,13 +14,10 @@ */ package com.amazonaws.services.dynamodbv2.datamodeling; -import java.util.Collections; -import java.util.EnumSet; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; +import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperConfig.SaveBehavior; import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMappingsRegistry.Mapping; import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMappingsRegistry.Mappings; import com.amazonaws.services.dynamodbv2.datamodeling.encryption.DoNotEncrypt; @@ -32,13 +29,18 @@ import com.amazonaws.services.dynamodbv2.datamodeling.encryption.TableAadOverride; import com.amazonaws.services.dynamodbv2.datamodeling.encryption.providers.EncryptionMaterialsProvider; import com.amazonaws.services.dynamodbv2.model.AttributeValue; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; /** * Encrypts all non-key fields prior to storing them in DynamoDB. + * It is critically important that this is only used with @{link SaveBehavior#CLOBBER}. Use of + * any other @{code SaveBehavior} may result in data-corruption. * * @author Greg Rubin */ public class AttributeEncryptor implements AttributeTransformer { + private static final Log LOG = LogFactory.getLog(AttributeEncryptor.class); private final DynamoDBEncryptor encryptor; private final Map, ModelClassMetadata> metadataCache = new ConcurrentHashMap<>(); @@ -58,9 +60,20 @@ public DynamoDBEncryptor getEncryptor() { public Map transform(final Parameters parameters) { // one map of attributeFlags per model class final ModelClassMetadata metadata = getModelClassMetadata(parameters); + + final Map attributeValues = parameters.getAttributeValues(); + if (metadata.doNotTouch) { + return attributeValues; + } + + if (parameters.isPartialUpdate()) { + LOG.error("Use of AttributeEncryptor without SaveBehavior.CLOBBER is an error and may result in data-corruption. " + + "This occured while trying to save " + parameters.getModelClass()); + } + try { return encryptor.encryptRecord( - parameters.getAttributeValues(), + attributeValues, metadata.getEncryptionFlags(), paramsToContext(parameters)); } catch (Exception ex) { diff --git a/src/test/java/com/amazonaws/services/dynamodbv2/datamodeling/TransformerHolisticTests.java b/src/test/java/com/amazonaws/services/dynamodbv2/datamodeling/TransformerHolisticTests.java index 7d1635d4..46f7eec0 100644 --- a/src/test/java/com/amazonaws/services/dynamodbv2/datamodeling/TransformerHolisticTests.java +++ b/src/test/java/com/amazonaws/services/dynamodbv2/datamodeling/TransformerHolisticTests.java @@ -115,6 +115,9 @@ public class TransformerHolisticTests { private static final EncryptionMaterialsProvider symWrappedProv; private AmazonDynamoDB client; + // AttributeEncryptor *must* be used with SaveBehavior.CLOBBER to avoid the risk of data corruption. + private static final DynamoDBMapperConfig CLOBBER_CONFIG = + DynamoDBMapperConfig.builder().withSaveBehavior(SaveBehavior.CLOBBER).build(); private static final BaseClass ENCRYPTED_TEST_VALUE = new BaseClass(); private static final Mixed MIXED_TEST_VALUE = new Mixed(); private static final SignOnly SIGNED_TEST_VALUE = new SignOnly(); @@ -292,7 +295,7 @@ public void setUp() { @Test public void simpleSaveLoad() { - DynamoDBMapper mapper = new DynamoDBMapper(client, DynamoDBMapperConfig.DEFAULT, new AttributeEncryptor(symProv)); + DynamoDBMapper mapper = new DynamoDBMapper(client, CLOBBER_CONFIG, new AttributeEncryptor(symProv)); Mixed obj = new Mixed(); obj.setHashKey(0); obj.setRangeKey(15); @@ -322,7 +325,7 @@ public void simpleSaveLoad() { @Test public void leadingAndTrailingZeros() { - DynamoDBMapper mapper = new DynamoDBMapper(client, DynamoDBMapperConfig.DEFAULT, new AttributeEncryptor(symProv)); + DynamoDBMapper mapper = new DynamoDBMapper(client, CLOBBER_CONFIG, new AttributeEncryptor(symProv)); Mixed obj = new Mixed(); obj.setHashKey(0); obj.setRangeKey(15); @@ -364,7 +367,7 @@ public void leadingAndTrailingZeros() { @Test public void simpleSaveLoadAsym() { - DynamoDBMapper mapper = new DynamoDBMapper(client, DynamoDBMapperConfig.DEFAULT, new AttributeEncryptor(asymProv)); + DynamoDBMapper mapper = new DynamoDBMapper(client, CLOBBER_CONFIG, new AttributeEncryptor(asymProv)); BaseClass obj = new BaseClass(); obj.setHashKey(0); @@ -394,7 +397,7 @@ public void simpleSaveLoadAsym() { @Test public void simpleSaveLoadHashOnly() { - DynamoDBMapper mapper = new DynamoDBMapper(client, DynamoDBMapperConfig.DEFAULT, new AttributeEncryptor( + DynamoDBMapper mapper = new DynamoDBMapper(client, CLOBBER_CONFIG, new AttributeEncryptor( symProv)); HashKeyOnly obj = new HashKeyOnly(""); @@ -411,7 +414,7 @@ public void simpleSaveLoadHashOnly() { @Test public void simpleSaveLoadKeysOnly() { - DynamoDBMapper mapper = new DynamoDBMapper(client, DynamoDBMapperConfig.DEFAULT, new AttributeEncryptor( + DynamoDBMapper mapper = new DynamoDBMapper(client, CLOBBER_CONFIG, new AttributeEncryptor( asymProv)); KeysOnly obj = new KeysOnly(); diff --git a/src/test/java/com/amazonaws/services/dynamodbv2/testing/FakeParameters.java b/src/test/java/com/amazonaws/services/dynamodbv2/testing/FakeParameters.java index 95889e6f..e82c2472 100644 --- a/src/test/java/com/amazonaws/services/dynamodbv2/testing/FakeParameters.java +++ b/src/test/java/com/amazonaws/services/dynamodbv2/testing/FakeParameters.java @@ -25,14 +25,20 @@ import com.amazonaws.services.dynamodbv2.model.AttributeValue; public class FakeParameters { + public static AttributeTransformer.Parameters getInstance(Class clazz, + Map attribs, DynamoDBMapperConfig config, String tableName, + String hashKeyName, String rangeKeyName) { + return getInstance(clazz, attribs, config, tableName, hashKeyName, rangeKeyName, false); + } + public static AttributeTransformer.Parameters getInstance(Class clazz, Map attribs, DynamoDBMapperConfig config, String tableName, - String hashKeyName, String rangeKeyName) { + String hashKeyName, String rangeKeyName, boolean isPartialUpdate) { // We use this relatively insane proxy setup so that modifications to the Parameters // interface doesn't break our tests (unless it actually impacts our code). FakeParameters fakeParams = new FakeParameters(clazz, attribs, config, tableName, - hashKeyName, rangeKeyName); + hashKeyName, rangeKeyName, isPartialUpdate); @SuppressWarnings("unchecked") AttributeTransformer.Parameters proxyObject = (AttributeTransformer.Parameters) Proxy .newProxyInstance(AttributeTransformer.class.getClassLoader(), @@ -65,9 +71,11 @@ public Object invoke(Object obj, Method method, Object[] args) throws Throwable private final String tableName; private final String hashKeyName; private final String rangeKeyName; + private final boolean isPartialUpdate; private FakeParameters(Class clazz, Map attribs, - DynamoDBMapperConfig config, String tableName, String hashKeyName, String rangeKeyName) { + DynamoDBMapperConfig config, String tableName, String hashKeyName, String rangeKeyName, + boolean isPartialUpdate) { super(); this.clazz = clazz; this.attrs = Collections.unmodifiableMap(attribs); @@ -75,6 +83,7 @@ private FakeParameters(Class clazz, Map attribs, this.tableName = tableName; this.hashKeyName = hashKeyName; this.rangeKeyName = rangeKeyName; + this.isPartialUpdate = isPartialUpdate; } public Map getAttributeValues() { @@ -100,4 +109,8 @@ public String getHashKeyName() { public String getRangeKeyName() { return rangeKeyName; } + + public boolean isPartialUpdate() { + return isPartialUpdate; + } } From b5f7fe9eea6cf6e8a0aad87ad6448dac6eb98ff5 Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Fri, 3 Aug 2018 12:00:24 -0700 Subject: [PATCH 2/6] Remove import-* in AttributeEncryptor.java --- .../dynamodbv2/datamodeling/AttributeEncryptor.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java index 92f22647..d16886df 100644 --- a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java +++ b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java @@ -14,7 +14,11 @@ */ package com.amazonaws.services.dynamodbv2.datamodeling; -import java.util.*; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperConfig.SaveBehavior; From 8a00b2a094d1e4f83bfd2a01a276e8117092ddce Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Fri, 3 Aug 2018 12:35:55 -0700 Subject: [PATCH 3/6] Improve wording around need of SaveBehavior.CLOBBER --- README.md | 5 +++-- .../com/amazonaws/examples/AwsKmsEncryptedObject.java | 2 +- .../dynamodbv2/datamodeling/AttributeEncryptor.java | 8 +++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 018fd157..928089de 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,8 @@ The **[Amazon DynamoDB][ddb] Client-side Encryption in Java** supports encryption and signing of your data when stored in Amazon DynamoDB. A typical use of this library is when you are using [DynamoDBMapper][ddbmapper], where transparent protection of all objects serialized through the mapper can be enabled via configuring an [AttributeEncryptor][attrencryptor]. -**Please note that it is critically important that you use `SaveBehavior.CLOBBER` when using AttributeEncryptor.** + +> Important: Use `SaveBehavior.CLOBBER` with `AttributeEncryptor`. If you do not do so you risk corrupting your signatures and encrypted data. For more advanced use cases where tighter control over the encryption and signing process is necessary, the low-level [DynamoDBEncryptor][ddbencryptor] can be used directly. @@ -75,7 +76,7 @@ To enable transparent encryption and signing, simply specify the necessary encry SecretKey cek = ...; // Content encrypting key SecretKey macKey = ...; // Signing key EncryptionMaterialsProvider provider = new SymmetricStaticProvider(cek, macKey); - mapper = new DynamoDBMapper(client, DynamoDBMapperConfig.DEFAULT, + mapper = new DynamoDBMapper(client, DynamoDBMapperConfig.builder().withSaveBehavior(SaveBehavior.CLOBBER).build(), new AttributeEncryptor(provider)); Book book = new Book(); book.setId(123); diff --git a/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java b/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java index 46987bf0..d2fc9d36 100644 --- a/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java +++ b/examples/com/amazonaws/examples/AwsKmsEncryptedObject.java @@ -52,7 +52,7 @@ public static void encryptRecord(final String cmkArn, final String region) { // Encryptor creation final DynamoDBEncryptor encryptor = DynamoDBEncryptor.getInstance(cmp); // Mapper Creation - // Please note the use of SaveBehavior.CLOBBER. Omitting this may result in data-corruption. + // Please note the use of SaveBehavior.CLOBBER. Omitting this can result in data-corruption. DynamoDBMapperConfig mapperConfig = DynamoDBMapperConfig.builder().withSaveBehavior(SaveBehavior.CLOBBER).build(); DynamoDBMapper mapper = new DynamoDBMapper(ddb, mapperConfig, new AttributeEncryptor(encryptor)); diff --git a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java index d16886df..983ffb68 100644 --- a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java +++ b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java @@ -38,8 +38,8 @@ /** * Encrypts all non-key fields prior to storing them in DynamoDB. - * It is critically important that this is only used with @{link SaveBehavior#CLOBBER}. Use of - * any other @{code SaveBehavior} may result in data-corruption. + * This must be used with @{link SaveBehavior#CLOBBER}. Use of + * any other @{code SaveBehavior} can result in data-corruption. * * @author Greg Rubin */ @@ -66,12 +66,14 @@ public Map transform(final Parameters parameters) { final ModelClassMetadata metadata = getModelClassMetadata(parameters); final Map attributeValues = parameters.getAttributeValues(); + // If this class is marked as "DoNotTouch" then we know our encryptor will not change it at all + // so we may as well fast-return and do nothing. This also avoids emitting errors when they would not apply. if (metadata.doNotTouch) { return attributeValues; } if (parameters.isPartialUpdate()) { - LOG.error("Use of AttributeEncryptor without SaveBehavior.CLOBBER is an error and may result in data-corruption. " + + LOG.error("Use of AttributeEncryptor without SaveBehavior.CLOBBER is an error and can result in data-corruption. " + "This occured while trying to save " + parameters.getModelClass()); } From c1f74d0ac94bf6efa5e283c9089700db2fee0eca Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Fri, 3 Aug 2018 13:31:19 -0700 Subject: [PATCH 4/6] Typo fix in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 928089de..42e810ea 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ The **[Amazon DynamoDB][ddb] Client-side Encryption in Java** supports encryptio A typical use of this library is when you are using [DynamoDBMapper][ddbmapper], where transparent protection of all objects serialized through the mapper can be enabled via configuring an [AttributeEncryptor][attrencryptor]. -> Important: Use `SaveBehavior.CLOBBER` with `AttributeEncryptor`. If you do not do so you risk corrupting your signatures and encrypted data. +**Important: Use `SaveBehavior.CLOBBER` with `AttributeEncryptor`. If you do not do so you risk corrupting your signatures and encrypted data.** For more advanced use cases where tighter control over the encryption and signing process is necessary, the low-level [DynamoDBEncryptor][ddbencryptor] can be used directly. From cd1ebb454fcbfd7dc7646ac6fb5170e2f669594d Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Fri, 3 Aug 2018 13:35:45 -0700 Subject: [PATCH 5/6] Improve explanation of issues when not using CLOBBER in AttributeEncryptor --- .../services/dynamodbv2/datamodeling/AttributeEncryptor.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java index 983ffb68..4ed31f25 100644 --- a/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java +++ b/src/main/java/com/amazonaws/services/dynamodbv2/datamodeling/AttributeEncryptor.java @@ -72,6 +72,10 @@ public Map transform(final Parameters parameters) { return attributeValues; } + // When AttributeEncryptor is used without SaveBehavior.CLOBBER, it is trying to transform only a subset + // of the actual fields stored in DynamoDB. This means that the generated signature will not cover any + // unmodified fields. Thus, upon untransform, the signature verification will fail as it won't cover all + // expected fields. if (parameters.isPartialUpdate()) { LOG.error("Use of AttributeEncryptor without SaveBehavior.CLOBBER is an error and can result in data-corruption. " + "This occured while trying to save " + parameters.getModelClass()); From 7530a1e8b0190c21b9a4bbb6ebc979b87593fae3 Mon Sep 17 00:00:00 2001 From: Greg Rubin Date: Fri, 3 Aug 2018 13:50:34 -0700 Subject: [PATCH 6/6] Improve explanation of SaveBehavior.CLOBBER in README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 42e810ea..9008060e 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,7 @@ The **[Amazon DynamoDB][ddb] Client-side Encryption in Java** supports encryptio A typical use of this library is when you are using [DynamoDBMapper][ddbmapper], where transparent protection of all objects serialized through the mapper can be enabled via configuring an [AttributeEncryptor][attrencryptor]. **Important: Use `SaveBehavior.CLOBBER` with `AttributeEncryptor`. If you do not do so you risk corrupting your signatures and encrypted data.** +When CLOBBER is not specified, fields that are present in the record may not be passed down to the encryptor, which results in fields being left out of the record signature. This in turn can result in records failing to decrypt. For more advanced use cases where tighter control over the encryption and signing process is necessary, the low-level [DynamoDBEncryptor][ddbencryptor] can be used directly.