Skip to content

Commit 2d514dc

Browse files
committed
Re-ack sessions when duplicate SYN packets are received
1 parent 69ed62e commit 2d514dc

File tree

2 files changed

+33
-11
lines changed

2 files changed

+33
-11
lines changed

app/src/main/java/tech/httptoolkit/android/vpn/SessionHandler.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,20 @@ private void sendAck(IPv4Header ipheader, TCPHeader tcpheader, int acceptedDataL
306306
writer.write(data);
307307
}
308308

309+
/**
310+
* resend the last acknowledgment packet to VPN client, e.g. when an unexpected out of order
311+
* packet arrives.
312+
* @param session Session
313+
*/
314+
private void resendAck(Session session){
315+
byte[] data = TCPPacketFactory.createResponseAckData(
316+
session.getLastIpHeader(),
317+
session.getLastTcpHeader(),
318+
session.getRecSequence()
319+
);
320+
writer.write(data);
321+
}
322+
309323
private void sendAckForDisorder(IPv4Header ipHeader, TCPHeader tcpheader, int acceptedDataLength) {
310324
long ackNumber = tcpheader.getSequenceNumber() + acceptedDataLength;
311325
Log.d(TAG,"sent disorder ack, ack# " + tcpheader.getSequenceNumber() +
@@ -378,6 +392,14 @@ private void replySynAck(IPv4Header ip, TCPHeader tcp) throws IOException {
378392
ip.getSourceIP(), tcp.getSourcePort()
379393
);
380394

395+
if (session.getLastIpHeader() != null) {
396+
// We have an existing session for this connection! We've somehow received a SYN
397+
// for an existing socket (or some kind of other race). We resend the last ACK
398+
// for this session, rejecting this SYN. Not clear why this happens, but it can.
399+
resendAck(session);
400+
return;
401+
}
402+
381403
synchronized (session) {
382404
session.setMaxSegmentSize(tcpheader.getMaxSegmentSize());
383405
session.setSendUnack(tcpheader.getSequenceNumber());

app/src/main/java/tech/httptoolkit/android/vpn/SessionManager.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,10 @@ public void closeSession(@NonNull Session session){
126126
public Session createNewUDPSession(int ip, int port, int srcIp, int srcPort) throws IOException {
127127
String keys = Session.getSessionKey(ip, port, srcIp, srcPort);
128128

129-
if (table.containsKey(keys)) {
130-
// For TCP, we freak out if you try to create an already existing session.
131-
// With UDP though, it's totally fine:
132-
return table.get(keys);
133-
}
129+
// For TCP, we freak out if you try to create an already existing session.
130+
// With UDP though, it's totally fine:
131+
Session existingSession = table.get(keys);
132+
if (existingSession != null) return existingSession;
134133

135134
Session session = new Session(srcIp, srcPort, ip, port, this);
136135

@@ -162,12 +161,13 @@ public Session createNewUDPSession(int ip, int port, int srcIp, int srcPort) thr
162161
@NotNull
163162
public Session createNewTCPSession(int ip, int port, int srcIp, int srcPort) throws IOException {
164163
String key = Session.getSessionKey(ip, port, srcIp, srcPort);
165-
if (table.containsKey(key)) {
166-
// This can happen if we receive two SYN packets somehow. That shouldn't happen, especially
167-
// given that our connection is local & should be 100% reliable, but it does. We probably
168-
// should RST, for now we just throw here (and so drop the packet entirely).
169-
throw new IllegalArgumentException("Can't create duplicate session");
170-
}
164+
165+
Session existingSession = table.get(key);
166+
167+
// This can happen if we receive two SYN packets somehow. That shouldn't happen,
168+
// given that our connection is local & should be 100% reliable, but it can.
169+
// We return the initialized session, which will be reacked to indicate rejection.
170+
if (existingSession != null) return existingSession;
171171

172172
Session session = new Session(srcIp, srcPort, ip, port, this);
173173

0 commit comments

Comments
 (0)