ovn-northd: LR respond ARP from valid subnet only.

Currently ovn LR datapath responds ARP requests even if the ARP
requestor's src IP doesn't belong to the LR port's subnets. This
may generate unnecessary ARP responses and there could also be
security concerns. This patch restricts the ARP response only if
the requestor's IP matches the LR port's subnets.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

ovn/northd/ovn-northd.8.xml

@@ -1077,11 +1077,15 @@ next;
 
         <p>
           These flows reply to ARP requests for the router's own IP address.
-          For each router port <var>P</var> that owns IP address <var>A</var>
+          The ARP requests are responded only if the requestor's IP belongs
+          to the same subnets of the logical router port.
+          For each router port <var>P</var> that owns IP address <var>A</var>,
+          which belongs to subnet <var>S</var> with prefix length <var>L</var>,
           and Ethernet address <var>E</var>, a priority-90 flow matches
-          <code>inport == <var>P</var> &amp;&amp; arp.op == 1 &amp;&amp;
-          arp.tpa == <var>A</var></code> (ARP request) with the following
-          actions:
+          <code>inport == <var>P</var> &amp;&amp;
+          arp.spa == <var>S</var>/<var>L</var> &amp;&amp; arp.op == 1
+          &amp;&amp; arp.tpa == <var>A</var></code> (ARP request) with the
+          following actions:
         </p>
 
         <pre>

ovn/northd/ovn-northd.c

@@ -4536,8 +4536,12 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
             ds_clear(&match);
             ds_put_format(&match,
-                          "inport == %s && arp.tpa == %s && arp.op == 1",
-                          op->json_key, op->lrp_networks.ipv4_addrs[i].addr_s);
+                          "inport == %s && arp.spa == %s/%u && arp.tpa == %s"
+                          " && arp.op == 1",
+                          op->json_key,
+                          op->lrp_networks.ipv4_addrs[i].network_s,
+                          op->lrp_networks.ipv4_addrs[i].plen,
+                          op->lrp_networks.ipv4_addrs[i].addr_s);
             if (op->od->l3dgw_port && op == op->od->l3dgw_port
                 && op->od->l3redirect_port) {
                 /* Traffic with eth.src = l3dgw_port->lrp_networks.ea_s

tests/ovn.at

@@ -2418,9 +2418,9 @@ test_arp() {
 # 5. Router replies to query for its MAC address from any random IP address
 #    in its subnet.
 #
-# 6. Router replies to query for its MAC address from another subnet.
+# 6. No reply to query for IP address other than router IP.
 #
-# 7. No reply to query for IP address other than router IP.
+# 7. No reply to query from another subnet.
 for i in 1 2 3; do
   for j in 1 2 3; do
     for k in 1 2 3; do
@@ -2429,10 +2429,21 @@ for i in 1 2 3; do
       rip=`ip_to_hex 192 168 $i$j 254`   # Router IP
       rmac=00000000ff$i$j                # Router MAC
       otherip=`ip_to_hex 192 168 $i$j 55` # Some other IP in subnet
-      test_arp $i$j$k $smac $sip        $rip        $rmac #4
-      test_arp $i$j$k $smac $otherip    $rip        $rmac #5
-      test_arp $i$j$k $smac 0a123456    $rip        $rmac #6
-      test_arp $i$j$k $smac $sip        $otherip          #7
+      externalip=`ip_to_hex 1 2 3 4`      # Some other IP not in subnet
+
+      test_arp $i$j$k $smac $sip        $rip        $rmac      #4
+      test_arp $i$j$k $smac $otherip    $rip        $rmac      #5
+      test_arp $i$j$k $smac $sip        $otherip               #6
+
+      # When rip is 192.168.33.254, ARP request from externalip won't be
+      # filtered, because 192.168.33.254 is configured to switch peer port
+      # for lrp33.
+      lrp33_rsp=
+      if test $i = 3 && test $j = 3; then
+        lrp33_rsp=$rmac
+      fi
+      test_arp $i$j$k $smac $externalip $rip        $lrp33_rsp #7
+
     done
   done
 done