@@ -2902,13 +2902,18 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows)
/* Ingress and Egress Pre-ACL Table (Priority 110).
*
- * Not to do conntrack on ND packets. */
+ * Not to do conntrack on ND and ICMP destination
+ * unreachable packets. */
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "nd", "next;");
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, "(nd_rs || nd_ra)",
"next;");
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+ "ip4 && icmp4.type == 3", "next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, "nd", "next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
"(nd_rs || nd_ra)", "next;");
+ ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
+ "ip4 && icmp4.type == 3", "next;");
/* Ingress and Egress Pre-ACL Table (Priority 100).
*
@@ -3089,6 +3094,46 @@ build_acl_log(struct ds *actions, const struct nbrec_acl *acl)
ds_put_cstr(actions, "); ");
}
+static void
+build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows,
+ enum ovn_stage stage, struct nbrec_acl *acl,
+ struct ds *extra_match, struct ds *extra_actions)
+{
+ struct ds match = DS_EMPTY_INITIALIZER;
+ struct ds actions = DS_EMPTY_INITIALIZER;
+ bool ingress = (stage == S_SWITCH_IN_ACL);
+
+ /* XXX: Treat TCP connections as "drop;" for now */
+ build_acl_log(&actions, acl);
+ if (extra_match->length > 0) {
+ ds_put_format(&match, "(%s) && ", extra_match->string);
+ }
+ ds_put_format(&match, "ip && tcp && (%s)", acl->match);
+ ds_put_cstr(&actions, "/* drop */");
+ ovn_lflow_add(lflows, od, stage, acl->priority + OVN_ACL_PRI_OFFSET,
+ ds_cstr(&match), ds_cstr(&actions));
+
+ /* IPv4 traffic */
+ ds_clear(&match);
+ ds_clear(&actions);
+ build_acl_log(&actions, acl);
+ if (extra_match->length > 0) {
+ ds_put_format(&match, "(%s) && ", extra_match->string);
+ }
+ ds_put_format(&match, "ip4 && (%s)", acl->match);
+ if (extra_actions->length > 0) {
+ ds_put_format(&actions, "%s ", extra_actions->string);
+ }
+ ds_put_format(&actions, "reg0 = 0; "
+ "eth.dst <-> eth.src; ip4.dst <-> ip4.src; "
+ "icmp4 { outport <-> inport; %s };",
+ ingress ? "output;" : "next(pipeline=ingress,table=0);");
+ ovn_lflow_add(lflows, od, stage, acl->priority + OVN_ACL_PRI_OFFSET,
+ ds_cstr(&match), ds_cstr(&actions));
+ ds_destroy(&match);
+ ds_destroy(&actions);
+}
+
static void
build_acls(struct ovn_datapath *od, struct hmap *lflows)
{
@@ -3268,29 +3313,26 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
struct ds match = DS_EMPTY_INITIALIZER;
struct ds actions = DS_EMPTY_INITIALIZER;
- /* XXX Need to support "reject", treat it as "drop;" for now. */
- if (!strcmp(acl->action, "reject")) {
- VLOG_INFO("reject is not a supported action");
- }
-
/* The implementation of "drop" differs if stateful ACLs are in
* use for this datapath. In that case, the actions differ
* depending on whether the connection was previously committed
* to the connection tracker with ct_commit. */
if (has_stateful) {
/* If the packet is not part of an established connection, then
- * we can simply drop it. */
- ds_put_format(&match,
- "(!ct.est || (ct.est && ct_label.blocked == 1)) "
- "&& (%s)",
- acl->match);
- build_acl_log(&actions, acl);
- ds_put_cstr(&actions, "/* drop */");
- ovn_lflow_add_with_hint(lflows, od, stage,
- acl->priority + OVN_ACL_PRI_OFFSET,
- ds_cstr(&match), ds_cstr(&actions),
- stage_hint);
-
+ * we can simply reject/drop it. */
+ ds_put_cstr(&match,
+ "(!ct.est || (ct.est && ct_label.blocked == 1))");
+ if (!strcmp(acl->action, "reject")) {
+ build_reject_acl_rules(od, lflows, stage, acl, &match,
+ &actions);
+ } else {
+ ds_put_format(&match, " && (%s)", acl->match);
+ build_acl_log(&actions, acl);
+ ds_put_cstr(&actions, "/* drop */");
+ ovn_lflow_add(lflows, od, stage,
+ acl->priority + OVN_ACL_PRI_OFFSET,
+ ds_cstr(&match), ds_cstr(&actions));
+ }
/* For an existing connection without ct_label set, we've
* encountered a policy change. ACLs previously allowed
* this connection and we committed the connection tracking
@@ -3300,30 +3342,37 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows)
* specifying "next;", we implicitly drop the packet after
* updating conntrack state. We would normally defer
* ct_commit() to the "stateful" stage, but since we're
- * dropping the packet, we go ahead and do it here. */
+ * rejecting/dropping the packet, we go ahead and do it here.
+ */
ds_clear(&match);
ds_clear(&actions);
- ds_put_format(&match,
- "ct.est && ct_label.blocked == 0 && (%s)",
- acl->match);
+ ds_put_cstr(&match, "ct.est && ct_label.blocked == 0");
ds_put_cstr(&actions, "ct_commit(ct_label=1/1); ");
- build_acl_log(&actions, acl);
- ds_put_cstr(&actions, "/* drop */");
- ovn_lflow_add_with_hint(lflows, od, stage,
- acl->priority + OVN_ACL_PRI_OFFSET,
- ds_cstr(&match), ds_cstr(&actions),
- stage_hint);
-
+ if (!strcmp(acl->action, "reject")) {
+ build_reject_acl_rules(od, lflows, stage, acl, &match,
+ &actions);
+ } else {
+ ds_put_format(&match, " && (%s)", acl->match);
+ build_acl_log(&actions, acl);
+ ds_put_cstr(&actions, "/* drop */");
+ ovn_lflow_add(lflows, od, stage,
+ acl->priority + OVN_ACL_PRI_OFFSET,
+ ds_cstr(&match), ds_cstr(&actions));
+ }
} else {
/* There are no stateful ACLs in use on this datapath,
- * so a "drop" ACL is simply the "drop" logical flow action
- * in all cases. */
- build_acl_log(&actions, acl);
- ds_put_cstr(&actions, "/* drop */");
- ovn_lflow_add_with_hint(lflows, od, stage,
- acl->priority + OVN_ACL_PRI_OFFSET,
- acl->match, ds_cstr(&actions),
- stage_hint);
+ * so a "reject/drop" ACL is simply the "reject/drop"
+ * logical flow action in all cases. */
+ if (!strcmp(acl->action, "reject")) {
+ build_reject_acl_rules(od, lflows, stage, acl, &match,
+ &actions);
+ } else {
+ build_acl_log(&actions, acl);
+ ds_put_cstr(&actions, "/* drop */");
+ ovn_lflow_add(lflows, od, stage,
+ acl->priority + OVN_ACL_PRI_OFFSET,
+ acl->match, ds_cstr(&actions));
+ }
}
ds_destroy(&match);
ds_destroy(&actions);
@@ -9339,3 +9339,91 @@ ra_test 000005dc 40 80 40 aef00000000000000000000000000000 30 fd0f00000000000000
OVN_CLEANUP([hv1],[hv2])
AT_CLEANUP
+
+AT_SETUP([ovn -- ACL reject rule test])
+AT_KEYWORDS([acl-reject])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# test_ip_packet INPORT HV ETH_SRC ETH_DST IPV4_SRC IPV4_DST IP_CHKSUM EXP_IP_CHKSUM EXP_ICMP_CHKSUM
+#
+# Causes a packet to be received on INPORT of the hypervisor HV. The packet is an IPv4 packet with
+# ETH_SRC, ETH_DST, IPV4_SRC, IPV4_DST, IP_CHKSUM as specified.
+# EXP_IP_CHKSUM and EXP_ICMP_CHKSUM are the ip and icmp checksums of the icmp destination
+# unreachable frame generated from ACL rule hit
+#
+# INPORT is a lport number, e.g. 11 for vif11.
+# HV is a hypervisor number
+# ETH_SRC and ETH_DST are each 12 hex digits.
+# IPV4_SRC and IPV4_DST are each 8 hex digits.
+# IP_CHKSUM, EXP_IP_CHSUM and EXP_ICMP_CHKSUM are each 4 hex digits
+test_ip_packet() {
+ local inport=$1 hv=$2 eth_src=$3 eth_dst=$4 ipv4_src=$5 ipv4_dst=$6 ip_chksum=$7
+ local exp_ip_chksum=$8 exp_icmp_chksum=$9
+ shift 9
+
+ local ip_ttl=ff
+ local packet=${eth_dst}${eth_src}08004500001400004000${ip_ttl}01${ip_chksum}${ipv4_src}${ipv4_dst}
+
+ local reply_icmp_ttl=ff
+ local icmp_type_code_response=0301
+ local icmp_data=00000000
+ local reply_icmp_payload=${icmp_type_code_response}${exp_icmp_chksum}${icmp_data}
+ local reply=${eth_src}${eth_dst}08004500001c00004000${reply_icmp_ttl}01${exp_ip_chksum}${ipv4_dst}${ipv4_src}${reply_icmp_payload}
+ echo $reply >> vif$inport.expected
+
+ as hv$hv ovs-appctl netdev-dummy/receive vif$inport $packet
+}
+
+# Create hypervisors hv[123].
+# Add vif1[123] to hv1, vif2[123] to hv2, vif3[123] to hv3.
+# Add all of the vifs to a single logical switch sw0.
+
+net_add n1
+ovn-nbctl ls-add sw0
+for i in 1 2 3; do
+ sim_add hv$i
+ as hv$i
+ ovs-vsctl add-br br-phys
+ ovn_attach n1 br-phys 192.168.0.$i
+
+ for j in 1 2 3; do
+ ovn-nbctl lsp-add sw0 sw0-p$i$j -- \
+ lsp-set-addresses sw0-p$i$j "00:00:00:00:00:$i$j 192.168.1.$i$j"
+
+ ovs-vsctl -- add-port br-int vif$i$j -- \
+ set interface vif$i$j \
+ external-ids:iface-id=sw0-p$i$j \
+ options:tx_pcap=hv$i/vif$i$j-tx.pcap \
+ options:rxq_pcap=hv$i/vif$i$j-rx.pcap \
+ ofport-request=$i$j
+ done
+done
+
+OVN_POPULATE_ARP
+# allow some time for ovn-northd and ovn-controller to catch up.
+sleep 1
+
+ip_to_hex() {
+ printf "%02x%02x%02x%02x" "$@"
+}
+
+for i in 1 2 3; do
+ : > vif${i}1.expected
+done
+
+ovn-nbctl --log acl-add sw0 to-lport 1000 "outport == \"sw0-p12\"" reject
+ovn-nbctl --log acl-add sw0 from-lport 1000 "inport == \"sw0-p11\"" reject
+ovn-nbctl --log acl-add sw0 from-lport 1000 "inport == \"sw0-p21\"" reject
+OVS_WAIT_UNTIL([test 3 = $(ovn-sbctl lflow-list | grep 'icmp4 {' | wc -l)])
+
+test_ip_packet 11 1 000000000011 000000000021 $(ip_to_hex 192 168 1 11) $(ip_to_hex 192 168 1 21) 0000 7d8d fcfe
+test_ip_packet 21 2 000000000021 000000000011 $(ip_to_hex 192 168 1 21) $(ip_to_hex 192 168 1 11) 0000 7d8d fcfe
+test_ip_packet 31 3 000000000031 000000000012 $(ip_to_hex 192 168 1 31) $(ip_to_hex 192 168 1 12) 0000 7d82 fcfe
+
+for i in 1 2 3; do
+ OVN_CHECK_PACKETS([hv$i/vif${i}1-tx.pcap], [vif${i}1.expected])
+done
+
+OVN_CLEANUP([hv1], [hv2], [hv3])
+AT_CLEANUP
Whenever the acl reject rule is hit send back an ICMPv4 destination unreachable packet and do not handle reject rule as drop one. Treat TCP connections as DROP for the moment since tcp_reset{} action has not been implemented yet. Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> --- ovn/northd/ovn-northd.c | 123 +++++++++++++++++++++++++++++++++--------------- tests/ovn.at | 88 ++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 37 deletions(-)