Skip to content

Commit 1da1cae

Browse files
kessplaskessplas
authored
fix: address some edge cases to fix async CBC ranged gets (#70)
* fix: address some edge cases to fix async CBC ranged gets
1 parent 3ac4609 commit 1da1cae

File tree

5 files changed

+142
-35
lines changed

5 files changed

+142
-35
lines changed

‎src/main/java/software/amazon/encryption/s3/internal/CipherPublisher.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public CipherPublisher(final Cipher cipher, final SdkPublisher<ByteBuffer> wrapp
3333
public void subscribe(Subscriber<? super ByteBuffer> subscriber) {
3434
// Wrap the (customer) subscriber in a CipherSubscriber, then subscribe it
3535
// to the wrapped (ciphertext) publisher
36-
Subscriber wrappedSubscriber = RangedGetUtils.adjustToDesiredRange(subscriber, range, contentRange, cipherTagLengthBits);
36+
Subscriber<? super ByteBuffer> wrappedSubscriber = RangedGetUtils.adjustToDesiredRange(subscriber, range, contentRange, cipherTagLengthBits);
3737
wrappedPublisher.subscribe(new CipherSubscriber(wrappedSubscriber, cipher, contentLength));
3838
}
3939
}

‎src/main/java/software/amazon/encryption/s3/internal/GetEncryptedObjectPipeline.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,8 @@ public void onStream(SdkPublisher<ByteBuffer> ciphertextPublisher) {
199199
case ALG_AES_256_GCM_IV12_TAG16_NO_KDF:
200200
cipher.init(Cipher.DECRYPT_MODE, contentKey, new GCMParameterSpec(tagLength, iv));
201201
break;
202-
case ALG_AES_256_CBC_IV16_NO_KDF:
203-
if (materials.s3Request().range() != null) {
204-
throw new UnsupportedOperationException();
205-
}
206202
case ALG_AES_256_CTR_IV16_TAG16_NO_KDF:
203+
case ALG_AES_256_CBC_IV16_NO_KDF:
207204
cipher.init(Cipher.DECRYPT_MODE, contentKey, new IvParameterSpec(iv));
208205
break;
209206
default:

‎src/main/java/software/amazon/encryption/s3/legacy/internal/AdjustedRangeSubscriber.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ private void initializeForRead(long rangeBeginning, long rangeEnd) {
2626
// To get to the left-most byte desired by a user, we must skip over the 16 bytes of the
2727
// preliminary cipher block, and then possibly skip a few more bytes into the next block
2828
// to where the left-most byte is located.
29-
3029
if (rangeBeginning < SYMMETRIC_CIPHER_BLOCK_SIZE_BYTES) {
3130
numBytesToSkip = (int) rangeBeginning;
3231
} else {
@@ -46,12 +45,19 @@ public void onSubscribe(Subscription s) {
4645

4746
@Override
4847
public void onNext(ByteBuffer byteBuffer) {
48+
// In edge cases where the beginning index exceeds the offset,
49+
// there is never valid data to read, so signal completion immediately.
50+
if (virtualAvailable <= 0) {
51+
wrappedSubscriber.onComplete();
52+
}
53+
4954
if (numBytesToSkip != 0) {
5055
byte[] buf = byteBuffer.array();
5156
if (numBytesToSkip > buf.length) {
57+
// If we need to skip past the available data,
58+
// we are returning nothing, so signal completion
5259
numBytesToSkip -= buf.length;
53-
// TODO: This Handles CBC mode Edge Condition but not working
54-
wrappedSubscriber.onNext(ByteBuffer.wrap(new byte[0]));
60+
wrappedSubscriber.onComplete();
5561
} else {
5662
outputBuffer = Arrays.copyOfRange(buf, numBytesToSkip, buf.length);
5763
numBytesToSkip = 0;
@@ -65,6 +71,13 @@ public void onNext(ByteBuffer byteBuffer) {
6571
virtualAvailable -= bytesToRead;
6672
wrappedSubscriber.onNext(ByteBuffer.wrap(outputBuffer, 0, Math.toIntExact(bytesToRead)));
6773
}
74+
75+
// Since we are skipping some bytes, we may need to signal onComplete
76+
// from within onNext to prevent the subscriber from waiting for more
77+
// data indefinitely
78+
if (virtualAvailable <= 0) {
79+
wrappedSubscriber.onComplete();
80+
}
6881
}
6982

7083
@Override

‎src/main/java/software/amazon/encryption/s3/legacy/internal/RangedGetUtils.java

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.io.ByteArrayInputStream;
88
import java.io.IOException;
99
import java.io.InputStream;
10+
import java.nio.ByteBuffer;
1011

1112
/**
1213
* Utilities for processing Ranged Get functions.
@@ -58,66 +59,72 @@ private static long getCipherBlockUpperBound(final long rightmostBytePosition) {
5859
return upperBound < 0 ? Long.MAX_VALUE : upperBound;
5960
}
6061

61-
public static Subscriber adjustToDesiredRange(Subscriber subscriber, long[] range, String contentRange, int cipherTagLengthBits) {
62-
if (range == null || contentRange == null) {
63-
return subscriber;
64-
}
62+
private static long calculateMaxOffset(String contentRange, int cipherTagLengthBits) {
6563
final long instanceLength;
6664
int pos = contentRange.lastIndexOf("/");
6765
instanceLength = Long.parseLong(contentRange.substring(pos + 1));
6866

69-
final long maxOffset = instanceLength - (cipherTagLengthBits / 8) - 1;
70-
if (range[1] > maxOffset) {
71-
range[1] = maxOffset;
72-
if (range[0] > range[1]) {
73-
// TODO: Find a way to handle subscriber similar to Empty InputStream (this resolves one edge case error in CBC)
67+
return instanceLength - (cipherTagLengthBits / 8) - 1;
68+
}
69+
70+
public static Subscriber<? super ByteBuffer> adjustToDesiredRange(Subscriber<? super ByteBuffer> subscriber, long[] cryptoRange, String contentRange, int cipherTagLengthBits) {
71+
if (cryptoRange == null || contentRange == null) {
72+
return subscriber;
73+
}
74+
75+
final long maxOffset = calculateMaxOffset(contentRange, cipherTagLengthBits);
76+
if (cryptoRange[1] > maxOffset) {
77+
cryptoRange[1] = maxOffset;
78+
if (cryptoRange[0] > cryptoRange[1]) {
79+
// When the beginning of the crypto range is after the max offset,
80+
// there is no data to read. The current implementation of
81+
// AdjustedRangeSubscriber handles this case itself,
82+
// but this might as well be a Null/Noop Subscriber
7483
try {
75-
return new AdjustedRangeSubscriber(subscriber, range[0], range[1]);
84+
return new AdjustedRangeSubscriber(subscriber, cryptoRange[0], cryptoRange[1]);
7685
} catch (IOException e) {
7786
throw new S3EncryptionClientException(e.getMessage());
7887
}
7988
}
8089
}
81-
if (range[0] > range[1]) {
90+
if (cryptoRange[0] > cryptoRange[1]) {
8291
// Make no modifications if range is invalid.
8392
return subscriber;
8493
}
8594
try {
86-
return new AdjustedRangeSubscriber(subscriber, range[0], range[1]);
95+
return new AdjustedRangeSubscriber(subscriber, cryptoRange[0], cryptoRange[1]);
8796
} catch (IOException e) {
8897
throw new S3EncryptionClientException("Error adjusting output to desired byte range: " + e.getMessage());
8998
}
9099
}
91100

92-
public static InputStream adjustToDesiredRange(InputStream plaintext, long[] range, String contentRange, int cipherTagLengthBits) {
93-
if (range == null || contentRange == null) {
101+
public static InputStream adjustToDesiredRange(InputStream plaintext, long[] cryptoRange, String contentRange, int cipherTagLengthBits) {
102+
if (cryptoRange == null || contentRange == null) {
94103
return plaintext;
95104
}
96-
final long instanceLength;
97-
int pos = contentRange.lastIndexOf("/");
98-
instanceLength = Long.parseLong(contentRange.substring(pos + 1));
99105

100-
final long maxOffset = instanceLength - (cipherTagLengthBits / 8) - 1;
101-
if (range[1] > maxOffset) {
102-
range[1] = maxOffset;
103-
if (range[0] > range[1]) {
106+
final long maxOffset = calculateMaxOffset(contentRange, cipherTagLengthBits);
107+
if (cryptoRange[1] > maxOffset) {
108+
cryptoRange[1] = maxOffset;
109+
if (cryptoRange[0] > cryptoRange[1]) {
104110
// Close existing input stream to avoid resource leakage,
105111
// return empty input stream
106112
try {
107-
if (plaintext != null)
113+
if (plaintext != null) {
108114
plaintext.close();
115+
}
109116
} catch (IOException e) {
110-
throw new RuntimeException("Error while closing the Input Stream" + e.getMessage());
117+
throw new S3EncryptionClientException("Error while closing the InputStream: " + e.getMessage());
111118
}
112119
return new ByteArrayInputStream(new byte[0]);
113120
}
114121
}
115-
if (range[0] > range[1]) {
122+
if (cryptoRange[0] > cryptoRange[1]) {
116123
// Make no modifications if range is invalid.
117124
return plaintext;
118125
}
119126
try {
120-
return new AdjustedRangeInputStream(plaintext, range[0], range[1]);
127+
return new AdjustedRangeInputStream(plaintext, cryptoRange[0], cryptoRange[1]);
121128
} catch (IOException e) {
122129
throw new S3EncryptionClientException("Error adjusting output to desired byte range: " + e.getMessage());
123130
}

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

Lines changed: 92 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import javax.crypto.SecretKey;
2323
import java.security.NoSuchAlgorithmException;
2424
import java.util.concurrent.CompletionException;
25-
import java.util.regex.Pattern;
2625

2726
import static org.junit.jupiter.api.Assertions.assertEquals;
2827
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -143,6 +142,83 @@ public void AsyncFailsOnRangeWhenLegacyModeDisabled() {
143142
asyncClient.close();
144143
}
145144

145+
@Test
146+
public void AsyncAesCbcV1toV3RangedGet() {
147+
final String objectKey = appendTestSuffix("aes-cbc-v1-to-v3-ranged-get-async");
148+
149+
// V1 Client
150+
EncryptionMaterialsProvider materialsProvider =
151+
new StaticEncryptionMaterialsProvider(new EncryptionMaterials(AES_KEY));
152+
CryptoConfiguration v1CryptoConfig =
153+
new CryptoConfiguration();
154+
AmazonS3Encryption v1Client = AmazonS3EncryptionClient.encryptionBuilder()
155+
.withCryptoConfiguration(v1CryptoConfig)
156+
.withEncryptionMaterials(materialsProvider)
157+
.build();
158+
159+
final String input = "0bcdefghijklmnopqrst0BCDEFGHIJKLMNOPQRST" +
160+
"1bcdefghijklmnopqrst1BCDEFGHIJKLMNOPQRST" +
161+
"2bcdefghijklmnopqrst2BCDEFGHIJKLMNOPQRST" +
162+
"3bcdefghijklmnopqrst3BCDEFGHIJKLMNOPQRST" +
163+
"4bcdefghijklmnopqrst4BCDEFGHIJKLMNOPQRST";
164+
165+
v1Client.putObject(BUCKET, objectKey, input);
166+
167+
// V3 Client
168+
S3AsyncClient v3Client = S3AsyncEncryptionClient.builder()
169+
.aesKey(AES_KEY)
170+
.enableLegacyUnauthenticatedModes(true)
171+
.build();
172+
173+
// Valid Range
174+
ResponseBytes<GetObjectResponse> objectResponse;
175+
176+
objectResponse = v3Client.getObject(builder -> builder
177+
.bucket(BUCKET)
178+
.range("bytes=10-20")
179+
.key(objectKey), AsyncResponseTransformer.toBytes()).join();
180+
String output;
181+
output = objectResponse.asUtf8String();
182+
assertEquals("klmnopqrst0", output);
183+
184+
// Valid start index within input and end index out of range, returns object from start index to End of Stream
185+
objectResponse = v3Client.getObject(builder -> builder
186+
.bucket(BUCKET)
187+
.range("bytes=190-300")
188+
.key(objectKey), AsyncResponseTransformer.toBytes()).join();
189+
output = objectResponse.asUtf8String();
190+
assertEquals("KLMNOPQRST", output);
191+
192+
// Invalid range start index range greater than ending index, returns entire object
193+
objectResponse = v3Client.getObject(builder -> builder
194+
.bucket(BUCKET)
195+
.range("bytes=100-50")
196+
.key(objectKey), AsyncResponseTransformer.toBytes()).join();
197+
output = objectResponse.asUtf8String();
198+
assertEquals(input, output);
199+
200+
// Invalid range format, returns entire object
201+
objectResponse = v3Client.getObject(builder -> builder
202+
.bucket(BUCKET)
203+
.range("10-20")
204+
.key(objectKey), AsyncResponseTransformer.toBytes()).join();
205+
output = objectResponse.asUtf8String();
206+
assertEquals(input, output);
207+
208+
// Invalid range starting index and ending index greater than object length but within Cipher Block size, returns empty object
209+
objectResponse = v3Client.getObject(builder -> builder
210+
.bucket(BUCKET)
211+
.range("bytes=216-217")
212+
.key(objectKey), AsyncResponseTransformer.toBytes()).join();
213+
output = objectResponse.asUtf8String();
214+
assertEquals("", output);
215+
216+
// Cleanup
217+
deleteObject(BUCKET, objectKey, v3Client);
218+
v3Client.close();
219+
}
220+
221+
146222
@Test
147223
public void failsOnRangeWhenLegacyModeDisabled() {
148224
final String objectKey = appendTestSuffix("fails-when-on-range-when-legacy-disabled");
@@ -314,6 +390,8 @@ public void AesCbcV1toV3RangedGet() {
314390
.withEncryptionMaterials(materialsProvider)
315391
.build();
316392

393+
// This string is 200 characters/bytes long
394+
// Due to padding, its ciphertext will be 208 bytes
317395
final String input = "0bcdefghijklmnopqrst0BCDEFGHIJKLMNOPQRST" +
318396
"1bcdefghijklmnopqrst1BCDEFGHIJKLMNOPQRST" +
319397
"2bcdefghijklmnopqrst2BCDEFGHIJKLMNOPQRST" +
@@ -360,14 +438,25 @@ public void AesCbcV1toV3RangedGet() {
360438
output = objectResponse.asUtf8String();
361439
assertEquals(input, output);
362440

363-
// Invalid range starting index and ending index greater than object length but within Cipher Block size, returns empty object
441+
// Invalid range starting index and ending index greater than object length
442+
// but within Cipher Block size, returns empty object
364443
objectResponse = v3Client.getObjectAsBytes(builder -> builder
365444
.bucket(BUCKET)
366445
.range("bytes=216-217")
367446
.key(objectKey));
368447
output = objectResponse.asUtf8String();
369448
assertEquals("", output);
370449

450+
// Invalid range starting index and ending index greater than object length
451+
// but within Cipher Block size, returns empty object
452+
objectResponse = v3Client.getObjectAsBytes(builder -> builder
453+
.bucket(BUCKET)
454+
.range("bytes=216-218")
455+
.key(objectKey));
456+
output = objectResponse.asUtf8String();
457+
assertEquals("", output);
458+
459+
371460
// Cleanup
372461
deleteObject(BUCKET, objectKey, v3Client);
373462
v3Client.close();
@@ -411,4 +500,5 @@ public void AesCbcV1toV3FailsRangeExceededObjectLength() {
411500
deleteObject(BUCKET, objectKey, v3Client);
412501
v3Client.close();
413502
}
503+
414504
}

0 commit comments

Comments
 (0)