Skip to content

slow: Add OSSP support #1363

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+46 −1
Open

slow: Add OSSP support #1363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e6fa76
slow: Add OSSP support
corubba Sep 23, 2025
0b37c57
oui: Add ITU-T
corubba Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions oui.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const struct tok oui_values[] = {
{ OUI_IANA, "IANA" },
{ OUI_NORTEL, "Nortel Networks SONMP" },
{ OUI_CISCO_90, "Cisco bridged" },
{ OUI_ITU_T, "ITU-T" },
{ OUI_RFC2684, "Ethernet bridged" },
{ OUI_ATM_FORUM, "ATM Forum" },
{ OUI_CABLE_BPDU, "DOCSIS Spanning Tree" },
Expand Down
1 change: 1 addition & 0 deletions oui.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ extern const struct tok smi_values[];
#define OUI_IANA 0x00005E /* IANA */
#define OUI_NORTEL 0x000081 /* Nortel SONMP */
#define OUI_CISCO_90 0x0000f8 /* Cisco bridging */
#define OUI_ITU_T 0x0019a7 /* International Telecommunication Union - Telecommunication Standardization Sector */
#define OUI_RFC2684 0x0080c2 /* RFC 2427/2684 bridged Ethernet */
#define OUI_ATM_FORUM 0x00A03E /* ATM Forum */
#define OUI_CABLE_BPDU 0x00E02F /* DOCSIS spanning tree BPDU */
Expand Down
39 changes: 38 additions & 1 deletion print-slow.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#define SLOW_PROTO_LACP 1
#define SLOW_PROTO_MARKER 2
#define SLOW_PROTO_OAM 3
#define SLOW_PROTO_OSSP 10

#define LACP_VERSION 1
#define MARKER_VERSION 1
Expand All @@ -42,6 +43,7 @@ static const struct tok slow_proto_values[] = {
{ SLOW_PROTO_LACP, "LACP" },
{ SLOW_PROTO_MARKER, "MARKER" },
{ SLOW_PROTO_OAM, "OAM" },
{ SLOW_PROTO_OSSP, "OSSP" },
{ 0, NULL}
};

Expand Down Expand Up @@ -239,6 +241,7 @@ struct lacp_marker_tlv_terminator_t {

static void slow_marker_lacp_print(netdissect_options *, const u_char *, u_int, u_int);
static void slow_oam_print(netdissect_options *, const u_char *, u_int);
static void slow_ossp_print(netdissect_options *, const u_char *, u_int);

void
slow_print(netdissect_options *ndo,
Expand Down Expand Up @@ -278,7 +281,8 @@ slow_print(netdissect_options *ndo,
print_version = 1;
break;

case SLOW_PROTO_OAM: /* fall through */
case SLOW_PROTO_OAM:
case SLOW_PROTO_OSSP:
print_version = 0;
break;

Expand Down Expand Up @@ -313,6 +317,13 @@ slow_print(netdissect_options *ndo,
default: /* should not happen */
break;

case SLOW_PROTO_OSSP:
/* skip subtype */
len -= 1;
pptr += 1;
slow_ossp_print(ndo, pptr, len);
break;

case SLOW_PROTO_OAM:
/* skip subtype */
len -= 1;
Expand Down Expand Up @@ -733,3 +744,29 @@ slow_oam_print(netdissect_options *ndo,
tooshort:
ND_PRINT("\n\t\t packet is too short");
}

/*
* Print Organization Specific Slow Protocol. (802.3 Annex 57B)
*/
static void
slow_ossp_print(netdissect_options *ndo,
const u_char *tptr, u_int tlen)
{
uint32_t oui;

ND_ICHECKMSG_U("length", tlen, <, 3);

oui = GET_BE_U_3(tptr);
ND_PRINT("\n\tOUI: %s (0x%06x)",
tok2str(oui_values, "Unknown", oui),
oui);
tlen -= 3;
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can see, this line can cause an out-of-bounds read: neither slow_ossp_print() nor its caller ensure that here tlen >= 3. Please add a length check.

Copy link
Author

Choose a reason for hiding this comment

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

GET_BE_U_3 does a length check, and will not return if there is not enough data. You can find the same pattern in other protocols, for example here, here and here. Worked fine when I tested it reducing the packet step by step one byte at a time.

Copy link
Member

Choose a reason for hiding this comment

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

In this context the GET_ macros guard the boundaries of the actually present (captured) data, and tlen is the declared data length. Please note:

  • the ND_ICHECK_U(len, <, 8); just before the data fetch in the first example,
  • the if (subtlv_len >= 6) around the data fetch in the second example, and
  • the if (len < length * NSH_HDR_WORD_SIZE) before the data fetch in the third example.

Copy link
Author

Choose a reason for hiding this comment

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

I don't really understand how tlen could ever be less than the captured data (the other way around sure), and thus tlen < 3 not always first trigger the GET_BE_U_3 longjump. But I will assume you know better and submit to your judgement.

Copy link
Author

Choose a reason for hiding this comment

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

I guess the reason is to have the correct message in the output, truncated packet (due to snaplen) vs invalid packet (simply too short).

Copy link
Member

Choose a reason for hiding this comment

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

Correctness of reporting is one reason, and consistency of code style is another.

Yet another reason is safety: any "length" recorded in the captured data generally cannot be trusted. In this case tlen is not obviously off: the EtherType/length field of the Ethernet frame stands for EtherType so in ethertype_print() it seems likely that length >= caplen, especially when the frame is read from a file and the length values have been sanitized.

However, if you consider how many protocols encapsulate Ethernet frames now and how many more will do that in future, it is almost certain that an encoding exists that has its own idea of an encapsulated Ethernet frame length and will present, let's say, 1KB of "captured" data and a value of length that will cause tlen here to be, let's say, 1. Then after the subtraction tlen would underflow to a very large value and print_unknown_data() will be printing a memory dump until it trips a boundary of the process' virtual memory and causes a segmentation fault. This is why the other three cases in this file and most if not all other decoders check both lengths.

tptr += 3;

print_unknown_data(ndo, tptr, "\n\t", tlen);

return;

invalid:
nd_print_invalid(ndo);
}
1 change: 1 addition & 0 deletions tests/TESTLIST
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ lldp_mud-v lldp_mudurl.pcap lldp_mudurl-v.out -e -v
lldp_mud-vv lldp_mudurl.pcap lldp_mudurl-vv.out -e -vv
lldp_8021_linkagg-v lldp_8021_linkagg.pcap lldp_8021_linkagg-v.out -v
lldp_8021_linkagg-vv lldp_8021_linkagg.pcap lldp_8021_linkagg-vv.out -vv
slow-ossp slow-ossp.pcap slow-ossp.out -v
# fuzzed pcap
udld-inf-loop-1-v udld-inf-loop-1.pcapng udld-inf-loop-1-v.out -v

Expand Down
5 changes: 5 additions & 0 deletions tests/slow-ossp.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
1 2025-09-23 15:00:00.000000 OSSP, length 52
OUI: ITU-T (0x0019a7)
0x0000: 0001 1000 0000 0100 0404 0000 0000 0000
0x0010: 0000 0000 0000 0000 0000 0000 0000 0000
0x0020: 0000 0000 0000 0000 0000 0000 0000 0000
Binary file added tests/slow-ossp.pcap
View file
Open in desktop
Binary file not shown.