Skip to content
This repository has been archived by the owner on Feb 8, 2022. It is now read-only.

nat/natpmp: add low-level Conn and Message APIs #3

Open
mdlayher opened this issue Aug 3, 2020 · 4 comments
Open

nat/natpmp: add low-level Conn and Message APIs #3

mdlayher opened this issue Aug 3, 2020 · 4 comments

Comments

@mdlayher
Copy link
Member

mdlayher commented Aug 3, 2020

Per the discussions with @danderson in #1, there is a need to enable sending/receiving NAT-PMP (and later PCP) messages over a single multiplexed UDP socket.

I think the best way to do this is to add a low-level Conn API (inspired by my NDP package) that looks something like so:

package natpmp

type Conn struct {
    // TODO: do or do not embed this directly to allow access to all deadline/raw byte I/O methods?
    net.PacketConn
}

func NewConn(pc net.PacketConn) (*Conn, error) {
    // Setup logic probably using x/net/ipv4. We still need to think about the multicast group case
    // where a NAT gateway can notify us of its new external IP.
}

type Message interface {
    Op() uint8
    encoding.BinaryMarshaler
    encoding.BinaryUnmarshaler
}

// All messages implement the Message interface.
type ExternalAddressRequest struct{}
type ExternalAddress struct{}
// other messages

// Marshaling to/from bytes while also dealing with message headers and I/O errors.
func ParseMessage(b []byte) (Message, error) {}
func MarshalMessage(m Message) ([]byte, error) {}

// Convenient APIs for dealing with Messages directly, while the underlying Conn also permits raw byte I/O
func (c *Conn) SendMessage(m Message, addr net.Addr) error {}
func (c *Conn) ReceiveMessage() (Message, net.Addr, error) {}

The existing Client can make use of this API in a very concise way and keep all the existing serialization and backoff/retry logic. It's unclear to me if any of that logic should live in Conn directly, but I'm leaning toward keeping it out.

For the Tailscale netcheck use case, it'd be easy to probe for NAT-PMP (and later PCP) using Conn.SendMessage, and then messages could be received using a raw Conn.ReadFrom combined with a call to ParseMessage.

Overall I think this approach provides significant flexibility while also allowing a nice low and high-level APIs. Thoughts, @danderson?

@danderson
Copy link
Member

There's no way in this API for me to deliver messages into the Conn, that's the main missing thing I think.

When using a multiplexed socket, the natpmp package can only use that socket for sending, never for receiving. Something else, in the caller's code, is receiving raw bytes and needs some way to tell "do these bytes look like a NAT-PMP message?" When that happens, it needs an API in this package to deliver those bytes, e.g.

func (c *Conn) DeliverPacket(b []byte, from netaddr.IPPort) {}

Then DeliverByte acts as if those bytes have just been read off the net.PacketConn, and processes them appropriately.

For the use of this package on a multiplexed socket, SendMessage and ReceiveMessage aren't really useful as exported types, in that we never expect to poke at NAT-PMP messages ourselves. The Conn also needs some kind of setting so that the rest of the library never reads from the PacketConn directly, or things will break.

ParseMessage can be used as the packet discriminator in the demultiplexer, but it's likely to be quite expensive, which is bad since it has to run on every packet received. It'd be nice to have some kind of ProbablyNATPMP(b []byte) bool, which only parses far enough into the packet to persuade itself that it might be NAT-PMP (e.g. version byte looks right, checksum if any...).

This is where it gets dicey implementing NAT-PMP as a general purpose library, because you have to co-design the protocol you're multiplexing with so that NAT-PMP is unambiguous compared to the packets in the main protocol.

@mdlayher
Copy link
Member Author

mdlayher commented Aug 3, 2020

I suppose I'm having a hard time understanding: you're saying that NAT-PMP/PCP/etc must be multiplexed with your application protocol on the same socket?

Taking a look at https://tools.ietf.org/html/rfc6886#section-3.3, it seems that you can specify your internal TCP/UDP port so the NAT negotations can happen out of band from the application protocol itself. This would allow the NAT negotiations to happen and open the port for the application protocol. Does this not really work in the real world?

@danderson
Copy link
Member

Hm. I'm puzzled. I was convinced that NAT-PMP and PCP both required the mapping request to come from the internal ip:port that the mapping should forward to. But looking again, I see that the target port is freely selectable in both protocols, and only the IP address is fixed (although PCP also has a THIRD_PARTY extension that allows editing mappings on behalf of some other IP).

So, I guess everything I said in #1 and here is not applicable. This library can run completely independently of anything else. Sorry!

@mdlayher
Copy link
Member Author

mdlayher commented Aug 3, 2020

It's all good! I was having a hard time discerning if your requirements were one or more of:

  • a Tailscale implementation requirement
  • an RFC requirement
  • a real world NAT gateway behavior requirement

I'll keep exploring and see what I can come up with. I do intend to make a type that multiplexes NAT-PMP and PCP as much as possible, but I haven't explored what that might look like yet.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants