Skip to content

Commit 41b094f

Browse files
AFaustAFaust
committed
[ACC-2372] Consolidate skip handling in CipherInputStream subclass
1 parent 4653838 commit 41b094f

File tree

8 files changed

+100
-61
lines changed

8 files changed

+100
-61
lines changed

‎contentgrid-appserver-contentstore-impl-encryption/src/main/java/com/contentgrid/appserver/contentstore/impl/encryption/engine/AesCtrEncryptionEngine.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
import com.contentgrid.appserver.contentstore.api.range.ResolvedContentRange;
77
import com.contentgrid.appserver.contentstore.impl.encryption.UndecryptableContentException;
88
import com.contentgrid.appserver.contentstore.impl.encryption.keys.KeyBytes;
9+
import com.contentgrid.appserver.contentstore.impl.utils.SkippableCipherInputStream;
910
import com.contentgrid.appserver.contentstore.impl.utils.SkippingInputStream;
1011
import com.contentgrid.appserver.contentstore.impl.utils.ZeroPrefixedInputStream;
12+
1113
import java.io.InputStream;
1214
import java.math.BigInteger;
1315
import java.security.InvalidAlgorithmParameterException;
@@ -18,10 +20,12 @@
1820
import java.util.List;
1921
import java.util.Objects;
2022
import java.util.stream.Collectors;
23+
2124
import javax.crypto.Cipher;
2225
import javax.crypto.CipherInputStream;
2326
import javax.crypto.NoSuchPaddingException;
2427
import javax.crypto.spec.IvParameterSpec;
28+
2529
import lombok.RequiredArgsConstructor;
2630
import lombok.SneakyThrows;
2731

