JoelBender / bacpypes

BACpypes provides a BACnet application layer and network layer written in Python for daemons, scripting, and graphical interfaces.
MIT License
303 stars 129 forks source link

Issues with ReadPropertyForeign and Unconfirmed-REQ #450

Open DB-CL opened 2 years ago

DB-CL commented 2 years ago

Description

When the BACnet network I'm dealing with is sending some random Unconfirmed-REQ, the ReadPropertyForeign client enter in a weird state and sends the packets to the wrong IP (or port, depending the setup).

How to reproduce

This is very easy to reproduce for me, but I think this is hard for anyone else because I have no idea where the Unconfirmed-REQ come from.

Details

Here is the wireshark when everything is going well :

10.0.0.1:47808 -> 10.0.0.20:37755 BVLC
10.0.0.20:37755 -> 10.0.0.1:47808 BVLC
---- here I launch the command read
10.0.0.1:47808 -> 10.0.0.20:37755 Who-is-router-to-network
10.0.0.20:37755 -> 10.0.0.1:47808 I-Am-router-to-network
10.0.0.1:47808 -> 10.0.0.20:37755 readProperty [ 1 ] analog-value,16 present value
10.0.0.20:37755 -> 10.0.0.1:47808 complex ACK

Here is the wireshark when it goes wrong :

10.0.0.1:47808 -> 10.0.0.20:37755 BVLC
10.0.0.20:37755 -> 10.0.0.1:47808 BVLC
10.0.0.20:37755 -> 10.0.0.1:47808 Unconfirmed-REQ who-Is 40629-40629
10.0.0.20:37755 -> 10.0.0.1:47808 Unconfirmed-REQ who-Is 30705-30705
10.0.0.20:37755 -> 10.0.0.1:47808 Unconfirmed-REQ who-Is 30711-30711
---- here I launch the command read
10.0.0.1:47808 -> 10.0.0.20:47808 readProperty [ 1 ] analog-value,16 present value
10.0.0.1:47808 -> 10.0.0.20:47808 readProperty [ 1 ] analog-value,16 present value
10.0.0.1:47808 -> 10.0.0.20:47808 readProperty [ 1 ] analog-value,16 present value

As you can see, the port is wrong

I don't know what's going on with these Unconfirmed-REQ. I'm using other bacnet clients and they are working well, so I think it's a bug on this ReadPropertyForeign client but I'm not sure.

JoelBender commented 2 years ago

The unconfirmed Who-Is gives a clue to what's going on. The client application has some expectations about the address of the router to network 1. You should see a Who-Is-Router-To-Network message when you first ask to send the request to 1:x, the application doesn't know what network it is on and assumes network 1 is some other network. The response for this is cached, if you turn on debugging for the RouterInfoCache you'll see some method calls to that effect.

When the Who-Is comes in, it probably has different NPDU contents which describe itself as being routed to get to your client, but the original source address of the PDU (at the IPv4 UDP layer) is a different address than what is cached, so the client updates its local cache to reflect the new topology. Again, you should see the cache updated when debugging is turned on.

You can force the client to update it's local cache with the rtn console command (in essence telling the stack "I'm giving you the address of the router to network 1, don't bother looking for it") or by turning on "route aware" and using the extended version of addresses (like 1:2@1.2.3.4) which bypasses the address-to-network resolving processes. The real solution is to fix the topology problems, and you might want to figure out where that packet is coming from! LOL!

DB-CL commented 2 years ago

Thanks, I will look into the route aware parameter to see if I can do it. I still feel like there is something odd with this client when it updates its local cache when it receives the Who-Is. Shouldn't be more controllable ? Because as soon as it receive a wrong packet, it is updating its cache with wrong information

The real solution is to fix the topology problems, and you might want to figure out where that packet is coming from! LOL!

I bet it is ^^ I cannot dig in that direction, I'm not aware of the real network topology and I can barely speak to the guys who made it ...

JoelBender commented 2 years ago

It would be nice to have a configuration parameter that says "this is the topology and don't let any incoming traffic try to say otherwise." Call it --static-routing or something.

DB-CL commented 2 years ago

Yes :)