libpcp / pcp

PCP client library
BSD 2-Clause "Simplified" License
69 stars 30 forks source link

Hardcoded source address selection per server considered harmful #10

Open fingon opened 10 years ago

fingon commented 10 years ago

I'm dealing mostly with multi-prefix home networks. In that case, on e.g. eth0, you may have 2001:db8:1::1/64 and 2001:db8:2::1/64 addresses on that same interface.

The pcp_server_discovery.c seems to assume that you can use exactly one address to communicate with a server (which is not obviously true). Any insight on how this could be fixed?

From my point of view, the correct way to handle this would be to bind to any socket, but then use specific from address that matches the 'internal' IP address; if that's not possible, avoid using that server.

fingon commented 10 years ago

It's bit more widespread than that I think. Example: Client choosing choosing to an use internal address X. But because X is not next-hop to any server S, client gets 'unable to create flow' and is not happy.

(And yes, address X is configured on the box).

Probably happens with multiple interfaces easily as well..

Correct approach would be not to have the src_ip in the server data-structure at all, but instead have it per-flow, and use sendmsg with explicit source address when sending packets to the server. (And check it when receiving).

The diff based on brief examination of the code would be bit too big for me to write (I'm just testing proxy-server infra), but would be nice to have :)

fingon commented 10 years ago

Also, if libpcp is long-lived, the hardcoded source address won't be valid necessarily all the time..

fingon commented 10 years ago

And in IPv6 case, default auto-discovered PCP server is linklocal typically => you will not be able to use it at all with IPv6 by default as any useful IPv6 internal address is not linklocal the code would want.. (next hop = linklocal most of the time)

libpcp commented 10 years ago

Thank for your input. You are right. I'll take a proper look at this.

fingon commented 10 years ago

On sendto -> sendmsg front, two examples: https://github.com/miniupnp/miniupnp/commit/93d7bb6ae29b8f9fe294f2847b61a40c8de42877 or https://github.com/fingon/minimalist-pcproxy/blob/master/udp46.c#L228 .. additionally decouple src_ip from pcp server structure, and it should 'just work', I think.

ziqin commented 1 month ago

10 years later, I found the same problem.

Is there any progress on this issue?