@@ -172,18 +176,17 @@ private static class DecryptingContentReader implements ContentReader {
172176
@Override
173177
public InputStream getContentInputStream() throws UnreadableContentException {
174178
return new ZeroPrefixedInputStream(
175-
new CipherInputStream(
179+
// CipherInputStream does not skip(n) into not-yet-decrypted data
180+
// Spring StreamUtils.copyRange needs that behaviour
181+
// so we use our SkippableCipherInputStream
182+
new SkippableCipherInputStream(
176183
new SkippingInputStream(
177184
delegate.getContentInputStream(),
178185
byteStartOffset
179186
),
180187
cipher
181188
),
182-
byteStartOffset,
183-
// CipherInputStream does not skip(n) into not-yet-decrypted data
184-
// Spring StreamUtils.copyRange needs that behaviour
185-
// so we have to tell prefixed input stream to use skipNBytes
186-
true
189+
byteStartOffset
187190
);
188191
}
189192

‎contentgrid-appserver-contentstore-impl-encryption/src/main/java/com/contentgrid/appserver/contentstore/impl/encryption/engine/AlfrescoCompatibleEncryptionEngine.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import com.contentgrid.appserver.contentstore.api.UnreadableContentException;
66
import com.contentgrid.appserver.contentstore.api.range.ResolvedContentRange;
77
import com.contentgrid.appserver.contentstore.impl.encryption.UndecryptableContentException;
8-
import com.contentgrid.appserver.contentstore.impl.utils.SkippingInputStream;
8+
import com.contentgrid.appserver.contentstore.impl.utils.SkippableCipherInputStream;
99

1010
import java.io.InputStream;
1111
import java.security.InvalidAlgorithmParameterException;
@@ -15,7 +15,6 @@
1515
import java.util.Map;
1616

1717
import javax.crypto.Cipher;
18-
import javax.crypto.CipherInputStream;
1918
import javax.crypto.KeyGenerator;
2019
import javax.crypto.NoSuchPaddingException;
2120
import javax.crypto.spec.IvParameterSpec;
@@ -144,8 +143,8 @@ public InputStream getContentInputStream() throws UnreadableContentException {
144143
InputStream raw = delegate.getContentInputStream();
145144
// CipherInputStream does not skip(n) into not-yet-decrypted data
146145
// Spring StreamUtils.copyRange needs that behaviour
147-
// so we have to tell prefixed input stream to use skipNBytes
148-
return new SkippingInputStream(new CipherInputStream(raw, cipher), 0, true);
146+
// so we use our SkippableCipherInputStream
147+
return new SkippableCipherInputStream(raw, cipher);
149148
}
150149

151150
@Override

‎contentgrid-appserver-contentstore-impl-utils/src/main/java/com/contentgrid/appserver/contentstore/impl/utils/SkippableCipherInputStream.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package com.contentgrid.appserver.contentstore.impl.utils;
2+
3+
import java.io.IOException;
4+
import java.io.InputStream;
5+
6+
import javax.crypto.Cipher;
7+
import javax.crypto.CipherInputStream;
8+
9+
public class SkippableCipherInputStream extends CipherInputStream
10+
{
11+
12+
public SkippableCipherInputStream(InputStream is, Cipher c)
13+
{
14+
super(is, c);
15+
}
16+
17+
/**
18+
* {@inheritDoc}
19+
*/
20+
@Override
21+
public long skip(long n) throws IOException
22+
{
23+
var skipped = super.skip(n);
24+
var remaining = n - skipped;
25+
var eof = false;
26+
while (!eof && remaining > 0) {
27+
if (read() == -1) {
28+
eof = true;
29+
} else {
30+
skipped += 1;
31+
remaining = n - skipped;
32+
}
33+
if (!eof && remaining > 0) {
34+
skipped += super.skip(remaining);
35+
remaining = n - skipped;
36+
}
37+
}
38+
39+
return skipped;
40+
}
41+
42+
43+
}

‎contentgrid-appserver-contentstore-impl-utils/src/main/java/com/contentgrid/appserver/contentstore/impl/utils/SkippingInputStream.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,8 @@ public class SkippingInputStream extends InputStream {
1313
@NonNull
1414
private final InputStream delegate;
1515
private final long skipBytes;
16-
// some delegates' skip may not skip into unread data
17-
// allow case-by-case option to use skipNBytes on delegate
18-
private boolean useDelegateSkipN = false;
1916
private boolean hasSkipped = false;
2017

21-
public SkippingInputStream(@NonNull InputStream delegate, long skipBytes, boolean useDelegateSkipN)
22-
{
23-
this(delegate, skipBytes);
24-
this.useDelegateSkipN = useDelegateSkipN;
25-
}
26-
2718
private void ensureSkipped() throws IOException {
2819
if(!hasSkipped) {
2920
delegate.skipNBytes(skipBytes);
@@ -37,10 +28,6 @@ public long skip(long n) throws IOException {
3728
return 0;
3829
}
3930
ensureSkipped();
40-
if (useDelegateSkipN) {
41-
delegate.skipNBytes(n);
42-
return n;
43-
}
4431
return delegate.skip(n);
4532
}
4633

‎contentgrid-appserver-contentstore-impl-utils/src/main/java/com/contentgrid/appserver/contentstore/impl/utils/ZeroPrefixedInputStream.java‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,11 @@
88
*/
99
public class ZeroPrefixedInputStream extends InputStream {
1010
private final InputStream delegate;
11-
// some delegates' skip may not skip into unread data
12-
// allow case-by-case option to use skipNBytes on delegate
13-
private final boolean useDelegateSkipN;
1411
private long prefixBytes;
1512

1613
public ZeroPrefixedInputStream(InputStream delegate, long prefixBytes) {
1714
this.delegate = delegate;
1815
this.prefixBytes = prefixBytes;
19-
this.useDelegateSkipN = false;
20-
}
21-
22-
public ZeroPrefixedInputStream(InputStream delegate, long prefixBytes, boolean useDelegateSkipN) {
23-
this.delegate = delegate;
24-
this.prefixBytes = prefixBytes;
25-
this.useDelegateSkipN = useDelegateSkipN;
2616
}
2717

2818
@Override
@@ -37,20 +27,12 @@ public long skip(long n) throws IOException {
3727
if(prefixBytes > 0) {
3828
n = n - prefixBytes; // Still skipping so many bytes from the offset
3929
try {
40-
if (useDelegateSkipN) {
41-
delegate.skipNBytes(n);
42-
return prefixBytes + n;
43-
}
4430
return prefixBytes + delegate.skip(n);
4531
} finally {
4632
prefixBytes = 0; // Now the whole offset is consumed; skip to the delegate
4733
}
4834
}
4935

50-
if (useDelegateSkipN) {
51-
delegate.skipNBytes(n);
52-
return n;
53-
}
5436
return delegate.skip(n);
5537
}
5638

‎contentgrid-appserver-contentstore-impl-utils/src/test/java/com/contentgrid/appserver/contentstore/impl/utils/AbstractDelegatingInputStreamTest.java‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,35 +21,36 @@ abstract class AbstractDelegatingInputStreamTest<T extends InputStream>
2121

2222
@Test
2323
void skipNegative() throws IOException {
24-
try (InputStream is = wrapDelegate(new ByteArrayInputStream(FULL_DATA), false)) {
24+
try (InputStream is = wrapDelegate(new ByteArrayInputStream(FULL_DATA))) {
2525
assertEquals(0, is.skip(-100));
2626
}
2727
}
2828

2929
@Test
30-
void skipNWithUnskippingDelegate() throws Exception {
30+
void skipWithUnskippingDelegate() throws Exception {
3131
// if no data was read yet (decrypted), the underlying cipher stream can not skip
32-
try (var sis = wrapDelegate(getUnskippingInputStream(), false)) {
32+
try (var sis = wrapDelegate(getUnskippableInputStream())) {
3333
assertEquals(0, sis.skip(5));
3434
}
3535

3636
// if some data was read, the underlying cipher stream can only skip up to the block size
3737
// AES block size is 16 byte, test data is longer
38-
try (var sis = wrapDelegate(getUnskippingInputStream(), false)) {
38+
try (var sis = wrapDelegate(getUnskippableInputStream())) {
3939
assertEquals(FULL_DATA[0], sis.read());
4040
assertEquals(5, sis.skip(5));
4141
assertEquals(10, sis.skip(12));
4242
}
43+
}
4344

44-
// with us forcing skipNBytes on the delegate, everything should be fine
45-
// this is only safe unless trying to skip beyond the limit, due to implicit read
46-
try (var sis = wrapDelegate(getUnskippingInputStream(), true)) {
45+
@Test
46+
void skipWithSkippingDelegate() throws Exception {
47+
try (var sis = wrapDelegate(getProperSkippableInputStream())) {
4748
assertEquals(5, sis.skip(5));
4849
assertEquals(15, sis.skip(15));
4950
}
5051
}
5152

52-
protected InputStream getUnskippingInputStream() throws Exception {
53+
protected InputStream getUnskippableInputStream() throws Exception {
5354
var bos = new ByteArrayOutputStream();
5455
var keygen = KeyGenerator.getInstance("AES");
5556
var key = keygen.generateKey();
@@ -67,5 +68,23 @@ protected InputStream getUnskippingInputStream() throws Exception {
6768
return new CipherInputStream(bis, cipher);
6869
}
6970

70-
protected abstract T wrapDelegate(InputStream is, boolean useDelegateSkipN);
71+
protected InputStream getProperSkippableInputStream() throws Exception {
72+
var bos = new ByteArrayOutputStream();
73+
var keygen = KeyGenerator.getInstance("AES");
74+
var key = keygen.generateKey();
75+
var cipher = Cipher.getInstance("AES");
76+
cipher.init(Cipher.ENCRYPT_MODE, key);
77+
78+
try (var cos = new CipherOutputStream(bos, cipher)) {
79+
cos.write(FULL_DATA);
80+
}
81+
82+
var bis = new ByteArrayInputStream(bos.toByteArray());
83+
cipher = Cipher.getInstance("AES");
84+
cipher.init(Cipher.DECRYPT_MODE, key);
85+
86+
return new SkippableCipherInputStream(bis, cipher);
87+
}
88+
89+
protected abstract T wrapDelegate(InputStream is);
7190
}

‎contentgrid-appserver-contentstore-impl-utils/src/test/java/com/contentgrid/appserver/contentstore/impl/utils/SkippingInputStreamTest.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,25 +41,28 @@ void onlySkipsOnce() throws IOException {
4141
}
4242

4343
@Test
44-
void skipNWithUnskippingDelegateAndPrefixSkip() throws Exception {
44+
void skipWithUnskippingDelegateAndPrefixSkip() throws Exception {
4545
// if some data was read, the underlying cipher stream can only skip up to the block size
4646
// AES block size is 16 byte, test data is longer
4747
// SkippingInputStream#ensureSkipped uses skipNBytes to read during initial skip
48-
try (var sis = new SkippingInputStream(getUnskippingInputStream(), 5, false)) {
48+
try (var sis = new SkippingInputStream(getUnskippableInputStream(), 5)) {
4949
assertEquals(5, sis.skip(5));
5050
// reaching 16 byte block size and not going beyond
5151
assertEquals(6, sis.skip(10));
5252
}
53+
}
5354

55+
@Test
56+
void skipWithSkippingDelegateAndPrefixSkip() throws Exception {
5457
// with us forcing skipNBytes on the delegate, everything should be fine
5558
// this is only safe unless trying to skip beyond the limit, due to implicit read
56-
try (var sis = new SkippingInputStream(getUnskippingInputStream(), 5, true)) {
59+
try (var sis = new SkippingInputStream(getProperSkippableInputStream(), 5)) {
5760
assertEquals(5, sis.skip(5));
5861
assertEquals(10, sis.skip(10));
5962
}
6063
}
6164

62-
protected SkippingInputStream wrapDelegate(InputStream is, boolean useDelegateSkipN) {
63-
return new SkippingInputStream(is, 0, useDelegateSkipN);
65+
protected SkippingInputStream wrapDelegate(InputStream is) {
66+
return new SkippingInputStream(is, 0);
6467
}
6568
}

‎contentgrid-appserver-contentstore-impl-utils/src/test/java/com/contentgrid/appserver/contentstore/impl/utils/ZeroPrefixedInputStreamTest.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,34 +65,37 @@ void skipsZeroBytesAndMore() throws IOException {
6565
}
6666

6767
@Test
68-
void skipNWithUnskippingDelegateAndPrefixSkip() throws Exception {
68+
void skipWithUnskippingDelegateAndPrefixSkip() throws Exception {
6969
// if no data was read yet (decrypted), the underlying cipher stream can not skip
7070
// skip is limited to the zero prefix
71-
try (var sis = new ZeroPrefixedInputStream(getUnskippingInputStream(), 5, false)) {
71+
try (var sis = new ZeroPrefixedInputStream(getUnskippableInputStream(), 5)) {
7272
assertEquals(5, sis.skip(10));
7373
}
7474

7575
// if some data was read, the underlying cipher stream can only skip up to the block size
7676
// AES block size is 16 byte, test data is longer
77-
try (var sis = new ZeroPrefixedInputStream(getUnskippingInputStream(), 5, false)) {
77+
try (var sis = new ZeroPrefixedInputStream(getUnskippableInputStream(), 5)) {
7878
// can only skip as much as zero prefix, but not into actual content
7979
assertEquals(5, sis.skip(10));
8080
assertEquals(FULL_DATA[0], sis.read());
8181
// can skip until end of decrypted block
8282
assertEquals(15, sis.skip(20));
8383
}
84+
}
8485

86+
@Test
87+
void skipWithSkippingDelegateAndPrefixSkip() throws Exception {
8588
// with us forcing skipNBytes on the delegate, everything should be fine
8689
// this is only safe unless trying to skip beyond the limit, due to implicit read
87-
try (var sis = new ZeroPrefixedInputStream(getUnskippingInputStream(), 5, true)) {
90+
try (var sis = new ZeroPrefixedInputStream(getProperSkippableInputStream(), 5)) {
8891
assertEquals(3, sis.skip(3));
8992
// this skips prefix and some real data
9093
assertEquals(3, sis.skip(3));
9194
assertEquals(10, sis.skip(10));
9295
}
9396
}
9497

95-
protected ZeroPrefixedInputStream wrapDelegate(InputStream is, boolean useDelegateSkipN) {
96-
return new ZeroPrefixedInputStream(is, 0, useDelegateSkipN);
98+
protected ZeroPrefixedInputStream wrapDelegate(InputStream is) {
99+
return new ZeroPrefixedInputStream(is, 0);
97100
}
98101
}

0 commit comments

Comments
 (0)