From e16a28b5854823e2d67099d49f7690235162b555 Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Wed, 1 Dec 2010 17:23:33 -0800 Subject: [PATCH] vswitch: Use "ipsec_gre" vport instead of "gre" with "other_config" Previously, a GRE-over-IPsec tunnel was created as an interface with a "type" of "gre" and the "other_config" column with "ipsec_cert" or "ipsec_psk" set. This could lead to a potential security problem if a user intended to create a GRE-over-IPsec tunnel, but misconfigured the "ipsec_*" config and created an unencrypted GRE tunnel. This commit defines an "ipsec_gre" tunnel type, which should prevent users from inadvertently establishing insecure tunnels. --- debian/ovs-monitor-ipsec | 33 ++++++----- include/openvswitch/tunnel.h | 1 + lib/dpif-linux.c | 28 ++++++--- lib/netdev-vport.c | 29 +++++++--- lib/odp-util.c | 1 + vswitchd/vswitch.xml | 108 ++++++++++++++++++++++++++++++----- 6 files changed, 157 insertions(+), 43 deletions(-) diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec index 1cea8009..27c15e8c 100755 --- a/debian/ovs-monitor-ipsec +++ b/debian/ovs-monitor-ipsec @@ -255,8 +255,7 @@ def monitor_uuid_schema_cb(schema): new_tables["Interface"] = keep_table_columns( schema, "Interface", {"name": string_type, "type": string_type, - "options": string_map_type, - "other_config": string_map_type}) + "options": string_map_type}) schema.tables = new_tables def usage(): @@ -308,38 +307,42 @@ def main(argv): new_interfaces = {} for rec in idl.data["Interface"].itervalues(): name = rec.name.as_scalar() - ipsec_cert = rec.other_config.get("ipsec_cert") - ipsec_psk = rec.other_config.get("ipsec_psk") + ipsec_cert = rec.options.get("ipsec_cert") + ipsec_psk = rec.options.get("ipsec_psk") is_ipsec = ipsec_cert or ipsec_psk - if rec.type.as_scalar() == "gre" and is_ipsec: - new_interfaces[name] = { + if rec.type.as_scalar() == "ipsec_gre": + if ipsec_cert or ipsec_psk: + new_interfaces[name] = { "remote_ip": rec.options.get("remote_ip"), "local_ip": rec.options.get("local_ip", "0.0.0.0/0"), "ipsec_cert": ipsec_cert, "ipsec_psk": ipsec_psk } + else: + s_log.warning( + "no ipsec_cert or ipsec_psk defined for %s" % name) if interfaces != new_interfaces: for name, vals in interfaces.items(): if name not in new_interfaces.keys(): ipsec.ipsec_cert_del(vals["local_ip"], vals["remote_ip"]) for name, vals in new_interfaces.items(): - if vals == interfaces.get(name): - s_log.warning( - "configuration changed for %s, need to delete " - "interface first" % name) + orig_vals = interfaces.get(name): + if orig_vals: + # Configuration for this host already exists. If + # it has changed, this is an error. + if vals != orig_vals: + s_log.warning( + "configuration changed for %s, need to delete " + "interface first" % name) continue if vals["ipsec_cert"]: ipsec.ipsec_cert_update(vals["local_ip"], vals["remote_ip"], vals["ipsec_cert"]) - elif vals["ipsec_psk"]: + else: ipsec.ipsec_psk_update(vals["local_ip"], vals["remote_ip"], vals["ipsec_psk"]) - else: - s_log.warning( - "no ipsec_cert or ipsec_psk defined for %s" % name) - continue interfaces = new_interfaces diff --git a/include/openvswitch/tunnel.h b/include/openvswitch/tunnel.h index d545e40e..44facfae 100644 --- a/include/openvswitch/tunnel.h +++ b/include/openvswitch/tunnel.h @@ -50,6 +50,7 @@ #define TNL_F_TTL_INHERIT (1 << 5) /* Inherit the TTL from the inner packet. */ #define TNL_F_PMTUD (1 << 6) /* Enable path MTU discovery. */ #define TNL_F_HDR_CACHE (1 << 7) /* Enable tunnel header caching. */ +#define TNL_F_IPSEC (1 << 8) /* Traffic is IPsec encrypted. */ /* This goes in the "config" member of struct odp_port for tunnel vports. */ struct tnl_port_config { diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index 870e03e2..66610cf2 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -37,6 +37,7 @@ #include "netdev.h" #include "netdev-vport.h" #include "ofpbuf.h" +#include "openvswitch/tunnel.h" #include "poll-loop.h" #include "rtnetlink.h" #include "shash.h" @@ -227,18 +228,31 @@ dpif_linux_set_drop_frags(struct dpif *dpif_, bool drop_frags) } static void -translate_vport_type_to_netdev_type(char *type, size_t size) +translate_vport_type_to_netdev_type(struct odp_port *port) { + char *type = port->type; + if (!strcmp(type, "netdev")) { - ovs_strlcpy(type, "system", size); + ovs_strlcpy(type, "system", sizeof port->type); + } else if (!strcmp(type, "gre")) { + const struct tnl_port_config *config; + + config = (struct tnl_port_config *)port->config; + if (config->flags & TNL_F_IPSEC) { + ovs_strlcpy(type, "ipsec_gre", sizeof port->type); + } } } static void -translate_netdev_type_to_vport_type(char *type, size_t size) +translate_netdev_type_to_vport_type(struct odp_port *port) { + char *type = port->type; + if (!strcmp(type, "system")) { - ovs_strlcpy(type, "netdev", size); + ovs_strlcpy(type, "netdev", sizeof port->type); + } else if (!strcmp(type, "ipsec_gre")) { + ovs_strlcpy(type, "gre", sizeof port->type); } } @@ -254,8 +268,8 @@ dpif_linux_port_add(struct dpif *dpif, struct netdev *netdev, memset(&port, 0, sizeof port); strncpy(port.devname, name, sizeof port.devname); strncpy(port.type, type, sizeof port.type); - translate_netdev_type_to_vport_type(port.type, sizeof port.type); netdev_vport_get_config(netdev, port.config); + translate_netdev_type_to_vport_type(&port); error = do_ioctl(dpif, ODP_VPORT_ATTACH, &port); if (!error) { @@ -277,7 +291,7 @@ dpif_linux_port_query__(const struct dpif *dpif, struct odp_port *port) { int error = do_ioctl(dpif, ODP_VPORT_QUERY, port); if (!error) { - translate_vport_type_to_netdev_type(port->type, sizeof port->type); + translate_vport_type_to_netdev_type(port); } return error; } @@ -323,7 +337,7 @@ dpif_linux_port_list(const struct dpif *dpif_, struct odp_port *ports, int n) for (i = 0; i < pv.n_ports; i++) { struct odp_port *port = &pv.ports[i]; - translate_vport_type_to_netdev_type(port->type, sizeof port->type); + translate_vport_type_to_netdev_type(port); } return pv.n_ports; } diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 13b1d930..681bc696 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -433,7 +433,8 @@ parse_tunnel_config(const struct netdev_dev *dev, const struct shash *args, { const char *name = netdev_dev_get_name(dev); const char *type = netdev_dev_get_type(dev); - bool is_gre = !strcmp(type, "gre"); + bool is_gre = false; + bool is_ipsec = false; struct tnl_port_config config; struct shash_node *node; bool ipsec_mech_set = false; @@ -442,6 +443,18 @@ parse_tunnel_config(const struct netdev_dev *dev, const struct shash *args, config.flags |= TNL_F_PMTUD; config.flags |= TNL_F_HDR_CACHE; + if (!strcmp(type, "gre")) { + is_gre = true; + } else if (!strcmp(type, "ipsec_gre")) { + is_gre = true; + is_ipsec = true; + + config.flags |= TNL_F_IPSEC; + + /* IPsec doesn't work when header caching is enabled. */ + config.flags &= ~TNL_F_HDR_CACHE; + } + SHASH_FOR_EACH (node, args) { if (!strcmp(node->name, "remote_ip")) { struct in_addr in_addr; @@ -501,8 +514,8 @@ parse_tunnel_config(const struct netdev_dev *dev, const struct shash *args, if (!strcmp(node->data, "false")) { config.flags &= ~TNL_F_HDR_CACHE; } - } else if (!strcmp(node->name, "ipsec_cert") - || !strcmp(node->name, "ipsec_psk")) { + } else if ((!strcmp(node->name, "ipsec_cert") + || !strcmp(node->name, "ipsec_psk")) && is_ipsec) { ipsec_mech_set = true; } else { VLOG_WARN("%s: unknown %s argument '%s'", @@ -510,11 +523,10 @@ parse_tunnel_config(const struct netdev_dev *dev, const struct shash *args, } } - /* IPsec doesn't work when header caching is enabled. Disable it if the - * IPsec local IP address and authentication mechanism have been defined. */ - if (ipsec_mech_set) { - VLOG_INFO("%s: header caching disabled due to use of IPsec", name); - config.flags &= ~TNL_F_HDR_CACHE; + if (is_ipsec && !ipsec_mech_set) { + VLOG_WARN("%s: IPsec requires an 'ipsec_cert' or ipsec_psk' argument", + name); + return EINVAL; } if (!config.daddr) { @@ -623,6 +635,7 @@ netdev_vport_register(void) { static const struct vport_class vport_classes[] = { { { "gre", VPORT_FUNCTIONS }, parse_tunnel_config }, + { { "ipsec_gre", VPORT_FUNCTIONS }, parse_tunnel_config }, { { "capwap", VPORT_FUNCTIONS }, parse_tunnel_config }, { { "patch", VPORT_FUNCTIONS }, parse_patch_config } }; diff --git a/lib/odp-util.c b/lib/odp-util.c index 1f6d93e9..29f536da 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -222,6 +222,7 @@ void format_odp_port_type(struct ds *ds, const struct odp_port *p) { if (!strcmp(p->type, "gre") + || !strcmp(p->type, "ipsec_gre") || !strcmp(p->type, "capwap")) { const struct tnl_port_config *config; diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index f78a5794..4cc29da0 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -755,9 +755,100 @@ bypass certain components of the IP stack (such as IP tables) and it may be useful to disable it if these features are required or as a debugging measure. Default is enabled, set to - false to disable. If IPsec is enabled through the - parameters, header caching will be - automatically disabled. + false to disable. + + +
ipsec_gre
+
An Ethernet over RFC 2890 Generic Routing Encapsulation over + IPv4 IPsec tunnel. Each tunnel (including those of type + gre) must be uniquely identified by the + combination of remote_ip and + local_ip. Note that if two ports are defined + that are the same except one has an optional identifier and + the other does not, the more specific one is matched first. + The following options may be specified in the + column: +
+
remote_ip
+
Required. The tunnel endpoint.
+
+
+
local_ip
+
Optional. The destination IP that received packets must + match. Default is to match all addresses.
+
+
+
ipsec_psk
+
Required. Specifies a pre-shared key for authentication + that must be identical on both sides of the tunnel.
+
+
+
in_key
+
Optional. The GRE key that received packets must contain. + It may either be a 32-bit number (no key and a key of 0 are + treated as equivalent) or the word flow. If + flow is specified then any key will be accepted + and the key will be placed in the tun_id field + for matching in the flow table. The ovs-ofctl manual page + contains additional information about matching fields in + OpenFlow flows. Default is no key.
+
+
+
out_key
+
Optional. The GRE key to be set on outgoing packets. It may + either be a 32-bit number or the word flow. If + flow is specified then the key may be set using + the set_tunnel Nicira OpenFlow vendor extension (0 + is used in the absence of an action). The ovs-ofctl manual + page contains additional information about the Nicira OpenFlow + vendor extensions. Default is no key.
+
+
+
key
+
Optional. Shorthand to set in_key and + out_key at the same time.
+
+
+
tos
+
Optional. The value of the ToS bits to be set on the + encapsulating packet. It may also be the word + inherit, in which case the ToS will be copied from + the inner packet if it is IPv4 or IPv6 (otherwise it will be + 0). Note that the ECN fields are always inherited. Default is + 0.
+
+
+
ttl
+
Optional. The TTL to be set on the encapsulating packet. + It may also be the word inherit, in which case the + TTL will be copied from the inner packet if it is IPv4 or IPv6 + (otherwise it will be the system default, typically 64). + Default is the system default TTL.
+
+
+
csum
+
Optional. Compute GRE checksums on outgoing packets. + Checksums present on incoming packets will be validated + regardless of this setting. Note that GRE checksums + impose a significant performance penalty as they cover the + entire packet. As the contents of the packet is typically + covered by L3 and L4 checksums, this additional checksum only + adds value for the GRE and encapsulated Ethernet headers. + Default is disabled, set to true to enable.
+
+
+
pmtud
+
Optional. Enable tunnel path MTU discovery. If enabled + ``ICMP destination unreachable - fragmentation'' needed + messages will be generated for IPv4 packets with the DF bit set + and IPv6 packets above the minimum MTU if the packet size + exceeds the path MTU minus the size of the tunnel headers. It + also forces the encapsulating packet DF bit to be set (it is + always set if the inner packet implies path MTU discovery). + Note that this option causes behavior that is typically + reserved for routers and therefore is not entirely in + compliance with the IEEE 802.1D specification for bridges. + Default is enabled, set to false to disable.
capwap
@@ -991,16 +1082,7 @@ Key-value pairs for rarely used interface features. Currently, - the only key is for configuring GRE-over-IPsec, which is only - available through the openvswitch-ipsec package for - Debian. The currently defined key-value pair is: -
-
ipsec_psk
-
Required key for GRE-over-IPsec interfaces. Specifies a - pre-shared key for authentication that must be identical on - both sides of the tunnel. Additionally, the - must be gre.
-
+ there are none defined. -- 2.30.2