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

Qiuyu Xiao qiuyu.xiao.qyx at gmail.com
Tue Jul 31 22:48:08 UTC 2018


This looks good. Thanks for fixing them!

-Qiuyu

On Tue, Jul 31, 2018 at 3:29 PM, Ben Pfaff <blp at ovn.org> wrote:
> 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