[GH-ISSUE #247] newt: ICMP not working when TCP or UDP port restrictions are set #1751

Closed
opened 2026-04-27 22:29:06 -05:00 by GiteaMirror · 1 comment
Owner

Originally created by @rskallies on GitHub (Feb 27, 2026).
Original GitHub issue: https://github.com/fosrl/newt/issues/247

Originally assigned to: @oschwartz10612 on GitHub.

Describe the Bug

Hi,

I use the latest newt 1.10.1. When adding a private resource in Pangolin and only allow a port or port range for TCP or UDP but still allow ICMO then ping does not work.

Environment

  • OS Type & Version: Debian 13
  • Pangolin Version: 1.16.0
  • Gerbil Version: latest
  • Traefik Version: latest
  • Newt Version: 1.10.1
  • Olm Version: latest

To Reproduce

Add a private resource e.g. for the new SSH feature, allow ICMP, allow only port 22. Try to ping the resource.
When the private resource setting is left at its defaults TCP any UDP any ICMP on then ping works.

Expected Behavior

ICMP should work without needing to allow all TCP / UDP ports.

Originally created by @rskallies on GitHub (Feb 27, 2026). Original GitHub issue: https://github.com/fosrl/newt/issues/247 Originally assigned to: @oschwartz10612 on GitHub. ### Describe the Bug Hi, I use the latest newt 1.10.1. When adding a private resource in Pangolin and only allow a port or port range for TCP or UDP but still allow ICMO then ping does not work. ### Environment - OS Type & Version: Debian 13 - Pangolin Version: 1.16.0 - Gerbil Version: latest - Traefik Version: latest - Newt Version: 1.10.1 - Olm Version: latest ### To Reproduce Add a private resource e.g. for the new SSH feature, allow ICMP, allow only port 22. Try to ping the resource. When the private resource setting is left at its defaults TCP any UDP any ICMP on then ping works. ### Expected Behavior ICMP should work without needing to allow all TCP / UDP ports.
GiteaMirror added the bug label 2026-04-27 22:29:06 -05:00
Author
Owner

@rskallies commented on GitHub (Feb 27, 2026):

newt-icmp-fix.patch

newt: ICMP not working when TCP or UDP port restrictions are set

Summary

ICMP (ping) access to a private resource fails whenever TCP or UDP port restrictions
are configured, even when the ICMP toggle is explicitly enabled in the Pangolin UI.
ICMP only works when both TCP and UDP are set to allow all ports.

Affected file

netstack2/proxy.go -- SubnetLookup.Match()

Root cause

The Match() function is the gatekeeper for all incoming packets. It decides whether
a packet should be forwarded to the proxy stack or dropped. For TCP and UDP it checks
port ranges. For ICMP it was supposed to check only the DisableIcmp flag.

The original code checked DisableIcmp first, then fell through to the port range
loop for all other cases, including ICMP:

// Original code
if rule.DisableIcmp && (proto == header.ICMPv4ProtocolNumber || proto == header.ICMPv6ProtocolNumber) {
    return nil
}

if len(rule.PortRanges) == 0 {
    return rule
}

for _, pr := range rule.PortRanges {
    if port >= pr.Min && port <= pr.Max {
        ...
    }
}
return nil

When DisableIcmp is false (ICMP allowed) and port ranges are non-empty, ICMP
packets reached the port range loop. ICMP has no port concept, so Match() is
called with port=0 and proto=ICMPv4ProtocolNumber. Port 0 never matched any
TCP or UDP range, so the function returned nil, blocking the ICMP packet.

When no port restrictions are set (PortRanges is empty), the early return at
len(rule.PortRanges) == 0 fired before the loop, so ICMP happened to work.

The Pangolin server correctly sends disableIcmp: false when the ICMP toggle is
enabled regardless of TCP/UDP port restrictions. The bug was entirely in Match().

Fix

Handle ICMP before the port range check. If the protocol is ICMP, return immediately
based solely on the DisableIcmp flag. Port ranges are irrelevant to ICMP.

if proto == header.ICMPv4ProtocolNumber || proto == header.ICMPv6ProtocolNumber {
    if rule.DisableIcmp {
        return nil
    }
    // ICMP is allowed; port ranges don't apply to ICMP
    return rule
}

Patch

The patch file is newt-icmp-fix.patch next to this document.
Apply with:

git apply newt-icmp-fix.patch

Additional change included in patch

Added disableIcmp to the existing log line in clients/clients.go (three
locations) to make the effective ICMP setting visible in logs:

Added target subnet from X to Y rewrite to Z with port ranges: [...] disableIcmp: false

Verification

After applying the fix, with TCP restricted to port 6565 and ICMP enabled:

newt | INFO: Added target subnet from 100.89.128.8/32 to 100.96.128.11/32
           rewrite to 192.168.17.61/32 with port ranges: [{6565 6565 tcp}] disableIcmp: false
newt | INFO: ICMP Handler: Echo Request from 100.89.128.8 to 192.168.17.61 (ident=973, seq=1)
newt | INFO: ICMP Handler: Matched subnet rule for 100.89.128.8 -> 192.168.17.61
newt | INFO: ICMP Handler: Ping successful to 192.168.17.61 using raw ICMP, injecting reply
newt | INFO: ICMP Handler: Queued echo reply packet for transmission
<!-- gh-comment-id:3973735219 --> @rskallies commented on GitHub (Feb 27, 2026): [newt-icmp-fix.patch](https://github.com/user-attachments/files/25608491/newt-icmp-fix.patch) # newt: ICMP not working when TCP or UDP port restrictions are set ## Summary ICMP (ping) access to a private resource fails whenever TCP or UDP port restrictions are configured, even when the ICMP toggle is explicitly enabled in the Pangolin UI. ICMP only works when both TCP and UDP are set to allow all ports. ## Affected file `netstack2/proxy.go` -- `SubnetLookup.Match()` ## Root cause The `Match()` function is the gatekeeper for all incoming packets. It decides whether a packet should be forwarded to the proxy stack or dropped. For TCP and UDP it checks port ranges. For ICMP it was supposed to check only the `DisableIcmp` flag. The original code checked `DisableIcmp` first, then fell through to the port range loop for all other cases, including ICMP: ```go // Original code if rule.DisableIcmp && (proto == header.ICMPv4ProtocolNumber || proto == header.ICMPv6ProtocolNumber) { return nil } if len(rule.PortRanges) == 0 { return rule } for _, pr := range rule.PortRanges { if port >= pr.Min && port <= pr.Max { ... } } return nil ``` When `DisableIcmp` is false (ICMP allowed) and port ranges are non-empty, ICMP packets reached the port range loop. ICMP has no port concept, so `Match()` is called with `port=0` and `proto=ICMPv4ProtocolNumber`. Port 0 never matched any TCP or UDP range, so the function returned `nil`, blocking the ICMP packet. When no port restrictions are set (`PortRanges` is empty), the early return at `len(rule.PortRanges) == 0` fired before the loop, so ICMP happened to work. The Pangolin server correctly sends `disableIcmp: false` when the ICMP toggle is enabled regardless of TCP/UDP port restrictions. The bug was entirely in `Match()`. ## Fix Handle ICMP before the port range check. If the protocol is ICMP, return immediately based solely on the `DisableIcmp` flag. Port ranges are irrelevant to ICMP. ```go if proto == header.ICMPv4ProtocolNumber || proto == header.ICMPv6ProtocolNumber { if rule.DisableIcmp { return nil } // ICMP is allowed; port ranges don't apply to ICMP return rule } ``` ## Patch The patch file is `newt-icmp-fix.patch` next to this document. Apply with: git apply newt-icmp-fix.patch ## Additional change included in patch Added `disableIcmp` to the existing log line in `clients/clients.go` (three locations) to make the effective ICMP setting visible in logs: Added target subnet from X to Y rewrite to Z with port ranges: [...] disableIcmp: false ## Verification After applying the fix, with TCP restricted to port 6565 and ICMP enabled: newt | INFO: Added target subnet from 100.89.128.8/32 to 100.96.128.11/32 rewrite to 192.168.17.61/32 with port ranges: [{6565 6565 tcp}] disableIcmp: false newt | INFO: ICMP Handler: Echo Request from 100.89.128.8 to 192.168.17.61 (ident=973, seq=1) newt | INFO: ICMP Handler: Matched subnet rule for 100.89.128.8 -> 192.168.17.61 newt | INFO: ICMP Handler: Ping successful to 192.168.17.61 using raw ICMP, injecting reply newt | INFO: ICMP Handler: Queued echo reply packet for transmission
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/newt#1751