[ovs-dev] [PATCH] ovn: ND security vulnerability.

nickcooper-zhangtonghao nickcooper-zhangtonghao at opencloud.tech
Wed Aug 17 16:29:29 UTC 2016


The the logical routers will populate the logical router's ND table when
receiving the NS/ND packets. If we continue to send ND advertisements or
solicitations to logical router, the MAC_Binding table will continue to
increase. That may reduce system performance and cause instability and crashes.

So,
1. When logical router receive a neighbor advertisements, we should check the
   packet's "ip6.dst" and "ip6.src".
2. The logical router uses a cache to store the neighbor solicitations which
   router sends. Only when logical routers send a neighbor solicitations,
   and get a corresponding neighbor advertisements, will the 'ovn-controller'
   update MAC_Binding table of SB database.

Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtonghao at opencloud.tech>
---
 include/ovn/actions.h    |  7 +++++
 ovn/controller/pinctrl.c | 81 ++++++++++++++++++++++++++++++++++++++++++++----
 ovn/lib/actions.c        | 28 +++++++++++++++++
 ovn/northd/ovn-northd.c  | 46 ++++++++++++++++++---------
 4 files changed, 141 insertions(+), 21 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 6f6f858..2e41675 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -60,6 +60,7 @@ struct simap;
     OVNACT(CT_SNAT,       ovnact_ct_nat)            \
     OVNACT(CT_LB,         ovnact_ct_lb)             \
     OVNACT(ARP,           ovnact_nest)              \
+    OVNACT(ND_NS,         ovnact_nest)              \
     OVNACT(ND_NA,         ovnact_nest)              \
     OVNACT(GET_ARP,       ovnact_get_mac_bind)      \
     OVNACT(PUT_ARP,       ovnact_put_mac_bind)      \
