Skip to content

Commit b315d4d

Browse files
tjcelayatjcelaya
committed
EncryptingEntity and EncryptedPartEntity should not close writeTo(OutputStream) (#333)
Stop closing supplied output streams in encrypting operations, it causes chunked requests to complete successfully in spite of exceptions being thrown. This resolves #332
1 parent babc7b4 commit b315d4d

File tree

4 files changed

+194
-73
lines changed

4 files changed

+194
-73
lines changed

‎java-manta-client/src/main/java/com/joyent/manta/client/crypto/EncryptingEntity.java‎

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -154,29 +154,26 @@ public void writeTo(final OutputStream httpOut) throws IOException {
154154

155155
OutputStream out = EncryptingEntityHelper.makeCipherOutputForStream(
156156
httpOut, encryptionContext);
157-
try {
158-
copyContentToOutputStream(out);
159-
/* We don't close quietly because we want the operation to fail if
160-
* there is an error closing out the CipherOutputStream. */
161-
out.close();
162-
163-
if (out instanceof HmacOutputStream) {
164-
final HMac hmac = ((HmacOutputStream) out).getHmac();
165-
final int hmacSize = hmac.getMacSize();
166-
final byte[] hmacBytes = new byte[hmacSize];
167-
hmac.doFinal(hmacBytes, 0);
168-
169-
Validate.isTrue(hmacBytes.length == hmacSize,
170-
"HMAC actual bytes doesn't equal the number of bytes expected");
171-
172-
if (LOGGER.isDebugEnabled()) {
173-
LOGGER.debug("HMAC: {}", Hex.encodeHexString(hmacBytes));
174-
}
175-
176-
httpOut.write(hmacBytes);
157+
158+
copyContentToOutputStream(out);
159+
/* We don't close quietly because we want the operation to fail if
160+
* there is an error closing out the CipherOutputStream. */
161+
out.close();
162+
163+
if (out instanceof HmacOutputStream) {
164+
final HMac hmac = ((HmacOutputStream) out).getHmac();
165+
final int hmacSize = hmac.getMacSize();
166+
final byte[] hmacBytes = new byte[hmacSize];
167+
hmac.doFinal(hmacBytes, 0);
168+
169+
Validate.isTrue(hmacBytes.length == hmacSize,
170+
"HMAC actual bytes doesn't equal the number of bytes expected");
171+
172+
if (LOGGER.isTraceEnabled()) {
173+
LOGGER.trace("HMAC: {}", Hex.encodeHexString(hmacBytes));
177174
}
178-
} finally {
179-
IOUtils.closeQuietly(httpOut);
175+
176+
httpOut.write(hmacBytes);
180177
}
181178
}
182179

‎java-manta-client/src/main/java/com/joyent/manta/client/crypto/EncryptingPartEntity.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ public void writeTo(final OutputStream httpOut) throws IOException {
140140
}
141141
}
142142
} finally {
143-
IOUtils.closeQuietly(httpOut);
144143
IOUtils.closeQuietly(contentStream);
145144
}
146145
}

‎java-manta-client/src/test/java/com/joyent/manta/client/crypto/EncryptingEntityTest.java‎

