From b375929cdbbaf664cf4e20bf27ac1957fea7336b Mon Sep 17 00:00:00 2001 From: Janus Varmarken Date: Tue, 10 Jul 2018 19:30:30 -0700 Subject: [PATCH] Bug-fix: Use seperate, direction-dependent sets of sequence numbers (used when determining if a segment is a retransmission) as client and server may (after a while) end up using a sequence number that the other party has already used for sending a packet in the opposite direction. --- .../java/edu/uci/iotproject/Conversation.java | 117 ++++++++++++++++-- 1 file changed, 107 insertions(+), 10 deletions(-) diff --git a/Code/Projects/SmartPlugDetector/src/main/java/edu/uci/iotproject/Conversation.java b/Code/Projects/SmartPlugDetector/src/main/java/edu/uci/iotproject/Conversation.java index 66e7804..a100894 100644 --- a/Code/Projects/SmartPlugDetector/src/main/java/edu/uci/iotproject/Conversation.java +++ b/Code/Projects/SmartPlugDetector/src/main/java/edu/uci/iotproject/Conversation.java @@ -43,15 +43,24 @@ public class Conversation { private final int mServerPort; /** - * The list of packets pertaining to this conversation. + * The list of packets (with payload) pertaining to this conversation. */ private final List mPackets; /** - * Contains the sequence numbers seen so far for this {@code Conversation}. + * Contains the sequence numbers used thus far by the host that is considered the client in this + * {@code Conversation}. * Used for filtering out retransmissions. */ - private final Set mSeqNumbers; + private final Set mSeqNumbersClient; + + /** + * Contains the sequence numbers used thus far by the host that is considered the server in this + * {@code Conversation}. + * Used for filtering out retransmissions. + */ + private final Set mSeqNumbersSrv; + /** * List of pairs FINs and their corresponding ACKs associated with this conversation. @@ -73,7 +82,10 @@ public class Conversation { this.mServerIp = serverIp; this.mServerPort = serverPort; this.mPackets = new ArrayList<>(); - this.mSeqNumbers = new HashSet<>(); + + this.mSeqNumbersClient = new HashSet<>(); + this.mSeqNumbersSrv = new HashSet<>(); + this.mFinPackets = new ArrayList<>(); } @@ -89,15 +101,12 @@ public class Conversation { public void addPacket(PcapPacket packet, boolean ignoreRetransmissions) { // Precondition: verify that packet does indeed pertain to conversation. onAddPrecondition(packet); - // For now we only support TCP flows. - TcpPacket tcpPacket = Objects.requireNonNull(packet.get(TcpPacket.class)); - int seqNo = tcpPacket.getHeader().getSequenceNumber(); - if (ignoreRetransmissions && mSeqNumbers.contains(seqNo)) { + if (ignoreRetransmissions && isRetransmission(packet)) { // Packet is a retransmission. Ignore it. return; } - // Update set of sequence numbers seen so far with sequence number of new packet. - mSeqNumbers.add(seqNo); + // Select direction-dependent set of sequence numbers seen so far and update it with sequence number of new packet. + addSeqNumber(packet); // Finally add packet to list of packets pertaining to this conversation. mPackets.add(packet); } @@ -216,4 +225,92 @@ public class Conversation { } } + /** + *

+ * Determines if the TCP packet contained in {@code packet} is a retransmission of a previously seen (logged) + * packet. + *

+ * + * + * TODO: + * the current implementation, which uses a set of previously seen sequence numbers, will consider a segment + * with a reused sequence number---occurring as a result of sequence number wrap around for a very long-lived + * connection---as a retransmission (and may therefore end up discarding it even though it is in fact NOT a + * retransmission). Ideas? + * + * + * @param packet The packet. + * @return {@code true} if {@code packet} was determined to be a retransmission, {@code false} otherwise. + */ + private boolean isRetransmission(PcapPacket packet) { + // Extract sequence number. + int seqNo = packet.get(TcpPacket.class).getHeader().getSequenceNumber(); + switch (getDirection(packet)) { + case CLIENT_TO_SERVER: + return mSeqNumbersClient.contains(seqNo); + case SERVER_TO_CLIENT: + return mSeqNumbersSrv.contains(seqNo); + default: + throw new RuntimeException(String.format("Unexpected value of enum '%s'", + Direction.class.getSimpleName())); + } + } + + /** + * Extracts the TCP sequence number from {@code packet} and adds it to the proper set of sequence numbers by + * analyzing the direction of the packet. + * @param packet A TCP packet (wrapped in a {@code PcapPacket}) that was added to this conversation and whose + * sequence number is to be recorded as seen. + */ + private void addSeqNumber(PcapPacket packet) { + // Note: below check is redundant if client code is correct as the call to check the precondition should already + // have been made by the addXPacket method that invokes this method. As such, the call below may be removed in + // favor of speed, but the improvement will be minor, hence the added safety may be worth it. + onAddPrecondition(packet); + // Extract sequence number. + int seqNo = packet.get(TcpPacket.class).getHeader().getSequenceNumber(); + // Determine direction of packet and add packet's sequence number to corresponding set of sequence numbers. + switch (getDirection(packet)) { + case CLIENT_TO_SERVER: + // Client to server packet. + mSeqNumbersClient.add(seqNo); + break; + case SERVER_TO_CLIENT: + // Server to client packet. + mSeqNumbersSrv.add(seqNo); + break; + default: + throw new RuntimeException(String.format("Unexpected value of enum '%s'", + Direction.class.getSimpleName())); + } + } + + /** + * Determine the direction of {@code packet}. + * @param packet The packet whose direction is to be determined. + * @return A {@link Direction} indicating the direction of the packet. + */ + private Direction getDirection(PcapPacket packet) { + IpV4Packet ipPacket = packet.get(IpV4Packet.class); + String ipSrc = ipPacket.getHeader().getSrcAddr().getHostAddress(); + String ipDst = ipPacket.getHeader().getDstAddr().getHostAddress(); + // Determine direction of packet. + if (ipSrc.equals(mClientIp) && ipDst.equals(mServerIp)) { + // Client to server packet. + return Direction.CLIENT_TO_SERVER; + } else if (ipSrc.equals(mServerIp) && ipDst.equals(mClientIp)) { + // Server to client packet. + return Direction.SERVER_TO_CLIENT; + } else { + throw new IllegalArgumentException("getDirection: packet not related to " + getClass().getSimpleName()); + } + } + + /** + * Utility enum for expressing the direction of a packet pertaining to this {@code Conversation}. + */ + private enum Direction { + CLIENT_TO_SERVER, SERVER_TO_CLIENT + } + } \ No newline at end of file -- 2.34.1