From 3084a6b0584ea35d34361ad1f3fe156e333ffd18 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Fri, 19 Aug 2011 14:47:16 +0200
Subject: [PATCH] Added optional padding to the frame format, so transports
 that are vulnerable to traffic analysis can frame their data independently of
 packet boundaries.

---
 .../briar/transport/ConnectionReaderImpl.java |  22 +++-
 .../briar/transport/ConnectionWriterImpl.java |   4 +-
 test/build.xml                                |   1 -
 test/net/sf/briar/FileReadWriteTest.java      |   3 +-
 .../transport/ConnectionReaderImplTest.java   | 116 ++++++++++++------
 .../transport/ConnectionWriterImplTest.java   |  40 +++---
 6 files changed, 123 insertions(+), 63 deletions(-)

diff --git a/components/net/sf/briar/transport/ConnectionReaderImpl.java b/components/net/sf/briar/transport/ConnectionReaderImpl.java
index f3d54a9a15..72f5db6ae3 100644
--- a/components/net/sf/briar/transport/ConnectionReaderImpl.java
+++ b/components/net/sf/briar/transport/ConnectionReaderImpl.java
@@ -31,8 +31,8 @@ implements ConnectionReader {
 		super(decrypter.getInputStream());
 		this.decrypter = decrypter;
 		this.mac = mac;
-		maxPayloadLength = MAX_FRAME_LENGTH - 6 - mac.getMacLength();
-		header = new byte[6];
+		maxPayloadLength = MAX_FRAME_LENGTH - 8 - mac.getMacLength();
+		header = new byte[8];
 		payload = new byte[maxPayloadLength];
 		footer = new byte[mac.getMacLength()];
 	}
