September 2017 - dev - lists.frrouting.org

Re: [dev] [PATCH] bgpd: fix length calculation of multi-segment AS_PATH UPDATE messages
by Donald Sharp 09 Sep '17

09 Sep '17

Andreas - Thanks for taking the time and solving this AS_PATH issue! I'm about to submit this patch for a Pull Request into FRR. I'm attributing the patch to you, before I can do so I need your 'Signed-off-by:...'. I'm including the relevant part of our documentation to see what we think it signifies: `Signed-off-by:` this is a developer's certification that he or she has the right to submit the patch for inclusion into the project. It is an agreement to the Developer's Certificate of Origin (below). Code without a proper signoff can not and will not be merged. If you are unfamiliar with this process, you should read the [official policy at kernel.org](http://www.kernel.org/doc/Documentation/SubmittingPatches) and you might find this article about [participating in the Linux community on the Linux Foundation website](http://www.linuxfoundation.org/content/how-participate-linux-commu… to be a helpful resource. In short, when you sign off on a commit, you assert your agreement to all of the following: > Developer's Certificate of Origin 1.1 > > By making a contribution to this project, I certify that: > > (a) The contribution was created in whole or in part by me and I > have the right to submit it under the open source license > indicated in the file; or > > (b) The contribution is based upon previous work that, to the best > of my knowledge, is covered under an appropriate open source > license and I have the right under that license to submit that > work with modifications, whether created in whole or in part > by me, under the same open source license (unless I am > permitted to submit under a different license), as indicated > in the file; or > > (c) The contribution was provided directly to me by some other > person who certified (a), (b) or (c) and I have not modified > it. > > (d) I understand and agree that this project and the contribution > are public and that a record of the contribution (including all > personal information I submit with it, including my sign-off) is > maintained indefinitely and may be redistributed consistent with > this project or the open source license(s) involved. thanks! donald On Thu, Sep 7, 2017 at 10:54 PM, Andreas Jaggi via dev <dev(a)lists.frrouting.org> wrote: > _______________________________________________ > dev mailing list > dev(a)lists.frrouting.org > https://lists.frrouting.org/listinfo/dev > > > ---------- Forwarded message ---------- > From: Andreas Jaggi <aj(a)open.ch> > To: quagga-dev(a)lists.quagga.net, dev(a)lists.frrouting.org > Cc: > Bcc: > Date: Fri, 8 Sep 2017 04:53:37 +0200 > Subject: [PATCH] bgpd: fix length calculation of multi-segment AS_PATH UPDATE messages > Hi > > There is a bug in the length calculation of mutli-segment AS_PATH UPDATE messages, which can make bgpd send out malformed BGP UPDATE packets and in turn causes that BGP sessions are reset. > > This thread on quagga-users shows how the problem manifests: https://lists.quagga.net/pipermail/quagga-users/2017-September/014830.html > > In our case we had an UPDATE message having an AS_PATH with 567 entries which was split into three segments (255+255+57). > This results in an AS_PATH attribute of the following length: 2274 bytes > (eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 57*4 segment length) > The AS_PATH attribute was written properly into the message, but the length written in the path attribute header by bgpd was 3294 bytes instead of 2274 (thus resulting in an invalid/malformed BGP UPDATE message). > > Turns out that the aspath_put function wrongly double-accumulates the written bytes and then ends up with these 3294 bytes instead of 2274. > (eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 510*4 segment length + 2 bytes segment header + 57*4 segment length). > > > The following patch (against the master branch of Quagga) fixes the length calculation in aspath_put: > > diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c > index b7af5e88..d813bfba 100644 > --- a/bgpd/bgp_aspath.c > +++ b/bgpd/bgp_aspath.c > @@ -903,7 +903,7 @@ aspath_put (struct stream *s, struct aspath *as, int use32bit ) > assegment_header_put (s, seg->type, AS_SEGMENT_MAX); > assegment_data_put (s, seg->as, AS_SEGMENT_MAX, use32bit); > written += AS_SEGMENT_MAX; > - bytes += ASSEGMENT_SIZE (written, use32bit); > + bytes += ASSEGMENT_SIZE (AS_SEGMENT_MAX, use32bit); > } > > /* write the final segment, probably is also the first */ > > > It looks like this bug has been slumbering for 10 years since its introduction in 2007 when it was added with the AS4 support changes (commit 0b2aa3a0). > > Which leads us to the following affected versions: > Quagga: 0.99.10 - 0.99.24.1, 1.0.*, 1.1.0, 1.1.1, 1.2.0, 1.2.1 > FRR: 2.0, 3.0-rc1 > > > Cheers > Andreas > > > -- > andreas jaggi > lead engineer network services > > open systems ag > raeffelstrasse 29 > ch-8045 zurich > t: +41 58 100 10 10 > f: +41 58 100 10 11 > aj(a)open.ch > > http://www.open.ch > >

