[ovs-dev] [PATCH v4 4/9] ipsec: reintroduce IPsec support for tunneling

Ben Pfaff blp at ovn.org
Tue Jul 31 22:29:59 UTC 2018


On Tue, Jul 31, 2018 at 02:08:49PM -0700, Qiuyu Xiao wrote:
> This patch reintroduces ovs-monitor-ipsec daemon that
> was previously removed by commit 2b02d770 ("openvswitch:
> Allow external IPsec tunnel management.")
> 
> After this patch, there are no IPsec flavored tunnels anymore.
> IPsec is enabled by setting up the right values in:
> 1. OVSDB:Interface:options column;
> 2. OVSDB:Open_vSwitch:other_config column;
> 3. OpenFlow pipeline.
> 
> GRE, VXLAN, GENEVE, and STT IPsec tunnels are supported. LibreSwan and
> StrongSwan IKE daemons are supported. User can choose pre-shared key,
> self-signed peer certificate, or CA-signed certificate as authentication
> method.
> 
> Signed-off-by: Qiuyu Xiao <qiuyu.xiao.qyx at gmail.com>
> Signed-off-by: Ansis Atteka <aatteka at ovn.org>
> Co-authored-by: Ansis Atteka <aatteka at ovn.org>

Thanks for the patch.

I think that you must not have flake8 installed.  It gave me a long list
of stylistic errors.  The following incremental fixes them.  Does it
look OK?

--8<--------------------------cut here-------------------------->8--

diff --git a/ipsec/ovs-monitor-ipsec b/ipsec/ovs-monitor-ipsec
index 580c8d8c80a6..163b04004f84 100755
--- a/ipsec/ovs-monitor-ipsec
+++ b/ipsec/ovs-monitor-ipsec
@@ -14,16 +14,12 @@
 # limitations under the License.
 
 import argparse
-import glob
-import os
 import re
 import subprocess
 import sys
 import copy
 from string import Template
 
-from ovs.db import error
-from ovs.db import types
 import ovs.daemon
 import ovs.db.idl
 import ovs.dirs
@@ -55,19 +51,19 @@ conn prevent_unencrypted_vxlan
     mark={0}
 
 """
-transp_tmpl = {"gre" : Template("""\
+transp_tmpl = {"gre": Template("""\
 conn $ifname-$version
 $auth_section
     leftprotoport=gre
     rightprotoport=gre
 
-"""), "gre64" : Template("""\
+"""), "gre64": Template("""\
 conn $ifname-$version
 $auth_section
     leftprotoport=gre
     rightprotoport=gre
 
-"""), "geneve" : Template("""\
+"""), "geneve": Template("""\
 conn $ifname-in-$version
 $auth_section
     leftprotoport=udp/6081
@@ -78,7 +74,7 @@ $auth_section
     leftprotoport=udp
     rightprotoport=udp/6081
 
-"""), "stt" : Template("""\
+"""), "stt": Template("""\
 conn $ifname-in-$version
 $auth_section
     leftprotoport=tcp/7471
@@ -89,7 +85,7 @@ $auth_section
     leftprotoport=tcp
     rightprotoport=tcp/7471
 
-"""), "vxlan" : Template("""\
+"""), "vxlan": Template("""\
 conn $ifname-in-$version
 $auth_section
     leftprotoport=udp/4789
@@ -106,6 +102,7 @@ exiting = False
 monitor = None
 xfrm = None
 
+
 class XFRM(object):
     """This class is a simple wrapper around ip-xfrm (8) command line
     utility.  We are using this class only for informational purposes
@@ -130,11 +127,11 @@ class XFRM(object):
             a = line.split(" ")
             if len(a) >= 4 and a[0] == "src" and a[2] == "dst":
                 dst = (a[3].split("/"))[0]
-                if not dst in policies:
+                if dst not in policies:
                     policies[dst] = []
                 policies[dst].append(line)
                 src = (a[3].split("/"))[0]
-                if not src in policies:
+                if src not in policies:
                     policies[src] = []
                 policies[src].append(line)
         return policies
@@ -155,14 +152,15 @@ class XFRM(object):
                     and a[1] == "src" and a[3] == "dst":
                 remote_ip = a[4].rstrip().split("/")[0]
                 local_ip = a[2].rstrip().split("/")[0]
-                if not remote_ip in securities:
+                if remote_ip not in securities:
                     securities[remote_ip] = []
                 securities[remote_ip].append(line)
-                if not local_ip in securities:
+                if local_ip not in securities:
                     securities[local_ip] = []
                 securities[local_ip].append(line)
         return securities
 
+
 class StrongSwanHelper(object):
     """This class does StrongSwan specific configurations."""
 
@@ -192,18 +190,18 @@ conn %%default
 
 """
 
-    auth_tmpl = {"psk" : Template("""\
+    auth_tmpl = {"psk": Template("""\
     left=$local_ip
     right=$remote_ip
     authby=psk"""),
-                 "pki_remote" : Template("""\
+                 "pki_remote": Template("""\
     left=$local_ip
     right=$remote_ip
     leftid=$local_name
     rightid=$remote_name
     leftcert=$certificate
     rightcert=$remote_cert"""),
-                 "pki_ca" : Template("""\
+                 "pki_ca": Template("""\
     left=$local_ip
     right=$remote_ip
     leftid=$local_name
@@ -256,7 +254,7 @@ conn %%default
             if not m:
                 continue
             ifname = m.group(1)
-            if not ifname in conns:
+            if ifname not in conns:
                 conns[ifname] = {}
             (conns[ifname])[tunnel_name[0]] = line
 
@@ -354,7 +352,10 @@ conn %%default
                 # interface name in IPsec "connection"
                 try:
                     ver = int(re.findall(r'\d+', conn[len(ifname):])[0])
-                except ValueError, IndexError:
+                except IndexError:
+                    vlog.err("%s does not contain version number")
+                    continue
+                except ValueError:
                     vlog.err("%s does not contain version number")
                     continue
 
@@ -362,6 +363,7 @@ conn %%default
                     vlog.info("%s is outdated %u" % (conn, ver))
                     subprocess.call([self.IPSEC, "stroke", "down-nb", conn])
 
+
 class LibreSwanHelper(object):
     """This class does LibreSwan specific configurations."""
     CONF_HEADER = """%s
@@ -378,11 +380,11 @@ conn %%default
 
 """ % (FILE_HEADER)
 
-    auth_tmpl = {"psk" : Template("""\
+    auth_tmpl = {"psk": Template("""\
     left=$local_ip
     right=$remote_ip
     authby=secret"""),
-                 "pki_remote" : Template("""\
+                 "pki_remote": Template("""\
     left=$local_ip
     right=$remote_ip
     leftid=@$local_name
@@ -390,7 +392,7 @@ conn %%default
     leftcert="$local_name"
     rightcert="$remote_name"
     leftrsasigkey=%cert"""),
-                 "pki_ca" : Template("""\
+                 "pki_ca": Template("""\
     left=$local_ip
     right=$remote_ip
     leftid=@$local_name
@@ -500,7 +502,10 @@ conn %%default
                 # interface name in IPsec "connection"
                 try:
                     ver = int(re.findall(r'\d+', conn[len(ifname):])[0])
-                except ValueError, IndexError:
+                except ValueError:
+                    vlog.err("%s does not contain version number")
+                    continue
+                except IndexError:
                     vlog.err("%s does not contain version number")
                     continue
 
@@ -577,7 +582,7 @@ conn %%default
                 continue
 
             ifname = m.group(1)
-            if not ifname in conns:
+            if ifname not in conns:
                 conns[ifname] = {}
             (conns[ifname])[conn] = line
 
@@ -665,11 +670,11 @@ conn %%default
                 raise Exception(proc.stderr.read())
 
             if cacert:
-                proc= subprocess.Popen(['certutil', '-A', '-a', '-i',
-                                       cacert, '-d', 'sql:/etc/ipsec.d/',
-                                       '-n', 'cacert', '-t', 'CT,,'],
-                                       stdout=subprocess.PIPE,
-                                       stderr=subprocess.PIPE)
+                proc = subprocess.Popen(['certutil', '-A', '-a', '-i',
+                                        cacert, '-d', 'sql:/etc/ipsec.d/',
+                                        '-n', 'cacert', '-t', 'CT,,'],
+                                        stdout=subprocess.PIPE,
+                                        stderr=subprocess.PIPE)
                 proc.wait()
                 if proc.returncode:
                     raise Exception(proc.stderr.read())
@@ -677,6 +682,7 @@ conn %%default
             vlog.err("Import cert and key failed.\n" + str(e))
         subprocess.call(['rm', '-f', '/tmp/%s.p12' % name])
 
+
 class IPsecTunnel(object):
     """This is the base class for IPsec tunnel."""
 
@@ -721,18 +727,18 @@ class IPsecTunnel(object):
             remote_name = monitor._get_cn_from_cert(remote_cert)
 
         new_conf = {
-            "ifname" : self.name,
-            "tunnel_type" : row.type,
-            "remote_ip" : options.get("remote_ip"),
-            "local_ip" : options.get("local_ip"),
-            "skb_mark" : monitor.conf["skb_mark"],
-            "certificate" : monitor.conf["pki"]["certificate"],
-            "private_key" : monitor.conf["pki"]["private_key"],
-            "ca_cert" : monitor.conf["pki"]["ca_cert"],
-            "remote_cert" : remote_cert,
-            "remote_name" : remote_name,
-            "local_name" : monitor.conf["pki"]["local_name"],
-            "psk" : options.get("psk")}
+            "ifname": self.name,
+            "tunnel_type": row.type,
+            "remote_ip": options.get("remote_ip"),
+            "local_ip": options.get("local_ip"),
+            "skb_mark": monitor.conf["skb_mark"],
+            "certificate": monitor.conf["pki"]["certificate"],
+            "private_key": monitor.conf["pki"]["private_key"],
+            "ca_cert": monitor.conf["pki"]["ca_cert"],
+            "remote_cert": remote_cert,
+            "remote_name": remote_name,
+            "local_name": monitor.conf["pki"]["local_name"],
+            "psk": options.get("psk")}
 
         if self.conf != new_conf:
             # Configuration was updated in OVSDB.  Validate it and figure
@@ -751,11 +757,11 @@ class IPsecTunnel(object):
                 self.state = "INVALID"
 
         new_status = {
-            "cfm_state" : "Up" if row.cfm_fault == [False] else
-                          "Down" if row.cfm_fault == [True] else
-                          "Disabled",
-            "ofport" : "Not assigned" if (row.ofport in [[], [-1]]) else
-                       row.ofport[0]}
+            "cfm_state": "Up" if row.cfm_fault == [False] else
+                         "Down" if row.cfm_fault == [True] else
+                         "Disabled",
+            "ofport": "Not assigned" if (row.ofport in [[], [-1]]) else
+                      row.ofport[0]}
 
         if self.status != new_status:
             # Tunnel has become unhealthy or ofport changed.  Simply log this.
@@ -837,6 +843,7 @@ class IPsecTunnel(object):
 
         return True
 
+
 class IPsecMonitor(object):
     """This class monitors and configures IPsec tunnels"""
 
@@ -846,13 +853,13 @@ class IPsecMonitor(object):
 
         # Global configuration shared by all tunnels
         self.conf = {
-            "pki" : {
-                "private_key" : None,
-                "certificate" : None,
-                "ca_cert" : None,
-                "local_name" : None
+            "pki": {
+                "private_key": None,
+                "certificate": None,
+                "ca_cert": None,
+                "local_name": None
             },
-            "skb_mark" : None
+            "skb_mark": None
         }
         self.conf_in_use = copy.deepcopy(self.conf)
 
@@ -963,7 +970,7 @@ class IPsecMonitor(object):
 
         # Mark for removal those tunnels that just disappeared from OVSDB
         for tunnel in self.tunnels.keys():
-            if not tunnel in ifaces:
+            if tunnel not in ifaces:
                 self.del_tunnel(tunnel)
 
     def read_ovsdb(self, data):
@@ -1037,21 +1044,25 @@ class IPsecMonitor(object):
 
         return m.group(1)
 
+
 def unixctl_xfrm_policies(conn, unused_argv, unused_aux):
     global xfrm
     policies = xfrm.get_policies()
     conn.reply(str(policies))
 
+
 def unixctl_xfrm_state(conn, unused_argv, unused_aux):
     global xfrm
     securities = xfrm.get_securities()
     conn.reply(str(securities))
 
+
 def unixctl_ipsec_status(conn, unused_argv, unused_aux):
     global monitor
     conns = monitor.ike_helper.get_active_conns()
     conn.reply(str(conns))
 
+
 def unixctl_show(conn, unused_argv, unused_aux):
     global monitor
     global xfrm
@@ -1059,11 +1070,13 @@ def unixctl_show(conn, unused_argv, unused_aux):
     securities = xfrm.get_securities()
     monitor.show(conn, policies, securities)
 
+
 def unixctl_refresh(conn, unused_argv, unused_aux):
     global monitor
     monitor.ike_helper.refresh(monitor)
     conn.reply(None)
 
+
 def unixctl_exit(conn, unused_argv, unused_aux):
     global monitor
     global exiting
@@ -1078,6 +1091,7 @@ def unixctl_exit(conn, unused_argv, unused_aux):
 
     conn.reply(None)
 
+
 def main():
     parser = argparse.ArgumentParser()
     parser.add_argument("database", metavar="DATABASE",
@@ -1147,6 +1161,7 @@ def main():
     unixctl_server.close()
     idl.close()
 
+
 if __name__ == '__main__':
     try:
         main()


More information about the dev mailing list