On 2015/10/13 18:21, Kevin Reay wrote:
> Fix a segfault when printing a malformed BGP AS_PATH update due to ASN
> extraction.
There are line-wrapping and whitespace issues in this patch.
> Better AS size extraction from AS paths: better heuristics (see
> bgp_attr_get_as_size).
That makes sense though the English in the comments relating to
this change is very awkward.
> Also fixes output support for 4-byte ASNs. For example;
> (AS_PATH[T] {500.500 513.65211})
> becomes:
> (AS_PATH[T] {500 500} 65211)
This example seems very wrong. I think what the diff is doing in this
area is switching from ASDOT to ASPLAIN which is alright (though it
should be a separate diff - one change, one diff - and I don't regard
it as "fix" but "change to the more common format") ... but that
doesn't match this example at all, which appears to be doing
something strange with as-sets.
> Collapses bgp_attr_print switch 2-byte and 4-byte path cases.
>
> bgp_attr_get_as_size function copied from tcpdump Git repo master.
The formatting of this function doesn't match style(9) or the code
in the rest of this file.
> This brings behavior in-line with newer versions on Linux.
>
> Finally, minor output tweak: add a space after BGP update attribute
> ORIGINATOR_ID's flags to make consistent with other attributes.
>
>
> Index: print-bgp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/tcpdump/print-bgp.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 print-bgp.c
> --- print-bgp.c 16 Jan 2015 06:40:21 -0000 1.17
> +++ print-bgp.c 14 Oct 2015 00:15:13 -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,63 @@ trunc:
> }
> #endif
>
> +/*
> + * bgp_attr_get_as_size
> + *
> + * Try to find the size of the ASs encoded in an as-path. It is not obvious,
> as
> + * both Old speakers that do not support 4 byte AS, and the new speakers
> that do
> + * support, exchange AS-Path with the same path-attribute type value 0x02.
> + */
> +static int
> +bgp_attr_get_as_size (u_int8_t bgpa_type, const u_char *pptr, int len)
> +{
> + const u_char *tptr = pptr;
> +
> + /*
> + * If the path attribute is the optional AS4 path type, then we already
> + * know, that ASs must be encoded in 4 byte format.
> + */
> + if (bgpa_type == BGPTYPE_AS4_PATH) {
> + return 4;
> + }
> +
> + /*
> + * Let us assume that ASs are of 2 bytes in size, and check if the
> AS-Path
> + * TLV is good. If not, ask the caller to try with AS encoded as 4 bytes
> + * each.
> + */
> + while (tptr < pptr + len) {
> + TCHECK(tptr[0]);
> +
> + /*
> + * If we do not find a valid segment type, our guess might be wrong.
> + */
> + if (tptr[0] < BGP_AS_SEG_TYPE_MIN || tptr[0] > BGP_AS_SEG_TYPE_MAX) {
> + goto trunc;
> + }
> + TCHECK(tptr[1]);
> + tptr += 2 + tptr[1] * 2;
> + }
> +
> + /*
> + * If we correctly reached end of the AS path attribute data content,
> + * then most likely ASs were indeed encoded as 2 bytes.
> + */
> + if (tptr == pptr + len) {
> + return 2;
> + }
> +
> +trunc:
> +
> + /*
> + * We can come here, either we did not have enough data, or if we
> + * try to decode 4 byte ASs in 2 byte format. Either way, return 4,
> + * so that calller can try to decode each AS as of 4 bytes. If indeed
> + * there was not enough data, it will crib and end the parse anyways.
> + */
> + return 4;
> +}
> +
> static int
> bgp_attr_print(const struct bgp_attr *attr, const u_char *dat, int len)
> {
> @@ -413,7 +473,6 @@ bgp_attr_print(const struct bgp_attr *at
>
> p = dat;
> tlen = len;
> - asn_bytes = 0;
>
> switch (attr->bgpa_type) {
> case BGPTYPE_ORIGIN:
> @@ -425,17 +484,8 @@ 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;
> @@ -444,11 +494,19 @@ bgp_attr_print(const struct bgp_attr *at
> printf(" empty");
> break;
> }
> +
> + /*
> + * BGP updates exchanged between New speakers that support 4
> + * byte AS, ASs are always encoded in 4 bytes. There is no
> + * definitive way to find this, just by the packet's
> + * contents. So, check for packet's TLV's sanity assuming
> + * 2 bytes first, and it does not pass, assume that ASs are
> + * encoded in 4 bytes format and move on.
> + */
> + asn_bytes = bgp_attr_get_as_size(attr->bgpa_type, dat, len);
> +
> while (p < dat + len) {
> TCHECK(p[0]);
> - if (asn_bytes == 0) {
> - asn_bytes = (len-2)/p[1];
> - }
> printf("%s",
> tok2str(bgp_as_path_segment_open_values,
> "?", p[0]));
> @@ -456,13 +514,10 @@ bgp_attr_print(const struct bgp_attr *at
> for (i = 0; i < p[1] * asn_bytes; i += asn_bytes) {
> TCHECK2(p[2 + i], asn_bytes);
> printf("%s", i == 0 ? "" : " ");
> - if (asn_bytes == 2 || EXTRACT_16BITS(&p[2 + i]))
> - printf("%u%s",
> - EXTRACT_16BITS(&p[2 + i]),
> - asn_bytes == 4 ? "." : "");
> - if (asn_bytes == 4)
> - printf("%u",
> - EXTRACT_16BITS(&p[2 + i + 2]));
> + printf("%u",
> + asn_bytes == 2 ?
> + EXTRACT_16BITS(&p[2 + i]) :
> + EXTRACT_32BITS(&p[2 + i]));
> }
> TCHECK(p[0]);
> printf("%s",
> @@ -549,7 +604,7 @@ bgp_attr_print(const struct bgp_attr *at
> break;
> }
> TCHECK2(p[0], 4);
> - printf("%s",getname(p));
> + printf(" %s", getname(p));
> break;
> case BGPTYPE_CLUSTER_LIST:
> if (len % 4) {
>