2 1

About API interface in FRRouting
by Wing Lin 08 Sep '17

08 Sep '17

Hi, I have been studying FRRouting recently. And I want to use FRRouting in my code, so I have one question below. Is there any API interface in FRRouting exposed to users? Looking forward to your reply. Thanks!

2 1

[PATCH] bgpd: fix length calculation of multi-segment AS_PATH UPDATE messages
by Andreas Jaggi 08 Sep '17

08 Sep '17

Hi There is a bug in the length calculation of mutli-segment AS_PATH UPDATE messages, which can make bgpd send out malformed BGP UPDATE packets and in turn causes that BGP sessions are reset. This thread on quagga-users shows how the problem manifests: https://lists.quagga.net/pipermail/quagga-users/2017-September/014830.html In our case we had an UPDATE message having an AS_PATH with 567 entries which was split into three segments (255+255+57). This results in an AS_PATH attribute of the following length: 2274 bytes (eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 57*4 segment length) The AS_PATH attribute was written properly into the message, but the length written in the path attribute header by bgpd was 3294 bytes instead of 2274 (thus resulting in an invalid/malformed BGP UPDATE message). Turns out that the aspath_put function wrongly double-accumulates the written bytes and then ends up with these 3294 bytes instead of 2274. (eg. 2 bytes segment header + 255*4 segment length + 2 bytes segment header + 510*4 segment length + 2 bytes segment header + 57*4 segment length). The following patch (against the master branch of Quagga) fixes the length calculation in aspath_put: diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index b7af5e88..d813bfba 100644 --- a/bgpd/bgp_aspath.c +++ b/bgpd/bgp_aspath.c @@ -903,7 +903,7 @@ aspath_put (struct stream *s, struct aspath *as, int use32bit ) assegment_header_put (s, seg->type, AS_SEGMENT_MAX); assegment_data_put (s, seg->as, AS_SEGMENT_MAX, use32bit); written += AS_SEGMENT_MAX; - bytes += ASSEGMENT_SIZE (written, use32bit); + bytes += ASSEGMENT_SIZE (AS_SEGMENT_MAX, use32bit); } /* write the final segment, probably is also the first */ It looks like this bug has been slumbering for 10 years since its introduction in 2007 when it was added with the AS4 support changes (commit 0b2aa3a0). Which leads us to the following affected versions: Quagga: 0.99.10 - 0.99.24.1, 1.0.*, 1.1.0, 1.1.1, 1.2.0, 1.2.1 FRR: 2.0, 3.0-rc1 Cheers Andreas -- andreas jaggi lead engineer network services open systems ag raeffelstrasse 29 ch-8045 zurich t: +41 58 100 10 10 f: +41 58 100 10 11 aj(a)open.ch http://www.open.ch

1 0

New Defects reported by Coverity Scan for freerangerouting/frr
by scan-admin＠coverity.com 07 Sep '17

07 Sep '17

