[ovs-dev] [PATCH 4/6] vswitch: Use "ipsec_gre" vport instead of "gre" with "other_config"
Justin Pettit
jpettit at nicira.com
Wed Dec 22 08:04:32 UTC 2010
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 1cea800..fc35ddb 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 vals["ipsec_psk"]:
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 d545e40..128d43b 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_IS_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 870e03e..9ce4282 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")) {
+ struct tnl_port_config config;
+
+ memcpy(&config, port->config, sizeof config);
+ if (config.flags & TNL_F_IS_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,7 +268,7 @@ 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);
+ translate_netdev_type_to_vport_type(&port);
netdev_vport_get_config(netdev, port.config);
error = do_ioctl(dpif, ODP_VPORT_ATTACH, &port);
@@ -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 13b1d93..ec02917 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_IS_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 e1ea976..8aeb98d 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -220,6 +220,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")) {
struct tnl_port_config config;
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 2f1a2b0..2dd6c93 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -702,9 +702,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
- <code>false</code> to disable. If IPsec is enabled through the
- <ref column="other_config"/> parameters, header caching will be
- automatically disabled.</dd>
+ <code>false</code> to disable.</dd>
+ </dl>
+ </dd>
+ <dt><code>ipsec_gre</code></dt>
+ <dd>An Ethernet over RFC 2890 Generic Routing Encapsulation over
+ IPv4 IPsec tunnel. Each tunnel (including those of type
+ <code>gre</code>) must be uniquely identified by the
+ combination of <code>remote_ip</code> and
+ <code>local_ip</code>. 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
+ <ref column="options"/> column:
+ <dl>
+ <dt><code>remote_ip</code></dt>
+ <dd>Required. The tunnel endpoint.</dd>
+ </dl>
+ <dl>
+ <dt><code>local_ip</code></dt>
+ <dd>Optional. The destination IP that received packets must
+ match. Default is to match all addresses.</dd>
+ </dl>
+ <dl>
+ <dt><code>ipsec_psk</code></dt>
+ <dd>Required. Specifies a pre-shared key for authentication
+ that must be identical on both sides of the tunnel.</dd>
+ </dl>
+ <dl>
+ <dt><code>in_key</code></dt>
+ <dd>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 <code>flow</code>. If
+ <code>flow</code> is specified then any key will be accepted
+ and the key will be placed in the <code>tun_id</code> 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.</dd>
+ </dl>
+ <dl>
+ <dt><code>out_key</code></dt>
+ <dd>Optional. The GRE key to be set on outgoing packets. It may
+ either be a 32-bit number or the word <code>flow</code>. If
+ <code>flow</code> is specified then the key may be set using
+ the <code>set_tunnel</code> 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.</dd>
+ </dl>
+ <dl>
+ <dt><code>key</code></dt>
+ <dd>Optional. Shorthand to set <code>in_key</code> and
+ <code>out_key</code> at the same time.</dd>
+ </dl>
+ <dl>
+ <dt><code>tos</code></dt>
+ <dd>Optional. The value of the ToS bits to be set on the
+ encapsulating packet. It may also be the word
+ <code>inherit</code>, 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.</dd>
+ </dl>
+ <dl>
+ <dt><code>ttl</code></dt>
+ <dd>Optional. The TTL to be set on the encapsulating packet.
+ It may also be the word <code>inherit</code>, 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.</dd>
+ </dl>
+ <dl>
+ <dt><code>csum</code></dt>
+ <dd>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 <code>true</code> to enable.</dd>
+ </dl>
+ <dl>
+ <dt><code>pmtud</code></dt>
+ <dd>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 <code>false</code> to disable.</dd>
</dl>
</dd>
<dt><code>capwap</code></dt>
@@ -938,16 +1029,7 @@
<column name="other_config">
Key-value pairs for rarely used interface features. Currently,
- the only key is for configuring GRE-over-IPsec, which is only
- available through the <code>openvswitch-ipsec</code> package for
- Debian. The currently defined key-value pair is:
- <dl>
- <dt><code>ipsec_psk</code></dt>
- <dd>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
- <ref column="type"/> must be <code>gre</code>.</dd>
- </dl>
+ there are none defined.
</column>
<column name="statistics">
--
1.7.1
More information about the dev
mailing list