Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

six Network Layer messages should be implemented #12

Open
duffy-ocraven opened this issue Aug 18, 2020 · 8 comments
Open

six Network Layer messages should be implemented #12

duffy-ocraven opened this issue Aug 18, 2020 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@duffy-ocraven
Copy link

duffy-ocraven commented Aug 18, 2020

There are six Network Layer messages:
X'00': Who-Is-Router-To-Network
X'01': I-Am-Router-To-Network
X'02': I-Could-Be-Router-To-Network
X'03': Reject-Message-To-Network
X'04': Router-Busy-To-Network
X'05': Router-Available-To-Network

which are in common use, and two more defined, which are deprecated:
X'06': Initialize-Routing-Table
X'07': Initialize-Routing-Table-Ack

These are at network level, thus below, so differently encoded from any APDU.

@NothinRandom
Copy link
Contributor

@duffy-corelight, it would really help if you also have pcaps for verification

@NothinRandom NothinRandom self-assigned this Aug 21, 2020
@NothinRandom
Copy link
Contributor

@duffy-corelight, looking through code and various test pcaps. I do see these pop up. By "implemented", is it safe to assume that you mean parsing? If so, then pcaps would be appreciated for these messages

@duffy-ocraven
Copy link
Author

Pkt#3 shows a correct one. And Pkt#4 shows a subtly incorrect one.
RouterBusyToNet_Port_IP.pcap.zip

@duffy-ocraven
Copy link
Author

Oh, I had formed an incorrect impression before I reported this, because the existing code:

            if (control == 0x80 ||
                control == 0x81) {
                local network_layer_message_type = bytestring_to_count(rest_of_data[rest_of_data_index]);
                rest_of_data_index += 1;
                data[data_index] = fmt("%s", network_layer_messages[network_layer_message_type]);

missed all the other 62 values which convey network_layer_messages. That should be:

if (control & 0x80) {

instead of

if (control == 0x80 ||
                control == 0x81) {

I mention 62 rather than 126 other values which can convey network_layer_messages, only because bit 4 is reserved and is always zero.

@duffy-ocraven
Copy link
Author

The subtly wrong pkt#4 is in two aspects. A) the standard mandates (and behaviorally only expects implementations to be coded this way) ip.dest_h is a broadcast address if-and-only-if the Original-Broadcast-NPDU is used and if the Original-Unicast-NPDU is used, then the ip.dest_h is a non-broadcast address. B) there is no Network Number 0 which can be reached through a router. The 0 value is semantically: "the network which comprises devices which can be reached without passing through a router". So no BACnet Network Layer packet would ever see a zero in a network number parameter.

@duffy-ocraven
Copy link
Author

Besides fixing the

if (control & 0x80) {

I would hope to see in the resolution of this issue also, that after the

data[data_index] = fmt("%s", network_layer_messages[network_layer_message_type]);

statement, the appending into data as string represented decimal (not hexadecimal, since decimal network numbers are the usual BACnet representation), of all the network number parameters (each is uint16) until the UDP/BVLC-Length. Don't be misled by Wireshark calling them Destination Network Address: They are not an address. They are a network number.

@NothinRandom
Copy link
Contributor

NothinRandom commented Sep 10, 2020

@duffy-corelight, latest update partially addresses this issue. Like the others, I'll let the customer close the issue if deemed as satisfied.

Edit: output looks like

#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	bvlc_function	bvlc_len	apdu_type	service_choice	data
#types	time	string	addr	port	addr	port	string	count	string	string	vector[string]
1300444502.084622	CBIcBy1P6pY1MIULAj	192.168.10.102	47808	192.168.10.202	47808	Original Unicast NPDU	21	Confirmed Service Request	Read Property Multiple	(empty)
1300444502.084847	CBIcBy1P6pY1MIULAj	192.168.10.102	47808	192.168.10.202	47808	Original Unicast NPDU	42	Complex Acknowledge	Read Property Multiple	(empty)
1300444505.364506	CLAJ2NTuA46jdaMQ4	192.168.10.102	47808	192.168.10.255	47808	Original Broadcast NPDU	7	-	-	network_layer_message=Router Busy To Network
1300444529.594316	CzBGF52VPzwJ6A1JSg	192.168.10.202	47808	192.168.10.255	47808	Original Unicast NPDU	13	-	-	network_layer_message=Router Busy To Network,network_numbers=6100;1000;0

@duffy-ocraven
Copy link
Author

Here are examples of a few of these messages, including a flawed implementation's pkt#5.
Flawed_pkt5_I-Am-Router.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants