Hi Ryan,
Thank you for your advice. The controller will drop the icmp4 packet whose
eth.src or eth.dst is
broadcast/multicast address and the type of icmp4 is “destination unreachable”.
That may reduce
the broadcast storm attack(e.g. icmp flood, dhcp flood). I write the rate
limiting of icmp4 into
my to-do list. Thanks.
ovn: the implementation of icmp4 reject actions.
It support icmp4 reject (e.g. icmp-net-unreachable, icmp-host-prohibited,
tcp-reset,
icmp-admin-prohibited, icmp-port-unreachable, icmp-net-prohibited,
icmp-host-unreachable,
and icmp-proto-unreachable). The icmp-net-unreachable is default. The "TCP RST"
function
will be completed soon. Reject action support only "from-lport" direction. In
general,
considering performance requirements, it might make sense to support only
"from-lport" direction.
Signed-off-by: nickcooper-zhangtonghao <nickcooper-zhangtong...@opencloud.tech>
---
lib/dp-packet.h | 16 +++++++
lib/packets.c | 21 +++++++++
lib/packets.h | 1 +
ovn/controller/pinctrl.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++
ovn/lib/actions.c | 4 +-
ovn/lib/actions.h | 6 +++
ovn/northd/ovn-northd.c | 37 +++++++++++++--
ovn/ovn-nb.ovsschema | 6 ++-
ovn/ovn-nb.xml | 44 ++++++++++++++++--
ovn/ovn-sb.xml | 4 --
ovn/utilities/ovn-nbctl.c | 41 +++++++++++++++--
11 files changed, 277 insertions(+), 16 deletions(-)
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 7c1e637..a6df36f 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -318,6 +318,15 @@ dp_packet_set_l3(struct dp_packet *b, void *l3)
b->l3_ofs = l3 ? (char *) l3 - (char *) dp_packet_data(b) : UINT16_MAX;
}
+static inline size_t
+dp_packet_l3_size(const struct dp_packet *b)
+{
+ return b->l3_ofs != UINT16_MAX
+ ? (const char *)dp_packet_tail(b) - (const char *)dp_packet_l3(b)
+ - dp_packet_l2_pad_size(b)
+ : 0;
+}
+
static inline void *
dp_packet_l4(const struct dp_packet *b)
{
@@ -342,6 +351,13 @@ dp_packet_l4_size(const struct dp_packet *b)
}
static inline const void *
+dp_packet_get_ip4_payload(const struct dp_packet *b)
+{
+ return OVS_LIKELY(dp_packet_l3_size(b) >= IP_HEADER_LEN)
+ ? (const char *)dp_packet_l3(b) + IP_HEADER_LEN : NULL;
+}
+
+static inline const void *
dp_packet_get_tcp_payload(const struct dp_packet *b)
{
size_t l4_size = dp_packet_l4_size(b);
diff --git a/lib/packets.c b/lib/packets.c
index a27264c..de90c9b 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -1251,6 +1251,9 @@ packet_format_tcp_flags(struct ds *s, uint16_t tcp_flags)
#define ARP_PACKET_SIZE (2 + ETH_HEADER_LEN + VLAN_HEADER_LEN + \
ARP_ETH_HEADER_LEN)
+#define ICMP4_PACKET_SIZE (2 + ETH_HEADER_LEN + VLAN_HEADER_LEN + \
+ IP_HEADER_LEN + ICMP_HEADER_LEN)
+
/* Clears 'b' and replaces its contents by an ARP frame with the specified
* 'arp_op', 'arp_sha', 'arp_tha', 'arp_spa', and 'arp_tpa'. The outer
* Ethernet frame is initialized with Ethernet source 'arp_sha' and destination
@@ -1299,6 +1302,24 @@ compose_arp__(struct dp_packet *b)
dp_packet_set_l3(b, arp);
}
+void
+compose_icmp4__(struct dp_packet *b)
+{
+ dp_packet_clear(b);
+ dp_packet_prealloc_tailroom(b, ICMP4_PACKET_SIZE);
+ dp_packet_reserve(b, 2 + VLAN_HEADER_LEN);
+
+ struct eth_header *eth = dp_packet_put_zeros(b, sizeof *eth);
+ eth->eth_type = htons(ETH_TYPE_IP);
+
+ struct ip_header *ip4 = dp_packet_put_zeros(b, sizeof *ip4);
+ struct icmp_header *icmp4 = dp_packet_put_zeros(b, sizeof *icmp4);
+
+ dp_packet_reset_offsets(b);
+ dp_packet_set_l4(b, icmp4);
+ dp_packet_set_l3(b, ip4);
+}
+
/* This function expect packet with ethernet header with correct
* l3 pointer set. */
static void *
diff --git a/lib/packets.h b/lib/packets.h
index 077ccfa..adfb02b 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1067,6 +1067,7 @@ void compose_arp(struct dp_packet *, uint16_t arp_op,
const struct eth_addr arp_sha,
const struct eth_addr arp_tha, bool broadcast,
ovs_be32 arp_spa, ovs_be32 arp_tpa);
+void compose_icmp4__(struct dp_packet *);
void compose_nd(struct dp_packet *, const struct eth_addr eth_src,
struct in6_addr *, struct in6_addr *);
void compose_na(struct dp_packet *,
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 1370301..7893872 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -72,6 +72,10 @@ static void send_garp_run(const struct ovsrec_bridge *,
static void pinctrl_handle_na(const struct flow *ip_flow,
const struct match *md,
struct ofpbuf *userdata);
+static void pinctrl_handle_icmp4(struct dp_packet *pkt_in,
+ const struct flow *ip_flow,
+ const struct match *md,
+ struct ofpbuf *userdata);
static void reload_metadata(struct ofpbuf *ofpacts,
const struct match *md);
@@ -412,6 +416,10 @@ process_packet_in(const struct ofp_header *msg)
case ACTION_OPCODE_NA:
pinctrl_handle_na(&headers, &pin.flow_metadata, &userdata);
break;
+
+ case ACTION_OPCODE_ICMP4:
+ pinctrl_handle_icmp4(&packet, &headers, &pin.flow_metadata, &userdata);
+ break;
default:
VLOG_WARN_RL(&rl, "unrecognized packet-in opcode %"PRIu32,
@@ -1003,3 +1011,108 @@ exit:
dp_packet_uninit(&packet);
ofpbuf_uninit(&ofpacts);
}
+
+static void
+pinctrl_handle_icmp4(struct dp_packet *pkt_in,
+ const struct flow *ip_flow,
+ const struct match *md,
+ struct ofpbuf *userdata)
+{
+ /* This action only works for ICMP packets, and the switch should only send
+ * us ICMP packets this way, but check here just to be sure. */
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
+ if (ip_flow->dl_type != htons(ETH_TYPE_IP) ||
+ ip_flow->nw_proto != IPPROTO_ICMP) {
+ VLOG_WARN_RL(&rl, "ICMP4 action on non-IP packet (Ethertype
%"PRIx16")"
+ " and (IP Protocol %"PRIx16")",
+ ntohs(ip_flow->dl_type),
+ ntohs(ip_flow->nw_proto));
+ return;
+ }
+
+ if (eth_addr_is_broadcast(ip_flow->dl_dst) ||
+ eth_addr_is_multicast(ip_flow->dl_dst)) {
+ VLOG_WARN_RL(&rl, "The ICMP4 packet's destination mac address is"
+ " broadcast or multicast address.");
+ return;
+ }
+
+ const struct icmp_header *icmp4_in = dp_packet_get_ip4_payload(pkt_in);
+ if (icmp4_in->icmp_type == 3) {
+ VLOG_WARN_RL(&rl, "The ICMP4 packets received indecate that the"
+ " destination is unreachable.");
+ return;
+ }
+
+ /* Compose an ICMP4 packet. */
+ uint64_t packet_stub[128 / 8];
+ struct dp_packet pkt_out;
+
+ dp_packet_use_stub(&pkt_out, packet_stub, sizeof packet_stub);
+ compose_icmp4__(&pkt_out);
+
+ struct eth_header *eth = dp_packet_l2(&pkt_out);
+ eth->eth_dst = ip_flow->dl_dst;
+ eth->eth_src = ip_flow->dl_src;
+
+ struct ip_header *ip4 = dp_packet_l3(&pkt_out);
+ ip4->ip_ihl_ver = IP_IHL_VER(5, 4);
+ ip4->ip_tos = ip_flow->nw_tos;
+ ip4->ip_tot_len = htons(sizeof *ip4 + sizeof *icmp4_in + sizeof *ip4 + 8);
+ ip4->ip_frag_off = htons(IP_DONT_FRAGMENT);
+ ip4->ip_ttl = MAXTTL;
+ ip4->ip_proto = IPPROTO_ICMP;
+ put_16aligned_be32(&ip4->ip_src, ip_flow->nw_src);
+ put_16aligned_be32(&ip4->ip_dst, ip_flow->nw_dst);
+ ip4->ip_csum = csum(ip4, sizeof *ip4);
+
+ /* Format of the ICMP destination unreachable message packet is
+ * ------------------------------------------------------------
+ * | IP HEADER | ICMP HEADER | ICMP DATA: IP Header + 8 Bytes |
+ * ------------------------------------------------------------
+ */
+ struct icmp_header *icmp4 = dp_packet_l4(&pkt_out);
+
+ /* IP header and first 8 bytes of original datagram's data'*/
+ dp_packet_put(&pkt_out, dp_packet_l3(pkt_in), IP_HEADER_LEN +8);
+ icmp4->icmp_csum = csum(icmp4, sizeof *icmp4);
+
+ if (ip_flow->vlan_tci & htons(VLAN_CFI)) {
+ eth_push_vlan(&pkt_out, htons(ETH_TYPE_VLAN_8021Q), ip_flow->vlan_tci);
+ }
+
+ /* Compose actions.
+ *
+ * First, copy metadata from 'md' into the packet-out via "set_field"
+ * actions, then add actions from 'userdata'.
+ */
+ uint64_t ofpacts_stub[4096 / 8];
+ struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
+ enum ofp_version version = rconn_get_version(swconn);
+
+ 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 icmp actions (%s)",
+ ofperr_to_string(error));
+ goto exit;
+ }
+
+ struct ofputil_packet_out po = {
+ .packet = dp_packet_data(&pkt_out),
+ .packet_len = dp_packet_size(&pkt_out),
+ .buffer_id = UINT32_MAX,
+ .in_port = OFPP_CONTROLLER,
+ .ofpacts = ofpacts.data,
+ .ofpacts_len = ofpacts.size,
+ };
+
+ enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
+ queue_msg(ofputil_encode_packet_out(&po, proto));
+
+exit:
+ dp_packet_uninit(&pkt_out);
+ ofpbuf_uninit(&ofpacts);
+}
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 3d10d61..a9b7c5a 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -249,7 +249,7 @@ put_controller_op(struct ofpbuf *ofpacts, enum
action_opcode opcode)
finish_controller_op(ofpacts, ofs);
}
-/* Implements the "arp" and "na" actions, which execute nested actions on a
+/* Implements the "arp", "na" and "icmp4" actions, which execute nested
actions on a
* packet derived from the one being processed. */
static void
parse_nested_action(struct action_context *ctx, enum action_opcode opcode,
@@ -1068,6 +1068,8 @@ parse_action(struct action_context *ctx)
parse_get_arp_action(ctx);
} else if (lexer_match_id(ctx->lexer, "put_arp")) {
parse_put_arp_action(ctx);
+ } else if (lexer_match_id(ctx->lexer, "icmp4")) {
+ parse_nested_action(ctx, ACTION_OPCODE_ICMP4, "ip4");
} else {
action_syntax_error(ctx, "expecting action");
}
diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h
index 48f0140..ed4ef5c 100644
--- a/ovn/lib/actions.h
+++ b/ovn/lib/actions.h
@@ -78,6 +78,12 @@ enum action_opcode {
* The actions, in OpenFlow 1.3 format, follow the action_header.
*/
ACTION_OPCODE_NA,
+
+ /* "icmp4 { ...actions... }".
+ *
+ * The actions, in OpenFlow 1.3 format, follow the action_header.
+ */
+ ACTION_OPCODE_ICMP4,
};
/* Header. */
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 6712d21..78679b1 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1625,11 +1625,42 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
acl->priority + OVN_ACL_PRI_OFFSET,
acl->match, "drop;");
} else if (!strcmp(acl->action, "reject")) {
- /* xxx Need to support "reject". */
- VLOG_INFO("reject is not a supported action");
+
+ /* ACLs support the "icmp-reject" actions.
+ * icmp-net-unreachable
+ * icmp-host-unreachable
+ * icmp-proto-unreachable
+ * icmp-port-unreachable
+ * icmp-net-prohibited
+ * icmp-host-prohibited
+ * icmp-admin-prohibited */
+
+ char reject_action[200];
+ uint8_t icmp_type = 3, icmp_code = 0;
+
+ if (!strcmp(acl->reject_action, "icmp-net-unreachable")) {
+ icmp_code = 0;
+ } else if (!strcmp(acl->reject_action, "icmp-host-unreachable")) {
+ icmp_code = 1;
+ } else if (!strcmp(acl->reject_action, "icmp-proto-unreachable")) {
+ icmp_code = 2;
+ } else if (!strcmp(acl->reject_action, "icmp-port-unreachable")) {
+ icmp_code = 3;
+ } else if (!strcmp(acl->reject_action, "icmp-net-prohibited")) {
+ icmp_code = 9;
+ } else if (!strcmp(acl->reject_action, "icmp-host-prohibited")) {
+ icmp_code = 10;
+ } else if (!strcmp(acl->reject_action, "icmp-admin-prohibited")) {
+ icmp_code = 13;
+ }
+
+ sprintf(reject_action, "icmp4 { eth.src<->eth.dst;
ip4.src<->ip4.dst;"
+ "icmp4.type = %d; icmp4.code = %d; outport = inport;
inport = \"\";output;};",
+ icmp_type, icmp_code);
+
ovn_lflow_add(lflows, od, stage,
acl->priority + OVN_ACL_PRI_OFFSET,
- acl->match, "drop;");
+ acl->match, reject_action);
}
}
}
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 460d5bd..9842664 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
"version": "5.0.0",
- "cksum": "849073644 7576",
+ "cksum": "2026671360 7919",
"tables": {
"Logical_Switch": {
"columns": {
@@ -87,6 +87,10 @@
"match": {"type": "string"},
"action": {"type": {"key": {"type": "string",
"enum": ["set", ["allow",
"allow-related", "drop", "reject"]]}}},
+ "reject_action": {"type": {"key": {"type": "string",
+ "enum": ["set",
["icmp-net-unreachable", "icmp-host-unreachable",
+ "icmp-proto-unreachable",
"icmp-port-unreachable", "icmp-net-prohibited",
+ "icmp-host-prohibited",
"icmp-admin-prohibited", "tcp-reset", ""]]}}},
"log": {"type": "boolean"},
"external_ids": {
"type": {"key": "string", "value": "string",
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index e571eeb..70d20da 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -681,13 +681,51 @@
</li>
<li>
- <code>reject</code>: Drop the packet, replying with a RST for TCP or
- ICMP unreachable message for other IP-based protocols.
- <code>Not implemented--currently treated as drop</code>
+ <code>reject</code>: Reject the packet, replying with a RST for TCP
or
+ ICMP unreachable message for other IP-based protocols. Reject action
support only <code>from-lport</code> direction.
</li>
</ul>
</column>
+ <column name="reject_action">
+ <p>The action to take when the reject ACL rule matches:</p>
+ <ul>
+ <li>
+ <code>icmp-net-unreachable</code>: ICMP network unreachable
(default).
+ </li>
+
+ <li>
+ <code>icmp-host-unreachable</code>: ICMP host unreachable.
+ </li>
+
+ <li>
+ <code>icmp-proto-unreachable</code>: ICMP protocol unreachable.
+ </li>
+
+ <li>
+ <code>icmp-port-unreachable</code>: ICMP port unreachable.
+ </li>
+
+ <li>
+ <code>icmp-net-prohibited</code>: ICMP network prohibited.
+ </li>
+
+ <li>
+ <code>icmp-host-prohibited</code>: ICMP host prohibited.
+ </li>
+
+ <li>
+ <code>icmp-admin-prohibited</code>: ICMP administratively prohibited.
+ </li>
+
+ <li>
+ <code>tcp-reset</code>: TCP RST packet.
+ <code>Not implemented--currently treated as
icmp-net-unreachable</code>
+ </li>
+
+ </ul>
+ </column>
+
<column name="log">
<p>
If set to <code>true</code>, packets that match the ACL will trigger a
diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
index 7b45bbb..73e9799 100644
--- a/ovn/ovn-sb.xml
+++ b/ovn/ovn-sb.xml
@@ -1251,10 +1251,6 @@
<li><code>icmp4.code = 1</code> (host unreachable)</li>
</ul>
- <p>
- Details TBD.
- </p>
-
<p><b>Prerequisite:</b> <code>ip4</code></p>
</dd>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 25916da..55cc1da 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -302,7 +302,7 @@ Logical switch commands:\n\
ls-list print the names of all logical switches\n\
\n\
ACL commands:\n\
- acl-add SWITCH DIRECTION PRIORITY MATCH ACTION [log]\n\
+ acl-add SWITCH DIRECTION PRIORITY MATCH ACTION [REJECT-ACTION] [log]\n\
add an ACL to SWITCH\n\
acl-del SWITCH [DIRECTION [PRIORITY MATCH]]\n\
remove ACLs from SWITCH\n\
@@ -1082,9 +1082,11 @@ nbctl_acl_list(struct ctl_context *ctx)
for (i = 0; i < ls->n_acls; i++) {
const struct nbrec_acl *acl = acls[i];
- ds_put_format(&ctx->output, "%10s %5"PRId64" (%s) %s%s\n",
+ ds_put_format(&ctx->output, "%10s %5"PRId64" (%s) %s %s %s\n",
acl->direction, acl->priority,
- acl->match, acl->action, acl->log ? " log" : "");
+ acl->match, acl->action,
+ strcmp(acl->action, "reject") ? "" : acl->reject_action,
+ acl->log ? "log" : "");
}
free(acls);
@@ -1120,6 +1122,7 @@ nbctl_acl_add(struct ctl_context *ctx)
{
const struct nbrec_logical_switch *ls;
const char *action = ctx->argv[5];
+ const char *reject_action = ctx->argv[6];
ls = ls_by_name_or_uuid(ctx, ctx->argv[1], true);
@@ -1133,6 +1136,35 @@ nbctl_acl_add(struct ctl_context *ctx)
"\"drop\", and \"reject\"", action);
return;
}
+
+ /* Validate reject action. */
+ if (strcmp(action, "reject")) {
+ reject_action = "";
+ } else {
+ if (strcmp(direction, "from-lport"))
+ ctl_fatal("%s: reject action support only \"from-lport\"
direction.",
+ direction);
+
+ if (ctx->argc == 6) {
+ reject_action = "icmp-net-unreachable";
+ } else if (ctx->argc == 7) {
+ if (strcmp(reject_action, "icmp-net-unreachable")
+ && strcmp(reject_action, "icmp-host-unreachable")
+ && strcmp(reject_action, "icmp-proto-unreachable")
+ && strcmp(reject_action, "icmp-port-unreachable")
+ && strcmp(reject_action, "icmp-net-prohibited")
+ && strcmp(reject_action, "icmp-host-prohibited")
+ && strcmp(reject_action, "icmp-admin-prohibited")
+ && strcmp(reject_action, "tcp-reset")) {
+ ctl_fatal("%s: reject action must be one of
\"icmp-net-unreachable\", "
+ "\"icmp-host-unreachable\",
\"icmp-proto-unreachable\", "
+ "\"icmp-port-unreachable\", \"icmp-net-prohibited\", "
+ "\"icmp-host-prohibited\", \"icmp-admin-prohibited\", "
+ "and \"tcp-reset\"", reject_action);
+ return;
+ }
+ }
+ }
/* Create the acl. */
struct nbrec_acl *acl = nbrec_acl_insert(ctx->txn);
@@ -1140,6 +1172,7 @@ nbctl_acl_add(struct ctl_context *ctx)
nbrec_acl_set_direction(acl, direction);
nbrec_acl_set_match(acl, ctx->argv[4]);
nbrec_acl_set_action(acl, action);
+ nbrec_acl_set_reject_action(acl, reject_action);
if (shash_find(&ctx->options, "--log") != NULL) {
nbrec_acl_set_log(acl, true);
}
@@ -2139,7 +2172,7 @@ static const struct ctl_command_syntax nbctl_commands[] =
{
{ "ls-list", 0, 0, "", NULL, nbctl_ls_list, NULL, "", RO },
/* acl commands. */
- { "acl-add", 5, 5, "SWITCH DIRECTION PRIORITY MATCH ACTION", NULL,
+ { "acl-add", 5, 6, "SWITCH DIRECTION PRIORITY MATCH ACTION
[REJECT-ACTIONS]", NULL,
nbctl_acl_add, NULL, "--log", RW },
{ "acl-del", 1, 4, "SWITCH [DIRECTION [PRIORITY MATCH]]", NULL,
nbctl_acl_del, NULL, "", RW },
--
1.8.3.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
