From db129a0d39a855df46b483d80edbae68768073aa Mon Sep 17 00:00:00 2001 From: Lorenzo Cogotti Date: Fri, 20 Aug 2021 22:37:20 +0200 Subject: [PATCH] [lonetix/*,tools/bgpgrep/*] Correct handling of extended timestamps during filtering, decode and dump --- lonetix/bgp/dump_isolario.c | 50 ++++++++++++++++++++++++------- lonetix/bgp/mrt.c | 33 +++++++++++--------- lonetix/include/df/bgp/mrt.h | 2 +- tools/bgpgrep/bgpgrep_dump.c | 10 ++++++- tools/bgpgrep/bgpgrep_timestamp.c | 6 ++-- 5 files changed, 72 insertions(+), 29 deletions(-) diff --git a/lonetix/bgp/dump_isolario.c b/lonetix/bgp/dump_isolario.c index eb6ac19..8536b41 100644 --- a/lonetix/bgp/dump_isolario.c +++ b/lonetix/bgp/dump_isolario.c @@ -153,6 +153,12 @@ static const char *Bgp_KnownCommunityToString(BgpCommCode code) } } +static void NormalizeExtendedTimestamp(Dumpfmtctx *ctx) +{ + ctx->timestamp += ctx->microsecs / 1000000; + ctx->microsecs %= 1000000; +} + static void DumpUnknown(Stmbuf *sb, BgpType type) { Bufio_Putc(sb, UNKNOWN_MARKER); @@ -420,7 +426,7 @@ static void DumpMrtInfoTrailer(Stmbuf *sb, const Dumpfmtctx *ctx) Bufio_Putu(sb, ctx->timestamp); if (ctx->microsecs > 0) { Utoa(ctx->microsecs, buf); - Df_strpadl(buf, '0', 9); + Df_strpadl(buf, '0', 6); Bufio_Putc(sb, '.'); Bufio_Puts(sb, buf); @@ -941,14 +947,25 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr *hdr, ctx.isAsn32bit = BGP4MP_ISASN32BIT(hdr->subtype); ctx.isAddPath = BGP4MP_ISADDPATH(hdr->subtype); ctx.timestamp = beswap32(hdr->timestamp); - if (hdr->type == MRT_BGP4MP_ET) + if (hdr->type == MRT_BGP4MP_ET) { + // Length must account for extended timestamp + if (len < 4) goto corrupt; + ctx.microsecs = beswap32(((const Mrthdrex *) hdr)->microsecs); + len -= 4; + + NormalizeExtendedTimestamp(&ctx); + } ctx.hdr = hdr; if (ctx.isAsn32bit) { + if (len < sizeof(bgp4mp->a32)) goto corrupt; + ctx.peerAs = beswap32(bgp4mp->a32.peerAs); if (bgp4mp->a32.afi == AFI_IP) { + if (len < sizeof(bgp4mp->a32v4)) goto corrupt; + ctx.peerAddr.family = IP4; ctx.peerAddr.v4 = bgp4mp->a32v4.peerAddr; @@ -956,14 +973,20 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr *hdr, } else { assert(bgp4mp->a32.afi == AFI_IP6); + if (len < sizeof(bgp4mp->a32v6)) goto corrupt; + ctx.peerAddr.family = IP6; ctx.peerAddr.v6 = bgp4mp->a32v6.peerAddr; offset = sizeof(bgp4mp->a32v6); } } else { + if (len < sizeof(bgp4mp->a16)) goto corrupt; + ctx.peerAs = beswap16(bgp4mp->a16.peerAs); if (bgp4mp->a16.afi == AFI_IP) { + if (len < sizeof(bgp4mp->a16v4)) goto corrupt; + ctx.peerAddr.family = IP4; ctx.peerAddr.v4 = bgp4mp->a16v4.peerAddr; @@ -971,6 +994,8 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr *hdr, } else { assert(bgp4mp->a16.afi == AFI_IP6); + if (len < sizeof(bgp4mp->a16v6)) goto corrupt; + ctx.peerAddr.family = IP6; ctx.peerAddr.v6 = bgp4mp->a16v6.peerAddr; @@ -984,10 +1009,13 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr *hdr, if (BGP4MP_ISSTATECHANGE(hdr->subtype)) { // Dump state change BgpFsmState chng[2]; - memcpy(chng, (Uint8 *) bgp4mp + offset, sizeof(chng)); Bufio_Putc(&sb, STATE_CHANGE_MARKER); Bufio_Putc(&sb, SEP_CHAR); + if (len < offset + sizeof(chng)) goto corrupt; + + memcpy(chng, (Uint8 *) bgp4mp + offset, sizeof(chng)); + Bufio_Putu(&sb, beswap16(chng[0])); Bufio_Putc(&sb, '-'); Bufio_Putu(&sb, beswap16(chng[1])); @@ -1000,17 +1028,14 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr *hdr, size_t nbytes = len - offset; nbytes = Bgp_CheckMsgHdr(msg, nbytes, /*allowExtendedSize=*/TRUE); - if (nbytes != 0) { - // Header is OK - nbytes -= BGP_HDRSIZ; - DumpBgp(&sb, msg->type, msg + 1, nbytes, table, &ctx); - } else { + if (nbytes == 0) { // Corrupted BGP4MP with invalid BGP data Bufio_Putc(&sb, UNKNOWN_MARKER); Bufio_Putc(&sb, SEP_CHAR); - WarnCorrupted(&sb, &ctx); - DumpMrtInfoTrailer(&sb, &ctx); + goto corrupt; } + + DumpBgp(&sb, msg->type, msg + 1, nbytes - BGP_HDRSIZ, table, &ctx); } else { // Deprecated/Unknown type Bufio_Putc(&sb, UNKNOWN_MARKER); @@ -1019,6 +1044,11 @@ static Sint64 Isolario_DumpBgp4mp(const Mrthdr *hdr, } return Bufio_Flush(&sb); + +corrupt: + WarnCorrupted(&sb, &ctx); + DumpMrtInfoTrailer(&sb, &ctx); + return Bufio_Flush(&sb); } static Sint64 Isolario_DumpZebra(const Mrthdr *hdr, diff --git a/lonetix/bgp/mrt.c b/lonetix/bgp/mrt.c index c93d5a6..b385ac8 100644 --- a/lonetix/bgp/mrt.c +++ b/lonetix/bgp/mrt.c @@ -16,9 +16,13 @@ #include #include -static void *MRT_DATAPTR(const Mrtrecord *rec) +static void *MRT_DATAPTR(const Mrtrecord *rec, size_t *len) { - return rec->buf + MRT_HDRSIZ + (MRT_ISEXHDRTYPE(MRT_HDR(rec)->type) << 2); + const Mrthdr *hdr = MRT_HDR(rec); + Uint32 off = MRT_ISEXHDRTYPE(hdr->type) << 2; + + *len = beswap32(hdr->len) - off; + return rec->buf + MRT_HDRSIZ + off; } Judgement Bgp_MrtFromBuf(Mrtrecord *rec, const void *buf, size_t nbytes) @@ -28,9 +32,9 @@ Judgement Bgp_MrtFromBuf(Mrtrecord *rec, const void *buf, size_t nbytes) const Mrthdr *hdr = (const Mrthdr *) buf; - size_t left = beswap32(hdr->len); - if (MRT_ISEXHDRTYPE(hdr->type)) - left += 4; // account for extended timestamp + size_t left = beswap32(hdr->len); // NOTE: Includes extended timestamp size, if any! + if (left < 4 && MRT_ISEXHDRTYPE(hdr->type)) // ...so check it + return Bgp_SetErrStat(BGPETRUNCMRT); size_t siz = sizeof(*hdr) + left; if (siz > nbytes) @@ -63,12 +67,12 @@ Judgement Bgp_ReadMrt(Mrtrecord *rec, void *streamp, const StmOps *ops) if ((size_t) n != sizeof(hdr)) return Bgp_SetErrStat(BGPEIO); - size_t left = beswap32(hdr.len); - if (MRT_ISEXHDRTYPE(hdr.type)) - left += 4; // account for extended timestamp + size_t left = beswap32(hdr.len); // NOTE: Includes extended timestamp size, if any! + if (left < 4 && MRT_ISEXHDRTYPE(hdr.type)) // ...so check it + return Bgp_SetErrStat(BGPETRUNCMRT); // Allocate buffer - // NOTE: MRT header length doesn't account for header size itself + // NOTE: ...but MRT header length doesn't account for header size itself size_t siz = sizeof(hdr) + left; const MemOps *memOps = MRT_MEMOPS(rec); @@ -517,11 +521,12 @@ Bgp4mphdr *Bgp_GetBgp4mpHdr(Mrtrecord *rec, size_t *nbytes) return NULL; } - size_t len = beswap32(hdr->len); - size_t offset = 0; + size_t len; Afi afi; - Bgp4mphdr *bgp4mp = (Bgp4mphdr *) MRT_DATAPTR(rec); + size_t offset = 0; + + Bgp4mphdr *bgp4mp = (Bgp4mphdr *) MRT_DATAPTR(rec, &len); if (BGP4MP_ISASN32BIT(hdr->subtype)) { offset += 2 * 4; offset += 2 + 2; @@ -572,8 +577,8 @@ Judgement Bgp_UnwrapBgp4mp(Mrtrecord *rec, Bgpmsg *dest, unsigned flags) if (!BGP4MP_ISMESSAGE(hdr->subtype)) return Bgp_SetErrStat(BGPEBADMRTTYPE); - Uint32 len = beswap32(hdr->len); - Uint8 *base = (Uint8 *) MRT_DATAPTR(rec); + size_t len; + Uint8 *base = (Uint8 *) MRT_DATAPTR(rec, &len); // Skip header size_t siz = BGP4MP_ISASN32BIT(hdr->subtype) ? 2*4 : 2*2; // skip ASN diff --git a/lonetix/include/df/bgp/mrt.h b/lonetix/include/df/bgp/mrt.h index fed679c..9950ae2 100755 --- a/lonetix/include/df/bgp/mrt.h +++ b/lonetix/include/df/bgp/mrt.h @@ -678,7 +678,7 @@ typedef ALIGNED(1, union) { FORCE_INLINE Bgp4mphdr *BGP4MP_HDR(const Mrthdr *hdr) { - return (hdr->subtype == MRT_BGP4MP_ET) ? + return (hdr->type == MRT_BGP4MP_ET) ? (Bgp4mphdr *) ((const Mrthdrex *) hdr + 1) : (Bgp4mphdr *) (hdr + 1); } diff --git a/tools/bgpgrep/bgpgrep_dump.c b/tools/bgpgrep/bgpgrep_dump.c index 5207927..e142656 100755 --- a/tools/bgpgrep/bgpgrep_dump.c +++ b/tools/bgpgrep/bgpgrep_dump.c @@ -65,6 +65,12 @@ static void FixBgpAttributeTableForRib(Bgpattrtab tab, Boolean isRibv2) } } +static void NormalizeExtendedTimestamp(void) +{ + S.timestampSecs += S.timestampMicrosecs / 1000000; + S.timestampMicrosecs %= 1000000; +} + static void OutputBgp4mp(const Mrthdr *hdr, Bgpattrtab tab) { S.lenientBgpErrors = TRUE; @@ -90,8 +96,10 @@ void BgpgrepD_Bgp4mp(void) S.peerAs = BGP4MP_GETPEERADDR(hdr->subtype, &S.peerAddr, bgp4mp); S.timestampSecs = beswap32(hdr->timestamp); S.timestampMicrosecs = 0; - if (hdr->subtype == MRT_BGP4MP_ET) + if (hdr->type == MRT_BGP4MP_ET) { S.timestampMicrosecs = beswap32(((const Mrthdrex *) hdr)->microsecs); + NormalizeExtendedTimestamp(); + } // Dump MRT data Bgp_UnwrapBgp4mp(&S.rec, &S.msg, /*flags=*/BGPF_UNOWNED); diff --git a/tools/bgpgrep/bgpgrep_timestamp.c b/tools/bgpgrep/bgpgrep_timestamp.c index cbcddca..c7fb1df 100644 --- a/tools/bgpgrep/bgpgrep_timestamp.c +++ b/tools/bgpgrep/bgpgrep_timestamp.c @@ -43,13 +43,13 @@ static Uint32 ParseFrac(const char *s, const char *p, char **ep) if (res != NCVENOERR) FATALF("%s: Expecting fractional portion of a second after '.'", s); - int ndigs = endp - p; - double frac = pow(10, ndigs); + int ndigs = endp - p; + long double frac = pow(10, ndigs); if (ep) *ep = endp; - return (Uint32) trunc((n / frac) * 100000.0); + return (Uint32) truncl(n / frac * 1000000.0L); } static Boolean ParseRfc3339(const char *s, Timestampop *dest)