From 29dda184e56ce3a1ec72db4612198f6b3ba84e82 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 5 Jan 2022 16:38:49 +0100 Subject: [PATCH 1/4] Conf: Fix parsing full-length IPv6 addresses Lexer expression for bytestring was too loose, accepting also full-length IPv6 addresses. It should be restricted such that colon is used between every byte or never. Fix the regex and also add some test cases for it. Thanks to Alexander Zubkov for the bugreport --- conf/cf-lex.l | 2 +- filter/test.conf | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/conf/cf-lex.l b/conf/cf-lex.l index 704a1750..c9d2f5a5 100644 --- a/conf/cf-lex.l +++ b/conf/cf-lex.l @@ -255,7 +255,7 @@ WHITE [ \t] return IP4; } -{XIGIT}{2}(:{XIGIT}{2}|{XIGIT}{2}){15,} { +{XIGIT}{2}((:{XIGIT}{2}){15,}|({XIGIT}{2}){15,}) { char *s = yytext; size_t len = 0, i; struct bytestring *bytes; diff --git a/filter/test.conf b/filter/test.conf index 6a28e4b3..f902f99f 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -335,6 +335,26 @@ ip p; p = 1234:5678::; bt_assert(!p.is_v4); bt_assert(p.mask(24) = 1234:5600::); + + p = 1:2:3:4:5:6:7:8; + bt_assert(!p.is_v4); + bt_assert(format(p) = "1:2:3:4:5:6:7:8"); + bt_assert(p.mask(64) = 1:2:3:4::); + + p = 10:20:30:40:50:60:70:80; + bt_assert(!p.is_v4); + bt_assert(format(p) = "10:20:30:40:50:60:70:80"); + bt_assert(p.mask(64) = 10:20:30:40::); + + p = 1090:20a0:30b0:40c0:50d0:60e0:70f0:8000; + bt_assert(!p.is_v4); + bt_assert(format(p) = "1090:20a0:30b0:40c0:50d0:60e0:70f0:8000"); + bt_assert(p.mask(64) = 1090:20a0:30b0:40c0::); + + p = ::fffe:6:c0c:936d:88c7:35d3; + bt_assert(!p.is_v4); + bt_assert(format(p) = "::fffe:6:c0c:936d:88c7:35d3"); + bt_assert(p.mask(64) = 0:0:fffe:6::); } bt_test_suite(t_ip, "Testing ip address"); From 77d032c71f62e44293a10ccc22f8c157442df179 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 5 Jan 2022 18:46:41 +0100 Subject: [PATCH 2/4] Netlink: Improve multipath parsing errors Function nl_parse_multipath() should handle errors internally. --- sysdep/linux/netlink.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c index fdf3f2db..1293df4d 100644 --- a/sysdep/linux/netlink.c +++ b/sysdep/linux/netlink.c @@ -681,7 +681,7 @@ nl_add_multipath(struct nlmsghdr *h, uint bufsize, struct nexthop *nh, int af, e } static struct nexthop * -nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, struct rtattr *ra, int af) +nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, const net_addr *n, struct rtattr *ra, int af) { struct rtattr *a[BIRD_RTA_MAX]; struct rtnexthop *nh = RTA_DATA(ra); @@ -695,7 +695,7 @@ nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, struct rtattr { /* Use RTNH_OK(nh,len) ?? */ if ((len < sizeof(*nh)) || (len < nh->rtnh_len)) - return NULL; + goto err; if (nh->rtnh_flags & RTNH_F_DEAD) goto next; @@ -706,7 +706,10 @@ nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, struct rtattr rv->weight = nh->rtnh_hops; rv->iface = if_find_by_index(nh->rtnh_ifindex); if (!rv->iface) - return NULL; + { + log(L_ERR "KRT: Received route %N with unknown ifindex %u", n, nh->rtnh_ifindex); + return NULL; + } /* Nonexistent RTNH_PAYLOAD ?? */ nl_attr_len = nh->rtnh_len - RTNH_LENGTH(0); @@ -714,18 +717,18 @@ nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, struct rtattr { case AF_INET: if (!nl_parse_attrs(RTNH_DATA(nh), nexthop_attr_want4, a, sizeof(a))) - return NULL; + goto err; break; case AF_INET6: if (!nl_parse_attrs(RTNH_DATA(nh), nexthop_attr_want6, a, sizeof(a))) - return NULL; + goto err; break; #ifdef HAVE_MPLS_KERNEL case AF_MPLS: if (!nl_parse_attrs(RTNH_DATA(nh), nexthop_attr_want_mpls, a, sizeof(a))) - return NULL; + goto err; if (a[RTA_NEWDST]) rv->labels = rta_get_mpls(a[RTA_NEWDST], rv->label); @@ -734,7 +737,7 @@ nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, struct rtattr #endif default: - return NULL; + goto err; } if (a[RTA_GATEWAY]) @@ -757,14 +760,19 @@ nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, struct rtattr nbr = neigh_find(&p->p, rv->gw, rv->iface, (rv->flags & RNF_ONLINK) ? NEF_ONLINK : 0); if (!nbr || (nbr->scope == SCOPE_HOST)) - return NULL; + { + log(L_ERR "KRT: Received route %N with strange next-hop %I", n, rv->gw); + return NULL; + } } #ifdef HAVE_MPLS_KERNEL if (a[RTA_ENCAP] && a[RTA_ENCAP_TYPE]) { - if (rta_get_u16(a[RTA_ENCAP_TYPE]) != LWTUNNEL_ENCAP_MPLS) { - log(L_WARN "KRT: Unknown encapsulation method %d in multipath", rta_get_u16(a[RTA_ENCAP_TYPE])); + if (rta_get_u16(a[RTA_ENCAP_TYPE]) != LWTUNNEL_ENCAP_MPLS) + { + log(L_WARN "KRT: Received route %N with unknown encapsulation method %d", + n, rta_get_u16(a[RTA_ENCAP_TYPE])); return NULL; } @@ -785,6 +793,10 @@ nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, struct rtattr first = nexthop_sort(first); return first; + +err: + log(L_ERR "KRT: Received strange multipath route %N", n); + return NULL; } static void @@ -1675,19 +1687,16 @@ nl_parse_route(struct nl_parse_state *s, struct nlmsghdr *h) if (a[RTA_MULTIPATH]) { - struct nexthop *nh = nl_parse_multipath(s, p, a[RTA_MULTIPATH], i->rtm_family); + struct nexthop *nh = nl_parse_multipath(s, p, n, a[RTA_MULTIPATH], i->rtm_family); if (!nh) - { - log(L_ERR "KRT: Received strange multipath route %N", net->n.addr); - return; - } + SKIP("strange RTA_MULTIPATH\n"); nexthop_link(ra, nh); break; } if (i->rtm_flags & RTNH_F_DEAD) - return; + SKIP("ignore RTNH_F_DEAD\n"); ra->nh.iface = if_find_by_index(oif); if (!ra->nh.iface) From f5c8fb5fba959d356ce1ea0fb5879223f76137f7 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 5 Jan 2022 19:25:42 +0100 Subject: [PATCH 3/4] Netlink: Do not ignore dead routes from BIRD Currently, BIRD ignores dead routes to consider them absent. But it also ignores its own routes and thus it can not correctly manage such routes in some cases. This patch makes an exception for routes with proto bird when ignoring dead routes, so they can be properly updated or removed. Thanks to Alexander Zubkov for the original patch. --- sysdep/linux/netlink.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c index 1293df4d..e127052a 100644 --- a/sysdep/linux/netlink.c +++ b/sysdep/linux/netlink.c @@ -681,7 +681,7 @@ nl_add_multipath(struct nlmsghdr *h, uint bufsize, struct nexthop *nh, int af, e } static struct nexthop * -nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, const net_addr *n, struct rtattr *ra, int af) +nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, const net_addr *n, struct rtattr *ra, int af, int krt_src) { struct rtattr *a[BIRD_RTA_MAX]; struct rtnexthop *nh = RTA_DATA(ra); @@ -697,7 +697,7 @@ nl_parse_multipath(struct nl_parse_state *s, struct krt_proto *p, const net_addr if ((len < sizeof(*nh)) || (len < nh->rtnh_len)) goto err; - if (nh->rtnh_flags & RTNH_F_DEAD) + if ((nh->rtnh_flags & RTNH_F_DEAD) && (krt_src != KRT_SRC_BIRD)) goto next; *last = rv = lp_allocz(s->pool, NEXTHOP_MAX_SIZE); @@ -1687,7 +1687,7 @@ nl_parse_route(struct nl_parse_state *s, struct nlmsghdr *h) if (a[RTA_MULTIPATH]) { - struct nexthop *nh = nl_parse_multipath(s, p, n, a[RTA_MULTIPATH], i->rtm_family); + struct nexthop *nh = nl_parse_multipath(s, p, n, a[RTA_MULTIPATH], i->rtm_family, krt_src); if (!nh) SKIP("strange RTA_MULTIPATH\n"); @@ -1695,7 +1695,7 @@ nl_parse_route(struct nl_parse_state *s, struct nlmsghdr *h) break; } - if (i->rtm_flags & RTNH_F_DEAD) + if ((i->rtm_flags & RTNH_F_DEAD) && (krt_src != KRT_SRC_BIRD)) SKIP("ignore RTNH_F_DEAD\n"); ra->nh.iface = if_find_by_index(oif); From bcb25084d31fdb90fcf1666f10e73fe0f863afc0 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 5 Jan 2022 20:07:27 +0100 Subject: [PATCH 4/4] Test: Activate some remaining build tests --- .gitlab-ci.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e7eaa66b..1325e9e8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -582,6 +582,11 @@ test-ospf-custom: variables: TEST_NAME: cf-ospf-custom +test-ospf-area: + <<: *test-base + variables: + TEST_NAME: cf-ospf-area + test-ospf-vrf: <<: *test-base variables: @@ -636,3 +641,8 @@ test-babel-auth: <<: *test-base variables: TEST_NAME: cf-babel-auth + +test-rip-base: + <<: *test-base + variables: + TEST_NAME: cf-rip-base