Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAT-PMP implementation violates RFC 6886. #97

Closed
Yawning opened this issue Oct 23, 2014 · 0 comments
Closed

NAT-PMP implementation violates RFC 6886. #97

Yawning opened this issue Oct 23, 2014 · 0 comments

Comments

Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
@Yawning
Copy link

@Yawning Yawning commented Oct 23, 2014

This is a moot point from my perspective (as a client implementer) since I can't rely on routers acutally updating, but for the sake of documentation, the mapping request processing in miniupnpd does not handle non-wildcard mapping removal correctly.

RFC 6886 states:

A client MAY also send an explicit packet to request deletion of a
mapping that is no longer needed. A client requests explicit
deletion of a mapping by sending a message to the NAT gateway
requesting the mapping, with the Requested Lifetime in Seconds set to
zero. The Suggested External Port MUST be set to zero by the client
on sending, and MUST be ignored by the gateway on reception.

So the correct layout of the UDP body for a single mapping removal (TCP) would be:

Vers: 00
OP: 01 Map TCP (Map UDP is equally broken)
Reserved: 00 00
Internal Port: <whatever>
Suggested External Port: 00 00
Requested Port Mapping Lifetime in Seconds: 00 00 00 00

natpmp.c:ProcessIncomingNATPMPPacket() will handle this like:

  • Look at eport and sets it to iport since eport is 0.
  • Remove the port forwarding entry based on proto (TCP) and eport, which is actually the client's internal (LAN side) port.

Assuming that eport == iport when handling mapping requests is flat out wrong, because there is no requirement anywhere that they must be equal. In the best case, the mapping removal fails with a Not Authorized/Refused status code. If another mapping that happens to use the client's iport as the eport exists, the incorrect mapping (possibly belonging to a different process on a different client host) will be removed.

The client side workaround, assuming every implementation of NAT-PMP is equally broken, would be to ignore the "The Suggested External Port MUST be set to zero by the client on sending" clause, but that's not an assumption I'm personally willing to make given that the clause has been present in the protocol specification from the initial IETF draft (Dating back to 2005).

The correct fix would be to remove the mapping based on the client address/iport pair, but I'm not sure how easy it would be to do (and I can't rely on that behavior).

@miniupnp miniupnp closed this in 350ca19 Oct 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment