Split the functionality of ConnectionReaderImpl into layers.

api/net/sf/briar/api/transport/ConnectionReaderFactory.java

@@ -12,7 +12,7 @@ public interface ConnectionReaderFactory {
 	 * returns.
 	 */
 	ConnectionReader createConnectionReader(InputStream in, byte[] secret,
-			byte[] tag);
+			byte[] bufferedTag);
 
 	/**
 	 * Creates a connection reader for a simplex connection or the initiator's
@@ -20,7 +20,7 @@ public interface ConnectionReaderFactory {
 	 * returns.
 	 */
 	ConnectionReader createConnectionReader(SegmentSource in, byte[] secret,
-			Segment buffered);
+			Segment bufferedSegment);
 
 	/**
 	 * Creates a connection reader for the responder's side of a duplex

components/net/sf/briar/transport/ConnectionReaderFactoryImpl.java

@@ -25,8 +25,8 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory {
 	}
 
 	public ConnectionReader createConnectionReader(InputStream in,
-			byte[] secret, byte[] tag) {
+			byte[] secret, byte[] bufferedTag) {
-		return createConnectionReader(in, secret, tag, true);
+		return createConnectionReader(in, secret, bufferedTag, true);
 	}
 
 	public ConnectionReader createConnectionReader(InputStream in,
@@ -35,7 +35,7 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory {
 	}
 
 	private ConnectionReader createConnectionReader(InputStream in,
-			byte[] secret, byte[] tag, boolean initiator) {
+			byte[] secret, byte[] bufferedTag, boolean initiator) {
 		// Derive the keys and erase the secret
 		ErasableKey tagKey = crypto.deriveTagKey(secret, initiator);
 		ErasableKey segKey = crypto.deriveSegmentKey(secret, initiator);
@@ -45,18 +45,24 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory {
 		Cipher tagCipher = crypto.getTagCipher();
 		Cipher segCipher = crypto.getSegmentCipher();
 		IncomingEncryptionLayer decrypter = new IncomingEncryptionLayerImpl(in,
-				tagCipher, segCipher, tagKey, segKey, false, tag);
+				tagCipher, segCipher, tagKey, segKey, false, bufferedTag);
 		// No error correction
 		IncomingErrorCorrectionLayer correcter =
 			new NullIncomingErrorCorrectionLayer(decrypter);
-		// Create the reader - don't tolerate errors
+		// Create the authenticator
 		Mac mac = crypto.getMac();
-		return new ConnectionReaderImpl(correcter, mac, macKey, false);
+		IncomingAuthenticationLayer authenticator =
+			new IncomingAuthenticationLayerImpl(correcter, mac, macKey);
+		// No reordering or retransmission
+		IncomingReliabilityLayer reliability =
+			new NullIncomingReliabilityLayer(authenticator);
+		// Create the reader - don't tolerate errors
+		return new ConnectionReaderImpl(reliability, false);
 	}
 
 	public ConnectionReader createConnectionReader(SegmentSource in,
-			byte[] secret, Segment buffered) {
+			byte[] secret, Segment bufferedSegment) {
-		return createConnectionReader(in, secret, buffered, true);
+		return createConnectionReader(in, secret, bufferedSegment, true);
 	}
 
 	public ConnectionReader createConnectionReader(SegmentSource in,
@@ -65,7 +71,7 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory {
 	}
 
 	private ConnectionReader createConnectionReader(SegmentSource in,
-			byte[] secret, Segment buffered, boolean initiator) {
+			byte[] secret, Segment bufferedSegment, boolean initiator) {
 		// Derive the keys and erase the secret
 		ErasableKey tagKey = crypto.deriveTagKey(secret, initiator);
 		ErasableKey segKey = crypto.deriveSegmentKey(secret, initiator);
@@ -76,12 +82,18 @@ class ConnectionReaderFactoryImpl implements ConnectionReaderFactory {
 		Cipher segCipher = crypto.getSegmentCipher();
 		IncomingEncryptionLayer decrypter =
 			new IncomingSegmentedEncryptionLayer(in, tagCipher, segCipher,
-					tagKey, segKey, false, buffered);
+					tagKey, segKey, false, bufferedSegment);
 		// No error correction
 		IncomingErrorCorrectionLayer correcter =
 			new NullIncomingErrorCorrectionLayer(decrypter);
-		// Create the reader - don't tolerate errors
+		// Create the authenticator
 		Mac mac = crypto.getMac();
-		return new ConnectionReaderImpl(correcter, mac, macKey, false);
+		IncomingAuthenticationLayer authenticator =
+			new IncomingAuthenticationLayerImpl(correcter, mac, macKey);
+		// No reordering or retransmission
+		IncomingReliabilityLayer reliability =
+			new NullIncomingReliabilityLayer(authenticator);
+		// Create the reader - don't tolerate errors
+		return new ConnectionReaderImpl(reliability, false);
 	}
 }

components/net/sf/briar/transport/ConnectionReaderImpl.java

@@ -1,45 +1,24 @@
 package net.sf.briar.transport;
 
 import static net.sf.briar.api.transport.TransportConstants.FRAME_HEADER_LENGTH;
-import static net.sf.briar.api.transport.TransportConstants.MAC_LENGTH;
-import static net.sf.briar.util.ByteUtils.MAX_32_BIT_UNSIGNED;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.security.InvalidKeyException;
-import java.util.Collection;
-import java.util.Collections;
-
-import javax.crypto.Mac;
 
 import net.sf.briar.api.FormatException;
-import net.sf.briar.api.crypto.ErasableKey;
 import net.sf.briar.api.transport.ConnectionReader;
 
 class ConnectionReaderImpl extends InputStream implements ConnectionReader {
 
-	private final IncomingErrorCorrectionLayer in;
+	private final IncomingReliabilityLayer 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,
+	ConnectionReaderImpl(IncomingReliabilityLayer in, boolean tolerateErrors) {
-			ErasableKey macKey, boolean tolerateErrors) {
 		this.in = in;
-		this.mac = mac;
 		this.tolerateErrors = tolerateErrors;
-		// Initialise the MAC
-		try {
-			mac.init(macKey);
-		} catch(InvalidKeyException e) {
-			throw new IllegalArgumentException(e);
-		}
-		macKey.erase();
-		if(mac.getMacLength() != MAC_LENGTH)
-			throw new IllegalArgumentException();
 		frame = new Frame();
 	}
 
@@ -72,49 +51,17 @@ class ConnectionReaderImpl extends InputStream implements ConnectionReader {
 	}
 
 	private boolean readValidFrame() throws IOException {
+		assert length == 0;
 		while(true) {
 			try {
-				return readFrame();
+				if(!in.readFrame(frame)) return false;
+				offset = FRAME_HEADER_LENGTH;
+				length = HeaderEncoder.getPayloadLength(frame.getBuffer());
+				return true;
 			} 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();
-		// 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 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 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 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 InvalidDataException();
-		}
-		offset = FRAME_HEADER_LENGTH;
-		length = payload;
-		frameNumber++;
-		return true;
-	}
 }

components/net/sf/briar/transport/IncomingAuthenticationLayer.java

@@ -0,0 +1,19 @@
+package net.sf.briar.transport;
+
+import java.io.IOException;
+import java.util.Collection;
+
+interface IncomingAuthenticationLayer {
+
+	/**
+	 * 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,
+	InvalidDataException;
+}

components/net/sf/briar/transport/IncomingAuthenticationLayerImpl.java

@@ -0,0 +1,64 @@
+package net.sf.briar.transport;
+
+import static net.sf.briar.api.transport.TransportConstants.FRAME_HEADER_LENGTH;
+import static net.sf.briar.api.transport.TransportConstants.MAC_LENGTH;
+
+import java.io.IOException;
+import java.security.InvalidKeyException;
+import java.util.Collection;
+
+import javax.crypto.Mac;
+
+import net.sf.briar.api.crypto.ErasableKey;
+
+class IncomingAuthenticationLayerImpl implements IncomingAuthenticationLayer {
+
+	private final IncomingErrorCorrectionLayer in;
+	private final Mac mac;
+
+	IncomingAuthenticationLayerImpl(IncomingErrorCorrectionLayer in, Mac mac,
+			ErasableKey macKey) {
+		this.in = in;
+		this.mac = mac;
+		// Initialise the MAC
+		try {
+			mac.init(macKey);
+		} catch(InvalidKeyException e) {
+			throw new IllegalArgumentException(e);
+		}
+		macKey.erase();
+		if(mac.getMacLength() != MAC_LENGTH)
+			throw new IllegalArgumentException();
+	}
+
+	public boolean readFrame(Frame f, Collection<Long> window)
+	throws IOException, InvalidDataException {
+		// Read a frame
+		if(!in.readFrame(f, window)) return false;
+		// Check that the length is legal
+		byte[] buf = f.getBuffer();
+		long frameNumber = HeaderEncoder.getFrameNumber(buf);
+		if(!HeaderEncoder.validateHeader(buf, frameNumber))
+			throw new InvalidDataException();
+		// Check that the payload and padding lengths are correct
+		int payload = HeaderEncoder.getPayloadLength(buf);
+		int padding = HeaderEncoder.getPaddingLength(buf);
+		if(f.getLength() != FRAME_HEADER_LENGTH + payload + padding
+				+ 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 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 InvalidDataException();
+		}
+		frameNumber++;
+		return true;
+	}
+}

components/net/sf/briar/transport/IncomingReliabilityLayer.java

@@ -0,0 +1,16 @@
+package net.sf.briar.transport;
+
+import java.io.IOException;
+
+interface IncomingReliabilityLayer {
+
+	/**
+	 * Reads a frame into the given buffer. 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) throws IOException, InvalidDataException;
+}

components/net/sf/briar/transport/NullIncomingReliabilityLayer.java

@@ -0,0 +1,24 @@
+package net.sf.briar.transport;
+
+import java.io.IOException;
+import java.util.Collection;
+import java.util.Collections;
+
+class NullIncomingReliabilityLayer implements IncomingReliabilityLayer {
+
+	private final IncomingAuthenticationLayer in;
+
+	private long frameNumber = 0L;
+
+	NullIncomingReliabilityLayer(IncomingAuthenticationLayer in) {
+		this.in = in;
+	}
+
+	public boolean readFrame(Frame f) throws IOException, InvalidDataException {
+		// Frames must be read in order
+		Collection<Long> window = Collections.singleton(frameNumber);
+		if(!in.readFrame(f, window)) return false;
+		frameNumber++;
+		return true;
+	}
+}

test/net/sf/briar/transport/ConnectionReaderImplTest.java

@@ -6,6 +6,7 @@ import static net.sf.briar.api.transport.TransportConstants.MAX_FRAME_LENGTH;
 import static org.junit.Assert.assertArrayEquals;
 
 import java.io.ByteArrayInputStream;
+import java.io.InputStream;
 
 import net.sf.briar.TestUtils;
 import net.sf.briar.api.FormatException;
@@ -14,6 +15,7 @@ import net.sf.briar.api.transport.ConnectionReader;
 import org.apache.commons.io.output.ByteArrayOutputStream;
 import org.junit.Test;
 
+// FIXME: This test covers too many classes
 public class ConnectionReaderImplTest extends TransportTest {
 
 	public ConnectionReaderImplTest() throws Exception {
@@ -32,11 +34,7 @@ public class ConnectionReaderImplTest extends TransportTest {
 		mac.doFinal(frame, FRAME_HEADER_LENGTH + payloadLength);
 		// Read the frame
 		ByteArrayInputStream in = new ByteArrayInputStream(frame);
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey,
-				false);
 		// There should be no bytes available before EOF
 		assertEquals(-1, r.getInputStream().read());
 	}
@@ -53,11 +51,7 @@ public class ConnectionReaderImplTest extends TransportTest {
 		mac.doFinal(frame, FRAME_HEADER_LENGTH + payloadLength);
 		// Read the frame
 		ByteArrayInputStream in = new ByteArrayInputStream(frame);
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		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());
@@ -82,11 +76,7 @@ public class ConnectionReaderImplTest extends TransportTest {
 		out.write(frame1);
 		// Read the first frame
 		ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		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,11 +109,7 @@ public class ConnectionReaderImplTest extends TransportTest {
 		out.write(frame1);
 		// Read the first frame
 		ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		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
@@ -148,11 +134,7 @@ public class ConnectionReaderImplTest extends TransportTest {
 		mac.doFinal(frame, FRAME_HEADER_LENGTH + payloadLength + paddingLength);
 		// Read the frame
 		ByteArrayInputStream in = new ByteArrayInputStream(frame);
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey,
-				false);
 		// The non-zero padding should be rejected
 		try {
 			r.getInputStream().read();
@@ -183,11 +165,7 @@ public class ConnectionReaderImplTest extends TransportTest {
 		out.write(frame1);
 		// Read the frames
 		ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey,
-				false);
 		byte[] read = new byte[payloadLength];
 		TestUtils.readFully(r.getInputStream(), read);
 		assertArrayEquals(new byte[payloadLength], read);
@@ -210,11 +188,7 @@ public class ConnectionReaderImplTest extends TransportTest {
 		frame[12] ^= 1;
 		// Try to read the frame - not a single byte should be read
 		ByteArrayInputStream in = new ByteArrayInputStream(frame);
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey,
-				false);
 		try {
 			r.getInputStream().read();
 			fail();
@@ -235,14 +209,21 @@ public class ConnectionReaderImplTest extends TransportTest {
 		frame[17] ^= 1;
 		// Try to read the frame - not a single byte should be read
 		ByteArrayInputStream in = new ByteArrayInputStream(frame);
-		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		ConnectionReader r = createConnectionReader(in);
-		IncomingErrorCorrectionLayer correcter =
-			new NullIncomingErrorCorrectionLayer(decrypter);
-		ConnectionReader r = new ConnectionReaderImpl(correcter, mac, macKey,
-				false);
 		try {
 			r.getInputStream().read();
 			fail();
 		} catch(FormatException expected) {}
 	}
+
+	private ConnectionReader createConnectionReader(InputStream in) {
+		IncomingEncryptionLayer decrypter = new NullIncomingEncryptionLayer(in);
+		IncomingErrorCorrectionLayer correcter =
+			new NullIncomingErrorCorrectionLayer(decrypter);
+		IncomingAuthenticationLayer authenticator =
+			new IncomingAuthenticationLayerImpl(correcter, mac, macKey);
+		IncomingReliabilityLayer reliability =
+			new NullIncomingReliabilityLayer(authenticator);
+		return new ConnectionReaderImpl(reliability, false);
+	}
 }

test/net/sf/briar/transport/FrameReadWriteTest.java

@@ -97,8 +97,11 @@ public class FrameReadWriteTest extends BriarTestCase {
 				tagCipher, segCipher, tagKey, segKey, false, recoveredTag);
 		IncomingErrorCorrectionLayer correcter1 =
 			new NullIncomingErrorCorrectionLayer(decrypter);
-		ConnectionReader reader = new ConnectionReaderImpl(correcter1, mac,
+		IncomingAuthenticationLayer authenticator =
-				macKey, false);
+			new IncomingAuthenticationLayerImpl(correcter1, mac, macKey);
+		IncomingReliabilityLayer reliability =
+			new NullIncomingReliabilityLayer(authenticator);
+		ConnectionReader reader = new ConnectionReaderImpl(reliability, false);
 		InputStream in1 = reader.getInputStream();
 		byte[] recovered = new byte[frame.length];
 		int offset = 0;