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 IndexInput slice(String sliceDescription, long offset, long length) throw } 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 void readBytes(byte[] b, int offset, int length) throws IOException { 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 DecryptingIndexInput clone() { 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. + *
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 void testSeekEmpty() throws Exception { 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; ) {