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 <hzh...@ebay.com>
---
 ovn/northd/ovn-northd.8.xml | 12 ++++++++----
 ovn/northd/ovn-northd.c     |  8 ++++++--
 tests/ovn.at                | 23 +++++++++++++++++------
 3 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index f1771c6..ffe2727 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -1123,11 +1123,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>
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index ba86bf5..4478cd3 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -5183,8 +5183,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
diff --git a/tests/ovn.at b/tests/ovn.at
index 70c6c50..663db22 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -2671,9 +2671,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
@@ -2682,10 +2682,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
-- 
2.1.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to