Skip to content

Commit f5aa55a

Browse files
committed
Don't report errors for ICMP type 3 packets
These are noisy and annoying, and although it's unclear where they come from, there's no sensible way to handle them other than simply dropping them, since we can't actually speak ICMP upstream.
1 parent 16dbcc3 commit f5aa55a

File tree

4 files changed

+24
-10
lines changed

4 files changed

+24
-10
lines changed

app/src/main/java/tech/httptoolkit/android/ProxyVpnService.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ class ProxyVpnService : VpnService(), IProtectSocket {
241241

242242
SocketProtector.getInstance().setProtector(this)
243243

244+
245+
246+
// TODO: Should we support *?
247+
244248
vpnRunnable = ProxyVpnRunnable(
245249
vpnInterface,
246250
proxyConfig.ip,

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,18 @@ private void handleICMPPacket(
424424
final ICMPPacket requestPacket = ICMPPacketFactory.parseICMPPacket(clientPacketData);
425425
Log.d(TAG, "Got an ICMP ping packet, type " + requestPacket.toString());
426426

427+
if (requestPacket.type == ICMPPacket.DESTINATION_UNREACHABLE_TYPE) {
428+
// This is a packet from the phone, telling somebody that a destination is unreachable.
429+
// Might be caused by issues on our end, but it's unclear what kind of issues. Regardless,
430+
// we can't send ICMP messages ourselves or react usefully, so we drop these silently.
431+
return;
432+
} else if (requestPacket.type != ICMPPacket.ECHO_REQUEST_TYPE) {
433+
// We only actually support outgoing ping packets. Loudly drop anything else:
434+
throw new PacketHeaderException(
435+
"Unknown ICMP type (" + requestPacket.type + "). Only echo requests are supported"
436+
);
437+
}
438+
427439
pingThreadpool.execute(new Runnable() {
428440
@Override
429441
public void run() {

app/src/main/java/tech/httptoolkit/android/vpn/transport/icmp/ICMPPacket.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,16 @@
33
import tech.httptoolkit.android.vpn.transport.PacketHeaderException;
44

55
public class ICMPPacket {
6+
// Two ICMP packets we can handle: simple ping & pong
67
public static final byte ECHO_REQUEST_TYPE = 8;
78
public static final byte ECHO_SUCCESS_TYPE = 0;
89

9-
final byte type;
10+
// One very common packet we ignore: connection rejection. Unclear why this happens,
11+
// random incoming connections that the phone tries to reply to? Nothing we can do though,
12+
// as we can't forward ICMP onwards, and we can't usefully respond or react.
13+
public static final byte DESTINATION_UNREACHABLE_TYPE = 3;
14+
15+
public final byte type;
1016
final byte code; // 0 for request, 0 for success, 0 - 15 for error subtypes
1117

1218
final int checksum;
@@ -23,10 +29,6 @@ public class ICMPPacket {
2329
int sequenceNumber,
2430
byte[] data
2531
) throws PacketHeaderException {
26-
if (type != ECHO_REQUEST_TYPE && type != ECHO_SUCCESS_TYPE) {
27-
throw new PacketHeaderException("ICMP packet with id must be request or response");
28-
}
29-
3032
this.type = (byte) type;
3133
this.code = (byte) code;
3234
this.checksum = checksum;

app/src/main/java/tech/httptoolkit/android/vpn/transport/icmp/ICMPPacketFactory.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,7 @@ public static ICMPPacket parseICMPPacket(@NonNull ByteBuffer stream) throws Pack
2424
final byte[] data = new byte[stream.remaining()];
2525
stream.get(data);
2626

27-
if (type == 8) {
28-
return new ICMPPacket(type, code, checksum, identifier, sequenceNumber, data);
29-
} else {
30-
throw new PacketHeaderException("Unknown ICMP type (" + type + "). Only echo requests are supported");
31-
}
27+
return new ICMPPacket(type, code, checksum, identifier, sequenceNumber, data);
3228
}
3329

3430
public static ICMPPacket buildSuccessPacket(ICMPPacket requestPacket) throws PacketHeaderException {

0 commit comments

Comments
 (0)