Hi, Please find the latest report on new defect(s) introduced to freerangerouting/frr found with Coverity Scan. 1 new defect(s) introduced to freerangerouting/frr found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 1455482: Incorrect expression (BAD_SIZEOF) /lib/thread.c: 62 in cpu_record_hash_key() ________________________________________________________________________________________________________ *** CID 1455482: Incorrect expression (BAD_SIZEOF) /lib/thread.c: 62 in cpu_record_hash_key() 56 static struct list *masters; 57 58 59 /* CLI start ---------------------------------------------------------------- */ 60 static unsigned int cpu_record_hash_key(struct cpu_thread_history *a) 61 { >>> CID 1455482: Incorrect expression (BAD_SIZEOF) >>> Taking the size of "&a->func", which is the address of an object, is suspicious. 62 int size = sizeof (&a->func); 63 64 return jhash(&a->func, size, 0); 65 } 66 67 static int cpu_record_hash_cmp(const struct cpu_thread_history *a, ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… To manage Coverity Scan email notifications for "frr(a)lists.nox.tf", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

1 0

Re: [dev] Regarding ospfv3 authentication
by Sutharsan S 07 Sep '17

07 Sep '17

Dear Renato, Thank you for the information. Can you please share us the patch? Probably we shall apply that and test. Best Regards, Sutharsan On Sep 6, 2017 21:45, "Renato Westphal" <renato(a)opensourcerouting.org> wrote: Hi Bala, Unfortunately it looks like I updated the changelog before getting the RFC 4552 code[1] merged in the repository. Long story short: I got everything working in the last year but my code relies on an external tool (iproute2) to install the IPSec SAs and policies in the Linux kernel, and this is not the ideal thing to do. I started converting the new lib/ipsec.c file to use Netlink instead but for some reason this work was sidetracked and I never finished it. I apologize for this mistake and I'll fix the chengelog until I actually open a Pull Request for this feature. In the meanwhile I can rebase [1] on top of master and send you a patch if you want. Let me know if that would help. Regards, Renato. [1] https://github.com/opensourcerouting/frr/commit/aebe82bb2 On Wed, Sep 6, 2017 at 9:00 AM, Bala Chandru <chandrubala141(a)gmail.com> wrote: > Hi, > > Does FRR 3.0 supports OSPFv3 authentication? > Though OSPFv3 doesn't have authentication header on its own similar to > OSPFv2, it should use IPSec for authentication. > > Though github claims that it supports RFC 4552. > > Reference Link: > > https://github.com/FRRouting/frr/wiki/FRR-2.0-%E2%86%92-FRR-3.0-Changelog > > But I couldn't see commands like "ipv6 ospf authentication .." etc., > Do I miss something? > > Regards, > Bala > > > > _______________________________________________ > dev mailing list > dev(a)lists.frrouting.org > https://lists.frrouting.org/listinfo/dev > -- Renato Westphal _______________________________________________ dev mailing list dev(a)lists.frrouting.org https://lists.frrouting.org/listinfo/dev

3 2

Regarding ospfv3 authentication
by Bala Chandru 06 Sep '17

06 Sep '17

Hi, Does FRR 3.0 supports OSPFv3 authentication? Though OSPFv3 doesn't have authentication header on its own similar to OSPFv2, it should use IPSec for authentication. Though github claims that it supports RFC 4552. Reference Link: https://github.com/FRRouting/frr/wiki/FRR-2.0-%E2%86%92-FRR-3.0-Changelog But I couldn't see commands like "ipv6 ospf authentication .." etc., Do I miss something? Regards, Bala

2 1

New Defects reported by Coverity Scan for freerangerouting/frr
by scan-admin＠coverity.com 05 Sep '17

05 Sep '17

Hi, Please find the latest report on new defect(s) introduced to freerangerouting/frr found with Coverity Scan. 1 new defect(s) introduced to freerangerouting/frr found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 1 of 1 defect(s) ** CID 1455380: (NULL_RETURNS) /bgpd/bgp_route.c: 1279 in bgp_attr_add_gshut_community() /bgpd/bgp_route.c: 1279 in bgp_attr_add_gshut_community() ________________________________________________________________________________________________________ *** CID 1455380: (NULL_RETURNS) /bgpd/bgp_route.c: 1279 in bgp_attr_add_gshut_community() 1273 struct community *old; 1274 struct community *new; 1275 struct community *merge; 1276 struct community *gshut; 1277 1278 old = attr->community; >>> CID 1455380: (NULL_RETURNS) >>> Assigning: "gshut" = null return value from "community_str2com". 1279 gshut = community_str2com("graceful-shutdown"); 1280 1281 if (old) { 1282 merge = community_merge(community_dup(old), gshut); 1283 1284 if (old->refcnt== 0) /bgpd/bgp_route.c: 1279 in bgp_attr_add_gshut_community() 1273 struct community *old; 1274 struct community *new; 1275 struct community *merge; 1276 struct community *gshut; 1277 1278 old = attr->community; >>> CID 1455380: (NULL_RETURNS) >>> Assigning: "gshut" = null return value from "community_str2com". 1279 gshut = community_str2com("graceful-shutdown"); 1280 1281 if (old) { 1282 merge = community_merge(community_dup(old), gshut); 1283 1284 if (old->refcnt== 0) ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… To manage Coverity Scan email notifications for "frr(a)lists.nox.tf", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

1 0

Today's Technical Meeting 11 am EDT
by Donald Sharp 05 Sep '17

05 Sep '17

https://bluejeans.com/9192742159 Agenda : 1) New Items 2) 3.0 stuff ( Mainly the crash in large communities ) 3) PR's 4) zebra rib_dest_t and rib_process discussion? Anything else? donald

1 0

New Defects reported by Coverity Scan for freerangerouting/frr
by scan-admin＠coverity.com 05 Sep '17

05 Sep '17

Hi, Please find the latest report on new defect(s) introduced to freerangerouting/frr found with Coverity Scan. 6 new defect(s) introduced to freerangerouting/frr found with Coverity Scan. New defect(s) Reported-by: Coverity Scan Showing 6 of 6 defect(s) ** CID 1455339: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_fsm.c: 416 in eigrp_fsm_event_keep_state() ________________________________________________________________________________________________________ *** CID 1455339: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_fsm.c: 416 in eigrp_fsm_event_keep_state() 410 prefix->distance = ne->distance; 411 prefix->reported_metric = 412 ne->total_metric; 413 if (msg->packet_type == EIGRP_OPC_QUERY) 414 eigrp_send_reply(msg->adv_router, prefix); 415 prefix->req_action |= EIGRP_FSM_NEED_UPDATE; >>> CID 1455339: Null pointer dereferences (NULL_RETURNS) >>> Dereferencing a null pointer "eigrp_lookup()". 416 listnode_add( 417 (eigrp_lookup())->topology_changes_internalIPV4, 418 prefix); 419 } 420 eigrp_topology_update_node_flags(prefix); 421 eigrp_update_routing_table(prefix); ** CID 1455338: (NULL_RETURNS) /eigrpd/eigrp_filter.c: 85 in eigrp_distribute_update() /eigrpd/eigrp_filter.c: 87 in eigrp_distribute_update() /eigrpd/eigrp_filter.c: 89 in eigrp_distribute_update() /eigrpd/eigrp_filter.c: 184 in eigrp_distribute_update() ________________________________________________________________________________________________________ *** CID 1455338: (NULL_RETURNS) /eigrpd/eigrp_filter.c: 85 in eigrp_distribute_update() 79 if (!dist->ifname) { 80 /* access list IN for whole process */ 81 if (dist->list[DISTRIBUTE_V4_IN]) { 82 alist = access_list_lookup( 83 AFI_IP, dist->list[DISTRIBUTE_V4_IN]); 84 if (alist) >>> CID 1455338: (NULL_RETURNS) >>> Dereferencing a null pointer "e". 85 e->list[EIGRP_FILTER_IN] = alist; 86 else 87 e->list[EIGRP_FILTER_IN] = NULL; 88 } else { 89 e->list[EIGRP_FILTER_IN] = NULL; 90 } /eigrpd/eigrp_filter.c: 87 in eigrp_distribute_update() 81 if (dist->list[DISTRIBUTE_V4_IN]) { 82 alist = access_list_lookup( 83 AFI_IP, dist->list[DISTRIBUTE_V4_IN]); 84 if (alist) 85 e->list[EIGRP_FILTER_IN] = alist; 86 else >>> CID 1455338: (NULL_RETURNS) >>> Dereferencing a null pointer "e". 87 e->list[EIGRP_FILTER_IN] = NULL; 88 } else { 89 e->list[EIGRP_FILTER_IN] = NULL; 90 } 91 92 /* access list OUT for whole process */ /eigrpd/eigrp_filter.c: 89 in eigrp_distribute_update() 83 AFI_IP, dist->list[DISTRIBUTE_V4_IN]); 84 if (alist) 85 e->list[EIGRP_FILTER_IN] = alist; 86 else 87 e->list[EIGRP_FILTER_IN] = NULL; 88 } else { >>> CID 1455338: (NULL_RETURNS) >>> Dereferencing a null pointer "e". 89 e->list[EIGRP_FILTER_IN] = NULL; 90 } 91 92 /* access list OUT for whole process */ 93 if (dist->list[DISTRIBUTE_V4_OUT]) { 94 alist = access_list_lookup( /eigrpd/eigrp_filter.c: 184 in eigrp_distribute_update() 178 179 /*struct eigrp_if_info * info = ifp->info; 180 ei = info->eigrp_interface;*/ 181 struct listnode *node, *nnode; 182 struct eigrp_interface *ei2; 183 /* Find proper interface */ >>> CID 1455338: (NULL_RETURNS) >>> Dereferencing a null pointer "e". 184 for (ALL_LIST_ELEMENTS(e->eiflist, node, nnode, ei2)) { 185 if (strcmp(ei2->ifp->name, ifp->name) == 0) { 186 ei = ei2; 187 break; 188 } 189 } ** CID 1455337: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_interface.c: 444 in eigrp_if_free() ________________________________________________________________________________________________________ *** CID 1455337: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_interface.c: 444 in eigrp_if_free() 438 THREAD_OFF(ei->t_hello); 439 eigrp_hello_send(ei, EIGRP_HELLO_GRACEFUL_SHUTDOWN, NULL); 440 } 441 442 dest_addr = *ei->connected->address; 443 apply_mask(&dest_addr); >>> CID 1455337: Null pointer dereferences (NULL_RETURNS) >>> Dereferencing a null pointer "eigrp". 444 pe = eigrp_topology_table_lookup_ipv4(eigrp->topology_table, 445 &dest_addr); 446 if (pe) 447 eigrp_prefix_entry_delete(eigrp->topology_table, pe); 448 449 eigrp_if_down(ei); ** CID 1455336: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_topology.c: 467 in eigrp_update_routing_table() ________________________________________________________________________________________________________ *** CID 1455336: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_topology.c: 467 in eigrp_update_routing_table() 461 } 462 } 463 464 void eigrp_update_routing_table(struct eigrp_prefix_entry *prefix) 465 { 466 struct eigrp *eigrp = eigrp_lookup(); >>> CID 1455336: Null pointer dereferences (NULL_RETURNS) >>> Dereferencing a null pointer "eigrp". 467 struct list *successors = 468 eigrp_topology_get_successor_max(prefix, eigrp->max_paths); 469 struct listnode *node; 470 struct eigrp_nexthop_entry *entry; 471 472 if (successors) { ** CID 1455335: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_topology.c: 446 in eigrp_topology_update_node_flags() ________________________________________________________________________________________________________ *** CID 1455335: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_topology.c: 446 in eigrp_topology_update_node_flags() 440 { 441 struct listnode *node; 442 struct eigrp_nexthop_entry *entry; 443 struct eigrp *eigrp = eigrp_lookup(); 444 445 for (ALL_LIST_ELEMENTS_RO(dest->entries, node, entry)) { >>> CID 1455335: Null pointer dereferences (NULL_RETURNS) >>> Dereferencing a null pointer "eigrp". 446 if (((uint64_t)entry->distance 447 <= (uint64_t)dest->distance * (uint64_t)eigrp->variance) 448 && entry->distance != EIGRP_MAX_METRIC) // is successor 449 { 450 entry->flags |= EIGRP_NEXTHOP_ENTRY_SUCCESSOR_FLAG; 451 entry->flags &= ~EIGRP_NEXTHOP_ENTRY_FSUCCESSOR_FLAG; ** CID 1455334: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_topology.c: 217 in eigrp_prefix_entry_delete() ________________________________________________________________________________________________________ *** CID 1455334: Null pointer dereferences (NULL_RETURNS) /eigrpd/eigrp_topology.c: 217 in eigrp_prefix_entry_delete() 211 struct eigrp *eigrp = eigrp_lookup(); 212 213 /* 214 * Emergency removal of the node from this list. 215 * Whatever it is. 216 */ >>> CID 1455334: Null pointer dereferences (NULL_RETURNS) >>> Dereferencing a null pointer "eigrp". 217 listnode_delete(eigrp->topology_changes_internalIPV4, node); 218 219 if (listnode_lookup(topology, node) != NULL) { 220 list_delete_all_node(node->entries); 221 list_free(node->entries); 222 list_free(node->rij); ________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0… To manage Coverity Scan email notifications for "frr(a)lists.nox.tf", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…

1 0

FRR, NVO3, VNC, .. what happens underneath?
by Raymond Burkholder 02 Sep '17

02 Sep '17

Hello, I have a network virtualisation scenario, and I think FRR might be a part of it, but I am not sure. I have taken a look at https://frrouting.org/user-guide/VNC-and-VNC_002dGW.html, and taken a quick skim of the NVO3 stuff. The NVO3 stuff references a bunch of possibilities. The FRR site provides command descriptions and a few examples. The bit missing from the FRR site is what happens underneath? What is actually used for encapsulation? How does FRR interact with the encapsulation layer? There was a one word reference to OpenFlow, but what is the glue that ties FRR, VNC, and the underpinnings together, particularly in a Linux environment? Are there any presentations, blog entries, black-magic or dark-art places describing example scenarios? Or is it really just that simple? An additional search problem is that VNC refers to screen virtualisation, so search parameters become delicate. -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.

2 4