[Bug]: LocalAddress is not used if peers have different EndPoint

cshiflet commented 2 years ago

What happened?

All clients and servers are using Netmaker v0.12.2. All clients and servers are running the official Docker images

Scenario

A single local network is connected to the Internet by WAN links. Two nodes on the local network use different WAN connections to talk to the Netmaker server.

WAN_1 (WAN_1_IP) <---> Local Network (e.g., 10.0.0.0/24) <---> WAN_2 (WAN_2_IP)
                       - Node_1 (e.g., 10.0.0.1/24)
                       - Node_2 (e.g., 10.0.0.2/24)

Behavior

Instead of the creating a direct WireGuard tunnel using the local address, the WAN endpoint addresses are used. Those values for the nodes are:

Node_1:

EndPoint: WAN_1_IP
LocalAddress: 10.0.0.1

Node_2:

EndPoint: WAN_2_IP
LocalAddress: 10.0.0.2

The output of wg on Node_1 shows:

peer: <Node_2>
  endpoint: WAN_2_IP:<port2>
...

The output of wg on Node_2 shows:

peer: <Node_1>
  endpoint: WAN_1_IP:<port1>
...

Expected Behavior

The two nodes on the same local network establish a direct WireGuard tunnel using their local addresses.

The output of wg would look like this instead. The output of wg on Node_1:

peer: <Node_2>
  endpoint: 10.0.0.2:<port2>
...

The output of wg on Node_2:

peer: <Node_1>
  endpoint: 10.0.0.1:<port1>
...

Cause

The decision to use the local address or end point address on a node is made by checking if the endpoint address of the peer is the same as the endpoint of the current node. In this scenario, the local addresses could be used but are not due to different endpoint values.

The following is the relevant excerpt of code is contained in netclient/server/grpc.go starting on line 152.

        if nodecfg.Endpoint == node.Endpoint {
            if nodecfg.LocalAddress != node.LocalAddress && node.LocalAddress != "" {
                node.Endpoint = node.LocalAddress
            } else {
                continue
            }
        }

Version

v0.12.2

What OS are you using?

Linux

Relevant log output

No response

Contributing guidelines

[X] Yes, I did.

afeiszli commented 2 years ago

Attempt to use a node's local address if the local address is a member of a connected subnet on the current node. Fallback to the endpoint address if a local address tunnel cannot be established.

I like this suggestion but is a bit more difficult than it sounds. We would need to configure the peer with the local address, send a test ping, and then reconfigure with the port if it doesn't work. Will consider but may not put this in.

cshiflet commented 2 years ago

Thanks for your consideration, and I think I understand the difficulty. I'm not intimately familiar with netmaker's architecture, but it looks like it is managing individual Wireguard tunnels. If so, I can see why it is not well suited to attempting multiple connections to a node on internal/external IPs simultaneously.

cshiflet commented 2 years ago

Sorry, I didn't mean to close this. Rather, it is probably best changed to a feature request in case it is of interest later.

mattkasun commented 11 months ago

https://github.com/gravitl/netclient/pull/569

abhishek9686 commented 4 months ago

@cshiflet we have now endpoint detection in place that identifies peers in the same LAN and sets their endpoints to use local addresses to communicate

gravitl / netmaker