From c185a12ef867ac5481e997f3fd69a76a0ff73522 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 29 Sep 2015 16:50:38 +0200 Subject: [PATCH] Verify actually written checksum in VerifyingIndexOutput today we don't verify that the actual checksum written to VerifyingIndexOutput is the actual checksum we are expecting. --- .../org/elasticsearch/index/store/Store.java | 50 +++++++++---- .../elasticsearch/index/store/StoreTests.java | 71 ++++++++++++++++++- 2 files changed, 106 insertions(+), 15 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/store/Store.java b/core/src/main/java/org/elasticsearch/index/store/Store.java index 0136c739761..c8394fc7a59 100644 --- a/core/src/main/java/org/elasticsearch/index/store/Store.java +++ b/core/src/main/java/org/elasticsearch/index/store/Store.java @@ -39,6 +39,7 @@ import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.lucene.Lucene; +import org.elasticsearch.common.lucene.store.ByteArrayIndexInput; import org.elasticsearch.common.lucene.store.InputStreamIndexInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; @@ -1259,27 +1260,42 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref private long writtenBytes; private final long checksumPosition; private String actualChecksum; + private final byte[] footerChecksum = new byte[8]; // this holds the actual footer checksum data written by to this output LuceneVerifyingIndexOutput(StoreFileMetaData metadata, IndexOutput out) { super(out); this.metadata = metadata; - checksumPosition = metadata.length() - 8; // the last 8 bytes are the checksum + checksumPosition = metadata.length() - 8; // the last 8 bytes are the checksum - we store it in footerChecksum } @Override public void verify() throws IOException { + String footerDigest = null; if (metadata.checksum().equals(actualChecksum) && writtenBytes == metadata.length()) { - return; + ByteArrayIndexInput indexInput = new ByteArrayIndexInput("checksum", this.footerChecksum); + footerDigest = digestToString(indexInput.readLong()); + if (metadata.checksum().equals(footerDigest)) { + return; + } } throw new CorruptIndexException("verification failed (hardware problem?) : expected=" + metadata.checksum() + - " actual=" + actualChecksum + " writtenLength=" + writtenBytes + " expectedLength=" + metadata.length() + + " actual=" + actualChecksum + " footer=" + footerDigest +" writtenLength=" + writtenBytes + " expectedLength=" + metadata.length() + " (resource=" + metadata.toString() + ")", "VerifyingIndexOutput(" + metadata.name() + ")"); } @Override public void writeByte(byte b) throws IOException { - if (writtenBytes++ == checksumPosition) { + final long writtenBytes = this.writtenBytes++; + if (writtenBytes == checksumPosition) { readAndCompareChecksum(); + } else if (writtenBytes > checksumPosition) { // we are writing parts of the checksum.... + final int index = Math.toIntExact(writtenBytes - checksumPosition); + if (index < footerChecksum.length) { + footerChecksum[index] = b; + } else { + verify(); // fail if we write more than expected + throw new AssertionError("write past EOF expected length: " + metadata.length() + " writtenBytes: " + writtenBytes); + } } out.writeByte(b); } @@ -1295,17 +1311,23 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref @Override public void writeBytes(byte[] b, int offset, int length) throws IOException { - if (writtenBytes + length > checksumPosition && actualChecksum == null) { - assert writtenBytes <= checksumPosition; - final int bytesToWrite = (int) (checksumPosition - writtenBytes); - out.writeBytes(b, offset, bytesToWrite); - readAndCompareChecksum(); - offset += bytesToWrite; - length -= bytesToWrite; - writtenBytes += bytesToWrite; + if (writtenBytes + length > checksumPosition) { + if (actualChecksum == null) { + assert writtenBytes <= checksumPosition; + final int bytesToWrite = (int) (checksumPosition - writtenBytes); + out.writeBytes(b, offset, bytesToWrite); + readAndCompareChecksum(); + offset += bytesToWrite; + length -= bytesToWrite; + writtenBytes += bytesToWrite; + } + for (int i = 0; i < length; i++) { + writeByte(b[offset+i]); + } + } else { + out.writeBytes(b, offset, length); + writtenBytes += length; } - out.writeBytes(b, offset, length); - writtenBytes += length; } } diff --git a/core/src/test/java/org/elasticsearch/index/store/StoreTests.java b/core/src/test/java/org/elasticsearch/index/store/StoreTests.java index 18ba33ff8c2..9386b6a20c0 100644 --- a/core/src/test/java/org/elasticsearch/index/store/StoreTests.java +++ b/core/src/test/java/org/elasticsearch/index/store/StoreTests.java @@ -149,16 +149,85 @@ public class StoreTests extends ESTestCase { } } Store.verify(verifyingOutput); - verifyingOutput.writeByte((byte) 0x0); + try { + appendRandomData(verifyingOutput); + fail("should be a corrupted index"); + } catch (CorruptIndexException | IndexFormatTooOldException | IndexFormatTooNewException ex) { + // ok + } try { Store.verify(verifyingOutput); fail("should be a corrupted index"); } catch (CorruptIndexException | IndexFormatTooOldException | IndexFormatTooNewException ex) { // ok } + IOUtils.close(indexInput, verifyingOutput, dir); } + public void testChecksumCorrupted() throws IOException { + Directory dir = newDirectory(); + IndexOutput output = dir.createOutput("foo.bar", IOContext.DEFAULT); + int iters = scaledRandomIntBetween(10, 100); + for (int i = 0; i < iters; i++) { + BytesRef bytesRef = new BytesRef(TestUtil.randomRealisticUnicodeString(random(), 10, 1024)); + output.writeBytes(bytesRef.bytes, bytesRef.offset, bytesRef.length); + } + output.writeInt(CodecUtil.FOOTER_MAGIC); + output.writeInt(0); + String checksum = Store.digestToString(output.getChecksum()); + output.writeLong(output.getChecksum() + 1); // write a wrong checksum to the file + output.close(); + + IndexInput indexInput = dir.openInput("foo.bar", IOContext.DEFAULT); + indexInput.seek(0); + BytesRef ref = new BytesRef(scaledRandomIntBetween(1, 1024)); + long length = indexInput.length(); + IndexOutput verifyingOutput = new Store.LuceneVerifyingIndexOutput(new StoreFileMetaData("foo1.bar", length, checksum), dir.createOutput("foo1.bar", IOContext.DEFAULT)); + while (length > 0) { + if (random().nextInt(10) == 0) { + verifyingOutput.writeByte(indexInput.readByte()); + length--; + } else { + int min = (int) Math.min(length, ref.bytes.length); + indexInput.readBytes(ref.bytes, ref.offset, min); + verifyingOutput.writeBytes(ref.bytes, ref.offset, min); + length -= min; + } + } + + try { + if (randomBoolean()) { + appendRandomData(verifyingOutput); + } else { + Store.verify(verifyingOutput); + } + fail("should be a corrupted index"); + } catch (CorruptIndexException | IndexFormatTooOldException | IndexFormatTooNewException ex) { + // ok + } + IOUtils.close(indexInput, verifyingOutput, dir); + } + + private void appendRandomData(IndexOutput output) throws IOException { + int numBytes = randomIntBetween(1, 1024); + final BytesRef ref = new BytesRef(scaledRandomIntBetween(1, numBytes)); + ref.length = ref.bytes.length; + while (numBytes > 0) { + if (random().nextInt(10) == 0) { + output.writeByte(randomByte()); + numBytes--; + } else { + for (int i = 0; i