This is an automated email from the ASF dual-hosted git repository.
bruno-roustant pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr-sandbox.git
The following commit(s) were added to refs/heads/main by this push:
new a72cc01 Fix invisible flaw in DecryptingIndexInput.seek() and extract
DecryptionBuffer (#131)
a72cc01 is described below
commit a72cc014bf14cc965515cdbd08dee183a39bb297
Author: Antoine Gruzelle <[email protected]>
AuthorDate: Fri May 22 15:21:27 2026 +0200
Fix invisible flaw in DecryptingIndexInput.seek() and extract
DecryptionBuffer (#131)
---
.../encryption/crypto/DecryptingIndexInput.java | 206 ++++++++++++++-------
.../crypto/DecryptingIndexInputTest.java | 37 ++++
2 files changed, 173 insertions(+), 70 deletions(-)
diff --git
a/encryption/src/main/java/org/apache/solr/encryption/crypto/DecryptingIndexInput.java
b/encryption/src/main/java/org/apache/solr/encryption/crypto/DecryptingIndexInput.java
index a459330..da1f3fa 100644
---
a/encryption/src/main/java/org/apache/solr/encryption/crypto/DecryptingIndexInput.java
+++
b/encryption/src/main/java/org/apache/solr/encryption/crypto/DecryptingIndexInput.java
@@ -51,14 +51,8 @@ public class DecryptingIndexInput extends FilterIndexInput {
private final long sliceOffset;
private final long sliceEnd;
private IndexInput indexInput;
- private AesCtrEncrypter encrypter;
- private byte[] inBuffer;
- private byte[] outBuffer;
+ private DecryptionBuffer buffer;
private byte[] oneByteBuf;
- private int inPos;
- private int outPos;
- private int outSize;
- private int padding;
private boolean closed;
/**
@@ -111,11 +105,7 @@ public class DecryptingIndexInput extends FilterIndexInput
{
this.sliceEnd = sliceOffset + sliceLength;
this.isClone = isClone;
this.indexInput = indexInput;
- this.encrypter = encrypter;
- encrypter.init(0);
- assert bufferCapacity % AES_BLOCK_SIZE == 0;
- inBuffer = new byte[bufferCapacity];
- outBuffer = new byte[bufferCapacity + AES_BLOCK_SIZE];
+ this.buffer = new DecryptionBuffer(encrypter, bufferCapacity);
oneByteBuf = new byte[1];
}
@@ -150,7 +140,7 @@ public class DecryptingIndexInput extends FilterIndexInput {
* Gets the current internal position in the delegate {@link IndexInput}. It
includes IV length.
*/
private long getPosition() {
- return indexInput.getFilePointer() - (outSize - outPos);
+ return indexInput.getFilePointer() - buffer.bufferedAhead();
}
@Override
@@ -163,27 +153,17 @@ public class DecryptingIndexInput extends
FilterIndexInput {
}
long targetPosition = position + sliceOffset;
long delegatePosition = indexInput.getFilePointer();
- long currentPosition = delegatePosition - (outSize - outPos);
- if (targetPosition >= currentPosition && targetPosition <=
delegatePosition) {
+ long currentPosition = delegatePosition - buffer.bufferedAhead();
+ if (buffer.hasData() && targetPosition >= currentPosition &&
targetPosition <= delegatePosition) {
// The target position is within the buffered output. Just move the
output buffer position.
- outPos += (int) (targetPosition - currentPosition);
- assert targetPosition == delegatePosition - (outSize - outPos);
+ buffer.skipBytes((int) (targetPosition - currentPosition));
+ assert targetPosition == delegatePosition - buffer.bufferedAhead();
} else {
indexInput.seek(targetPosition);
- setPosition(targetPosition);
+ buffer.seek(targetPosition, delegateOffset);
}
}
- private void setPosition(long position) {
- outPos = outSize = 0;
- // Compute the counter by ignoring the IV and the delegate offset, if any.
- long delegatePosition = position - delegateOffset;
- long counter = delegatePosition / AES_BLOCK_SIZE;
- encrypter.init(counter);
- padding = (int) (delegatePosition & AES_BLOCK_SIZE_MOD_MASK);
- inPos = padding;
- }
-
/**
* Returns the number of encrypted/decrypted bytes in the file.
* <p>It is the logical length of the file, not the physical length. It
excludes the IV added artificially to manage
@@ -203,7 +183,7 @@ public class DecryptingIndexInput extends FilterIndexInput {
}
DecryptingIndexInput slice = new DecryptingIndexInput(
getFullSliceDescription(sliceDescription), delegateOffset, sliceOffset +
offset, length, true,
- indexInput.clone(), encrypter.clone(), inBuffer.length);
+ indexInput.clone(), buffer.cloneEncrypter(), buffer.capacity());
slice.seek(0);
return slice;
}
@@ -224,42 +204,7 @@ public class DecryptingIndexInput extends FilterIndexInput
{
throw new EOFException("Read beyond EOF (position=" + (getPosition() -
sliceOffset) + ", arrayLength=" + length
+ ", fileLength=" + length() + ") in " + this);
}
- while (length > 0) {
- // Transfer decrypted bytes from outBuffer.
- int outRemaining = outSize - outPos;
- if (outRemaining > 0) {
- if (length <= outRemaining) {
- System.arraycopy(outBuffer, outPos, b, offset, length);
- outPos += length;
- return;
- }
- System.arraycopy(outBuffer, outPos, b, offset, outRemaining);
- outPos += outRemaining;
- offset += outRemaining;
- length -= outRemaining;
- }
- readToFillBuffer(length);
- decryptBuffer();
- }
- }
-
- private void readToFillBuffer(int length) throws IOException {
- assert length > 0;
- int inRemaining = inBuffer.length - inPos;
- if (inRemaining > 0) {
- int numBytesToRead = Math.min(inRemaining, length);
- indexInput.readBytes(inBuffer, inPos, numBytesToRead);
- inPos += numBytesToRead;
- }
- }
-
- private void decryptBuffer() {
- assert inPos > padding : "inPos=" + inPos + " padding=" + padding;
- encrypter.process(inBuffer, 0, inPos, outBuffer, 0);
- outSize = inPos;
- inPos = 0;
- outPos = padding;
- padding = 0;
+ buffer.readDecrypted(indexInput, b, offset, length);
}
@Override
@@ -268,12 +213,133 @@ public class DecryptingIndexInput extends
FilterIndexInput {
clone.isClone = true;
clone.indexInput = indexInput.clone();
assert clone.indexInput.getFilePointer() == indexInput.getFilePointer();
- clone.encrypter = encrypter.clone();
- clone.inBuffer = new byte[inBuffer.length];
- clone.outBuffer = new byte[outBuffer.length];
+ clone.buffer = buffer.clone();
clone.oneByteBuf = new byte[1];
// The clone must be initialized.
- clone.setPosition(getPosition());
+ clone.buffer.seek(getPosition(), delegateOffset);
return clone;
}
-}
\ No newline at end of file
+
+ /**
+ * Manages the AES-CTR decryption buffers and encrypter state.
+ * <p>Reads encrypted bytes from a delegate {@link IndexInput} into an input
buffer,
+ * decrypts them into an output buffer, and serves decrypted bytes to the
caller.
+ */
+ private static class DecryptionBuffer implements Cloneable {
+
+ AesCtrEncrypter encrypter;
+ byte[] inBuffer;
+ byte[] outBuffer;
+ int inPos;
+ int outPos;
+ int outSize;
+ int padding;
+
+ DecryptionBuffer(AesCtrEncrypter encrypter, int bufferCapacity) {
+ assert bufferCapacity % AES_BLOCK_SIZE == 0;
+ this.encrypter = encrypter;
+ encrypter.init(0);
+ inBuffer = new byte[bufferCapacity];
+ outBuffer = new byte[bufferCapacity + AES_BLOCK_SIZE];
+ }
+
+ /** Whether there are decrypted bytes available in the output buffer. */
+ boolean hasData() {
+ return outSize > 0;
+ }
+
+ /** Number of decrypted bytes in the output buffer not yet consumed. */
+ int bufferedAhead() {
+ return outSize - outPos;
+ }
+
+ /** The input buffer capacity (used when creating slices with the same
buffer size). */
+ int capacity() {
+ return inBuffer.length;
+ }
+
+ /**
+ * Advances the read position within the already-decrypted output buffer.
+ * The caller must ensure the target falls within the buffered range.
+ */
+ void skipBytes(int bytesCount) {
+ outPos += bytesCount;
+ assert outPos <= outSize;
+ }
+
+ /**
+ * Resets the buffer state and initializes the AES-CTR counter and padding
for the given
+ * absolute position in the delegate file.
+ *
+ * @param absolutePosition position in the delegate {@link IndexInput}
(includes IV offset)
+ * @param delegateOffset the position right after the IV — the "zero" of
the AES-CTR stream
+ */
+ void seek(long absolutePosition, long delegateOffset) {
+ outPos = outSize = 0;
+ long relativePosition = absolutePosition - delegateOffset;
+ long counter = relativePosition / AES_BLOCK_SIZE;
+ encrypter.init(counter);
+ padding = (int) (relativePosition & AES_BLOCK_SIZE_MOD_MASK);
+ inPos = padding;
+ }
+
+ /**
+ * Reads and decrypts bytes from the delegate into the target array.
+ * Serves from the output buffer first, then reads and decrypts more as
needed.
+ */
+ void readDecrypted(IndexInput delegate, byte[] target, int offset, int
length) throws IOException {
+ while (length > 0) {
+ int outRemaining = outSize - outPos;
+ if (outRemaining > 0) {
+ if (length <= outRemaining) {
+ System.arraycopy(outBuffer, outPos, target, offset, length);
+ outPos += length;
+ return;
+ }
+ System.arraycopy(outBuffer, outPos, target, offset, outRemaining);
+ outPos += outRemaining;
+ offset += outRemaining;
+ length -= outRemaining;
+ }
+ readFromDelegate(delegate, length);
+ decrypt();
+ }
+ }
+
+ private void readFromDelegate(IndexInput delegate, int length) throws
IOException {
+ assert length > 0;
+ int inRemaining = inBuffer.length - inPos;
+ if (inRemaining > 0) {
+ int numBytesToRead = Math.min(inRemaining, length);
+ delegate.readBytes(inBuffer, inPos, numBytesToRead);
+ inPos += numBytesToRead;
+ }
+ }
+
+ private void decrypt() {
+ assert inPos > padding : "inPos=" + inPos + " padding=" + padding;
+ encrypter.process(inBuffer, 0, inPos, outBuffer, 0);
+ outSize = inPos;
+ inPos = 0;
+ outPos = padding;
+ padding = 0;
+ }
+
+ AesCtrEncrypter cloneEncrypter() {
+ return encrypter.clone();
+ }
+
+ @Override
+ public DecryptionBuffer clone() {
+ try {
+ DecryptionBuffer clone = (DecryptionBuffer) super.clone();
+ clone.encrypter = encrypter.clone();
+ clone.inBuffer = new byte[inBuffer.length];
+ clone.outBuffer = new byte[outBuffer.length];
+ return clone;
+ } catch (CloneNotSupportedException e) {
+ throw new AssertionError(e);
+ }
+ }
+ }
+}
diff --git
a/encryption/src/test/java/org/apache/solr/encryption/crypto/DecryptingIndexInputTest.java
b/encryption/src/test/java/org/apache/solr/encryption/crypto/DecryptingIndexInputTest.java
index 0e6337d..e820cf3 100644
---
a/encryption/src/test/java/org/apache/solr/encryption/crypto/DecryptingIndexInputTest.java
+++
b/encryption/src/test/java/org/apache/solr/encryption/crypto/DecryptingIndexInputTest.java
@@ -151,6 +151,43 @@ public class DecryptingIndexInputTest extends
RandomizedTest {
indexInput.close();
}
+ /**
+ * Reproduces a bug where creating a sub-slice at offset 0 from a slice at a
non-zero offset
+ * would produce corrupted decrypted data. This is the pattern used by
+ * {@link IndexInput#randomAccessSlice(long, long)}, which internally calls
+ * slice("randomaccess", 0, length).
+ *
+ * The root cause: when slice() creates a new DecryptingIndexInput, the
+ * constructor sets encrypter.init(0) (AES counter = 0) and the subsequent
+ * seek(0) is supposed to call setPosition() to initialize the correct
+ * AES counter and padding. But when the cloned delegate's file pointer
already equals the
+ * target position (which happens with offset=0), seek() took a buffer
shortcut
+ * and skipped setPosition(), leaving the counter and padding at wrong
values.
+ *
+ * With prefixSize=17: counter should be 17/16=1 (not 0) and padding should
be 17%16=1 (not 0).
+ * Both are wrong without the fix, triggering both dimensions of the bug.
+ */
+ @Test
+ public void testRandomAccessSlice() throws Exception {
+ ByteBuffersDataOutput dataOutput = new ByteBuffersDataOutput();
+ EncryptingIndexOutput indexOutput =
createEncryptingIndexOutput(dataOutput);
+ int bytesBeforeFirstSlice = 17;
+ byte[] prefix = new byte[bytesBeforeFirstSlice];
+ indexOutput.writeBytes(prefix, 0, prefix.length);
+ String data = "Hello, world!";
+ indexOutput.writeBytes(data.getBytes(), 0, data.length());
+ indexOutput.close();
+
+ DecryptingIndexInput root = createDecryptingIndexInput(dataOutput, 0);
+ // Simulates reading a sub-file (.tip for example) at a non-zero offset
inside a compound file (.cfs)
+ IndexInput firstSlice = root.slice(".tip", bytesBeforeFirstSlice,
data.length());
+ // Simulates randomAccessSlice(0, length) - creates a sub-slice at offset 0
+ IndexInput subSlice = firstSlice.slice("randomaccess", 0, data.length());
+ byte[] result = new byte[data.length()];
+ subSlice.readBytes(result, 0, data.length());
+ assertEquals(data, new String(result));
+ }
+
@Test
public void testSeek() throws Exception {
for (int reps = randomIntBetween(1, 200); --reps > 0; ) {
