Skip to content

Commit

Permalink
updates to address issues: #12, #13, #14, #15, #16, #17
Browse files Browse the repository at this point in the history
  • Loading branch information
@NothinRandom
NothinRandom committed Sep 10, 2020
1 parent 4e37e51 commit 4f91d6a
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 3 deletions.
36 changes: 35 additions & 1 deletion scripts/consts.zeek
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export {
[4] = "Segment Acknowledge",
[5] = "Error",
[6] = "Reject",
[7] = "Abort ABORT"
[7] = "Abort",
} &default=function(i: count):string { return fmt("apdu_type (%x)", i); } &redef;

## BACnet service choices
Expand Down Expand Up @@ -546,6 +546,7 @@ export {
const limit_enable = {
[1] = "Event Low Limit Enable",
[2] = "Event High Limit Enable",
[3] = "Event Both Limits Enable",
} &default=function(i: count):string { return fmt("limit_enable (%x)", i); } &redef;

## BACnet action
Expand All @@ -562,6 +563,9 @@ export {

## BACnet segmentations
const segmentation_supports = {
[0] = "Segmented Both",
[1] = "Segmented Transmit",
[2] = "Segmented Receive",
[3] = "No Segmentation",
} &default=function(i: count):string { return fmt("segmentation (%x)", i); } &redef;

Expand Down Expand Up @@ -757,6 +761,36 @@ export {
[138] = "Invalid Value In This State",
} &default=function(i: count):string { return fmt("error_code (%x)", i); } &redef;

## reject reasons
const rejects = {
[0] = "Other",
[1] = "Buffer Overflow",
[2] = "Inconsistent Parameters",
[3] = "Invalid Parameter Data Type",
[4] = "Invalid Tag",
[5] = "Missing Required Parameter",
[6] = "Parameter Out Of Range",
[7] = "Too Many Arguments",
[8] = "Undefined Enumeration",
[9] = "Unrecognized Service",
} &default=function(i: count):string { return fmt("reject (%x)", i); } &redef;

## abort reasons
const aborts = {
[0] = "Other",
[1] = "Buffer Overflow",
[2] = "Invalid APDU In This State",
[3] = "Preempted By Higher Priority Task",
[4] = "Segmentation Not Supported",
[5] = "Security Error",
[6] = "Insufficient Security",
[7] = "Window Size Out Of Range",
[8] = "Application Exceeded Reply Time",
[9] = "Out Of Resources",
[10] = "TSM Timeout",
[11] = "APDU Too Long",
} &default=function(i: count):string { return fmt("abort (%x)", i); } &redef;

## vendors
const vendors = {
[0] = "ASHRAE",
Expand Down
72 changes: 70 additions & 2 deletions scripts/main.zeek
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,45 @@ event bacnet(c:connection, is_orig:bool,
if (control == 0x80 ||
control == 0x81) {
local network_layer_message_type = bytestring_to_count(rest_of_data[rest_of_data_index]);
data[data_index] = fmt("network_layer_message=%s", network_layer_messages[network_layer_message_type]);
rest_of_data_index += 1;
data[data_index] = fmt("%s", network_layer_messages[network_layer_message_type]);
data_index += 1;
##! type, functiom, blvc len makes up 4
bvlc_len -= 4;
##! check if more data to parse
if (rest_of_data_index < bvlc_len) {
switch(network_layer_message_type) {
case 0x00: ##! Who Is Router To Network
break;
case 0x01: ##! I Am Router To Network
break;
case 0x02: ##! I Could Be Router To Network
break;
case 0x03, ##! Reject Message To Network
0x04: ##! Router Busy To Network
if (network_layer_message_type == 0x03) {
##! could enumerate if available: http://www.bacnet.org/Addenda/Add-135-2010ao.pdf#page=9
data[data_index] = fmt("reason=%d", bytestring_to_count(rest_of_data[rest_of_data_index]));
data_index += 1;
}
local network_numbers: string = "";
while (rest_of_data_index < bvlc_len) {
network_numbers += fmt("%d", bytestring_to_count(rest_of_data[rest_of_data_index: rest_of_data_index+2]));
rest_of_data_index += 2;
if (rest_of_data_index < bvlc_len) {
network_numbers += ";";
}
}
data[data_index] = fmt("network_numbers=%s", network_numbers);
break;
case 0x05: ##! Router Available To Network
break;
case 0x06: ##! Initialize Routing Table
break;
case 0x07: ##! Initialize Available To Network
break;
}
}
break;
}
else if (control == 0x08 ||
Expand Down Expand Up @@ -224,7 +261,7 @@ event bacnet(c:connection, is_orig:bool,
data[data_index] = fmt("low_limit=%d", bytes_to_count(len, rest_of_data[rest_of_data_index:rest_of_data_index+len]));
}
else {
data[data_index] = fmt("time=%d:%02d:%02d.%d", bytestring_to_count(rest_of_data[rest_of_data_index]),
data[data_index] = fmt("time=%d:%02d:%02d.%02d", bytestring_to_count(rest_of_data[rest_of_data_index]),
bytestring_to_count(rest_of_data[rest_of_data_index+1]),
bytestring_to_count(rest_of_data[rest_of_data_index+2]),
bytestring_to_count(rest_of_data[rest_of_data_index+3]));
Expand All @@ -234,6 +271,19 @@ event bacnet(c:connection, is_orig:bool,
}
break;
case 0x04: ##! segment ack
##! parse data if negative ack is true
if ((apdu_type & 2) > 0) {
##! from previous increment
rest_of_data_index -= 2;
##! invoke id
data[data_index] = fmt("invoke_id=%d", bytestring_to_count(rest_of_data[rest_of_data_index]));
data_index += 1;
rest_of_data_index += 1;
data[data_index] = fmt("sequence_number=%d", bytestring_to_count(rest_of_data[rest_of_data_index]));
data_index += 1;
rest_of_data_index += 1;
data[data_index] = fmt("window_size=%d", bytestring_to_count(rest_of_data[rest_of_data_index]));
}
break;
case 0x05: ##! error
c$bacnet$service_choice = confirmed_services[serviceChoice];
Expand All @@ -248,6 +298,24 @@ event bacnet(c:connection, is_orig:bool,
rest_of_data_index += 1;
data[data_index] = fmt("code=%s", error_codes[bytes_to_count(len, rest_of_data[rest_of_data_index:rest_of_data_index+len])]);
break;
case 0x06: ##! reject
##! from previous increment
rest_of_data_index -= 2;
##! invoke id
data[data_index] = fmt("invoke_id=%d", bytestring_to_count(rest_of_data[rest_of_data_index]));
data_index += 1;
rest_of_data_index += 1;
data[data_index] = fmt("reason=%d", rejects[bytestring_to_count(rest_of_data[rest_of_data_index])]);
break;
case 0x07: ##! abort
##! from previous increment
rest_of_data_index -= 2;
##! invoke id
data[data_index] = fmt("invoke_id=%d", bytestring_to_count(rest_of_data[rest_of_data_index]));
data_index += 1;
rest_of_data_index += 1;
data[data_index] = fmt("reason=%d", aborts[bytestring_to_count(rest_of_data[rest_of_data_index])]);
break;
default:
c$bacnet$service_choice = confirmed_services[serviceChoice];
switch(serviceChoice) {
Expand Down

6 comments on commit 4f91d6a

@duffy-ocraven
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 159 case 0x07: ##! Initialize Available To Network
has wrong comment. The packets with 6 and higher are practically vestigial, and mostly did not see industry-adoption, so as to be not worthy of anything more than appearing in comments, and maybe enumeration text-strings, but here are the accurate ones which have been defined.

X'06': Initialize-Routing-Table
X'07': Initialize-Routing-Table-Ack
X'08': Establish-Connection-To-Network
X'09': Disconnect-Connection-To-Network
X'0A': Challenge-Request
X'0B': Security-Payload
X'0C': Security-Response
X'0D': Request-Key-Update
X'0E': Update-Key-Set
X'0F': Update-Distribution-Key
X'10': Request-Master-Key
X'11': Set-Master-Key

X'12' and X'13' are seeing adoption. What seeing them should tell a protocol parser to do, is a subject worthy of its own ticket.

X'12': What-Is-Network-Number
X'13': Network-Number-Is

@NothinRandom
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha! Good thing it was only a comment. It'll get updated on the next round of commits, along with adding these new cases. If possible, could you provide insights into these other cases? Only 3 and 4 are covered as of now.

@duffy-ocraven
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In

if (network_layer_message_type == 0x03) {
                                ##! could enumerate if available: http://www.bacnet.org/Addenda/Add-135-2010ao.pdf#page=9
                                data[data_index] = fmt("reason=%d", bytestring_to_count(rest_of_data[rest_of_data_index]));
                                data_index += 1;
                                }

the code also needs to advance

rest_of_data_index += 1;

@duffy-ocraven
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The packet would be malformed if encoded transmitting a 1-byte network number, but


                            while (rest_of_data_index < bvlc_len) {
                                network_numbers += fmt("%d", bytestring_to_count(rest_of_data[rest_of_data_index: rest_of_data_index+2]));
                                rest_of_data_index += 2;
                                if (rest_of_data_index < bvlc_len) {
                                    ...

is not defensively coded against the possibility, and would access beyond the end of the buffer.

@duffy-ocraven
Copy link

@duffy-ocraven duffy-ocraven commented on 4f91d6a Sep 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop construct is the proper way to capture what is in the actual packet (after provisioning for the reason in 0x03 and for a potential malformed 1-octet final network number in all) for all of network_layer_message_type == 0x00 through 0x05. This additionally gives an excellent opportunity for our design to decide its architecture for weirdness. In addition to a 1-octet final network number being malformed, when the (network_layer_message_type == 0x03), then any content other than exactly one network number, is malformed. Note, however: when the (network_layer_message_type == 0x04) or the (network_layer_message_type == 0x05), then any content can be fewer or more than one network number (both situations are stipulated in the standard) and yet is still properly well-formed. For (network_layer_message_type == 0x00), then any content can be zero or one network number (both situations are stipulated in the standard) and yet is still properly well-formed. For (network_layer_message_type == 0x01), then any content can be one or more than one network number (both situations are stipulated in the standard) and yet is still properly well-formed. For (network_layer_message_type == 0x02), the content is required to be exactly one network number to be properly well-formed.

@duffy-ocraven
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 121/122

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

still haven't corrected per #12 comment: missed all the other 62 values which convey network_layer_messages. That should be:

if (control & 0x80) {

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

Please sign in to comment.