Skip to content

fix(packet): allow privileged local ports - #611

Closed
Komzpa wants to merge 2 commits into
traviscross:masterfrom
Komzpa:darafei/allow-low-localport
Closed

fix(packet): allow privileged local ports#611
Komzpa wants to merge 2 commits into
traviscross:masterfrom
Komzpa:darafei/allow-low-localport

Conversation

@Komzpa

@Komzpa Komzpa commented May 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #421.

Stacked on #647.

Summary

  • accept --localport values in the full UDP port range 1..65535 instead of rejecting ports below 1024 in the UI and mtr-packet parser
  • keep shared port bounds in packet/ports.h so UI validation, packet parsing, source-port rotation, and permission checks use the same definitions
  • require raw UDP probes with an explicit privileged local source port to pass a real UDP bind() preflight after capability dropping
  • report a focused error when a low UDP source port is requested but the OS denies the bind
  • document that ports below 1024 only work when the operating system allows the packet helper to bind privileged local ports
  • cover low UDP local-port handling in the existing UDP probe tests

Security note

This branch no longer keeps or re-enables CAP_NET_BIND_SERVICE after startup. With #647 below it, mtr-packet drops all capabilities permanently after privileged socket setup.

Raw UDP packet construction can otherwise place a privileged source port directly in the UDP header. To avoid bypassing the kernel's local-port policy, this branch performs a real UDP bind() check for explicit low local ports on the raw-socket path. A low local port therefore succeeds only if the OS allows the bind without later capability re-enabling; otherwise it fails cleanly with permission-denied.

Validation

@Komzpa
Komzpa force-pushed the darafei/allow-low-localport branch 2 times, most recently from 1d8ec96 to 42c8504 Compare May 8, 2026 03:18
@rewolff

rewolff commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Not entirely sure what you want to do.

My reasoning is that it should not be possible to leverage the elevated privileges that MTR requires to open raw sockets to obtain privileged ports.

So for this to be accepted you need to convince me that this is the case. Just outright refusing to use privileged ports is IMHO safe and reduces functionality only a very little bit. (the "rsh" protocol relies on a privileged source port. And this is no longer used. What are the chances that a port specific routing issue needs to be diagnosed going forward?)

@Komzpa
Komzpa force-pushed the darafei/allow-low-localport branch 2 times, most recently from 63b8cfe to ba6701d Compare May 9, 2026 08:45
@Komzpa

Komzpa commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, that concern makes sense.

This PR came from #421 rather than from a strong preference of mine. The intended behavior is only: if an operator explicitly asks for --localport 80 (or another low UDP source port), mtr should pass that request through instead of rejecting it before the kernel sees it. It is not meant to make mtr-packet's raw-socket privilege sufficient to bind a privileged source port.

The safety boundary I was relying on is the OS bind permission itself: with only the usual raw-socket capability, binding low UDP source ports still fails; it only succeeds when mtr-packet also runs as root or has CAP_NET_BIND_SERVICE. I updated the branch to document that explicitly and added a UDP probe test for local-port 80 under the root-run probe tests.

That said, I do not have a strong stake in changing this policy. If you prefer the existing conservative behavior, rejecting this PR is fine. If #421's use case is worth supporting, this PR is also ready to merge. It is independent of the other packet/port changes; either direction can be accepted or rejected separately.

@Komzpa
Komzpa force-pushed the darafei/allow-low-localport branch 2 times, most recently from 005c567 to 44f2a93 Compare May 9, 2026 09:13
@Komzpa
Komzpa force-pushed the darafei/allow-low-localport branch from 44f2a93 to 401d569 Compare May 9, 2026 10:28
@Komzpa
Komzpa force-pushed the darafei/allow-low-localport branch from 401d569 to 9b4ffa9 Compare May 9, 2026 10:34
@Komzpa
Komzpa force-pushed the darafei/allow-low-localport branch from 9b4ffa9 to a76de3a Compare May 9, 2026 11:12
@rewolff

rewolff commented May 25, 2026

Copy link
Copy Markdown
Collaborator

If we were to allow this, mtr would have to be audited that people don't use mtr to elevate privileges or "mess up the network" (denial of service) by sending "bad" packets onto the network. People will have to find their own solution if they find it necessary to use a privileged port.

@rewolff rewolff closed this May 25, 2026
@Komzpa

Komzpa commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@rewolff I have no preference here but if you do then please also deal with #421 which lead to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

why MinPort?

2 participants