Lines changed: 87 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import com.joyent.manta.client.MantaObjectInputStream;
1111
import com.joyent.manta.client.MantaObjectResponse;
12+
import com.joyent.manta.config.DefaultsConfigContext;
1213
import com.joyent.manta.exception.MantaClientEncryptionException;
1314
import com.joyent.manta.http.MantaHttpHeaders;
1415
import com.joyent.manta.http.entity.ExposedStringEntity;
@@ -17,11 +18,13 @@
1718
import org.apache.commons.io.FileUtils;
1819
import org.apache.commons.io.IOUtils;
1920
import org.apache.commons.io.input.BoundedInputStream;
21+
import org.apache.commons.io.input.BrokenInputStream;
2022
import org.apache.commons.lang3.RandomStringUtils;
2123
import org.apache.commons.lang3.RandomUtils;
2224
import org.apache.http.HttpEntity;
2325
import org.apache.http.client.methods.CloseableHttpResponse;
2426
import org.apache.http.conn.EofSensorInputStream;
27+
import org.apache.http.entity.InputStreamEntity;
2528
import org.bouncycastle.jcajce.io.CipherInputStream;
2629
import org.mockito.Mockito;
2730
import org.testng.Assert;
@@ -34,6 +37,7 @@
3437
import java.io.FileInputStream;
3538
import java.io.FileOutputStream;
3639
import java.io.IOException;
40+
import java.io.OutputStream;
3741
import java.net.URL;
3842
import java.nio.charset.Charset;
3943
import java.nio.charset.StandardCharsets;
@@ -116,6 +120,89 @@ public void canSurviveNetworkFailuresInAesCbc() throws Exception {
116120
canBeWrittenIdempotently(AesCbcCipherDetails.INSTANCE_128_BIT);
117121
}
118122

123+
/* Cipher-generic tests */
124+
125+
private void canBeWrittenIdempotently(final SupportedCipherDetails cipherDetails) throws Exception {
126+
final SecretKey secretKey = SecretKeyUtils.generate(cipherDetails);
127+
final String content = RandomStringUtils.randomAlphanumeric(RandomUtils.nextInt(500, 1500));
128+
final ExposedStringEntity contentEntity = new ExposedStringEntity(
129+
content,
130+
StandardCharsets.UTF_8);
131+
132+
final ByteArrayOutputStream referenceEncrypted = new ByteArrayOutputStream(content.length());
133+
{
134+
135+
final EncryptingEntity referenceEntity = new EncryptingEntity(
136+
secretKey,
137+
cipherDetails,
138+
contentEntity);
139+
140+
referenceEntity.writeTo(referenceEncrypted);
141+
validateCiphertext(
142+
cipherDetails,
143+
secretKey,
144+
contentEntity.getBackingBuffer().array(),
145+
referenceEntity.getCipher().getIV(),
146+
referenceEncrypted.toByteArray());
147+
}
148+
149+
final ByteArrayOutputStream retryEncrypted = new ByteArrayOutputStream(content.length());
150+
final FailingOutputStream output = new FailingOutputStream(retryEncrypted, content.length() / 2);
151+
{
152+
final EncryptingEntity retryingEntity = new EncryptingEntity(
153+
secretKey,
154+
cipherDetails,
155+
contentEntity);
156+
157+
Assert.assertThrows(IOException.class, () -> {
158+
retryingEntity.writeTo(output);
159+
});
160+
161+
// clear the data written so we only see what makes it into the second request
162+
retryEncrypted.reset();
163+
output.setMinimumBytes(FailingOutputStream.NO_FAILURE);
164+
165+
retryingEntity.writeTo(output);
166+
validateCiphertext(
167+
cipherDetails,
168+
secretKey,
169+
contentEntity.getBackingBuffer().array(),
170+
retryingEntity.getCipher().getIV(),
171+
retryEncrypted.toByteArray());
172+
}
173+
}
174+
175+
public void doesNotCloseSuppliedOutputStreamWhenWrittenSuccessfully() throws Exception {
176+
final SupportedCipherDetails cipherDetails = DefaultsConfigContext.DEFAULT_CIPHER;
177+
final SecretKey secretKey = SecretKeyUtils.generate(cipherDetails);
178+
final String content = RandomStringUtils.randomAlphanumeric(RandomUtils.nextInt(500, 1500));
179+
final ExposedStringEntity contentEntity = new ExposedStringEntity(
180+
content,
181+
StandardCharsets.UTF_8);
182+
183+
final EncryptingEntity encryptingEntity = new EncryptingEntity(
184+
secretKey,
185+
cipherDetails,
186+
contentEntity);
187+
188+
final OutputStream output = Mockito.mock(OutputStream.class);
189+
encryptingEntity.writeTo(output);
190+
Mockito.verify(output, Mockito.never()).close();
191+
}
192+
193+
public void doesNotCloseSuppliedOutputStreamWhenFailureOccurs() throws Exception {
194+
final SupportedCipherDetails cipherDetails = DefaultsConfigContext.DEFAULT_CIPHER;
195+
final SecretKey secretKey = SecretKeyUtils.generate(cipherDetails);
196+
final EncryptingEntity encryptingEntity = new EncryptingEntity(
197+
secretKey,
198+
cipherDetails,
199+
new InputStreamEntity(new BrokenInputStream(new IOException("bad input"))));
200+
201+
final OutputStream output = Mockito.mock(OutputStream.class);
202+
Assert.assertThrows(IOException.class, () -> encryptingEntity.writeTo(output));
203+
Mockito.verify(output, Mockito.never()).close();
204+
}
205+
119206
/* Test helper methods */
120207

