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_SHUT 2
+#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(&bgpn, 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 += 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]");
}