@@ -83,9 +83,11 @@ implements ConnectionReader {
 		// Check that the frame has the expected frame number
 		if(ByteUtils.readUint32(header, 0) != frame)
 			throw new FormatException();
-		// Check that the payload length is legal
+		// Check that the payload and padding lengths are legal
 		payloadLen = ByteUtils.readUint16(header, 4);
-		if(payloadLen == 0 || payloadLen > maxPayloadLength)
+		int paddingLen = ByteUtils.readUint16(header, 6);
+		if(payloadLen + paddingLen == 0) throw new FormatException();
+		if(payloadLen + paddingLen > maxPayloadLength)
 			throw new FormatException();
 		frame++;
 		// Read the payload
@@ -97,6 +99,18 @@ implements ConnectionReader {
 			offset += read;
 		}
 		payloadOff = 0;
+		// Read the padding
+		while(offset < payloadLen + paddingLen) {
+			int read = in.read(payload, offset,
+					payloadLen + paddingLen - offset);
+			if(read == -1) throw new EOFException(); // Unexpected EOF
+			mac.update(payload, offset, read);
+			offset += read;
+		}
+		// Check that the padding is all zeroes
+		for(int i = payloadLen; i < payloadLen + paddingLen; i++) {
+			if(payload[i] != 0) throw new FormatException();
+		}
 		// Read the MAC
 		byte[] expectedMac = mac.doFinal();
 		decrypter.readMac(footer);
diff --git a/components/net/sf/briar/transport/ConnectionWriterImpl.java b/components/net/sf/briar/transport/ConnectionWriterImpl.java
index 882176a11d..ee7ef1b41a 100644
--- a/components/net/sf/briar/transport/ConnectionWriterImpl.java
+++ b/components/net/sf/briar/transport/ConnectionWriterImpl.java
@@ -28,9 +28,9 @@ implements ConnectionWriter {
 		super(encrypter.getOutputStream());
 		this.encrypter = encrypter;
 		this.mac = mac;
-		maxPayloadLength = MAX_FRAME_LENGTH - 6 - mac.getMacLength();
+		maxPayloadLength = MAX_FRAME_LENGTH - 8 - mac.getMacLength();
 		buf = new ByteArrayOutputStream(maxPayloadLength);
-		header = new byte[6];
+		header = new byte[8];
 	}
 
 	public OutputStream getOutputStream() {
diff --git a/test/build.xml b/test/build.xml
index f6bf3ddb10..5af97c568d 100644
--- a/test/build.xml
+++ b/test/build.xml
@@ -31,7 +31,6 @@
 			<test name='net.sf.briar.protocol.ConsumersTest'/>
 			<test name='net.sf.briar.protocol.ProtocolReadWriteTest'/>
 			<test name='net.sf.briar.protocol.RequestReaderTest'/>
-			<test name='net.sf.briar.protocol.SigningDigestingOutputStreamTest'/>
 			<test name='net.sf.briar.protocol.writers.RequestWriterImplTest'/>
 			<test name='net.sf.briar.serial.ReaderImplTest'/>
 			<test name='net.sf.briar.serial.WriterImplTest'/>
diff --git a/test/net/sf/briar/FileReadWriteTest.java b/test/net/sf/briar/FileReadWriteTest.java
index a831946ca5..5c0ca77c2f 100644
--- a/test/net/sf/briar/FileReadWriteTest.java
+++ b/test/net/sf/briar/FileReadWriteTest.java
@@ -173,8 +173,7 @@ public class FileReadWriteTest extends TestCase {
 		TransportWriter t = protocolWriterFactory.createTransportWriter(out);
 		t.writeTransports(transports, timestamp);
 
-		w.getOutputStream().flush();
-		w.getOutputStream().close();
+		out.close();
 		assertTrue(file.exists());
 		assertTrue(file.length() > message.getSize());
 	}
diff --git a/test/net/sf/briar/transport/ConnectionReaderImplTest.java b/test/net/sf/briar/transport/ConnectionReaderImplTest.java
index b0ce604359..d701ed999e 100644
--- a/test/net/sf/briar/transport/ConnectionReaderImplTest.java
+++ b/test/net/sf/briar/transport/ConnectionReaderImplTest.java
@@ -27,6 +27,7 @@ import com.google.inject.Injector;
 public class ConnectionReaderImplTest extends TestCase {
 
 	private final Mac mac;
+	private final int headerLength = 8, macLength;
 
 	public ConnectionReaderImplTest() throws Exception {
 		super();
@@ -34,15 +35,17 @@ public class ConnectionReaderImplTest extends TestCase {
 		CryptoComponent crypto = i.getInstance(CryptoComponent.class);
 		mac = crypto.getMac();
 		mac.init(crypto.generateSecretKey());
+		macLength = mac.getMacLength();
 	}
 
 	@Test
 	public void testLengthZero() throws Exception {
-		// Six bytes for the header, none for the payload
-		byte[] frame = new byte[6 + mac.getMacLength()];
+		int payloadLength = 0;
+		byte[] frame = new byte[headerLength + payloadLength + macLength];
+		writeHeader(frame, 0L, payloadLength, 0);
 		// Calculate the MAC
-		mac.update(frame, 0, 6);
-		mac.doFinal(frame, 6);
+		mac.update(frame, 0, headerLength + payloadLength);
+		mac.doFinal(frame, headerLength + payloadLength);
 		// Read the frame
 		ByteArrayInputStream in = new ByteArrayInputStream(frame);
 		ConnectionDecrypter d = new NullConnectionDecrypter(in);
@@ -55,12 +58,12 @@ public class ConnectionReaderImplTest extends TestCase {
 
 	@Test
 	public void testLengthOne() throws Exception {
-		// Six bytes for the header, one for the payload
-		byte[] frame = new byte[6 + 1 + mac.getMacLength()];
-		ByteUtils.writeUint16(1, frame, 4); // Frame number 0, length 1
+		int payloadLength = 1;
+		byte[] frame = new byte[headerLength + payloadLength + macLength];
+		writeHeader(frame, 0L, payloadLength, 0);
 		// Calculate the MAC
-		mac.update(frame, 0, 6 + 1);
-		mac.doFinal(frame, 6 + 1);
+		mac.update(frame, 0, headerLength + payloadLength);
+		mac.doFinal(frame, headerLength + payloadLength);
 		// Read the frame
 		ByteArrayInputStream in = new ByteArrayInputStream(frame);
 		ConnectionDecrypter d = new NullConnectionDecrypter(in);
@@ -72,18 +75,17 @@ public class ConnectionReaderImplTest extends TestCase {
 
 	@Test
 	public void testMaxLength() throws Exception {
-		int maxPayloadLength = MAX_FRAME_LENGTH - 6 - mac.getMacLength();
+		int maxPayloadLength = MAX_FRAME_LENGTH - headerLength - macLength;
 		// First frame: max payload length
-		byte[] frame = new byte[6 + maxPayloadLength + mac.getMacLength()];
-		ByteUtils.writeUint16(maxPayloadLength, frame, 4);
-		mac.update(frame, 0, 6 + maxPayloadLength);
-		mac.doFinal(frame, 6 + maxPayloadLength);
+		byte[] frame = new byte[MAX_FRAME_LENGTH];
+		writeHeader(frame, 0L, maxPayloadLength, 0);
+		mac.update(frame, 0, headerLength + maxPayloadLength);
+		mac.doFinal(frame, headerLength + maxPayloadLength);
 		// Second frame: max payload length plus one
-		byte[] frame1 = new byte[6 + maxPayloadLength + 1 + mac.getMacLength()];
-		ByteUtils.writeUint32(1, frame1, 0);
-		ByteUtils.writeUint16(maxPayloadLength + 1, frame1, 4);
-		mac.update(frame1, 0, 6 + maxPayloadLength + 1);
-		mac.doFinal(frame1, 6 + maxPayloadLength + 1);
+		byte[] frame1 = new byte[MAX_FRAME_LENGTH + 1];
+		writeHeader(frame1, 1L, maxPayloadLength + 1, 0);
+		mac.update(frame1, 0, headerLength + maxPayloadLength + 1);
+		mac.doFinal(frame1, headerLength + maxPayloadLength + 1);
 		// Concatenate the frames
 		ByteArrayOutputStream out = new ByteArrayOutputStream();
 		out.write(frame);
@@ -102,19 +104,51 @@ public class ConnectionReaderImplTest extends TestCase {
 		} catch(FormatException expected) {}
 	}
 
+	@Test
+	public void testMaxLengthWithPadding() throws Exception {
+		int maxPayloadLength = MAX_FRAME_LENGTH - headerLength - macLength;
+		int paddingLength = 10;
+		// First frame: max payload length, including padding
+		byte[] frame = new byte[MAX_FRAME_LENGTH];
+		writeHeader(frame, 0L, maxPayloadLength - paddingLength, paddingLength);
+		mac.update(frame, 0, headerLength + maxPayloadLength);
+		mac.doFinal(frame, headerLength + maxPayloadLength);
+		// Second frame: max payload length plus one, including padding
+		byte[] frame1 = new byte[MAX_FRAME_LENGTH + 1];
+		writeHeader(frame1, 1L, maxPayloadLength + 1 - paddingLength,
+				paddingLength);
+		mac.update(frame1, 0, headerLength + maxPayloadLength + 1);
+		mac.doFinal(frame1, headerLength + maxPayloadLength + 1);
+		// Concatenate the frames
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		out.write(frame);
+		out.write(frame1);
+		// Read the first frame
+		ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
+		ConnectionDecrypter d = new NullConnectionDecrypter(in);
+		ConnectionReader r = new ConnectionReaderImpl(d, mac);
+		byte[] read = new byte[maxPayloadLength - paddingLength];
+		TestUtils.readFully(r.getInputStream(), read);
+		// Try to read the second frame
+		byte[] read1 = new byte[maxPayloadLength + 1 - paddingLength];
+		try {
+			TestUtils.readFully(r.getInputStream(), read1);
+			fail();
+		} catch(FormatException expected) {}
+	}
+
 	@Test
 	public void testMultipleFrames() throws Exception {
 		// First frame: 123-byte payload
-		byte[] frame = new byte[6 + 123 + mac.getMacLength()];
-		ByteUtils.writeUint16(123, frame, 4); // Frame number 0, length 123
-		mac.update(frame, 0, 6 + 123);
-		mac.doFinal(frame, 6 + 123);
+		byte[] frame = new byte[8 + 123 + mac.getMacLength()];
+		writeHeader(frame, 0L, 123, 0);
+		mac.update(frame, 0, 8 + 123);
+		mac.doFinal(frame, 8 + 123);
 		// Second frame: 1234-byte payload
-		byte[] frame1 = new byte[6 + 1234 + mac.getMacLength()];
-		ByteUtils.writeUint32(1, frame1, 0); // Frame number 1
-		ByteUtils.writeUint16(1234, frame1, 4); // Length 1234
-		mac.update(frame1, 0, 6 + 1234);
-		mac.doFinal(frame1, 6 + 1234);
+		byte[] frame1 = new byte[8 + 1234 + mac.getMacLength()];
+		writeHeader(frame1, 1L, 1234, 0);
+		mac.update(frame1, 0, 8 + 1234);
+		mac.doFinal(frame1, 8 + 1234);
 		// Concatenate the frames
 		ByteArrayOutputStream out = new ByteArrayOutputStream();
 		out.write(frame);
@@ -133,12 +167,12 @@ public class ConnectionReaderImplTest extends TestCase {
 
 	@Test
 	public void testCorruptPayload() throws Exception {
-		// Six bytes for the header, eight for the payload
-		byte[] frame = new byte[6 + 8 + mac.getMacLength()];
-		ByteUtils.writeUint16(8, frame, 4); // Frame number 0, length 8
+		int payloadLength = 8;
+		byte[] frame = new byte[headerLength + payloadLength + macLength];
+		writeHeader(frame, 0L, payloadLength, 0);
 		// Calculate the MAC
-		mac.update(frame, 0, 6 + 8);
-		mac.doFinal(frame, 6 + 8);
+		mac.update(frame, 0, headerLength + payloadLength);
+		mac.doFinal(frame, headerLength + payloadLength);
 		// Modify the payload
 		frame[12] ^= 1;
 		// Try to read the frame - not a single byte should be read
@@ -153,12 +187,12 @@ public class ConnectionReaderImplTest extends TestCase {
 
 	@Test
 	public void testCorruptMac() throws Exception {
-		// Six bytes for the header, eight for the payload
-		byte[] frame = new byte[6 + 8 + mac.getMacLength()];
-		ByteUtils.writeUint16(8, frame, 4); // Frame number 0, length 8
+		int payloadLength = 8;
+		byte[] frame = new byte[headerLength + payloadLength + macLength];
+		writeHeader(frame, 0L, payloadLength, 0);
 		// Calculate the MAC
-		mac.update(frame, 0, 6 + 8);
-		mac.doFinal(frame, 6 + 8);
+		mac.update(frame, 0, headerLength + payloadLength);
+		mac.doFinal(frame, headerLength + payloadLength);
 		// Modify the MAC
 		frame[17] ^= 1;
 		// Try to read the frame - not a single byte should be read
@@ -171,6 +205,12 @@ public class ConnectionReaderImplTest extends TestCase {
 		} catch(FormatException expected) {}
 	}
 
+	private void writeHeader(byte[] b, long frame, int payload, int padding) {
+		ByteUtils.writeUint32(frame, b, 0);
+		ByteUtils.writeUint16(payload, b, 4);
+		ByteUtils.writeUint16(padding, b, 6);
+	}
+
 	/** A ConnectionDecrypter that performs no decryption. */
 	private static class NullConnectionDecrypter
 	implements ConnectionDecrypter {
diff --git a/test/net/sf/briar/transport/ConnectionWriterImplTest.java b/test/net/sf/briar/transport/ConnectionWriterImplTest.java
index 13677f4a9b..50dbd389dc 100644
--- a/test/net/sf/briar/transport/ConnectionWriterImplTest.java
+++ b/test/net/sf/briar/transport/ConnectionWriterImplTest.java
@@ -23,6 +23,7 @@ import com.google.inject.Injector;
 public class ConnectionWriterImplTest extends TestCase {
 
 	private final Mac mac;
+	private final int headerLength = 8, macLength;
 
 	public ConnectionWriterImplTest() throws Exception {
 		super();
@@ -30,6 +31,7 @@ public class ConnectionWriterImplTest extends TestCase {
 		CryptoComponent crypto = i.getInstance(CryptoComponent.class);
 		mac = crypto.getMac();
 		mac.init(crypto.generateSecretKey());
+		macLength = mac.getMacLength();
 	}
 
 	@Test
@@ -45,12 +47,12 @@ public class ConnectionWriterImplTest extends TestCase {
 
 	@Test
 	public void testSingleByteFrame() throws Exception {
-		// Six bytes for the header, one for the payload
-		byte[] frame = new byte[6 + 1 + mac.getMacLength()];
-		ByteUtils.writeUint16(1, frame, 4); // Payload length = 1
+		int payloadLength = 1;
+		byte[] frame = new byte[headerLength + payloadLength + macLength];
+		writeHeader(frame, 0L, payloadLength, 0);
 		// Calculate the MAC
-		mac.update(frame, 0, 6 + 1);
-		mac.doFinal(frame, 6 + 1);
+		mac.update(frame, 0, headerLength + payloadLength);
+		mac.doFinal(frame, headerLength + payloadLength);
 		// Check that the ConnectionWriter gets the same results
 		ByteArrayOutputStream out = new ByteArrayOutputStream();
 		ConnectionEncrypter e = new NullConnectionEncrypter(out);
@@ -62,7 +64,7 @@ public class ConnectionWriterImplTest extends TestCase {
 
 	@Test
 	public void testFrameIsWrittenAtMaxLength() throws Exception {
-		int maxPayloadLength = MAX_FRAME_LENGTH - 6 - mac.getMacLength();
+		int maxPayloadLength = MAX_FRAME_LENGTH - headerLength - macLength;
 		ByteArrayOutputStream out = new ByteArrayOutputStream();
 		ConnectionEncrypter e = new NullConnectionEncrypter(out);
 		ConnectionWriter w = new ConnectionWriterImpl(e, mac);
@@ -75,22 +77,22 @@ public class ConnectionWriterImplTest extends TestCase {
 		assertEquals(MAX_FRAME_LENGTH, out.size());
 		// Flushing the stream should write a single-byte frame
 		out1.flush();
-		assertEquals(MAX_FRAME_LENGTH + 6 + 1 + mac.getMacLength(), out.size());
+		assertEquals(MAX_FRAME_LENGTH + headerLength + 1 + macLength,
+				out.size());
 	}
 
 	@Test
 	public void testMultipleFrames() throws Exception {
 		// First frame: 123-byte payload
-		byte[] frame = new byte[6 + 123 + mac.getMacLength()];
-		ByteUtils.writeUint16(123, frame, 4);
-		mac.update(frame, 0, 6 + 123);
-		mac.doFinal(frame, 6 + 123);
+		byte[] frame = new byte[headerLength + 123 + macLength];
+		writeHeader(frame, 0L, 123, 0);
+		mac.update(frame, 0, headerLength + 123);
+		mac.doFinal(frame, headerLength + 123);
 		// Second frame: 1234-byte payload
-		byte[] frame1 = new byte[6 + 1234 + mac.getMacLength()];
-		ByteUtils.writeUint32(1, frame1, 0);
-		ByteUtils.writeUint16(1234, frame1, 4);
-		mac.update(frame1, 0, 6 + 1234);
-		mac.doFinal(frame1, 6 + 1234);
+		byte[] frame1 = new byte[headerLength + 1234 + macLength];
+		writeHeader(frame1, 1L, 1234, 0);
+		mac.update(frame1, 0, headerLength + 1234);
+		mac.doFinal(frame1, headerLength + 1234);
 		// Concatenate the frames
 		ByteArrayOutputStream out = new ByteArrayOutputStream();
 		out.write(frame);
@@ -108,6 +110,12 @@ public class ConnectionWriterImplTest extends TestCase {
 		assertTrue(Arrays.equals(expected, actual));
 	}
 
+	private void writeHeader(byte[] b, long frame, int payload, int padding) {
+		ByteUtils.writeUint32(frame, b, 0);
+		ByteUtils.writeUint16(payload, b, 4);
+		ByteUtils.writeUint16(padding, b, 6);
+	}
+
 	/** A ConnectionEncrypter that performs no encryption. */
 	private static class NullConnectionEncrypter
 	implements ConnectionEncrypter {
-- 
GitLab