121208
private void canCountBytesFromStreamWithUnknownLength(SupportedCipherDetails cipherDetails)
@@ -228,56 +315,6 @@ private static void verifyEncryptionWorksRoundTrip(byte[] keyBytes,
228315
}
229316
}
230317

231-
private void canBeWrittenIdempotently(final SupportedCipherDetails cipherDetails) throws Exception {
232-
final SecretKey secretKey = SecretKeyUtils.generate(cipherDetails);
233-
final String content = RandomStringUtils.randomAlphanumeric(RandomUtils.nextInt(500, 1500));
234-
final ExposedStringEntity contentEntity = new ExposedStringEntity(
235-
content,
236-
StandardCharsets.UTF_8);
237-
238-
final ByteArrayOutputStream referenceEncrypted = new ByteArrayOutputStream(content.length());
239-
{
240-
241-
final EncryptingEntity referenceEntity = new EncryptingEntity(
242-
secretKey,
243-
cipherDetails,
244-
contentEntity);
245-
246-
referenceEntity.writeTo(referenceEncrypted);
247-
validateCiphertext(
248-
cipherDetails,
249-
secretKey,
250-
contentEntity.getBackingBuffer().array(),
251-
referenceEntity.getCipher().getIV(),
252-
referenceEncrypted.toByteArray());
253-
}
254-
255-
final ByteArrayOutputStream retryEncrypted = new ByteArrayOutputStream(content.length());
256-
final FailingOutputStream output = new FailingOutputStream(retryEncrypted, content.length() / 2);
257-
{
258-
final EncryptingEntity retryingEntity = new EncryptingEntity(
259-
secretKey,
260-
cipherDetails,
261-
contentEntity);
262-
263-
Assert.assertThrows(IOException.class, () -> {
264-
retryingEntity.writeTo(output);
265-
});
266-
267-
// clear the data written so we only see what makes it into the second request
268-
retryEncrypted.reset();
269-
output.setMinimumBytes(FailingOutputStream.NO_FAILURE);
270-
271-
retryingEntity.writeTo(output);
272-
validateCiphertext(
273-
cipherDetails,
274-
secretKey,
275-
contentEntity.getBackingBuffer().array(),
276-
retryingEntity.getCipher().getIV(),
277-
retryEncrypted.toByteArray());
278-
}
279-
}
280-
281318
private void validateCiphertext(SupportedCipherDetails cipherDetails,
282319
SecretKey secretKey,
283320
byte[] plaintext,

‎java-manta-client/src/test/java/com/joyent/manta/client/multipart/EncryptingPartEntityTest.java‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Copyright (c) 2017, Joyent, Inc. All rights reserved.
3+
*
4+
* This Source Code Form is subject to the terms of the Mozilla Public
5+
* License, v. 2.0. If a copy of the MPL was not distributed with this
6+
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
7+
*/
8+
package com.joyent.manta.client.multipart;
9+
10+
import com.joyent.manta.client.crypto.EncryptingEntityHelper;
11+
import com.joyent.manta.client.crypto.EncryptingPartEntity;
12+
import com.joyent.manta.client.crypto.EncryptionContext;
13+
import com.joyent.manta.client.crypto.SecretKeyUtils;
14+
import com.joyent.manta.client.crypto.SupportedCipherDetails;
15+
import com.joyent.manta.config.DefaultsConfigContext;
16+
import com.joyent.manta.http.entity.ExposedStringEntity;
17+
import org.apache.commons.io.input.BrokenInputStream;
18+
import org.apache.commons.lang3.RandomStringUtils;
19+
import org.apache.commons.lang3.RandomUtils;
20+
import org.apache.http.entity.InputStreamEntity;
21+
import org.mockito.Mockito;
22+
import org.testng.Assert;
23+
import org.testng.annotations.BeforeMethod;
24+
import org.testng.annotations.Test;
25+
26+
import java.io.ByteArrayOutputStream;
27+
import java.io.IOException;
28+
import java.io.OutputStream;
29+
import java.nio.charset.StandardCharsets;
30+
31+
@Test
32+
public class EncryptingPartEntityTest {
33+
34+
private EncryptionState state;
35+
36+
private static final ByteArrayOutputStream BYTE_ARRAY_OUTPUT_STREAM_EMPTY = new ByteArrayOutputStream(0);
37+
38+
private static final EncryptingPartEntity.LastPartCallback CALLBACK_NOOP =
39+
new EncryptingPartEntity.LastPartCallback() {
40+
@Override
41+
public ByteArrayOutputStream call(final long uploadedBytes) throws IOException {
42+
return BYTE_ARRAY_OUTPUT_STREAM_EMPTY;
43+
}
44+
};
45+
46+
@BeforeMethod
47+
public void setup() throws Exception {
48+
final SupportedCipherDetails cipherDetails = DefaultsConfigContext.DEFAULT_CIPHER;
49+
state = new EncryptionState(new EncryptionContext(SecretKeyUtils.generate(cipherDetails), cipherDetails));
50+
51+
state.setMultipartStream(
52+
new MultipartOutputStream(
53+
state.getEncryptionContext().getCipherDetails().getBlockSizeInBytes()));
54+
state.setCipherStream(
55+
EncryptingEntityHelper.makeCipherOutputForStream(
56+
state.getMultipartStream(),
57+
state.getEncryptionContext()));
58+
}
59+
60+
public void doesNotCloseSuppliedOutputStreamWhenFailureOccurs() throws Exception {
61+
final ExposedStringEntity contentEntity = new ExposedStringEntity(
62+
RandomStringUtils.randomAlphanumeric(RandomUtils.nextInt(500, 1500)),
63+
StandardCharsets.UTF_8);
64+
65+
final EncryptingPartEntity encryptingPartEntity = new EncryptingPartEntity(
66+
state.getCipherStream(),
67+
state.getMultipartStream(),
68+
contentEntity,
69+
CALLBACK_NOOP);
70+
71+
final OutputStream output = Mockito.mock(OutputStream.class);
72+
encryptingPartEntity.writeTo(output);
73+
Mockito.verify(output, Mockito.never()).close();
74+
}
75+
76+
public void doesNotCloseSuppliedOutputStreamWhenWrittenSuccessfully() throws Exception {
77+
final EncryptingPartEntity encryptingPartEntity = new EncryptingPartEntity(
78+
state.getCipherStream(),
79+
state.getMultipartStream(),
80+
new InputStreamEntity(new BrokenInputStream(new IOException("bad input"))),
81+
CALLBACK_NOOP);
82+
83+
final OutputStream output = Mockito.mock(OutputStream.class);
84+
Assert.assertThrows(IOException.class, () -> encryptingPartEntity.writeTo(output));
85+
Mockito.verify(output, Mockito.never()).close();
86+
}
87+
88+
}

0 commit comments

Comments
 (0)