@@ -321,6 +322,12 @@ enum action_opcode {
      */
     ACTION_OPCODE_PUT_DHCP_OPTS,
 
+    /* "nd_ns { ...actions... }".
+     *
+     * The actions, in OpenFlow 1.3 format, follow the action_header.
+     */
+    ACTION_OPCODE_ND_NS,
+
     /* "nd_na { ...actions... }".
      *
      * The actions, in OpenFlow 1.3 format, follow the action_header.
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index bba7b29..9728e12 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -77,6 +77,9 @@ static void send_garp_run(const struct ovsrec_bridge *,
                           const char *chassis_id,
                           const struct lport_index *lports,
                           struct hmap *local_datapaths);
+static void pinctrl_handle_nd_ns(const struct flow *ip_flow,
+                                 const struct match *md,
+                                 struct ofpbuf *userdata);
 static void pinctrl_handle_nd_na(const struct flow *ip_flow,
                                  const struct match *md,
                                  struct ofpbuf *userdata);
@@ -715,6 +718,10 @@ process_packet_in(const struct ofp_header *msg)
         pinctrl_handle_put_dhcp_opts(&packet, &pin, &userdata, &continuation);
         break;
 
+    case ACTION_OPCODE_ND_NS:
+        pinctrl_handle_nd_ns(&headers, &pin.flow_metadata, &userdata);
+        break;
+
     case ACTION_OPCODE_ND_NA:
         pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata);
         break;
@@ -1006,12 +1013,9 @@ pinctrl_handle_put_mac_binding(const struct flow *md,
 
     /* Check to see whether the corresponding ARP request exists in the cache.
      * If we don't find it, return directly. */
-    if (is_arp) {
-        bool update = pinctrl_find_delete_arp_request_cache(dp_key, port_key, ip_s);
-
-        if (!update) {
-            return;
-        }
+    bool update = pinctrl_find_delete_arp_request_cache(dp_key, port_key, ip_s);
+    if (!update) {
+        return;
     }
 
     uint32_t hash = hash_string(ip_s, hash_2words(dp_key, port_key));
@@ -1398,6 +1402,71 @@ reload_metadata(struct ofpbuf *ofpacts, const struct match *md)
 }
 
 static void
+pinctrl_handle_nd_ns(const struct flow *ip_flow, const struct match *md,
+                     struct ofpbuf *userdata)
+{
+    /* This action only works for IPv6 ND packets, and the switch should only
+     * send us ND packets this way, but check here just to be sure. */
+    if (ip_flow->dl_type != htons(ETH_TYPE_IPV6)) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+        VLOG_WARN_RL(&rl, "NS action on non-ND packet");
+        return;
+    }
+
+    enum ofp_version version = rconn_get_version(swconn);
+    enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
+
+    uint64_t packet_stub[128 / 8];
+    struct dp_packet packet;
+    dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
+
+    compose_nd_ns(&packet, ip_flow->dl_src,
+                &ip_flow->ipv6_src, &ip_flow->ipv6_dst);
+
+    /* Reload previous packet metadata. */
+    uint64_t ofpacts_stub[4096 / 8];
+    struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
+    reload_metadata(&ofpacts, md);
+
+    enum ofperr error = ofpacts_pull_openflow_actions(userdata, userdata->size,
+                                                      version, &ofpacts);
+    if (error) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+        VLOG_WARN_RL(&rl, "failed to parse actions for 'na' (%s)",
+                     ofperr_to_string(error));
+        goto exit;
+    }
+
+    struct ofputil_packet_out po = {
+        .packet = dp_packet_data(&packet),
+        .packet_len = dp_packet_size(&packet),
+        .buffer_id = UINT32_MAX,
+        .in_port = OFPP_CONTROLLER,
+        .ofpacts = ofpacts.data,
+        .ofpacts_len = ofpacts.size,
+    };
+
+    queue_msg(ofputil_encode_packet_out(&po, proto));
+
+    /* The logical router uses a cache to store the neighbor
+     * solicitations. Only when logical routers send a NS packet,
+     * and get a corresponding NA packet, will the 'ovn-controller'
+     * update mac_binding table of SB database. */
+    uint32_t dp_key = ntohll(md->flow.metadata);
+    uint32_t port_key = md->flow.regs[MFF_LOG_OUTPORT - MFF_REG0];
+    char ip_s[INET6_ADDRSTRLEN];
+
+    ovs_be128 ip6 = hton128(flow_get_xxreg(&md->flow, 0));
+    inet_ntop(AF_INET6, &ip6, ip_s, sizeof(ip_s));
+
+    pinctrl_handle_arp_request_cache(dp_key, port_key, ip_s);
+
+exit:
+    dp_packet_uninit(&packet);
+    ofpbuf_uninit(&ofpacts);
+}
+
+static void
 pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
                      struct ofpbuf *userdata)
 {
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 4beddda..e4e10f6 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1065,6 +1065,12 @@ parse_ARP(struct action_context *ctx)
 }
 
 static void
+parse_ND_NS(struct action_context *ctx)
+{
+    parse_nested_action(ctx, OVNACT_ND_NS, "ip6");
+}
+
+static void
 parse_ND_NA(struct action_context *ctx)
 {
     parse_nested_action(ctx, OVNACT_ND_NA, "nd_ns");
@@ -1086,6 +1092,12 @@ format_ARP(const struct ovnact_nest *nest, struct ds *s)
 }
 
 static void
+format_ND_NS(const struct ovnact_nest *nest, struct ds *s)
+{
+    format_nested_action(nest, "nd_ns", s);
+}
+
+static void
 format_ND_NA(const struct ovnact_nest *nest, struct ds *s)
 {
     format_nested_action(nest, "nd_na", s);
@@ -1124,6 +1136,14 @@ encode_ARP(const struct ovnact_nest *on,
 }
 
 static void
+encode_ND_NS(const struct ovnact_nest *on,
+             const struct ovnact_encode_params *ep,
+             struct ofpbuf *ofpacts)
+{
+    encode_nested_actions(on, ep, ACTION_OPCODE_ND_NS, ofpacts);
+}
+
+static void
 encode_ND_NA(const struct ovnact_nest *on,
              const struct ovnact_encode_params *ep,
              struct ofpbuf *ofpacts)
@@ -1145,6 +1165,12 @@ free_ARP(struct ovnact_nest *nest)
 }
 
 static void
