Skip to content

Commit 7d8dda5

Browse files
author
Anirav Kareddy
committed
made changes according to first round of PR
1 parent 5b90bd4 commit 7d8dda5

File tree

5 files changed

+183
-70
lines changed

5 files changed

+183
-70
lines changed

‎pom.xml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,18 @@
168168
<scope>test</scope>
169169
</dependency>
170170

171+
<dependency>
172+
<groupId>org.slf4j</groupId>
173+
<artifactId>jcl-over-slf4j</artifactId>
174+
<version>2.1.0-alpha1</version>
175+
</dependency>
176+
177+
<dependency>
178+
<groupId>ch.qos.logback</groupId>
179+
<artifactId>logback-classic</artifactId>
180+
<version>1.5.18</version>
181+
</dependency>
182+
171183
</dependencies>
172184

173185
<build>

‎src/main/java/software/amazon/encryption/s3/S3EncryptionClient.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package software.amazon.encryption.s3;
44

55
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
6+
import org.apache.commons.logging.LogFactory;
67
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
78
import software.amazon.awssdk.awscore.AwsRequestOverrideConfiguration;
89
import software.amazon.awssdk.awscore.exception.AwsServiceException;
@@ -1069,10 +1070,10 @@ public S3EncryptionClient build() {
10691070
if (!onlyOneNonNull(_cryptoMaterialsManager, _keyring, _aesKey, _rsaKeyPair, _kmsKeyId)) {
10701071
throw new S3EncryptionClientException("Exactly one must be set of: crypto materials manager, keyring, AES key, RSA key pair, KMS key id");
10711072
}
1072-
if (_enableLegacyWrappingAlgorithms && _keyring !=null && _keyring instanceof S3Keyring) {
1073+
if (_enableLegacyWrappingAlgorithms && _keyring !=null) {
10731074
S3Keyring keyring = (S3Keyring) _keyring;
1074-
if (!keyring.enableLegacyWrappingAlgorithms()) {
1075-
throw new S3EncryptionClientException("Legacy wrapping algorithms are not enabled for this keyring");
1075+
if (!keyring.areLegacyWrappingAlgorithmsEnabled()) {
1076+
LogFactory.getLog(getClass()).warn("enableLegacyWrappingAlgorithms is set on the client, but is not set on the keyring provided. In order to enable legacy wrapping algorithms, set enableLegacyWrappingAlgorithms to true in the keyring's builder.");
10761077
}
10771078
}
10781079

‎src/main/java/software/amazon/encryption/s3/materials/S3Keyring.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ protected S3Keyring(Builder<?, ?> builder) {
3636
/**
3737
* @return true if legacy wrapping algorithms are enabled, false otherwise
3838
*/
39-
public boolean enableLegacyWrappingAlgorithms() {
40-
return _enableLegacyWrappingAlgorithms;
41-
}
39+
public boolean areLegacyWrappingAlgorithmsEnabled() { return _enableLegacyWrappingAlgorithms;}
4240

4341
/**
4442
* Generates a data key using the provided EncryptionMaterials and the configured DataKeyGenerator.

‎src/test/java/software/amazon/encryption/s3/S3EncryptionClientCompatibilityTest.java‎

Lines changed: 149 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package software.amazon.encryption.s3;
44

5+
import ch.qos.logback.classic.Logger;
6+
import ch.qos.logback.classic.spi.ILoggingEvent;
7+
import ch.qos.logback.core.read.ListAppender;
58
import com.amazonaws.services.kms.AWSKMS;
69
import com.amazonaws.services.kms.AWSKMSClientBuilder;
710
import com.amazonaws.services.s3.AmazonS3Encryption;
@@ -15,17 +18,12 @@
1518
import com.amazonaws.services.s3.model.EncryptedPutObjectRequest;
1619
import com.amazonaws.services.s3.model.EncryptionMaterials;
1720
import com.amazonaws.services.s3.model.EncryptionMaterialsProvider;
18-
import com.amazonaws.services.s3.model.GetObjectMetadataRequest;
19-
import com.amazonaws.services.s3.model.InitiateMultipartUploadRequest;
20-
import com.amazonaws.services.s3.model.InitiateMultipartUploadResult;
2121
import com.amazonaws.services.s3.model.KMSEncryptionMaterials;
2222
import com.amazonaws.services.s3.model.KMSEncryptionMaterialsProvider;
23-
import com.amazonaws.services.s3.model.ObjectMetadata;
2423
import com.amazonaws.services.s3.model.StaticEncryptionMaterialsProvider;
25-
import com.amazonaws.services.s3.model.StorageClass;
26-
import com.amazonaws.services.s3.model.UploadObjectRequest;
2724
import org.junit.jupiter.api.BeforeAll;
2825
import org.junit.jupiter.api.Test;
26+
import org.slf4j.LoggerFactory;
2927
import software.amazon.awssdk.core.ResponseBytes;
3028
import software.amazon.awssdk.core.sync.RequestBody;
3129
import software.amazon.awssdk.services.s3.S3Client;
@@ -44,17 +42,15 @@
4442
import javax.crypto.SecretKey;
4543
import java.io.ByteArrayInputStream;
4644
import java.io.IOException;
47-
import java.io.InputStream;
4845
import java.nio.charset.StandardCharsets;
4946
import java.security.KeyPair;
5047
import java.security.KeyPairGenerator;
5148
import java.security.NoSuchAlgorithmException;
5249
import java.util.HashMap;
5350
import java.util.Map;
54-
import java.util.concurrent.ExecutionException;
5551

5652
import static org.junit.jupiter.api.Assertions.assertEquals;
57-
import static org.junit.jupiter.api.Assertions.assertNotEquals;
53+
import static org.junit.jupiter.api.Assertions.assertFalse;
5854
import static org.junit.jupiter.api.Assertions.assertThrows;
5955
import static org.junit.jupiter.api.Assertions.assertTrue;
6056

@@ -983,69 +979,158 @@ public void nullMaterialDescriptionV3() {
983979
}
984980

985981
@Test
986-
public void validateAgainstSettingLegacyWrappingOnClientWithAesKeyringPassedV1toV3() {
987-
try {
988-
AesKeyring aesKeyring = AesKeyring.builder()
989-
.wrappingKey(AES_KEY)
990-
.build();
991-
992-
S3Client wrappedClient = S3Client.create();
993-
S3Client v3Client = S3EncryptionClient.builder()
994-
.keyring(aesKeyring)
995-
.wrappedClient(wrappedClient)
996-
.enableLegacyWrappingAlgorithms(true)
997-
.enableLegacyUnauthenticatedModes(true)
998-
.build();
999-
throw new RuntimeException("Expected failure");
1000-
} catch (S3EncryptionClientException e) {
1001-
assertTrue(e.getMessage().contains("Legacy wrapping algorithms are not enabled for this keyring"));
1002-
}
982+
public void LegacyWrappingEnabledOnClientButNotOnAesKeyring() {
983+
Logger logger = (Logger) LoggerFactory.getLogger(S3EncryptionClient.class);
984+
ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
985+
listAppender.start();
986+
logger.addAppender(listAppender);
987+
988+
AesKeyring aesKeyring = AesKeyring.builder()
989+
.wrappingKey(AES_KEY)
990+
.build();
991+
992+
S3Client wrappedClient = S3Client.create();
993+
S3Client v3Client = S3EncryptionClient.builder()
994+
.keyring(aesKeyring)
995+
.wrappedClient(wrappedClient)
996+
.enableLegacyWrappingAlgorithms(true)
997+
.enableLegacyUnauthenticatedModes(true)
998+
.build();
1003999

1000+
assertTrue(listAppender.list.stream().anyMatch(event -> event.getMessage().contains("enableLegacyWrappingAlgorithms is set on the client, but is not set on the keyring provided. In order to enable legacy wrapping algorithms, set enableLegacyWrappingAlgorithms to true in the keyring's builder.")));
1001+
logger.detachAppender(listAppender);
10041002
}
10051003

10061004
@Test
1007-
public void validateAgainstSettingLegacyWrappingOnClientWithRsaKeyringPassedV1toV3() {
1008-
try {
1009-
PartialRsaKeyPair partialRsaKeyPair = PartialRsaKeyPair.builder()
1010-
.publicKey(RSA_KEY_PAIR.getPublic())
1011-
.privateKey(RSA_KEY_PAIR.getPrivate())
1012-
.build();
1013-
1014-
RsaKeyring rsaKeyring = RsaKeyring.builder()
1015-
.wrappingKeyPair(partialRsaKeyPair)
1016-
.build();
1017-
1018-
S3Client wrappedClient = S3Client.create();
1019-
S3Client v3Client = S3EncryptionClient.builder()
1020-
.keyring(rsaKeyring)
1021-
.wrappedClient(wrappedClient)
1022-
.enableLegacyWrappingAlgorithms(true)
1023-
.enableLegacyUnauthenticatedModes(true)
1024-
.build();
1025-
throw new RuntimeException("Expected failure");
1026-
} catch (S3EncryptionClientException e) {
1027-
assertTrue(e.getMessage().contains("Legacy wrapping algorithms are not enabled for this keyring"));
1028-
}
1005+
public void LegacyWrappingEnabledOnClientButNotOnRsaKeyring() {
1006+
Logger logger = (Logger) LoggerFactory.getLogger(S3EncryptionClient.class);
1007+
ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
1008+
listAppender.start();
1009+
logger.addAppender(listAppender);
1010+
1011+
PartialRsaKeyPair partialRsaKeyPair = PartialRsaKeyPair.builder()
1012+
.publicKey(RSA_KEY_PAIR.getPublic())
1013+
.privateKey(RSA_KEY_PAIR.getPrivate())
1014+
.build();
1015+
1016+
RsaKeyring rsaKeyring = RsaKeyring.builder()
1017+
.wrappingKeyPair(partialRsaKeyPair)
1018+
.build();
1019+
1020+
S3Client wrappedClient = S3Client.create();
1021+
S3Client v3Client = S3EncryptionClient.builder()
1022+
.keyring(rsaKeyring)
1023+
.wrappedClient(wrappedClient)
1024+
.enableLegacyWrappingAlgorithms(true)
1025+
.enableLegacyUnauthenticatedModes(true)
1026+
.build();
1027+
1028+
assertTrue(listAppender.list.stream().anyMatch(event -> event.getMessage().contains("enableLegacyWrappingAlgorithms is set on the client, but is not set on the keyring provided. In order to enable legacy wrapping algorithms, set enableLegacyWrappingAlgorithms to true in the keyring's builder.")));
1029+
logger.detachAppender(listAppender);
1030+
10291031
}
10301032

10311033
@Test
1032-
public void validateAgainstSettingLegacyWrappingOnClientWithKmsKeyringPassedV1toV3() {
1033-
try {
1034-
KmsKeyring kmsKeyring = KmsKeyring.builder()
1035-
.wrappingKeyId(KMS_KEY_ID)
1036-
.build();
1037-
1038-
S3Client wrappedClient = S3Client.create();
1039-
S3Client v3Client = S3EncryptionClient.builder()
1040-
.keyring(kmsKeyring)
1041-
.wrappedClient(wrappedClient)
1042-
.enableLegacyWrappingAlgorithms(true)
1043-
.enableLegacyUnauthenticatedModes(true)
1044-
.build();
1045-
throw new RuntimeException("Expected failure");
1046-
} catch (S3EncryptionClientException e) {
1047-
assertTrue(e.getMessage().contains("Legacy wrapping algorithms are not enabled for this keyring"));
1034+
public void LegacyWrappingEnabledOnClientButNotOnKmsKeyring() {
1035+
Logger logger = (Logger) LoggerFactory.getLogger(S3EncryptionClient.class);
1036+
ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
1037+
listAppender.start();
1038+
logger.addAppender(listAppender);
1039+
1040+
KmsKeyring kmsKeyring = KmsKeyring.builder()
1041+
.wrappingKeyId(KMS_KEY_ID)
1042+
.build();
1043+
1044+
S3Client wrappedClient = S3Client.create();
1045+
S3Client v3Client = S3EncryptionClient.builder()
1046+
.keyring(kmsKeyring)
1047+
.wrappedClient(wrappedClient)
1048+
.enableLegacyWrappingAlgorithms(true)
1049+
.enableLegacyUnauthenticatedModes(true)
1050+
.build();
1051+
1052+
assertTrue(listAppender.list.stream().anyMatch(event -> event.getMessage().contains("enableLegacyWrappingAlgorithms is set on the client, but is not set on the keyring provided. In order to enable legacy wrapping algorithms, set enableLegacyWrappingAlgorithms to true in the keyring's builder.")));
1053+
logger.detachAppender(listAppender);
1054+
10481055
}
1056+
1057+
@Test
1058+
public void LegacyWrappingEnabledOnBothClientAndAesKeyring() {
1059+
Logger logger = (Logger) LoggerFactory.getLogger(S3EncryptionClient.class);
1060+
ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
1061+
listAppender.start();
1062+
logger.addAppender(listAppender);
1063+
1064+
AesKeyring aesKeyring = AesKeyring.builder()
1065+
.wrappingKey(AES_KEY)
1066+
.enableLegacyWrappingAlgorithms(true)
1067+
.build();
1068+
1069+
S3Client wrappedClient = S3Client.create();
1070+
S3Client v3Client = S3EncryptionClient.builder()
1071+
.keyring(aesKeyring)
1072+
.wrappedClient(wrappedClient)
1073+
.enableLegacyWrappingAlgorithms(true)
1074+
.enableLegacyUnauthenticatedModes(true)
1075+
.build();
1076+
1077+
assertFalse(listAppender.list.stream()
1078+
.anyMatch(event -> event.getMessage().contains("enableLegacyWrappingAlgorithms is set on the client, but is not set on the keyring provided. In order to enable legacy wrapping algorithms, set enableLegacyWrappingAlgorithms to true in the keyring's builder.")));
1079+
logger.detachAppender(listAppender);
1080+
}
1081+
1082+
@Test
1083+
public void LegacyWrappingEnabledOnBothClientAndRsaKeyring() {
1084+
Logger logger = (Logger) LoggerFactory.getLogger(S3EncryptionClient.class);
1085+
ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
1086+
listAppender.start();
1087+
logger.addAppender(listAppender);
1088+
1089+
PartialRsaKeyPair partialRsaKeyPair = PartialRsaKeyPair.builder()
1090+
.publicKey(RSA_KEY_PAIR.getPublic())
1091+
.privateKey(RSA_KEY_PAIR.getPrivate())
1092+
.build();
1093+
1094+
RsaKeyring rsaKeyring = RsaKeyring.builder()
1095+
.wrappingKeyPair(partialRsaKeyPair)
1096+
.enableLegacyWrappingAlgorithms(true)
1097+
.build();
1098+
1099+
S3Client wrappedClient = S3Client.create();
1100+
S3Client v3Client = S3EncryptionClient.builder()
1101+
.keyring(rsaKeyring)
1102+
.wrappedClient(wrappedClient)
1103+
.enableLegacyWrappingAlgorithms(true)
1104+
.enableLegacyUnauthenticatedModes(true)
1105+
.build();
1106+
1107+
assertFalse(listAppender.list.stream()
1108+
.anyMatch(event -> event.getMessage().contains("enableLegacyWrappingAlgorithms is set on the client, but is not set on the keyring provided. In order to enable legacy wrapping algorithms, set enableLegacyWrappingAlgorithms to true in the keyring's builder.")));
1109+
logger.detachAppender(listAppender);
10491110
}
10501111

1112+
@Test
1113+
public void LegacyWrappingEnabledOnBothClientAndKmsKeyring() {
1114+
Logger logger = (Logger) LoggerFactory.getLogger(S3EncryptionClient.class);
1115+
ListAppender<ILoggingEvent> listAppender = new ListAppender<>();
1116+
listAppender.start();
1117+
logger.addAppender(listAppender);
1118+
1119+
KmsKeyring kmsKeyring = KmsKeyring.builder()
1120+
.wrappingKeyId(KMS_KEY_ID)
1121+
.enableLegacyWrappingAlgorithms(true)
1122+
.build();
1123+
1124+
S3Client wrappedClient = S3Client.create();
1125+
S3Client v3Client = S3EncryptionClient.builder()
1126+
.keyring(kmsKeyring)
1127+
.wrappedClient(wrappedClient)
1128+
.enableLegacyWrappingAlgorithms(true)
1129+
.enableLegacyUnauthenticatedModes(true)
1130+
.build();
1131+
1132+
assertFalse(listAppender.list.stream()
1133+
.anyMatch(event -> event.getMessage().contains("enableLegacyWrappingAlgorithms is set on the client, but is not set on the keyring provided. In order to enable legacy wrapping algorithms, set enableLegacyWrappingAlgorithms to true in the keyring's builder.")));
1134+
logger.detachAppender(listAppender);
1135+
}
10511136
}

‎src/test/resources/logback-test.xml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<configuration>
3+
<appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
4+
<encoder>
5+
<pattern>%highlight(WARNING: %msg%n)</pattern>
6+
</encoder>
7+
<filter class="ch.qos.logback.classic.filter.LevelFilter">
8+
<level>WARN</level>
9+
<onMatch>ACCEPT</onMatch>
10+
<onMismatch>DENY</onMismatch>
11+
</filter>
12+
</appender>
13+
14+
<root level="warn">
15+
<appender-ref ref="CONSOLE" />
16+
</root>
17+
</configuration>

0 commit comments

Comments
 (0)