Re: tcpdump: decode BGP Administrative Shutdown Communication
On Wed, 19 Apr 2017, Job Snijders wrote: > The realisation that a shutdown communication may contain \0 (since NUL is a > valid UTF-8 char) \0 isn't a valid UTF-8 character. UTF-8 sets the MSB on code points > 127: https://en.wikipedia.org/wiki/UTF-8#Description
Re: tcpdump: decode BGP Administrative Shutdown Communication
> The realisation that a shutdown communication may contain \0 (since NUL is a > valid UTF-8 char), led me to alter the proposed changes. A debugging tool like > tcpdump should display trash too. This 0003 patch avoids the memset/memcpy and > can deal with trash in the shutdown communication through a new util > putlchars(). What kind of trash do you mean? Any kind of trash? How about an xterm set-title ANSI sequence, or something worse. Personally, I think this should use vis.
Re: tcpdump: decode BGP Administrative Shutdown Communication
On Mon, Apr 17, 2017 at 01:56:17PM -0600, Theo de Raadt wrote:
> + memset(string, 0, 129);
> + memcpy(string, p+1, shutdown_comm_length);
> + safeputs(string);
>
> Please don't copy numbers like that. If this is a string, why not use
> string functions that gaurantee truncation and truncation detection...
The payload of the shutdown communication (which starts at p+1) is not a
C string: the content of the field wich is memcpy'd, is not
null-terminated. Instead the field's size is available through
'shutdown_comm_length'.
The realisation that a shutdown communication may contain \0 (since NUL is a
valid UTF-8 char), led me to alter the proposed changes. A debugging tool like
tcpdump should display trash too. This 0003 patch avoids the memset/memcpy and
can deal with trash in the shutdown communication through a new util
putlchars().
example output:
BGP (NOTIFICATION: error Cease, subcode #2, Shutdown Communication
(length: 52): "This is a test of the sh\000\000\000\000wn communication
system.") (DF) [tos 0xc0] (ttl 255, id 40416, len 126)
Kind regards,
Job
diff --git a/usr.sbin/tcpdump/print-bgp.c b/usr.sbin/tcpdump/print-bgp.c
index d028d671893..e25fdbd930a 100644
--- a/usr.sbin/tcpdump/print-bgp.c
+++ b/usr.sbin/tcpdump/print-bgp.c
@@ -228,9 +228,13 @@ static const char *bgpnotify_minor_update[] = {
/* RFC 4486 */
#define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
+/* draft-ietf-idr-shutdown-07 */
+#define BGP_NOTIFY_MINOR_CEASE_SHUT2
+#define BGP_NOTIFY_MINOR_CEASE_RESET 4
+#define BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN 128
static const char *bgpnotify_minor_cease[] = {
- NULL, "Maximum Number of Prefixes Reached", "Administratively Shutdown",
- "Peer De-configured", "Administratively Reset", "Connection Rejected",
+ NULL, "Maximum Number of Prefixes Reached", "Administrative Shutdown",
+ "Peer De-configured", "Administrative Reset", "Connection Rejected",
"Other Configuration Change", "Connection Collision Resolution",
"Out of Resources",
};
@@ -302,6 +306,21 @@ static const char *afnumber[] = AFNUM_NAME_STR;
sizeof(afnumber)/sizeof(afnumber[0]), (x)))
+static void
+print_hex(const u_char *p, u_int len)
+{
+ while (len--)
+ printf("%02x", *p++);
+}
+
+static void
+putlchars(const u_char *str, u_int len)
+{
+ while (len--)
+ safeputchar(*str++);
+}
+
+
static const char *
num_or_str(const char **table, size_t siz, int value)
{
@@ -996,6 +1015,8 @@ bgp_notification_print(const u_char *dat, int length)
u_int16_t af;
u_int8_t safi;
const u_char *p;
+ uint8_t shutdown_comm_length;
+ uint8_t remainder_offset;
TCHECK2(dat[0], BGP_NOTIFICATION_SIZE);
memcpy(, dat, BGP_NOTIFICATION_SIZE);
@@ -1026,9 +1047,54 @@ bgp_notification_print(const u_char *dat, int length)
printf(" Max Prefixes: %u", EXTRACT_32BITS(p+3));
}
+
+ /*
+* draft-ietf-idr-shutdown describes a method to send a
+* message intended for human consumption regarding the
+* Administrative Shutdown or Reset event. This is called
+* the "Shutdown Communication". The communication is
+* UTF-8 encoded and may be no longer than 128 bytes.
+*/
+
+ if ((bgpn.bgpn_minor == BGP_NOTIFY_MINOR_CEASE_SHUT ||
+ bgpn.bgpn_minor == BGP_NOTIFY_MINOR_CEASE_RESET) &&
+ (length >= BGP_NOTIFICATION_SIZE + 1)) {
+ p = dat + BGP_NOTIFICATION_SIZE;
+ TCHECK2(*p, 1);
+ shutdown_comm_length = *(p);
+ remainder_offset = 0;
+ /* if we received garbage, make sure we hexdump it all
*/
+ if (shutdown_comm_length >
+ BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN ||
+ shutdown_comm_length > (length -
BGP_NOTIFICATION_SIZE) + 1)
+ printf(", invalid Shutdown Communication
length");
+ else if (shutdown_comm_length == 0) {
+ printf(", empty Shutdown Communication");
+ remainder_offset += 1;
+ }
+ /* a proper shutdown communication */
+ else {
+ TCHECK2(*(p+1), shutdown_comm_length);
+ printf(", Shutdown Communication (length: %u):
\"",
+ shutdown_comm_length);
+ putlchars(p+1, shutdown_comm_length);
+ printf("\"");
+ remainder_offset +=
Re: tcpdump: decode BGP Administrative Shutdown Communication
+ memset(string, 0, 129); + memcpy(string, p+1, shutdown_comm_length); + safeputs(string); Please don't copy numbers like that. If this is a string, why not use string functions that gaurantee truncation and truncation detection...
Re: tcpdump: decode BGP Administrative Shutdown Communication
Hi all,
Daan Keuper (Computest) was kind enough to review the diff, he pointed
out the following:
safeputs() expects a null-terminated string. Since shutdown_comm_length
won't exceed BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN (128), the
following will ensure a null-terminated string is passed to safeputs().
> + char string[128];
^^^
> + memset(string, 0, 128);
^^^
> + memcpy(string, p+1, shutdown_comm_length);
> + safeputs(string);
128 should be 129. oops!
I'd like to defer to more experienced programmers on the aesthetics of
using "129" rather then "BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN + 1",
or perhaps even an entirely other approach to ensure the string is
null-terminated.
Also added an additional check "shutdown_comm_length > (length -
BGP_NOTIFICATION_SIZE)" which is useful to prevent in case the shutdown
communication claims to be longer then the BGP message actually is.
Kind regards,
Job
diff --git a/usr.sbin/tcpdump/print-bgp.c b/usr.sbin/tcpdump/print-bgp.c
index d028d671893..0886304926e 100644
--- a/usr.sbin/tcpdump/print-bgp.c
+++ b/usr.sbin/tcpdump/print-bgp.c
@@ -228,9 +228,13 @@ static const char *bgpnotify_minor_update[] = {
/* RFC 4486 */
#define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
+/* draft-ietf-idr-shutdown-07 */
+#define BGP_NOTIFY_MINOR_CEASE_SHUT2
+#define BGP_NOTIFY_MINOR_CEASE_RESET 4
+#define BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN 128
static const char *bgpnotify_minor_cease[] = {
- NULL, "Maximum Number of Prefixes Reached", "Administratively Shutdown",
- "Peer De-configured", "Administratively Reset", "Connection Rejected",
+ NULL, "Maximum Number of Prefixes Reached", "Administrative Shutdown",
+ "Peer De-configured", "Administrative Reset", "Connection Rejected",
"Other Configuration Change", "Connection Collision Resolution",
"Out of Resources",
};
@@ -302,6 +306,13 @@ static const char *afnumber[] = AFNUM_NAME_STR;
sizeof(afnumber)/sizeof(afnumber[0]), (x)))
+static void
+print_hex(const u_char *p, u_int len)
+{
+ while (len--)
+ printf("%02x", *p++);
+}
+
static const char *
num_or_str(const char **table, size_t siz, int value)
{
@@ -996,6 +1007,9 @@ bgp_notification_print(const u_char *dat, int length)
u_int16_t af;
u_int8_t safi;
const u_char *p;
+ uint8_t shutdown_comm_length;
+ uint8_t remainder_offset;
+ char string[129];
TCHECK2(dat[0], BGP_NOTIFICATION_SIZE);
memcpy(, dat, BGP_NOTIFICATION_SIZE);
@@ -1026,9 +1040,56 @@ bgp_notification_print(const u_char *dat, int length)
printf(" Max Prefixes: %u", EXTRACT_32BITS(p+3));
}
+
+ /*
+* draft-ietf-idr-shutdown describes a method to send a
+* message intended for human consumption regarding the
+* Administrative Shutdown or Reset event. This is called
+* the "Shutdown Communication". The communication is
+* UTF-8 encoded and may be no longer than 128 bytes.
+*/
+
+ if ((bgpn.bgpn_minor == BGP_NOTIFY_MINOR_CEASE_SHUT ||
+ bgpn.bgpn_minor == BGP_NOTIFY_MINOR_CEASE_RESET) &&
+ (length >= BGP_NOTIFICATION_SIZE + 1)) {
+ p = dat + BGP_NOTIFICATION_SIZE;
+ TCHECK2(*p, 1);
+ shutdown_comm_length = *(p);
+ remainder_offset = 0;
+ /* seems we received garbage, hexdump it all */
+ if (shutdown_comm_length >
+ BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN ||
+ shutdown_comm_length > (length -
BGP_NOTIFICATION_SIZE))
+ printf(", invalid Shutdown Communication
length");
+ else if (shutdown_comm_length == 0) {
+ printf(", empty Shutdown Communication");
+ remainder_offset += 1;
+ }
+ /* a proper shutdown communication */
+ else {
+ TCHECK2(*(p+1), shutdown_comm_length);
+ printf(", Shutdown Communication (length: %u):
\"",
+ shutdown_comm_length);
+ memset(string, 0, 129);
+ memcpy(string, p+1, shutdown_comm_length);
+ safeputs(string);
+ printf("\"");
+ remainder_offset += shutdown_comm_length + 1;
+ }
+ /* if there is trailing data, hexdump it */
+
Re: tcpdump: decode BGP Administrative Shutdown Communication
Hi OpenBSD,
bgpd(8) as shipped in OpenBSD 6.1 supports draft-ietf-idr-shutdown-07.
The below patch adds support to tcpdump(8) to decode such shutdown
communication.
This is an improved version of the patch proposal I sent in January.
Kind regards,
Job
diff --git a/usr.sbin/tcpdump/print-bgp.c b/usr.sbin/tcpdump/print-bgp.c
index d028d671893..2871b92909f 100644
--- a/usr.sbin/tcpdump/print-bgp.c
+++ b/usr.sbin/tcpdump/print-bgp.c
@@ -228,9 +228,13 @@ static const char *bgpnotify_minor_update[] = {
/* RFC 4486 */
#define BGP_NOTIFY_MINOR_CEASE_MAXPRFX 1
+/* draft-ietf-idr-shutdown-07 */
+#define BGP_NOTIFY_MINOR_CEASE_SHUT2
+#define BGP_NOTIFY_MINOR_CEASE_RESET 4
+#define BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN 128
static const char *bgpnotify_minor_cease[] = {
- NULL, "Maximum Number of Prefixes Reached", "Administratively Shutdown",
- "Peer De-configured", "Administratively Reset", "Connection Rejected",
+ NULL, "Maximum Number of Prefixes Reached", "Administrative Shutdown",
+ "Peer De-configured", "Administrative Reset", "Connection Rejected",
"Other Configuration Change", "Connection Collision Resolution",
"Out of Resources",
};
@@ -302,6 +306,13 @@ static const char *afnumber[] = AFNUM_NAME_STR;
sizeof(afnumber)/sizeof(afnumber[0]), (x)))
+static void
+print_hex(const u_char *p, u_int len)
+{
+ while (len--)
+ printf("%02x", *p++);
+}
+
static const char *
num_or_str(const char **table, size_t siz, int value)
{
@@ -996,6 +1007,9 @@ bgp_notification_print(const u_char *dat, int length)
u_int16_t af;
u_int8_t safi;
const u_char *p;
+ uint8_t shutdown_comm_length;
+ uint8_t remainder_offset;
+ char string[128];
TCHECK2(dat[0], BGP_NOTIFICATION_SIZE);
memcpy(, dat, BGP_NOTIFICATION_SIZE);
@@ -1026,9 +1040,55 @@ bgp_notification_print(const u_char *dat, int length)
printf(" Max Prefixes: %u", EXTRACT_32BITS(p+3));
}
+
+ /*
+* draft-ietf-idr-shutdown describes a method to send a
+* message intended for human consumption regarding the
+* Administrative Shutdown or Reset event. This is called
+* the "Shutdown Communication". The communication is
+* UTF-8 encoded and may be no longer than 128 bytes.
+*/
+
+ if ((bgpn.bgpn_minor == BGP_NOTIFY_MINOR_CEASE_SHUT ||
+ bgpn.bgpn_minor == BGP_NOTIFY_MINOR_CEASE_RESET) &&
+ (length >= BGP_NOTIFICATION_SIZE + 1)) {
+ p = dat + BGP_NOTIFICATION_SIZE;
+ TCHECK2(*p, 1);
+ shutdown_comm_length = *(p);
+ remainder_offset = 0;
+ /* seems we received garbage, hexdump it all */
+ if (shutdown_comm_length >
+ BGP_NOTIFY_MINOR_CEASE_ADMIN_SHUTDOWN_LEN)
+ printf(", invalid Shutdown Communication
length");
+ else if (shutdown_comm_length == 0) {
+ printf(", empty Shutdown Communication");
+ remainder_offset += 1;
+ }
+ /* a proper shutdown communication */
+ else {
+ TCHECK2(*(p+1), shutdown_comm_length);
+ printf(", Shutdown Communication (length: %u):
\"",
+ shutdown_comm_length);
+ memset(string, 0, 128);
+ memcpy(string, p+1, shutdown_comm_length);
+ safeputs(string);
+ printf("\"");
+ remainder_offset += shutdown_comm_length + 1;
+ }
+ /* if there is trailing data, hexdump it */
+ if (length -
+ (remainder_offset + BGP_NOTIFICATION_SIZE) > 0) {
+ printf(", Data: (length: %u) ",
+ length - (remainder_offset +
+ BGP_NOTIFICATION_SIZE));
+ print_hex(p + remainder_offset, length -
+ (remainder_offset + BGP_NOTIFICATION_SIZE));
+ }
+ }
}
return;
+
trunc:
printf("[|BGP]");
}