+free_ND_NS(struct ovnact_nest *nest)
+{
+    free_nested_actions(nest);
+}
+
+static void
 free_ND_NA(struct ovnact_nest *nest)
 {
     free_nested_actions(nest);
@@ -1672,6 +1698,8 @@ parse_action(struct action_context *ctx)
         parse_ct_lb_action(ctx);
     } else if (lexer_match_id(ctx->lexer, "arp")) {
         parse_ARP(ctx);
+    } else if (lexer_match_id(ctx->lexer, "nd_ns")) {
+        parse_ND_NS(ctx);
     } else if (lexer_match_id(ctx->lexer, "nd_na")) {
         parse_ND_NA(ctx);
     } else if (lexer_match_id(ctx->lexer, "get_arp")) {
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 02be5e6..0186b39 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3156,17 +3156,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30,
                       ds_cstr(&match), "drop;");
 
-        /* ND advertisement handling.  Use advertisements to populate
-         * the logical router's ARP/ND table. */
-        ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 90, "nd_na",
-                      "put_nd(inport, nd.target, nd.tll);");
-
-        /* Lean from neighbor solicitations that were not directed at
-         * us.  (A priority-90 flow will respond to requests to us and
-         * learn the sender's mac address. */
-        ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 80, "nd_ns",
-                      "put_nd(inport, ip6.src, nd.sll);");
-
         /* Pass other traffic not already handled to the next table for
          * routing. */
         ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 0, "1", "next;");
@@ -3372,9 +3361,24 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                           ds_cstr(&match), "drop;");
         }
 
-        /* ND reply.  These flows reply to ND solicitations for the
-         * router's own IP address. */
         for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
+            /* ND reply handling.  Use Use advertisements to populate the logical
+             * router's ND table. */
+            ds_clear(&match);
+            ds_put_format(&match,
+                    "inport == %s && nd_na && ip6.dst == {%s, %s} "
+                    "&& ip6.src == %s/%d",
+                    op->json_key,
+                    op->lrp_networks.ipv6_addrs[i].addr_s,
+                    op->lrp_networks.ipv6_addrs[i].sn_addr_s,
+                    op->lrp_networks.ipv6_addrs[i].network_s,
+                    op->lrp_networks.ipv6_addrs[i].plen);
+
+            ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
+                          ds_cstr(&match), "put_nd(inport, nd.target, nd.tll);");
+
+             /* ND reply.  These flows reply to ND solicitations for the
+             * router's own IP address. */
             ds_clear(&match);
             ds_put_format(&match,
                     "inport == %s && nd_ns && ip6.dst == {%s, %s} "
@@ -3386,7 +3390,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
 
             ds_clear(&actions);
             ds_put_format(&actions,
-                          "put_nd(inport, ip6.src, nd.sll); "
                           "nd_na { "
                           "eth.src = %s; "
                           "ip6.src = %s; "
@@ -3782,13 +3785,26 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
         }
 
         ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 100,
-                      "eth.dst == 00:00:00:00:00:00",
+                      "ip && eth.dst == 00:00:00:00:00:00",
                       "arp { "
                       "eth.dst = ff:ff:ff:ff:ff:ff; "
                       "arp.spa = reg1; "
                       "arp.op = 1; " /* ARP request */
                       "output; "
                       "};");
+
+        ds_clear(&actions);
+        ds_put_format(&actions,
+                      "nd_ns { "
+                      "nd.target = xxreg0; "
+                      "ip6.src = xxreg1; "
+                      "output; "
+                      "};");
+
+        ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 100,
+                      "ip6 && eth.dst == 00:00:00:00:00:00",
+                      ds_cstr(&actions));
+
         ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 0, "1", "output;");
     }
 
-- 
1.8.3.1






More information about the dev mailing list