| Message ID | 1587160465-1540-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,ovn] ovn-northd: Limit IPv6 ND NS/RA/RS to the local network. | expand |
On Sat, Apr 18, 2020 at 3:24 AM Dumitru Ceara <dceara@redhat.com> wrote: > > Neighbor solicitation packets for router owned IPs are replied to in > table IN_IP_INPUT at a higher priority than flows relay IPv6 multicast > traffic when needed. All other NS/NA packets received at this point can > be safely dropped. > > However, router advertisement and router solicitation packets are > processed at a later stage, in ND_RA_OPTIONS/ND_RA_RESPONSE. These > packets need to be allowed in table IN_IP_INPUT. > > Commit 677a3ba4d66b incorrectly allowed all IPv6 multicast traffic > destined to all-nodes in table IN_IP_INPUT. Instead, only ND_RA and > ND_RS packets should be allowed. All others were either already > processed or should be dropped. If multicast relay is enabled then IPv6 > multicast traffic that's not destined to reserved groups should also be > allowed. > > Furthermore, router solicitation and advertisement packets that don't > get processed in tables ND_RA_OPTIONS/ND_RA_RESPONSE should be dropped > in IN_IP_ROUTING because they should never be routed. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1825334 > Reported-by: Jakub Libosvar <jlibosva@redhat.com> > Fixes: 677a3ba4d66b ("ovn: Add MLD support.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > northd/ovn-northd.8.xml | 49 ++++++++++++++++++++++++++++++++----------------- > northd/ovn-northd.c | 43 ++++++++++++++++++++++++++++++------------- > 2 files changed, 62 insertions(+), 30 deletions(-) Thanks Dumitru for the fix. I tested locally too and confirm that IPv6 RA packets which entered the router pipeline are dropped. I applied this patch to master and branch-20.03. Thanks Numan > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 82c86f6..efcc4b7 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -1670,22 +1670,6 @@ next; > > <li> > <p> > - A priority-87 flow explicitly allows IPv6 multicast traffic that is > - supposed to reach the router pipeline (e.g., neighbor solicitations > - and traffic destined to the All-Routers multicast group). > - </p> > - </li> > - > - <li> > - <p> > - A priority-86 flow allows IP multicast traffic if > - <ref column="options" table="Logical_Router"/>:mcast_relay='true', > - otherwise drops it. > - </p> > - </li> > - > - <li> > - <p> > ICMP echo reply. These flows reply to ICMP echo requests received > for the router's IP address. Let <var>A</var> be an IP address > owned by a router port. Then, for each <var>A</var> that is > @@ -1946,6 +1930,29 @@ nd.tll = <var>external_mac</var>; > > <li> > <p> > + A priority-84 flow explicitly allows IPv6 multicast traffic that is > + supposed to reach the router pipeline (i.e., router solicitation > + and router advertisement packets). > + </p> > + </li> > + > + <li> > + <p> > + A priority-83 flow explicitly drops IPv6 multicast traffic that is > + destined to reserved multicast groups. > + </p> > + </li> > + > + <li> > + <p> > + A priority-82 flow allows IP multicast traffic if > + <ref column="options" table="Logical_Router"/>:mcast_relay='true', > + otherwise drops it. > + </p> > + </li> > + > + <li> > + <p> > UDP port unreachable. Priority-80 flows generate ICMP port > unreachable messages in reply to UDP datagrams directed to the > router's IP address, except in the special case of gateways, > @@ -2442,6 +2449,13 @@ output; > <ul> > <li> > <p> > + Priority-550 flow that drops IPv6 Router Solicitation/Advertisement > + packets that were not processed in previous tables. > + </p> > + </li> > + > + <li> > + <p> > Priority-500 flows that match IP multicast traffic destined to > groups registered on any of the attached switches and sets > <code>outport</code> to the associated multicast group that will > @@ -2457,7 +2471,8 @@ output; > multicast group, which <code>ovn-northd</code> populates with the > logical ports that have > <ref column="options" table="Logical_Router_Port"/> > - <code>:mcast_flood='true'</code>. > + <code>:mcast_flood='true'</code>. If no router ports are configured > + to flood multicast traffic the packets are dropped. > </p> > </li> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 616e52b..3248350 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -8006,17 +8006,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > > /* Priority-90 flows reply to ARP requests and ND packets. */ > > - /* Allow IPv6 multicast traffic that's supposed to reach the > - * router pipeline (e.g., neighbor solicitations). > - */ > - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 87, "ip6.mcast_flood", > - "next;"); > - > - /* Allow multicast if relay enabled (priority 86). */ > - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 86, > - "ip4.mcast || ip6.mcast", > - od->mcast_info.rtr.relay ? "next;" : "drop;"); > - > /* Drop ARP packets (priority 85). ARP request packets for router's own > * IPs are handled with priority-90 flows. > * Drop IPv6 ND packets (priority 85). ND NA packets for router's own > @@ -8025,6 +8014,21 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 85, > "arp || nd", "drop;"); > > + /* Allow IPv6 multicast traffic that's supposed to reach the > + * router pipeline (e.g., router solicitations). > + */ > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 84, "nd_rs || nd_ra", > + "next;"); > + > + /* Drop other reserved multicast. */ > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 83, > + "ip6.mcast_rsvd", "drop;"); > + > + /* Allow other multicast if relay enabled (priority 82). */ > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 82, > + "ip4.mcast || ip6.mcast", > + od->mcast_info.rtr.relay ? "next;" : "drop;"); > + > /* Drop Ethernet local broadcast. By definition this traffic should > * not be forwarded.*/ > ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 50, > @@ -9545,7 +9549,17 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > * advance to next table (priority 500). > */ > HMAP_FOR_EACH (od, key_node, datapaths) { > - if (!od->nbr || !od->mcast_info.rtr.relay) { > + if (!od->nbr) { > + continue; > + } > + > + /* Drop IPv6 multicast traffic that shouldn't be forwarded, > + * i.e., router solicitation and router advertisement. > + */ > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 550, > + "nd_rs || nd_ra", "drop;"); > + > + if (!od->mcast_info.rtr.relay) { > continue; > } > > @@ -9576,7 +9590,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > } > > /* If needed, flood unregistered multicast on statically configured > - * ports. > + * ports. Otherwise drop any multicast traffic. > */ > if (od->mcast_info.rtr.flood_static) { > ds_clear(&actions); > @@ -9587,6 +9601,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, > "ip.ttl--; " > "next; " > "};"); > + } else { > + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 450, > + "ip4.mcast || ip6.mcast", "drop;"); > } > } > > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 4/20/20 5:13 PM, Numan Siddique wrote: > On Sat, Apr 18, 2020 at 3:24 AM Dumitru Ceara <dceara@redhat.com> wrote: >> >> Neighbor solicitation packets for router owned IPs are replied to in >> table IN_IP_INPUT at a higher priority than flows relay IPv6 multicast >> traffic when needed. All other NS/NA packets received at this point can >> be safely dropped. >> >> However, router advertisement and router solicitation packets are >> processed at a later stage, in ND_RA_OPTIONS/ND_RA_RESPONSE. These >> packets need to be allowed in table IN_IP_INPUT. >> >> Commit 677a3ba4d66b incorrectly allowed all IPv6 multicast traffic >> destined to all-nodes in table IN_IP_INPUT. Instead, only ND_RA and >> ND_RS packets should be allowed. All others were either already >> processed or should be dropped. If multicast relay is enabled then IPv6 >> multicast traffic that's not destined to reserved groups should also be >> allowed. >> >> Furthermore, router solicitation and advertisement packets that don't >> get processed in tables ND_RA_OPTIONS/ND_RA_RESPONSE should be dropped >> in IN_IP_ROUTING because they should never be routed. >> >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1825334 >> Reported-by: Jakub Libosvar <jlibosva@redhat.com> >> Fixes: 677a3ba4d66b ("ovn: Add MLD support.") >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- >> northd/ovn-northd.8.xml | 49 ++++++++++++++++++++++++++++++++----------------- >> northd/ovn-northd.c | 43 ++++++++++++++++++++++++++++++------------- >> 2 files changed, 62 insertions(+), 30 deletions(-) > > Thanks Dumitru for the fix. > I tested locally too and confirm that IPv6 RA packets which entered > the router pipeline > are dropped. > > I applied this patch to master and branch-20.03. > > Thanks > Numan > Thanks Numan! I'll also try to send a patch soon to add a unit test for this issue. Regards, Dumitru
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 82c86f6..efcc4b7 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -1670,22 +1670,6 @@ next; <li> <p> - A priority-87 flow explicitly allows IPv6 multicast traffic that is - supposed to reach the router pipeline (e.g., neighbor solicitations - and traffic destined to the All-Routers multicast group). - </p> - </li> - - <li> - <p> - A priority-86 flow allows IP multicast traffic if - <ref column="options" table="Logical_Router"/>:mcast_relay='true', - otherwise drops it. - </p> - </li> - - <li> - <p> ICMP echo reply. These flows reply to ICMP echo requests received for the router's IP address. Let <var>A</var> be an IP address owned by a router port. Then, for each <var>A</var> that is @@ -1946,6 +1930,29 @@ nd.tll = <var>external_mac</var>; <li> <p> + A priority-84 flow explicitly allows IPv6 multicast traffic that is + supposed to reach the router pipeline (i.e., router solicitation + and router advertisement packets). + </p> + </li> + + <li> + <p> + A priority-83 flow explicitly drops IPv6 multicast traffic that is + destined to reserved multicast groups. + </p> + </li> + + <li> + <p> + A priority-82 flow allows IP multicast traffic if + <ref column="options" table="Logical_Router"/>:mcast_relay='true', + otherwise drops it. + </p> + </li> + + <li> + <p> UDP port unreachable. Priority-80 flows generate ICMP port unreachable messages in reply to UDP datagrams directed to the router's IP address, except in the special case of gateways, @@ -2442,6 +2449,13 @@ output; <ul> <li> <p> + Priority-550 flow that drops IPv6 Router Solicitation/Advertisement + packets that were not processed in previous tables. + </p> + </li> + + <li> + <p> Priority-500 flows that match IP multicast traffic destined to groups registered on any of the attached switches and sets <code>outport</code> to the associated multicast group that will @@ -2457,7 +2471,8 @@ output; multicast group, which <code>ovn-northd</code> populates with the logical ports that have <ref column="options" table="Logical_Router_Port"/> - <code>:mcast_flood='true'</code>. + <code>:mcast_flood='true'</code>. If no router ports are configured + to flood multicast traffic the packets are dropped. </p> </li> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 616e52b..3248350 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -8006,17 +8006,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, /* Priority-90 flows reply to ARP requests and ND packets. */ - /* Allow IPv6 multicast traffic that's supposed to reach the - * router pipeline (e.g., neighbor solicitations). - */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 87, "ip6.mcast_flood", - "next;"); - - /* Allow multicast if relay enabled (priority 86). */ - ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 86, - "ip4.mcast || ip6.mcast", - od->mcast_info.rtr.relay ? "next;" : "drop;"); - /* Drop ARP packets (priority 85). ARP request packets for router's own * IPs are handled with priority-90 flows. * Drop IPv6 ND packets (priority 85). ND NA packets for router's own @@ -8025,6 +8014,21 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 85, "arp || nd", "drop;"); + /* Allow IPv6 multicast traffic that's supposed to reach the + * router pipeline (e.g., router solicitations). + */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 84, "nd_rs || nd_ra", + "next;"); + + /* Drop other reserved multicast. */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 83, + "ip6.mcast_rsvd", "drop;"); + + /* Allow other multicast if relay enabled (priority 82). */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 82, + "ip4.mcast || ip6.mcast", + od->mcast_info.rtr.relay ? "next;" : "drop;"); + /* Drop Ethernet local broadcast. By definition this traffic should * not be forwarded.*/ ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 50, @@ -9545,7 +9549,17 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * advance to next table (priority 500). */ HMAP_FOR_EACH (od, key_node, datapaths) { - if (!od->nbr || !od->mcast_info.rtr.relay) { + if (!od->nbr) { + continue; + } + + /* Drop IPv6 multicast traffic that shouldn't be forwarded, + * i.e., router solicitation and router advertisement. + */ + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 550, + "nd_rs || nd_ra", "drop;"); + + if (!od->mcast_info.rtr.relay) { continue; } @@ -9576,7 +9590,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } /* If needed, flood unregistered multicast on statically configured - * ports. + * ports. Otherwise drop any multicast traffic. */ if (od->mcast_info.rtr.flood_static) { ds_clear(&actions); @@ -9587,6 +9601,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, "ip.ttl--; " "next; " "};"); + } else { + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 450, + "ip4.mcast || ip6.mcast", "drop;"); } }
Neighbor solicitation packets for router owned IPs are replied to in table IN_IP_INPUT at a higher priority than flows relay IPv6 multicast traffic when needed. All other NS/NA packets received at this point can be safely dropped. However, router advertisement and router solicitation packets are processed at a later stage, in ND_RA_OPTIONS/ND_RA_RESPONSE. These packets need to be allowed in table IN_IP_INPUT. Commit 677a3ba4d66b incorrectly allowed all IPv6 multicast traffic destined to all-nodes in table IN_IP_INPUT. Instead, only ND_RA and ND_RS packets should be allowed. All others were either already processed or should be dropped. If multicast relay is enabled then IPv6 multicast traffic that's not destined to reserved groups should also be allowed. Furthermore, router solicitation and advertisement packets that don't get processed in tables ND_RA_OPTIONS/ND_RA_RESPONSE should be dropped in IN_IP_ROUTING because they should never be routed. Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1825334 Reported-by: Jakub Libosvar <jlibosva@redhat.com> Fixes: 677a3ba4d66b ("ovn: Add MLD support.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- northd/ovn-northd.8.xml | 49 ++++++++++++++++++++++++++++++++----------------- northd/ovn-northd.c | 43 ++++++++++++++++++++++++++++++------------- 2 files changed, 62 insertions(+), 30 deletions(-)