diff --git a/api/net/sf/briar/api/transport/TransportConstants.java b/api/net/sf/briar/api/transport/TransportConstants.java index b8158078bd597ed2eae8c7217fe93ac361ec5224..5e65a43c1d93eb03763b6adbc11461023e1c855d 100644 --- a/api/net/sf/briar/api/transport/TransportConstants.java +++ b/api/net/sf/briar/api/transport/TransportConstants.java @@ -26,4 +26,7 @@ public interface TransportConstants { /** The size of the connection reordering window. */ static final int CONNECTION_WINDOW_SIZE = 32; + + /** The size of the frame reordering window. */ + static final int FRAME_WINDOW_SIZE = 32; } diff --git a/components/net/sf/briar/transport/ConnectionWindowImpl.java b/components/net/sf/briar/transport/ConnectionWindowImpl.java index 09faa909aa986a0cbc197e3fe9fb7dee56527c4d..d20eca411de139ec13e7a44e25e29101c86ee498 100644 --- a/components/net/sf/briar/transport/ConnectionWindowImpl.java +++ b/components/net/sf/briar/transport/ConnectionWindowImpl.java @@ -83,12 +83,12 @@ class ConnectionWindowImpl implements ConnectionWindow { } // Returns the lowest value contained in a window with the given centre - private long getBottom(long centre) { + private static long getBottom(long centre) { return Math.max(0, centre - CONNECTION_WINDOW_SIZE / 2); } // Returns the highest value contained in a window with the given centre - private long getTop(long centre) { + private static long getTop(long centre) { return Math.min(ByteUtils.MAX_32_BIT_UNSIGNED, centre + CONNECTION_WINDOW_SIZE / 2 - 1); } diff --git a/components/net/sf/briar/transport/FrameWindow.java b/components/net/sf/briar/transport/FrameWindow.java new file mode 100644 index 0000000000000000000000000000000000000000..dcb1d75a9e81b78df78113940e8ab8c61355b2aa --- /dev/null +++ b/components/net/sf/briar/transport/FrameWindow.java @@ -0,0 +1,22 @@ +package net.sf.briar.transport; + +interface FrameWindow { + + /** Returns true if the given number is in the window. */ + boolean contains(long frameNumber); + + /** + * Advances the window so it is centred on the given number, unless the + * centre is already greater than the number. Returns false if the window + * is unmodifiable. + */ + boolean advance(long frameNumber); + + /** + * Removes the given number from the window and, if the number is greater + * than or equal to the window's centre, advances the centre to one greater + * than the given number. Returns false if the given number is not in the + * window or the window is unmodifiable. + */ + boolean remove(long frameNumber); +} diff --git a/components/net/sf/briar/transport/FrameWindowImpl.java b/components/net/sf/briar/transport/FrameWindowImpl.java new file mode 100644 index 0000000000000000000000000000000000000000..3348ce73f4fe18a16d86fb9ab3922638b1f16fb8 --- /dev/null +++ b/components/net/sf/briar/transport/FrameWindowImpl.java @@ -0,0 +1,63 @@ +package net.sf.briar.transport; + +import static net.sf.briar.api.transport.TransportConstants.FRAME_WINDOW_SIZE; +import static net.sf.briar.util.ByteUtils.MAX_32_BIT_UNSIGNED; + +import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; + +class FrameWindowImpl implements FrameWindow { + + private final Collection<Long> window; + + private long centre; + + FrameWindowImpl() { + window = new HashSet<Long>(); + for(long l = 0; l < FRAME_WINDOW_SIZE / 2; l++) window.add(l); + centre = 0; + } + + public boolean contains(long frameNumber) { + if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED) + throw new IllegalArgumentException(); + return window.contains(frameNumber); + } + + public boolean advance(long frameNumber) { + if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED) + throw new IllegalArgumentException(); + if(frameNumber <= centre) return true; + // Remove values that have passed out of the window + long newBottom = getBottom(frameNumber); + Iterator<Long> it = window.iterator(); + while(it.hasNext()) if(it.next() < newBottom) it.remove(); + // Add values that have passed into the window + long fillFrom = Math.max(newBottom, getTop(centre) + 1); + long newTop = getTop(frameNumber); + for(long l = fillFrom; l <= newTop; l++) window.add(l); + centre = frameNumber; + return true; + } + + public boolean remove(long frameNumber) { + if(frameNumber < 0 || frameNumber > MAX_32_BIT_UNSIGNED) + throw new IllegalArgumentException(); + if(!window.remove(frameNumber)) return false; + if(frameNumber >= centre && frameNumber < MAX_32_BIT_UNSIGNED) + advance(frameNumber + 1); + return true; + } + + // Returns the lowest value contained in a window with the given centre + private static long getBottom(long centre) { + return Math.max(0, centre - FRAME_WINDOW_SIZE / 2); + } + + // Returns the highest value contained in a window with the given centre + private static long getTop(long centre) { + return Math.min(MAX_32_BIT_UNSIGNED, + centre + FRAME_WINDOW_SIZE / 2 - 1); + } +} diff --git a/components/net/sf/briar/transport/HeaderEncoder.java b/components/net/sf/briar/transport/HeaderEncoder.java index b1d47a6c1b5bed306dc2b8db82751b02ba80d76b..18b3aecef2546883fcf1d719c663701bd09dac0d 100644 --- a/components/net/sf/briar/transport/HeaderEncoder.java +++ b/components/net/sf/briar/transport/HeaderEncoder.java @@ -22,9 +22,8 @@ class HeaderEncoder { ByteUtils.writeUint16(padding, header, 6); } - static boolean validateHeader(byte[] header, long frameNumber) { + static boolean validateHeader(byte[] header) { if(header.length < FRAME_HEADER_LENGTH) return false; - if(ByteUtils.readUint32(header, 0) != frameNumber) return false; int payload = ByteUtils.readUint16(header, 4); int padding = ByteUtils.readUint16(header, 6); int frameLength = FRAME_HEADER_LENGTH + payload + padding + MAC_LENGTH; diff --git a/components/net/sf/briar/transport/IncomingAuthenticationLayer.java b/components/net/sf/briar/transport/IncomingAuthenticationLayer.java index c9559537fb9ef780be1e5e937898c643bb37f369..22dd0df36510f5961c3cbc5012b66c5d204eabb1 100644 --- a/components/net/sf/briar/transport/IncomingAuthenticationLayer.java +++ b/components/net/sf/briar/transport/IncomingAuthenticationLayer.java @@ -1,7 +1,6 @@ package net.sf.briar.transport; import java.io.IOException; -import java.util.Collection; interface IncomingAuthenticationLayer { @@ -14,6 +13,6 @@ interface IncomingAuthenticationLayer { * @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, FrameWindow window) throws IOException, InvalidDataException; } diff --git a/components/net/sf/briar/transport/IncomingAuthenticationLayerImpl.java b/components/net/sf/briar/transport/IncomingAuthenticationLayerImpl.java index 14320757c345e9c09ada122b0dc3d125604cd6f4..4c33c29e155d0ae961ce0c8737ee3d1944a4c5e9 100644 --- a/components/net/sf/briar/transport/IncomingAuthenticationLayerImpl.java +++ b/components/net/sf/briar/transport/IncomingAuthenticationLayerImpl.java @@ -5,7 +5,6 @@ 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; @@ -31,15 +30,13 @@ class IncomingAuthenticationLayerImpl implements IncomingAuthenticationLayer { throw new IllegalArgumentException(); } - public boolean readFrame(Frame f, Collection<Long> window) - throws IOException, InvalidDataException { + public boolean readFrame(Frame f, FrameWindow 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(); + if(!HeaderEncoder.validateHeader(buf)) throw new InvalidDataException(); // Check that the payload and padding lengths are correct int payload = HeaderEncoder.getPayloadLength(buf); int padding = HeaderEncoder.getPaddingLength(buf); @@ -58,7 +55,6 @@ class IncomingAuthenticationLayerImpl implements IncomingAuthenticationLayer { if(expectedMac[i] != buf[macStart + i]) throw new InvalidDataException(); } - frameNumber++; return true; } } diff --git a/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java b/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java index bd555c150cd66c33757e20d1bccf91ff4cd8adfc..e49e0cbb9fe5ca344c29459695ec31f8e66cd521 100644 --- a/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java +++ b/components/net/sf/briar/transport/IncomingErrorCorrectionLayer.java @@ -1,7 +1,6 @@ package net.sf.briar.transport; import java.io.IOException; -import java.util.Collection; interface IncomingErrorCorrectionLayer { @@ -14,6 +13,6 @@ interface IncomingErrorCorrectionLayer { * @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, FrameWindow window) throws IOException, InvalidDataException; } diff --git a/components/net/sf/briar/transport/NullFrameWindow.java b/components/net/sf/briar/transport/NullFrameWindow.java new file mode 100644 index 0000000000000000000000000000000000000000..c8560de86161b2ecca76a325d2b00255d6bff3c0 --- /dev/null +++ b/components/net/sf/briar/transport/NullFrameWindow.java @@ -0,0 +1,20 @@ +package net.sf.briar.transport; + +class NullFrameWindow implements FrameWindow { + + private long centre = 0L; + + public boolean contains(long frameNumber) { + return frameNumber == centre; + } + + public boolean advance(long frameNumber) { + return frameNumber == centre; + } + + public boolean remove(long frameNumber) { + if(frameNumber != centre) return false; + centre++; + return true; + } +} diff --git a/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java b/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java index 7d344a592909cbe11cffa291a00edcb71e1386de..87ce8d4b358ee0bb4d2dc76b248fc8a4d9e37c69 100644 --- a/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java +++ b/components/net/sf/briar/transport/NullIncomingErrorCorrectionLayer.java @@ -1,7 +1,6 @@ package net.sf.briar.transport; import java.io.IOException; -import java.util.Collection; import net.sf.briar.api.transport.Segment; @@ -15,7 +14,7 @@ class NullIncomingErrorCorrectionLayer implements IncomingErrorCorrectionLayer { segment = new SegmentImpl(); } - public boolean readFrame(Frame f, Collection<Long> window) + public boolean readFrame(Frame f, FrameWindow window) throws IOException, InvalidDataException { while(true) { if(!in.readSegment(segment)) return false; diff --git a/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java b/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java index b8b965fc17bac7a238239337b3cbcb8e1141ef66..3ecd84502c6bd9081f2767c1bfa7a774144903b5 100644 --- a/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java +++ b/components/net/sf/briar/transport/NullIncomingReliabilityLayer.java @@ -1,24 +1,22 @@ 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; + private final FrameWindow window; NullIncomingReliabilityLayer(IncomingAuthenticationLayer in) { this.in = in; + window = new NullFrameWindow(); } 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++; + long frameNumber = HeaderEncoder.getFrameNumber(f.getBuffer()); + if(!window.remove(frameNumber) && window.advance(frameNumber)) + throw new RuntimeException(); return true; } } diff --git a/test/build.xml b/test/build.xml index a92959b5055890ff9efb1a7f154ce7561edbd5e2..3998eadb1f6db632eed0e39e80c54f451b3184ab 100644 --- a/test/build.xml +++ b/test/build.xml @@ -56,6 +56,7 @@ <test name='net.sf.briar.transport.ConnectionWriterImplTest'/> <test name='net.sf.briar.transport.ConnectionWriterTest'/> <test name='net.sf.briar.transport.FrameReadWriteTest'/> + <test name='net.sf.briar.transport.FrameWindowImplTest'/> <test name='net.sf.briar.transport.IncomingEncryptionLayerImplTest'/> <test name='net.sf.briar.transport.IncomingSegmentedEncryptionLayerTest'/> <test name='net.sf.briar.transport.OutgoingEncryptionLayerImplTest'/> diff --git a/test/net/sf/briar/transport/FrameWindowImplTest.java b/test/net/sf/briar/transport/FrameWindowImplTest.java new file mode 100644 index 0000000000000000000000000000000000000000..1cc08245701af5501d48d0df6d78d4db22cb7df9 --- /dev/null +++ b/test/net/sf/briar/transport/FrameWindowImplTest.java @@ -0,0 +1,89 @@ +package net.sf.briar.transport; + +import net.sf.briar.BriarTestCase; +import static net.sf.briar.api.transport.TransportConstants.FRAME_WINDOW_SIZE; +import static net.sf.briar.util.ByteUtils.MAX_32_BIT_UNSIGNED; + +import org.junit.Test; + +public class FrameWindowImplTest extends BriarTestCase { + + @Test + public void testWindowSliding() { + FrameWindow w = new FrameWindowImpl(); + for(int i = 0; i < 100; i++) { + assertTrue(w.contains(i)); + w.remove(i); + assertFalse(w.contains(i)); + } + } + + @Test + public void testWindowJumping() { + FrameWindow w = new FrameWindowImpl(); + for(int i = 0; i < 100; i += 13) { + assertTrue(w.contains(i)); + w.remove(i); + assertFalse(w.contains(i)); + } + } + + @Test + public void testWindowUpperLimit() { + FrameWindow w = new FrameWindowImpl(); + // Centre is 0, highest value in window is 15 + for(int i = 0; i < 16; i++) assertTrue(w.contains(i)); + assertFalse(w.remove(16)); + assertTrue(w.remove(15)); + assertFalse(w.remove(15)); + // Centre is 16, highest value in window is 31 + for(int i = 0; i < 32; i++) assertEquals(i != 15, w.contains(i)); + assertFalse(w.remove(32)); + assertTrue(w.remove(31)); + // Values greater than 2^32 - 1 should never be allowed + assertTrue(w.advance(MAX_32_BIT_UNSIGNED - 1)); + assertTrue(w.remove(MAX_32_BIT_UNSIGNED - 1)); + assertTrue(w.remove(MAX_32_BIT_UNSIGNED)); + try { + w.remove(MAX_32_BIT_UNSIGNED + 1); + fail(); + } catch(IllegalArgumentException expected) {} + } + + @Test + public void testAdvance() { + FrameWindow w = new FrameWindowImpl(); + long centre = 0; + for(int i = 0; i < 10; i++) { + long bottom = Math.max(0, centre - FRAME_WINDOW_SIZE / 2); + long top = Math.min(MAX_32_BIT_UNSIGNED, + centre + FRAME_WINDOW_SIZE / 2 - 1); + if(bottom > 0) assertFalse(w.contains(bottom - 1)); + for(long j = bottom; j <= top; j++) assertTrue(w.contains(j)); + if(top < MAX_32_BIT_UNSIGNED) assertFalse(w.contains(top + 1)); + centre += 12345; + assertTrue(w.advance(centre)); + } + } + + @Test + public void testWindowLowerLimit() { + FrameWindow w = new FrameWindowImpl(); + // Centre is 0, negative values should never be allowed + try { + w.remove(-1); + fail(); + } catch(IllegalArgumentException expected) {} + // Slide the window twice + assertTrue(w.remove(15)); + assertTrue(w.remove(16)); + // Centre is 17, lowest value in window is 1 + assertFalse(w.remove(0)); + assertTrue(w.remove(1)); + // Slide the window + assertTrue(w.remove(25)); + // Centre is 26, lowest value in window is 10 + assertFalse(w.remove(9)); + assertTrue(w.remove(10)); + } +} \ No newline at end of file