Adopt an updated version of the tcpdump.org ASN size calculation for
BGP UPDATE message AS_PATHs. This corrects some bad behaviour due to
incorrect ASN size calculations.
I believe that the current way of calculating the ASN size for an
UPDATE AS_PATH attribute is flawed.
Currently, the ASN length (2 or 4 bytes) is calculated by dividing the
total length of the AS_PATH attribute by the number of ASNs (p[1]) in
the *first set encountered*:
asn_bytes = (len-2)/p[1];
The assumption that this first segment length describes the entire
attribute is incorrect; there could be multiple path segment lengths
in the attribute. The current method only uses the first encountered
segment length (while using the entire attribute's byte length).
This very often works fine, and when the calculation is incorrect the
printf code only prints the first 2 bytes due to the way the if
statements are structured, so it usually appears to work.
(Sometimes the calculated ASN length isn't even 2 or 4 bytes, often
causing 2 separate ASNs to be printed together as one ASN in ASDOT
notation.)
Example: here's the original OpenBSD output for a 2-byte encoded
AS_PATH:
(AS_PATH[T] 30)
here's tcpdump.org 4.5.1 on Linux:
AS Path (2), length: 10, Flags [T]: 30 { 10 20 }
and here's OpenBSD with the changes in this patch:
(AS_PATH[T] 30 {10 20})
A comment in the original code claims:
...
* To identify which is the case, compare the length of the path
* segment value in bytes, with the path segment length from the
* message (counted in # of AS)
This incorrectly describes the calculation being performed; the length
being compared isn't just the length of the "path segment value" but of
the entire attribute.
I've attached an updated version of the upstream's heuristics function
for calculating the ASN size: bgp_attr_get_as_size(). The updated
version contains style(9) changes, simplifications, an additional
validity check (non-zero segment lengths), and comment re-writes.
I'm interested in hearing any feedback. I'm also interested if anyone
with experience using tcpdump with BGP packets can report seeing the
incorrect behaviour described here in the past.
Index: print-bgp.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
retrieving revision 1.18
diff -u -p -r1.18 print-bgp.c
--- print-bgp.c 20 Oct 2015 11:29:07 -0000 1.18
+++ print-bgp.c 24 Oct 2015 18:21:55 -0000
@@ -140,6 +140,9 @@ struct bgp_attr {
#define BGP_CONFED_AS_SEQUENCE 3 /* draft-ietf-idr-rfc3065bis-01 */
#define BGP_CONFED_AS_SET 4 /* draft-ietf-idr-rfc3065bis-01 */
+#define BGP_AS_SEG_TYPE_MIN BGP_AS_SET
+#define BGP_AS_SEG_TYPE_MAX BGP_CONFED_AS_SET
+
static struct tok bgp_as_path_segment_open_values[] = {
{ BGP_AS_SET, " {" },
{ BGP_AS_SEQUENCE, " " },
@@ -400,6 +403,55 @@ trunc:
}
#endif
+/*
+ * Try to determine the size of the ASs encoded in an AS-path. It is
+ * not obvious as both speaker types exchange AS-paths with the same
+ * path-attribute type.
+ */
+static int
+bgp_attr_get_as_size(u_int8_t bgpa_type, const u_char *dat, int len)
+{
+ const u_char *p;
+
+ p = dat;
+
+ /* AS4 path types are always encoded in 4-byte format */
+ if (bgpa_type == BGPTYPE_AS4_PATH) {
+ return 4;
+ }
+
+ /*
+ * Start by assuming 2-byte ASs. Iterate through the path data using the
+ * segment length values. Switch to 4-bytes if we encounter an invalid
+ * field value/if the AS-Path length is invalid for the assumed size.
+ */
+ while (p < dat + len) {
+ TCHECK(p[0]);
+
+ /* check segment type: invalid value means wrong size */
+ if (p[0] < BGP_AS_SEG_TYPE_MIN || p[0] > BGP_AS_SEG_TYPE_MAX)
+ goto trunc;
+
+ TCHECK(p[1]);
+
+ /* check segment length: invalid indicates wrong size */
+ if (p[1] == 0)
+ goto trunc;
+
+ p += 2 + p[1] * 2;
+ }
+
+ /* matching length: it's very likely the ASs were encoded as 2-bytes */
+ if (p == dat + len)
+ return 2;
+trunc:
+ /*
+ * Either there was not enough data or we tried to decode 4-byte ASs
+ * with an incorrect size of 2-bytes.
+ */
+ return 4;
+}
+
static int
bgp_attr_print(const struct bgp_attr *attr, const u_char *dat, int len)
{
@@ -425,17 +477,7 @@ bgp_attr_print(const struct bgp_attr *at
}
break;
case BGPTYPE_AS4_PATH:
- asn_bytes = 4;
- /* FALLTHROUGH */
case BGPTYPE_AS_PATH:
- /*
- * 2-byte speakers will receive AS4_PATH as well AS_PATH (2-byte).
- * 4-byte speakers will only receive AS_PATH but it will be 4-byte.
- * To identify which is the case, compare the length of the path
- * segment value in bytes, with the path segment length from the
- * message (counted in # of AS)
- */
-
if (len % 2) {
printf(" invalid len");
break;
@@ -445,15 +487,13 @@ bgp_attr_print(const struct bgp_attr *at
printf(" empty");
break;
}
+ asn_bytes = bgp_attr_get_as_size(attr->bgpa_type, dat, len);
while (p < dat + len) {
TCHECK2(p[0], 2);
- if (asn_bytes == 0) {
- if (p[1] == 0) {
- /* invalid: segment contains one or more AS */
- printf(" malformed");
- break;
- }
- asn_bytes = (len-2)/p[1];
+ if (p[1] == 0) {
+ /* invalid: segment contains no AS */
+ printf(" malformed");
+ break;
}
printf("%s",
tok2str(bgp_as_path_segment_open_values,
