pymodbus-dev / pymodbus

A full modbus protocol written in python
Other
2.22k stars 907 forks source link

Bugfix Fix: default source_address invalid for IPv6 #1914

Closed couling closed 8 months ago

couling commented 8 months ago

This PR seeks to remove hardcoded IP addresses from pymodbus wherever possible, though it doesn't quite succeed.

The two addresses which were hard-coded were:

0.0.0.0

This has a complex history.

In this context is was being used to mean "any address" but this was incorrect, especially when used with a client. As it's an IPv4 address, it cannot be used to mean "any source address" when connecting to an IPv6 server. See issue #1881. The most compatible way to connect a client using any source address at the low level is to skip calling bind(). Python will do this if source_address is not set or set to None and python also determins the family (AF_INET or AF_INET6) automatically from the server's address.

For servers the story is a lot more complex. You cannot simply skip binding a socket before you listen. Nor can you entirely avoid picking AF_INET or AF_INET6 as the socket must exist listening to one or the other.

I'm still unclear on what the right way to listen on any IPv6 or IPv4 address is:

127.0.0.1

This is the IPv4 loopback address. OS are beginning to use IPv6 by default over IPv4 meaning that localhost can translate to ::1 instead of 127.0.0.1. Hard coding the address to 127.0.0.1 creates a trip hazard especially for the server where users may spin up a server expecting it to be available on localhost and finding their connections getting rejected as if the server doesn't exist.

Here it is much better to use the hostname and allow the OS to be consistent in which IP stack to use.

Covered by this PR:

Not covered by this PR

janiversen commented 8 months ago

Please do not try to remake all of pymodbus in one pull request, that makes it hard to bisect if we later have a problem.

Simulator and http would be better in seperate pull requests

couling commented 8 months ago

Rest assured I'm not: I'm trying to remove the same problem from multiple places, I'll trim this down a bit because I don't want to include anything for the HTTP server. That was over zealous of me.

Theres a couple of knock on issues I'm having with the modbus server because it doesn't interpret None the same way the client does for source_address and the same config class is used for both with the source_address reinterpreted as listening address.

I think I know the right way to get the modbus server to behave when it sees None but it needs a lot more testing.

I'll set the PR to "ready for review" when done.

janiversen commented 8 months ago

Good luck, looking forward to make the formal review

couling commented 8 months ago

It looks like the 3.12 tests are broken in the CI.

janiversen commented 8 months ago

Seems the python action have a problem, investigating.