[ovs-dev] [PATCHv2 2/2] ovn: Add l3 port security for IPv4 and IPv6
Ben Pfaff
blp at ovn.org
Fri Feb 26 19:27:19 UTC 2016
On Mon, Feb 22, 2016 at 04:00:01PM +0530, Numan Siddique wrote:
> For every port security defined for a logical port, add following lflows
> in "ls_in_port_sec" and "ls_out_port_sec" stage
> - A priority 90 flow to allow ipv4 traffic for known ip addresses
> and (broadcast ip - for ingress, mainly for dhcp)
> - A priority 80 flow to drop all ipv4 traffic.
> - For ingress, a priority 90 flow to allow arp traffic for known
> ip addresses and priority 80 flow to drop all arp traffic
> - A priority 90 flow to allow ipv6 traffic for known ipv6 addresses if
> port security has ipv6 address(es) defined
> - A priority 80 flow to drop all ipv6 traffic.
> - A priority 50 flow to allow all traffic on that port with the matching
> eth address
>
> Eg. if the port security is "00:00:00:00:00:01 10.0.0.2 fe80::eaff:fe28:3390"
>
> priority=90, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
> && arp && arp.sha == 00:00:00:00:00:01 && (arp.spa == 10.0.0.2)), action=(next;)
>
> priority=90, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
> && ip4 && ((ip4.src == 0.0.0.0 && ip4.dst == 255.255.255.255) ||
> ip4.src == 10.0.0.3)), action=(next;)
>
> priority=90, match=(inport == "portname" && eth.src == 00:00:00:00:00:01 && ip6
> && (ip6.src == fe80::200:ff:fe00:1 || ip6.src == :: ||
> ip6.src == fe80::eaff:fe28:3390)), action=(next;)
>
> priority=80, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
> && (arp || ip4)), action=(drop;)
>
> priority=80, match=(inport == "portname" && eth.src == 00:00:00:00:00:01
> && ip6), action=(drop;)
>
> priority=50, match=(inport == "portname" && eth.src == 00:00:00:00:00:01),
> action=(next;)
>
> Co-Authored-by: Ben Pfaff <blp at ovn.org>
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
It looks to me like the following example is not implemented correctly.
That's fine, let's just delete that example:
<dt><code>00:23:20:00:00:00/ff:ff:ff:00:00:00</code></dt>
<dd>
Similar to the first example, except that any Ethernet address in the
Nicira OUI is allowed.
</dd>
It looks to me like when there's only an L2 address, though, this does
not implement this part of the documentation:
It also restricts the inner source MAC addresses that the host
may send in ARP and IPv6 Neighbor Discovery packets.
I see some ways to simplify code, improve the wording of the
documentation, and other improvements. I'm appending some suggested
changes to fold in.
I see some XXXs that it would be nice to fix before getting this in.
I'll look forward to v3.
--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index e416a07..11b61d5 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1042,26 +1042,20 @@ build_port_security_ipv4_flow(enum ovn_pipeline pipeline, struct ds *match,
struct ipv4_netaddr *ipv4_addrs,
int n_ipv4_addrs)
{
- char *ipv4_addr_field;
if (pipeline == P_IN) {
- ipv4_addr_field = "ip4.src";
- /* For ingress pipeline allow broadcast traffic (for dhcp) */
- ds_put_cstr(match, " && ip4 && ((ip4.src == 0.0.0.0 &&"
- " ip4.dst == 255.255.255.255) || ");
- }
- else {
- ipv4_addr_field = "ip4.dst";
- ds_put_cstr(match, " && ip4 && (");
+ ds_put_cstr(match, " && ip4.src == {0.0.0.0, 255.255.255.255, ");
+ } else {
+ ds_put_cstr(match, " && ip4.dst == {");
}
for(int i = 0; i < n_ipv4_addrs; i++) {
- if (i) {
- ds_put_cstr(match, " || ");
- }
- ds_put_format(match, "%s == "IP_FMT, ipv4_addr_field,
- IP_ARGS(ipv4_addrs[i].addr));
+ ds_put_format(match, IP_FMT", ", IP_ARGS(ipv4_addrs[i].addr));
}
- ds_put_cstr(match, ")");
+
+ /* Replace ", " by "}". */
+ ds_chomp(match, ' ');
+ ds_chomp(match, ',');
+ ds_put_cstr(match, "}");
}
static void
@@ -1069,35 +1063,24 @@ build_port_security_ipv6_flow(
enum ovn_pipeline pipeline, struct ds *match, struct eth_addr ea,
struct ipv6_netaddr *ipv6_addrs, int n_ipv6_addrs)
{
- char *ip6_addr_field;
- if (pipeline == P_IN) {
- ip6_addr_field = "ip6.src";
- }
- else {
- ip6_addr_field = "ip6.dst";
- }
-
char ip6_str[INET6_ADDRSTRLEN + 1];
- /* Allow link local address */
+
+ ds_put_format(match, " && %s == {",
+ pipeline == P_IN ? "ip6.src" : "ip6.dst");
+
+ /* Allow link-local address. */
struct in6_addr lla;
in6_generate_lla(ea, &lla);
- memset(ip6_str, 0, sizeof(ip6_str));
ipv6_string_mapped(ip6_str, &lla);
- ds_put_format(match, " && ip6 && (%s == %s", ip6_addr_field, ip6_str);
+ ds_put_format(match, "%s, ", ip6_str);
/* Allow ip6.src=:: and ip6.dst=ff00::/8 for ND packets */
- if (pipeline == P_IN) {
- ds_put_cstr(match, " || ip6.src == ::");
- }
- else {
- ds_put_cstr(match, " || ip6.dst == ff00::/8");
- }
+ ds_put_cstr(match, pipeline == P_IN ? "::" : "ff00::/8");
for(int i = 0; i < n_ipv6_addrs; i++) {
- memset(ip6_str, 0, sizeof(ip6_str));
ipv6_string_mapped(ip6_str, &ipv6_addrs[i].addr);
- ds_put_format(match, " || %s == %s", ip6_addr_field, ip6_str);
+ ds_put_format(match, ", %s", ip6_str);
}
- ds_put_cstr(match, ")");
+ ds_put_cstr(match, "}");
}
/*
@@ -1165,9 +1148,9 @@ build_port_security(enum ovn_pipeline pipeline, struct ovn_port *op,
ds_destroy(&match);
if (!ps.n_ipv4_addrs && !ps.n_ipv6_addrs) {
- /* port security option has only l2 address.
- * Add port security constraints only on eth address which the above
- * priority 50 flow takes care of*/
+ /* port security option has only L2 address. Add port security
+ * constraints only on eth address which the above priority 50 flow
+ * takes care of. */
continue;
}
@@ -1195,7 +1178,7 @@ build_port_security(enum ovn_pipeline pipeline, struct ovn_port *op,
free(ps.ipv4_addrs);
}
- /* Drop all ip and arp packets (for ingress pipline) with
+ /* Drop all IP and ARP packets (for ingress pipeline) with
* priority 80. */
ds_init(&match);
ds_put_format(&match, "%s == %s && %s == "ETH_ADDR_FMT,
@@ -1203,15 +1186,14 @@ build_port_security(enum ovn_pipeline pipeline, struct ovn_port *op,
ETH_ADDR_ARGS(ps.ea));
if (pipeline == P_IN) {
ds_put_cstr(&match, " && (arp || ip4)");
- }
- else {
+ } else {
ds_put_cstr(&match, " && ip4");
}
ovn_lflow_add(lflows, op->od, stage, 80, ds_cstr(&match), "drop;");
ds_destroy(&match);
if (ps.n_ipv6_addrs) {
- /* XXX Need to add port security flows for ipv6 ND */
+ /* XXX Need to add port security flows for IPv6 ND. */
ds_init(&match);
ds_put_format(&match, "%s == %s && %s == "ETH_ADDR_FMT,
port_direction, op->json_key, eth_addr_field,
@@ -1603,12 +1585,10 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
if (lport_is_enabled(op->nbs)) {
build_port_security(P_OUT, op, lflows);
} else {
- struct ds match = DS_EMPTY_INITIALIZER;
- ds_put_format(&match, "outport == %s", op->json_key);
-
+ char *match = xasprintf("outport == %s", op->json_key);
ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC, 150,
- ds_cstr(&match), "drop;");
- ds_destroy(&match);
+ match, "drop;");
+ free(match);
}
}
}
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index cdddda5..72cc511 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -356,9 +356,8 @@
</p>
<p>
- Each element in the set must contain one Ethernet address optionally
- masked followed by zero or more IPv4 or IPv6 addresses (or both).
- It would restrict the host to sending packets from and receiving
+ Each element in the set must begin with one Ethernet address.
+ This would restrict the host to sending packets from and receiving
packets to the ethernet addresses defined in the logical port's
<ref column="port_security"/> column. It also restricts the inner
source MAC addresses that the host may send in ARP and IPv6
@@ -371,9 +370,9 @@
IPv6 addresses (or both), with optional masks. If a mask is given, it
must be a CIDR mask. In addition to the restrictions described for
Ethernet addresses above, such an element restricts the IPv4 or IPv6
- addresses from the host may send and to which it may receive to packets
- to the specified addresses. A masked address, if the host part is
- zero, indicates that the host is allowed to use any addresses in the
+ addresses from which the host may send and to which it may receive
+ packets to the specified addresses. A masked address, if the host part
+ is zero, indicates that the host is allowed to use any address in the
subnet; if the host part is nonzero, the mask simply indicates the size
of the subnet. In addition:
</p>
@@ -443,12 +442,6 @@
other than the one specified.
</dd>
- <dt><code>00:23:20:00:00:00/ff:ff:ff:00:00:00</code></dt>
- <dd>
- Similar to the first example, except that any Ethernet address in the
- Nicira OUI is allowed.
- </dd>
-
<dt><code>80:fa:5b:06:72:b7 192.168.1.10/24</code></dt>
<dd>
This adds further restrictions to the first example. The host may
@@ -463,15 +456,15 @@
<dt><code>"80:fa:5b:12:42:ba", "80:fa:5b:06:72:b7 192.168.1.10/24"</code></dt>
<dd>
- In this case, the host may send traffic from and receive traffic to the
- specified MAC addresses - "80:fa:5b:12:42:ba" and "80:fa:5b:06:72:b7", and
+ The host may send traffic from and receive traffic to the
+ specified MAC addresses, and
to receive traffic to Ethernet multicast and broadcast addresses,
- but not otherwise. With mac "80:fa:5b:12:42:ba", host may
- send traffic from and receive traffic to any l3 address.
- With mac "80:fa:5b:06:72:b7" the host may send IPv4 packets from or
+ but not otherwise. With MAC 80:fa:5b:12:42:ba, the host may
+ send traffic from and receive traffic to any L3 address.
+ With MAC 80:fa:5b:06:72:b7, the host may send IPv4 packets from or
receive IPv4 packets to only 192.168.1.10, except that it may also
receive IPv4 packets to 192.168.1.255 (based on the subnet mask),
- 255.255.255.255, and any address n 224.0.0.0/4 and the host may not
+ 255.255.255.255, and any address in 224.0.0.0/4. The host may not
send or receive any IPv6 (including IPv6 Neighbor Discovery) traffic.
</dd>
</dl>
More information about the dev
mailing list