diff --git a/api/net/sf/briar/api/FormatException.java b/api/net/sf/briar/api/FormatException.java index b48f8ba51f08f7b38f0d229dbf01c16e4635db9d..6796df2b7fe6d86cdef4e69b4f5e771f2e78791b 100644 --- a/api/net/sf/briar/api/FormatException.java +++ b/api/net/sf/briar/api/FormatException.java @@ -2,8 +2,8 @@ package net.sf.briar.api; import java.io.IOException; +/** An exception that indicates an unrecoverable formatting error. */ public class FormatException extends IOException { private static final long serialVersionUID = 2274966775687766337L; - } diff --git a/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java b/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java index b2f296ef74671e852e68d83538cfc72fe0952892..7ef9d7adff038cdfe0224c9cb35d5c1a976c92b0 100644 --- a/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java +++ b/components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java @@ -49,9 +49,9 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory { // No error correction IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - // Create the reader + // Create the reader - don't tolerate errors Mac mac = crypto.getMac(); - return new ConnectionReaderImpl(correcter, mac, macKey); + return new ConnectionReaderImpl(correcter, mac, macKey, false); } public ConnectionReader createConnectionReader(SegmentSource in, @@ -80,8 +80,8 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory { // No error correction IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - // Create the reader + // Create the reader - don't tolerate errors Mac mac = crypto.getMac(); - return new ConnectionReaderImpl(correcter, mac, macKey); + return new ConnectionReaderImpl(correcter, mac, macKey, false); } } diff --git a/components/net/sf/briar/transport/ConnectionReaderImpl.java b/components/net/sf/briar/transport/ConnectionReaderImpl.java index 0241369d4559a5e3c005dad9fe18657bfc0ebb33..e02905813cca46b9161fbb292940ca430ba1f252 100644 --- a/components/net/sf/briar/transport/ConnectionReaderImpl.java +++ b/components/net/sf/briar/transport/ConnectionReaderImpl.java @@ -20,15 +20,17 @@ class ConnectionReaderImpl extends InputStream implements ConnectionReader { private final IncomingErrorCorrectionLayer in; private final Mac mac; + private final boolean tolerateErrors; private final Frame frame; private long frameNumber = 0L; private int offset = 0, length = 0; ConnectionReaderImpl(IncomingErrorCorrectionLayer in, Mac mac, - ErasableKey macKey) { + ErasableKey macKey, boolean tolerateErrors) { this.in = in; this.mac = mac; + this.tolerateErrors = tolerateErrors; // Initialise the MAC try { mac.init(macKey); @@ -47,7 +49,7 @@ class ConnectionReaderImpl extends InputStream implements ConnectionReader { @Override public int read() throws IOException { - while(length == 0) if(!readFrame()) return -1; + while(length == 0) if(!readValidFrame()) return -1; int b = frame.getBuffer()[offset] & 0xff; offset++; length--; @@ -61,7 +63,7 @@ class ConnectionReaderImpl extends InputStream implements ConnectionReader { @Override public int read(byte[] b, int off, int len) throws IOException { - while(length == 0) if(!readFrame()) return -1; + while(length == 0) if(!readValidFrame()) return -1; len = Math.min(len, length); System.arraycopy(frame.getBuffer(), offset, b, off, len); offset += len; @@ -69,33 +71,46 @@ class ConnectionReaderImpl extends InputStream implements ConnectionReader { return len; } - private boolean readFrame() throws IOException { + private boolean readValidFrame() throws IOException { + while(true) { + try { + return readFrame(); + } catch(InvalidDataException e) { + if(tolerateErrors) continue; + throw new FormatException(); + } + } + } + + private boolean readFrame() throws IOException, InvalidDataException { assert length == 0; // Don't allow more than 2^32 frames to be read - if(frameNumber > MAX_32_BIT_UNSIGNED) throw new IllegalStateException(); + if(frameNumber > MAX_32_BIT_UNSIGNED) + throw new IllegalStateException(); // Read a frame Collection<Long> window = Collections.singleton(frameNumber); if(!in.readFrame(frame, window)) return false; // Check that the frame number is correct and the length is legal byte[] buf = frame.getBuffer(); if(!HeaderEncoder.validateHeader(buf, frameNumber)) - throw new FormatException(); + throw new InvalidDataException(); // Check that the payload and padding lengths are correct int payload = HeaderEncoder.getPayloadLength(buf); int padding = HeaderEncoder.getPaddingLength(buf); if(frame.getLength() != FRAME_HEADER_LENGTH + payload + padding - + MAC_LENGTH) throw new FormatException(); + + MAC_LENGTH) throw new InvalidDataException(); // Check that the padding is all zeroes int paddingStart = FRAME_HEADER_LENGTH + payload; for(int i = paddingStart; i < paddingStart + padding; i++) { - if(buf[i] != 0) throw new FormatException(); + if(buf[i] != 0) throw new InvalidDataException(); } // Check the MAC int macStart = FRAME_HEADER_LENGTH + payload + padding; mac.update(buf, 0, macStart); byte[] expectedMac = mac.doFinal(); for(int i = 0; i < expectedMac.length; i++) { - if(expectedMac[i] != buf[macStart + i]) throw new FormatException(); + if(expectedMac[i] != buf[macStart + i]) + throw new InvalidDataException(); } offset = FRAME_HEADER_LENGTH; length = payload; diff --git a/components/net/sf/briar/transport/IncomingEncryptionLayer.java b/components/net/sf/briar/transport/IncomingEncryptionLayer.java index 84c8776c1164a99a91f27404c4e3a741c6422616..3521ce46e6b71dd8a8b00fe9eb4ed1be8da59a0d 100644 --- a/components/net/sf/briar/transport/IncomingEncryptionLayer.java +++ b/components/net/sf/briar/transport/IncomingEncryptionLayer.java @@ -9,6 +9,10 @@ interface IncomingEncryptionLayer { /** * Reads a segment, excluding its tag, into the given buffer. Returns false * if no more segments can be read from the connection. + * @throws IOException if an unrecoverable error occurs and the connection + * must be closed. + * @throws InvalidDataException if a recoverable error occurs. The caller + * may choose whether to retry the read or close the connection. */ - boolean readSegment(Segment s) throws IOException; + boolean readSegment(Segment s) throws IOException, InvalidDataException; } diff --git a/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java b/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java index 446c7a3ef52d5f64beccf4574702f54eb36e2a7c..bd555c150cd66c33757e20d1bccf91ff4cd8adfc 100644 --- a/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java +++ b/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java @@ -9,6 +9,11 @@ interface IncomingErrorCorrectionLayer { * Reads a frame into the given buffer. The frame number must be contained * in the given window. Returns false if no more frames can be read from * the connection. + * @throws IOException if an unrecoverable error occurs and the connection + * must be closed. + * @throws InvalidDataException if a recoverable error occurs. The caller + * may choose whether to retry the read or close the connection. */ - boolean readFrame(Frame f, Collection<Long> window) throws IOException; + boolean readFrame(Frame f, Collection<Long> window) throws IOException, + InvalidDataException; } diff --git a/components/net/sf/briar/transport/IncomingSegmentedEncryptionLayer.java b/components/net/sf/briar/transport/IncomingSegmentedEncryptionLayer.java index 89815dc1e6f91fe687ea6fc3c730200ac442460b..f2a2ad40a3ddda8e77b7c258355f6dce51d498b9 100644 --- a/components/net/sf/briar/transport/IncomingSegmentedEncryptionLayer.java +++ b/components/net/sf/briar/transport/IncomingSegmentedEncryptionLayer.java @@ -11,7 +11,6 @@ import java.security.GeneralSecurityException; import javax.crypto.Cipher; import javax.crypto.spec.IvParameterSpec; -import net.sf.briar.api.FormatException; import net.sf.briar.api.crypto.ErasableKey; import net.sf.briar.api.plugins.SegmentSource; import net.sf.briar.api.transport.Segment; @@ -32,22 +31,23 @@ class IncomingSegmentedEncryptionLayer implements IncomingEncryptionLayer { IncomingSegmentedEncryptionLayer(SegmentSource in, Cipher tagCipher, Cipher segCipher, ErasableKey tagKey, ErasableKey segKey, - boolean tagEverySegment, Segment s) { + boolean tagEverySegment, Segment bufferedSegment) { this.in = in; this.tagCipher = tagCipher; this.segCipher = segCipher; this.tagKey = tagKey; this.segKey = segKey; this.tagEverySegment = tagEverySegment; + this.bufferedSegment = bufferedSegment; blockSize = segCipher.getBlockSize(); if(blockSize < FRAME_HEADER_LENGTH) throw new IllegalArgumentException(); iv = IvEncoder.encodeIv(0L, blockSize); segment = new SegmentImpl(); - bufferedSegment = s; } - public boolean readSegment(Segment s) throws IOException { + public boolean readSegment(Segment s) throws IOException, + InvalidDataException { boolean expectTag = tagEverySegment || firstSegment; firstSegment = false; try { @@ -62,14 +62,14 @@ class IncomingSegmentedEncryptionLayer implements IncomingEncryptionLayer { } int offset = expectTag ? TAG_LENGTH : 0; int length = segment.getLength(); - if(length > MAX_SEGMENT_LENGTH) throw new FormatException(); + if(length > MAX_SEGMENT_LENGTH) throw new InvalidDataException(); if(length < offset + FRAME_HEADER_LENGTH + MAC_LENGTH) - throw new FormatException(); + throw new InvalidDataException(); byte[] ciphertext = segment.getBuffer(); // If a tag is expected then decrypt and validate it if(expectTag) { long seg = TagEncoder.decodeTag(ciphertext, tagCipher, tagKey); - if(seg == -1) throw new FormatException(); + if(seg == -1) throw new InvalidDataException(); segmentNumber = seg; } // Decrypt the segment diff --git a/components/net/sf/briar/transport/InvalidDataException.java b/components/net/sf/briar/transport/InvalidDataException.java new file mode 100644 index 0000000000000000000000000000000000000000..34580d4340feeb2239993105f8c02e8b859b02a2 --- /dev/null +++ b/components/net/sf/briar/transport/InvalidDataException.java @@ -0,0 +1,7 @@ +package net.sf.briar.transport; + +/** An exception that indicates a recoverable formatting error. */ +class InvalidDataException extends Exception { + + private static final long serialVersionUID = 4455775710413826953L; +} diff --git a/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java b/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java index 53898dc9f953dd40d7522082ca70e5d87c0db7fa..7d344a592909cbe11cffa291a00edcb71e1386de 100644 --- a/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java +++ b/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java @@ -16,7 +16,7 @@ class NullIncomingErrorCorrectionLayer implements IncomingErrorCorrectionLayer { } public boolean readFrame(Frame f, Collection<Long> window) - throws IOException { + throws IOException, InvalidDataException { while(true) { if(!in.readSegment(segment)) return false; byte[] buf = segment.getBuffer(); diff --git a/test/net/sf/briar/transport/ConnectionReaderImplTest.java b/test/net/sf/briar/transport/ConnectionReaderImplTest.java index 344561e799c53a367719f71391c7fefdd35937f5..33358ba418960d35a2165a1f008244138fa3e037 100644 --- a/test/net/sf/briar/transport/ConnectionReaderImplTest.java +++ b/test/net/sf/briar/transport/ConnectionReaderImplTest.java @@ -35,7 +35,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); // There should be no bytes available before EOF assertEquals(-1, r.getInputStream().read()); } @@ -55,7 +56,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); // There should be one byte available before EOF assertEquals(0, r.getInputStream().read()); assertEquals(-1, r.getInputStream().read()); @@ -83,7 +85,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); byte[] read = new byte[MAX_PAYLOAD_LENGTH]; TestUtils.readFully(r.getInputStream(), read); // Try to read the second frame @@ -119,7 +122,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); byte[] read = new byte[MAX_PAYLOAD_LENGTH - paddingLength]; TestUtils.readFully(r.getInputStream(), read); // Try to read the second frame @@ -147,7 +151,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); // The non-zero padding should be rejected try { r.getInputStream().read(); @@ -181,7 +186,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); byte[] read = new byte[payloadLength]; TestUtils.readFully(r.getInputStream(), read); assertArrayEquals(new byte[payloadLength], read); @@ -207,7 +213,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); try { r.getInputStream().read(); fail(); @@ -231,7 +238,8 @@ public class ConnectionReaderImplTest extends TransportTest { IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in); IncomingErrorCorrectionLayer correcter = new NullIncomingErrorCorrectionLayer(decrypter); - ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey); + ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey, + false); try { r.getInputStream().read(); fail(); diff --git a/test/net/sf/briar/transport/FrameReadWriteTest.java b/test/net/sf/briar/transport/FrameReadWriteTest.java index 9c10894c4ad53a22b3d89ae9e41324f42ef5f653..ad25f196bb497fcfa4d21164faf069b3e30f4d43 100644 --- a/test/net/sf/briar/transport/FrameReadWriteTest.java +++ b/test/net/sf/briar/transport/FrameReadWriteTest.java @@ -98,7 +98,7 @@ public class FrameReadWriteTest extends BriarTestCase { IncomingErrorCorrectionLayer correcter1 = new NullIncomingErrorCorrectionLayer(decrypter); ConnectionReader reader = new ConnectionReaderImpl(correcter1, mac, - macKey); + macKey, false); InputStream in1 = reader.getInputStream(); byte[] recovered = new byte[frame.length]; int offset = 0; diff --git a/test/net/sf/briar/transport/NullIncomingEncryptionLayer.java b/test/net/sf/briar/transport/NullIncomingEncryptionLayer.java index 6be55a3a74ec8080643d7a3a93d00083fe6e2b5f..def5509b8381ce1017533f5ecea85fdd6d527306 100644 --- a/test/net/sf/briar/transport/NullIncomingEncryptionLayer.java +++ b/test/net/sf/briar/transport/NullIncomingEncryptionLayer.java @@ -39,7 +39,7 @@ class NullIncomingEncryptionLayer implements IncomingEncryptionLayer { int padding = HeaderEncoder.getPaddingLength(buf); length = FRAME_HEADER_LENGTH + payload + padding + MAC_LENGTH; if(length > MAX_FRAME_LENGTH) throw new FormatException(); - // Read the remainder of the frame/segment + // Read the remainder of the frame while(offset < length) { int read = in.read(buf, offset, length - offset); if(read == -1) throw new EOFException();