[ovs-dev] [PATCH v5 2/6] ipsec: reintroduce IPsec support for tunneling

Qiuyu Xiao qiuyu.xiao.qyx at gmail.com
Wed Sep 19 21:38:22 UTC 2018


On Wed, Aug 29, 2018 at 3:41 PM Ansis Atteka <ansisatteka at gmail.com> wrote:
>
> On Sun, 19 Aug 2018 at 20:31, Qiuyu Xiao <qiuyu.xiao.qyx at gmail.com> wrote:
> >
> > On Mon, Aug 13, 2018 at 2:33 AM, Ansis Atteka <ansisatteka at gmail.com> wrote:
> > > On Tue, 7 Aug 2018 at 09:43, Qiuyu Xiao <qiuyu.xiao.qyx at gmail.com> 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.
> > > s/mehod/methods
> > >>
> > >> 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>
> > >> ---
> Sorry for responding so late. I was off for the last two weeks.
> > >
> > > I have two high level comments that we privately discussed earlier on Friday:
> > > 1. the local_ip should be wildcardable. Otherwise, if routes change,
> > > then then packets may leak out unencrypted before local_ip gets
> > > explicitly updated by administrator as well.
> >
> > I did some test. The IP can be set as %defaultroute in LibreSwan so
> > that no fixed IP is required. I will use this and avoid local_ip
> > setting on the next revision.
> Great to hear you figured this out.
> >
> > > 2. the strongSwan/Ubuntu and libreswan/Fedora compatibility issue due
> > > to integrity check. I know that this could be strongswan or libreswan
> > > bug, but perhaps we could use some alternate configuration that works?
> > > Did you find one?
> >
> > I didn't find alternate configuration that works. I might just send a
> > email to the upstream mailing list.
> ok
>
> >
> > > Other than that see small implementation details
> > >
> > >>  Makefile.am             |    1 +
> > >>  ipsec/automake.mk       |   10 +
> > >>  ipsec/ovs-monitor-ipsec | 1173 +++++++++++++++++++++++++++++++++++++++
> > >>  3 files changed, 1184 insertions(+)
> > >>  create mode 100644 ipsec/automake.mk
> > >>  create mode 100755 ipsec/ovs-monitor-ipsec
> > >>
> > >> diff --git a/Makefile.am b/Makefile.am
> > >> index 788972804..aeb2d108f 100644
> > >> --- a/Makefile.am
> > >> +++ b/Makefile.am
> > >> @@ -481,6 +481,7 @@ include tests/automake.mk
> > >>  include include/automake.mk
> > >>  include third-party/automake.mk
> > >>  include debian/automake.mk
> > >> +include ipsec/automake.mk
> > >>  include vswitchd/automake.mk
> > >>  include ovsdb/automake.mk
> > >>  include rhel/automake.mk
> > >> diff --git a/ipsec/automake.mk b/ipsec/automake.mk
> > >> new file mode 100644
> > >> index 000000000..1e530cb42
> > >> --- /dev/null
> > >> +++ b/ipsec/automake.mk
> > >> @@ -0,0 +1,10 @@
> > >> +# Copyright (C) 2017 Nicira, Inc.
> > >> +#
> > >> +# Copying and distribution of this file, with or without modification,
> > >> +# are permitted in any medium without royalty provided the copyright
> > >> +# notice and this notice are preserved.  This file is offered as-is,
> > >> +# without warranty of any kind.
> > >> +
> > >> +EXTRA_DIST += \
> > >> +        ipsec/ovs-monitor-ipsec
> > >> +FLAKE8_PYFILES += ipsec/ovs-monitor-ipsec
> > >> diff --git a/ipsec/ovs-monitor-ipsec b/ipsec/ovs-monitor-ipsec
> > >> new file mode 100755
> > >> index 000000000..163b04004
> > >> --- /dev/null
> > >> +++ b/ipsec/ovs-monitor-ipsec
> > >> @@ -0,0 +1,1173 @@
> > >> +#!/usr/bin/env python
> > >> +# Copyright (c) 2017 Nicira, Inc.
> > >> +#
> > >> +# Licensed under the Apache License, Version 2.0 (the "License");
> > >> +# you may not use this file except in compliance with the License.
> > >> +# You may obtain a copy of the License at:
> > >> +#
> > >> +#     http://www.apache.org/licenses/LICENSE-2.0
> > >> +#
> > >> +# Unless required by applicable law or agreed to in writing, software
> > >> +# distributed under the License is distributed on an "AS IS" BASIS,
> > >> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > >> +# See the License for the specific language governing permissions and
> > >> +# limitations under the License.
> > >> +
> > >> +import argparse
> > >> +import re
> > >> +import subprocess
> > >> +import sys
> > >> +import copy
> > >> +from string import Template
> > >> +
> > >> +import ovs.daemon
> > >> +import ovs.db.idl
> > >> +import ovs.dirs
> > >> +import ovs.unixctl
> > >> +import ovs.unixctl.server
> > >> +import ovs.util
> > >> +import ovs.vlog
> > >> +
> > >> +
> > >> +FILE_HEADER = "# Generated by ovs-monitor-ipsec...do not modify by hand!\n\n"
> > >> +SHUNT_POLICY = """conn prevent_unencrypted_gre
> > >> +    type=drop
> > >> +    leftprotoport=gre
> > >> +    mark={0}
> > >> +
> > >> +conn prevent_unencrypted_geneve
> > >> +    type=drop
> > >> +    leftprotoport=udp/6081
> > >> +    mark={0}
> > >> +
> > >> +conn prevent_unencrypted_stt
> > >> +    type=drop
> > >> +    leftprotoport=tcp/7471
> > >> +    mark={0}
> > >> +
> > >> +conn prevent_unencrypted_vxlan
> > >> +    type=drop
> > >> +    leftprotoport=udp/4789
> > >> +    mark={0}
> > >> +
> > >> +"""
> > >> +transp_tmpl = {"gre": Template("""\
> > >> +conn $ifname-$version
> > >> +$auth_section
> > >> +    leftprotoport=gre
> > >> +    rightprotoport=gre
> > >> +
> > >> +"""), "gre64": Template("""\
> > >> +conn $ifname-$version
> > >> +$auth_section
> > >> +    leftprotoport=gre
> > >> +    rightprotoport=gre
> > >> +
> > >> +"""), "geneve": Template("""\
> > >> +conn $ifname-in-$version
> > >> +$auth_section
> > >> +    leftprotoport=udp/6081
> > >> +    rightprotoport=udp
> > >> +
> > >> +conn $ifname-out-$version
> > >> +$auth_section
> > >> +    leftprotoport=udp
> > >> +    rightprotoport=udp/6081
> > >> +
> > >> +"""), "stt": Template("""\
> > >> +conn $ifname-in-$version
> > >> +$auth_section
> > >> +    leftprotoport=tcp/7471
> > >> +    rightprotoport=tcp
> > >> +
> > >> +conn $ifname-out-$version
> > >> +$auth_section
> > >> +    leftprotoport=tcp
> > >> +    rightprotoport=tcp/7471
> > >> +
> > >> +"""), "vxlan": Template("""\
> > >> +conn $ifname-in-$version
> > >> +$auth_section
> > >> +    leftprotoport=udp/4789
> > >> +    rightprotoport=udp
> > >> +
> > >> +conn $ifname-out-$version
> > >> +$auth_section
> > >> +    leftprotoport=udp
> > >> +    rightprotoport=udp/4789
> > >> +
> > >> +""")}
> > >> +vlog = ovs.vlog.Vlog("ovs-monitor-ipsec")
> > >> +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
> > >> +    so that ovs-monitor-ipsec could verify that IKE keying daemon has
> > >> +    installed IPsec policies and security associations into kernel as
> > >> +    expected."""
> > >> +
> > >> +    def __init__(self, ip_root_prefix):
> > >> +        self.IP = ip_root_prefix + "/sbin/ip"
> > >> +
> > >> +    def get_policies(self):
> > >> +        """This function returns IPsec policies (from kernel) in a dictionary
> > >> +        where <key> is destination IPv4 address and <value> is SELECTOR of
> > >> +        the IPsec policy."""
> > >> +        policies = {}
> > >> +        proc = subprocess.Popen([self.IP, 'xfrm', 'policy'],
> > >> +                                stdout=subprocess.PIPE)
> > >> +        while True:
> > >> +            line = proc.stdout.readline().strip()
> > >> +            if line == '':
> > >> +                break
> > >> +            a = line.split(" ")
> > >> +            if len(a) >= 4 and a[0] == "src" and a[2] == "dst":
> > >> +                dst = (a[3].split("/"))[0]
> > >> +                if dst not in policies:
> > >> +                    policies[dst] = []
> > >> +                policies[dst].append(line)
> > >> +                src = (a[3].split("/"))[0]
> > >> +                if src not in policies:
> > >> +                    policies[src] = []
> > >> +                policies[src].append(line)
> > >> +        return policies
> > >> +
> > >> +    def get_securities(self):
> > >> +        """This function returns IPsec security associations (from kernel)
> > >> +        in a dictionary where <key> is destination IPv4 address and <value>
> > >> +        is SELECTOR."""
> > >> +        securities = {}
> > >> +        proc = subprocess.Popen([self.IP, 'xfrm', 'state'],
> > >> +                                stdout=subprocess.PIPE)
> > >> +        while True:
> > >> +            line = proc.stdout.readline().strip()
> > >> +            if line == '':
> > >> +                break
> > >> +            a = line.split(" ")
> > >> +            if len(a) >= 4 and a[0] == "sel" \
> > >> +                    and a[1] == "src" and a[3] == "dst":
> > >> +                remote_ip = a[4].rstrip().split("/")[0]
> > >> +                local_ip = a[2].rstrip().split("/")[0]
> > >> +                if remote_ip not in securities:
> > >> +                    securities[remote_ip] = []
> > >> +                securities[remote_ip].append(line)
> > >> +                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."""
> > >> +
> > >> +    STRONGSWAN_CONF = """%s
> > >> +charon.plugins.kernel-netlink.set_proto_port_transport_sa = yes
> > >> +charon.plugins.kernel-netlink.xfrm_ack_expires = 10
> > >> +charon.load_modular = yes
> > >> +charon.plugins.gcm.load = yes
> > >> +""" % (FILE_HEADER)
> > >> +
> > >> +    CONF_HEADER = """%s
> > >> +config setup
> > >> +    uniqueids=yes
> > >> +
> > >> +conn %%default
> > >> +    keyingtries=%%forever
> > >> +    type=transport
> > >> +    keyexchange=ikev2
> > >> +    auto=route
> > >> +    ike=aes256gcm16-sha256-modp2048
> > >> +    esp=aes256gcm16-modp2048
> > >> +
> > >> +""" % (FILE_HEADER)
> > >> +
> > >> +    CA_SECTION = """ca ca_auth
> > >> +    cacert=%s
> > >> +
> > >> +"""
> > >> +
> > >> +    auth_tmpl = {"psk": Template("""\
> > >> +    left=$local_ip
> > >> +    right=$remote_ip
> > >> +    authby=psk"""),
> > >> +                 "pki_remote": Template("""\
> > >> +    left=$local_ip
> > >> +    right=$remote_ip
> > >> +    leftid=$local_name
> > >> +    rightid=$remote_name
> > >> +    leftcert=$certificate
> > >> +    rightcert=$remote_cert"""),
> > >> +                 "pki_ca": Template("""\
> > >> +    left=$local_ip
> > >> +    right=$remote_ip
> > >> +    leftid=$local_name
> > >> +    rightid=$remote_name
> > >> +    leftcert=$certificate""")}
> > >> +
> > >> +    def __init__(self, root_prefix):
> > >> +        self.CHARON_CONF = root_prefix + "/etc/strongswan.d/ovs.conf"
> > >> +        self.IPSEC = root_prefix + "/usr/sbin/ipsec"
> > >> +        self.IPSEC_CONF = root_prefix + "/etc/ipsec.conf"
> > >> +        self.IPSEC_SECRETS = root_prefix + "/etc/ipsec.secrets"
> > >> +        self.conf_file = None
> > >> +        self.secrets_file = None
> > >> +
> > >> +    def start_ike_daemon(self):
> > >> +        """This function starts StrongSwan."""
> > >> +        f = open(self.CHARON_CONF, "w")
> > >> +        f.write(self.STRONGSWAN_CONF)
> > >> +        f.close()
> > >> +
> > >> +        f = open(self.IPSEC_CONF, "w")
> > >> +        f.write(self.CONF_HEADER)
> > >> +        f.close()
> > >> +
> > >> +        f = open(self.IPSEC_SECRETS, "w")
> > >> +        f.write(FILE_HEADER)
> > >> +        f.close()
> > >> +
> > >> +        vlog.info("Starting StrongSwan")
> > >> +        subprocess.call([self.IPSEC, "start"])
> > >> +
> > >> +    def get_active_conns(self):
> > >> +        """This function parses output from 'ipsec status' command.
> > >> +        It returns dictionary where <key> is interface name (as in OVSDB)
> > >> +        and <value> is another dictionary.  This another dictionary
> > >> +        uses strongSwan connection name as <key> and more detailed
> > >> +        sample line from the parsed outpus as <value>. """
> > >> +
> > >> +        conns = {}
> > >> +        proc = subprocess.Popen([self.IPSEC, 'status'], stdout=subprocess.PIPE)
> > >> +
> > >> +        while True:
> > >> +            line = proc.stdout.readline().strip()
> > >> +            if line == '':
> > >> +                break
> > >> +            tunnel_name = line.split(":")
> > >> +            if len(tunnel_name) < 2:
> > >> +                continue
> > >> +            m = re.match(r"(.*)(-in-\d+|-out-\d+).*", tunnel_name[0])
> > >> +            if not m:
> > >> +                continue
> > >> +            ifname = m.group(1)
> > >> +            if ifname not in conns:
> > >> +                conns[ifname] = {}
> > >> +            (conns[ifname])[tunnel_name[0]] = line
> > >> +
> > >> +        return conns
> > >> +
> > >> +    def config_init(self):
> > >> +        self.conf_file = open(self.IPSEC_CONF, "w")
> > >> +        self.secrets_file = open(self.IPSEC_SECRETS, "w")
> > >> +        self.conf_file.write(self.CONF_HEADER)
> > >> +        self.secrets_file.write(FILE_HEADER)
> > >> +
> > >> +    def config_global(self, monitor):
> > >> +        """Configure the global state of IPsec tunnels."""
> > >> +        needs_refresh = False
> > >> +
> > >> +        if monitor.conf_in_use != monitor.conf:
> > >> +            monitor.conf_in_use = copy.deepcopy(monitor.conf)
> > >> +            needs_refresh = True
> > >> +
> > >> +        # Configure the shunt policy
> > >> +        if monitor.conf_in_use["skb_mark"]:
> > >> +            skb_mark = monitor.conf_in_use["skb_mark"]
> > >> +            self.conf_file.write(SHUNT_POLICY.format(skb_mark))
> > >> +
> > >> +        # Configure the CA cert
> > >> +        if monitor.conf_in_use["pki"]["ca_cert"]:
> > >> +            cacert = monitor.conf_in_use["pki"]["ca_cert"]
> > >> +            self.conf_file.write(self.CA_SECTION % cacert)
> > >> +
> > >> +        return needs_refresh
> > >> +
> > >> +    def config_tunnel(self, tunnel):
> > >> +        if tunnel.conf["psk"]:
> > >> +            self.secrets_file.write('%s %s : PSK "%s"\n' %
> > >> +                            (tunnel.conf["local_ip"], tunnel.conf["remote_ip"],
> > >
> > > local_ip should be wildcardable, if user wants to. Otherwise, if
> > > routes change then packets may sneak out in plain.
> > >> +                             tunnel.conf["psk"]))
> > >> +            auth_section = self.auth_tmpl["psk"].substitute(tunnel.conf)
> > >> +        else:
> > >> +            self.secrets_file.write("%s %s : RSA %s\n" %
> > >> +                            (tunnel.conf["local_ip"], tunnel.conf["remote_ip"],
> > >> +                             tunnel.conf["private_key"]))
> > >> +            if tunnel.conf["remote_cert"]:
> > >> +                tmpl = self.auth_tmpl["pki_remote"]
> > >> +                auth_section = tmpl.substitute(tunnel.conf)
> > >> +            else:
> > >> +                tmpl = self.auth_tmpl["pki_ca"]
> > >> +                auth_section = tmpl.substitute(tunnel.conf)
> > >> +
> > >> +        vals = tunnel.conf.copy()
> > >> +        vals["auth_section"] = auth_section
> > >> +        vals["version"] = tunnel.version
> > >> +        conf_text = transp_tmpl[tunnel.conf["tunnel_type"]].substitute(vals)
> > >> +        self.conf_file.write(conf_text)
> > >> +
> > >> +    def config_fini(self):
> > >> +        self.secrets_file.close()
> > >> +        self.conf_file.close()
> > >> +        self.secrets_file = None
> > >> +        self.conf_file = None
> > >> +
> > >> +    def refresh(self, monitor):
> > >> +        """This functions refreshes strongSwan configuration.  Behind the
> > >> +        scenes this function calls:
> > >> +        1. once "ipsec update" command that tells strongSwan to load
> > >> +           all new tunnels from "ipsec.conf"; and
> > >> +        2. once "ipsec rereadsecrets" command that tells strongswan to load
> > >> +           secrets from "ipsec.conf" file
> > >> +        3. for every removed tunnel "ipsec stroke down-nb <tunnel>" command
> > >> +           that removes old tunnels.
> > >> +        Once strongSwan vici bindings will be distributed with major
> > >> +        Linux distributions this function could be simplified."""
> > >> +        vlog.info("Refreshing StrongSwan configuration")
> > >> +        subprocess.call([self.IPSEC, "update"])
> > >> +        subprocess.call([self.IPSEC, "rereadsecrets"])
> > >> +        # "ipsec update" command does not remove those tunnels that were
> > >> +        # updated or that disappeared from the ipsec.conf file.  So, we have
> > >> +        # to manually remove them by calling "ipsec stroke down-nb <tunnel>"
> > >> +        # command.  We use <version> number to tell apart tunnels that
> > >> +        # were just updated.
> > >> +        # "ipsec down-nb" command is designed to be non-blocking (opposed
> > >> +        # to "ipsec down" command).  This means that we should not be concerned
> > >> +        # about possibility of ovs-monitor-ipsec to block for each tunnel
> > >> +        # while strongSwan sends IKE messages over Internet.
> > >> +        conns_dict = self.get_active_conns()
> > >> +        for ifname, conns in conns_dict.iteritems():
> > >> +            tunnel = monitor.tunnels.get(ifname)
> > >> +            for conn in conns:
> > >> +                # IPsec "connection" names that we choose in strongswan
> > >> +                # must start with Interface name
> > >> +                if not conn.startswith(ifname):
> > >> +                    vlog.err("%s does not start with %s" % (conn, ifname))
> > >> +                    continue
> > >> +
> > >> +                # version number should be the first integer after
> > >> +                # interface name in IPsec "connection"
> > >> +                try:
> > >> +                    ver = int(re.findall(r'\d+', conn[len(ifname):])[0])
> > >> +                except IndexError:
> > >> +                    vlog.err("%s does not contain version number")
> > >> +                    continue
> > >> +                except ValueError:
> > >> +                    vlog.err("%s does not contain version number")
> > >> +                    continue
> > >> +
> > >> +                if not tunnel or tunnel.version != ver:
> > >> +                    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
> > >> +config setup
> > >> +    uniqueids=yes
> > >> +
> > >> +conn %%default
> > >> +    keyingtries=%%forever
> > >> +    type=transport
> > >> +    auto=route
> > >> +    ike=aes_gcm256-sha2_256
> > >> +    esp=aes_gcm256
> > >> +    ikev2=insist
> > >> +
> > >> +""" % (FILE_HEADER)
> > >> +
> > >> +    auth_tmpl = {"psk": Template("""\
> > >> +    left=$local_ip
> > >> +    right=$remote_ip
> > >> +    authby=secret"""),
> > >> +                 "pki_remote": Template("""\
> > >> +    left=$local_ip
> > >> +    right=$remote_ip
> > >> +    leftid=@$local_name
> > >> +    rightid=@$remote_name
> > >> +    leftcert="$local_name"
> > >> +    rightcert="$remote_name"
> > >> +    leftrsasigkey=%cert"""),
> > >> +                 "pki_ca": Template("""\
> > >> +    left=$local_ip
> > >> +    right=$remote_ip
> > >> +    leftid=@$local_name
> > >> +    rightid=@$remote_name
> > >> +    leftcert="$local_name"
> > >> +    leftrsasigkey=%cert
> > >> +    rightca=%same""")}
> > >> +
> > >> +    def __init__(self, libreswan_root_prefix):
> > >> +        self.IPSEC = libreswan_root_prefix + "/usr/sbin/ipsec"
> > >> +        self.IPSEC_CONF = libreswan_root_prefix + "/etc/ipsec.conf"
> > >> +        self.IPSEC_SECRETS = libreswan_root_prefix + "/etc/ipsec.secrets"
> > >> +        self.conf_file = None
> > >> +        self.secrets_file = None
> > >> +
> > >> +    def start_ike_daemon(self):
> > >> +        """This function starts LibreSwan."""
> > >> +        f = open(self.IPSEC_CONF, "w")
> > >> +        f.write(self.CONF_HEADER)
> > >> +        f.close()
> > >> +
> > >> +        f = open(self.IPSEC_SECRETS, "w")
> > >> +        f.write(FILE_HEADER)
> > >> +        f.close()
> > >> +
> > >> +        vlog.info("Starting LibreSwan")
> > >> +        subprocess.call([self.IPSEC, "start"])
> > > Should you start or restart here?
> >
> > 'restart' makes more sense. 'start' won't affect IKE daemon if the IKE
> > daemon is already running. I will change this.
> >
> > >
> > > With libreswan. is there possibility of having some stale config after
> > > restart that may need to be wiped  out?
> >
> > If the stale config is not ovs-ipsec-monior's config, `ipsec restart`
> > won't delete it. The administrator should manage this properly. And I
> > don't think ovs-ipsec-monitor should delete other keys and certs in
> > NSS database which are not configured by itself.
> >
> > If admin uses `systemctl restart/stop openvswitch-ipsec.service`, the
> > stale config belongs to ovs-ipsec-monitor will be deleted by the
> > `unixctl_exit` method.
> It is not guaranteed that unixctl_exit method is invoked, is it? The
> ovs-monitor-ipsec process can be abruptly killed (eg type kill
> -SIGKILL in bash). The new process instance should be able to recover.
>
> However, if you do "ipsec restart", perhaps it may be enough.

Actually, we still need to remove stale config in NSS if the daemon
exits abnormally. In the new revision, I set a prefix to the name of
the certs and private keys added by ovs-monitor-ipsec. When
ovs-monitor-ipsec restarts, it checks whether stale certs and private
keys exist in NSS database. If so, those stale states will be deleted.

> >
> > >
> > >
> > >> +
> > >> +    def config_init(self):
> > >> +        self.conf_file = open(self.IPSEC_CONF, "w")
> > >> +        self.secrets_file = open(self.IPSEC_SECRETS, "w")
> > >> +        self.conf_file.write(self.CONF_HEADER)
> > >> +        self.secrets_file.write(FILE_HEADER)
> > >> +
> > >> +    def config_global(self, monitor):
> > >> +        """Configure the global state of IPsec tunnels."""
> > >> +        needs_refresh = False
> > >> +
> > >> +        if monitor.conf_in_use["pki"] != monitor.conf["pki"]:
> > >> +            # Clear old state
> > >> +            if monitor.conf_in_use["pki"]["certificate"]:
> > >> +                self._delete_local_certs_and_key(monitor.conf_in_use["pki"])
> > >> +
> > >> +            # Load new state
> > >> +            if monitor.conf["pki"]["certificate"]:
> > >> +                self._import_local_certs_and_key(monitor.conf["pki"])
> > >> +
> > >> +            monitor.conf_in_use["pki"] = copy.deepcopy(monitor.conf["pki"])
> > >> +            needs_refresh = True
> > >> +
> > >> +        # Configure the shunt policy
> > >> +        if monitor.conf["skb_mark"]:
> > >> +            self.conf_file.write(SHUNT_POLICY.format(monitor.conf["skb_mark"]))
> > >> +
> > >> +        if monitor.conf_in_use["skb_mark"] != monitor.conf["skb_mark"]:
> > >> +            monitor.conf_in_use["skb_mark"] = monitor.conf["skb_mark"]
> > >> +            needs_refresh = True
> > >> +
> > >> +        return needs_refresh
> > >> +
> > >> +    def config_tunnel(self, tunnel):
> > >> +        if tunnel.conf["psk"]:
> > >> +            self.secrets_file.write('%s %s : PSK "%s"\n' %
> > >> +                            (tunnel.conf["local_ip"], tunnel.conf["remote_ip"],
> > >> +                             tunnel.conf["psk"]))
> > >> +            auth_section = self.auth_tmpl["psk"].substitute(tunnel.conf)
> > >> +        elif tunnel.conf["remote_cert"]:
> > >> +            auth_section = self.auth_tmpl["pki_remote"].substitute(tunnel.conf)
> > >> +            self._import_remote_cert(tunnel.conf["remote_cert"],
> > >> +                                     tunnel.conf["remote_name"])
> > >> +        else:
> > >> +            auth_section = self.auth_tmpl["pki_ca"].substitute(tunnel.conf)
> > >> +
> > >> +        vals = tunnel.conf.copy()
> > >> +        vals["auth_section"] = auth_section
> > >> +        vals["version"] = tunnel.version
> > >> +        conf_text = transp_tmpl[tunnel.conf["tunnel_type"]].substitute(vals)
> > >> +        self.conf_file.write(conf_text)
> > >> +
> > >> +    def config_fini(self):
> > >> +        self.secrets_file.close()
> > >> +        self.conf_file.close()
> > >> +        self.secrets_file = None
> > >> +        self.conf_file = None
> > >> +
> > >> +    def clear_tunnel_state(self, tunnel):
> > >> +        if tunnel.conf["remote_cert"]:
> > >> +            self._delete_remote_cert(tunnel.conf["remote_name"])
> > >> +
> > >> +    def refresh(self, monitor):
> > >> +        vlog.info("Refreshing LibreSwan configuration")
> > >> +        subprocess.call([self.IPSEC, "auto", "--rereadsecrets"])
> > >> +        tunnels = set(monitor.tunnels.keys())
> > >> +
> > >> +        # Delete old connections
> > >> +        conns_dict = self.get_active_conns()
> > >> +        for ifname, conns in conns_dict.iteritems():
> > >> +            tunnel = monitor.tunnels.get(ifname)
> > >> +
> > >> +            for conn in conns:
> > >> +                # IPsec "connection" names must start with Interface name
> > >> +                if not conn.startswith(ifname):
> > >> +                    vlog.err("%s does not start with %s" % (conn, ifname))
> > >> +                    continue
> > >> +
> > >> +                # version number should be the first integer after
> > >> +                # interface name in IPsec "connection"
> > >> +                try:
> > >> +                    ver = int(re.findall(r'\d+', conn[len(ifname):])[0])
> > >> +                except ValueError:
> > >> +                    vlog.err("%s does not contain version number")
> > >> +                    continue
> > >> +                except IndexError:
> > >> +                    vlog.err("%s does not contain version number")
> > >> +                    continue
> > >> +
> > >> +                if not tunnel or tunnel.version != ver:
> > >> +                    vlog.info("%s is outdated %u" % (conn, ver))
> > >> +                    subprocess.call([self.IPSEC, "auto", "--delete", conn])
> > >> +                elif ifname in tunnels:
> > >> +                    tunnels.remove(ifname)
> > >> +
> > >> +        # Activate new connections
> > >> +        for name in tunnels:
> > >> +            ver = monitor.tunnels[name].version
> > >> +            conn_in = "%s-in-%s" % (name, ver)
> > >> +            conn_out = "%s-out-%s" % (name, ver)
> > >> +
> > >> +            # In a corner case, LibreSwan daemon restarts for some reason and
> > >> +            # the "ipsec auto --start" command is lost. Just retry to make sure
> > >> +            # the command is received by LibreSwan.
> > >> +            while True:
> > >> +                proc = subprocess.Popen([self.IPSEC, "auto", "--start",
> > >> +                                        "--asynchronous", conn_in],
> > >> +                                        stdout=subprocess.PIPE,
> > >> +                                        stderr=subprocess.PIPE)
> > >> +                perr = str(proc.stderr.read())
> > >> +                pout = str(proc.stdout.read())
> > >> +                if not re.match(r".*Connection refused.*", perr) and \
> > >> +                        not re.match(r".*need --listen.*", pout):
> > >> +                    break
> > >> +
> > >> +            while True:
> > >> +                proc = subprocess.Popen([self.IPSEC, "auto", "--start",
> > >> +                                        "--asynchronous", conn_out],
> > >> +                                        stdout=subprocess.PIPE,
> > >> +                                        stderr=subprocess.PIPE)
> > >> +                perr = str(proc.stderr.read())
> > >> +                pout = str(proc.stdout.read())
> > >> +                if not re.match(r".*Connection refused.*", perr) and \
> > >> +                        not re.match(r".*need --listen.*", pout):
> > >> +                    break
> > >> +
> > >> +        # Activate shunt policy if configured
> > >> +        if monitor.conf["skb_mark"]:
> > > Should you call these commands only if skb_mark changed? Also, I am
> > > surprised you need to invoke them in the first place - would adding
> > > auto=start for libreswan shunt policies make this happen
> > > automatically(just like with strongSwan)? Maybe libreswan does not
> > > support that?
> >
> > LibreSwan doesn't have `ipsec refresh` command. Activating new
> > connection requires `ipsec auto --add/up/start`. Actually I tested
> > LibreSwan again with these shunt policies. Even though the shunt
> > connections are added to LibreSwan with `ipsec auto --add`, the kernel
> > xfrm policies won't be installed. If I restart LibreSwan, xfrm
> > policies can be installed (but we don't want to restart LibreSwan). I
> > think this is a bug in LibreSwan, I will send an email to the mailing
> > list.
>
> Agree, It is strange that explicit "ipsec auto --add" command is
> required for libreswan.
>
> >
> > >> +            subprocess.call([self.IPSEC, "auto", "--start",
> > >> +                            "--asynchronous", "prevent_unencrypted_gre"])
> > >> +            subprocess.call([self.IPSEC, "auto", "--start",
> > >> +                            "--asynchronous", "prevent_unencrypted_geneve"])
> > >> +            subprocess.call([self.IPSEC, "auto", "--start",
> > >> +                            "--asynchronous", "prevent_unencrypted_stt"])
> > >> +            subprocess.call([self.IPSEC, "auto", "--start",
> > >> +                            "--asynchronous", "prevent_unencrypted_vxlan"])
> > >> +
> > >> +    def get_active_conns(self):
> > >> +        """This function parses output from 'ipsec status' command.
> > >> +        It returns dictionary where <key> is interface name (as in OVSDB)
> > >> +        and <value> is another dictionary.  This another dictionary
> > >> +        uses LibreSwan connection name as <key> and more detailed
> > >> +        sample line from the parsed outpus as <value>. """
> > >> +
> > >> +        conns = {}
> > >> +        proc = subprocess.Popen([self.IPSEC, 'status'], stdout=subprocess.PIPE)
> > >> +
> > >> +        while True:
> > >> +            line = proc.stdout.readline().strip()
> > >> +            if line == '':
> > >> +                break
> > >> +
> > >> +            m = re.search(r"#\d+: \"(.*)\".*", line)
> > >> +            if not m:
> > >> +                continue
> > >> +
> > >> +            conn = m.group(1)
> > >> +            m = re.match(r"(.*)(-in-\d+|-out-\d+)", conn)
> > >> +            if not m:
> > >> +                continue
> > >> +
> > >> +            ifname = m.group(1)
> > >> +            if ifname not in conns:
> > >> +                conns[ifname] = {}
> > >> +            (conns[ifname])[conn] = line
> > >> +
> > >> +        return conns
> > >> +
> > >> +    def _delete_remote_cert(self, remote_name):
> > >> +        """Delete remote certiticate from the NSS database."""
> > >> +        try:
> > >> +            proc = subprocess.Popen(['certutil', '-D', '-d',
> > >> +                                    'sql:/etc/ipsec.d/', '-n', remote_name],
> > >> +                                    stdout=subprocess.PIPE,
> > >> +                                    stderr=subprocess.PIPE)
> > >> +            proc.wait()
> > >> +            if proc.returncode:
> > >> +                raise Exception(proc.stderr.read())
> > >> +        except Exception as e:
> > >> +            vlog.err("Delete remote certificate failed.\n" + str(e))
> > > s/Delete remote certificate failed/Failed to delete remote
> > > ceretificate XXX from NSS: +str(e)
> > >
> > >> +
> > >> +    def _import_remote_cert(self, cert, remote_name):
> > >> +        """Import remote certificate to the NSS database."""
> > >> +        try:
> > >> +            proc = subprocess.Popen(['certutil', '-A', '-a', '-i', cert,
> > >> +                                    '-d', 'sql:/etc/ipsec.d/', '-n',
> > >> +                                    remote_name, '-t', 'P,P,P'],
> > >> +                                    stdout=subprocess.PIPE,
> > >> +                                    stderr=subprocess.PIPE)
> > >> +            proc.wait()
> > >> +            if proc.returncode:
> > >> +                raise Exception(proc.stderr.read())
> > >> +        except Exception as e:
> > >> +            vlog.err("Import remote certificate failed.\n" + str(e))
> > > I would write "Failed to import remote certificate XXX into NSS:" + str(e)
> > >
> > >> +
> > >> +    def _delete_local_certs_and_key(self, pki):
> > >> +        """Delete certs and key from the NSS database."""
> > >> +        name = pki["local_name"]
> > >> +        cacert = pki["ca_cert"]
> > >> +
> > >> +        try:
> > >> +            # Delete certificate and private key
> > >> +            proc = subprocess.Popen(['certutil', '-F', '-d',
> > >> +                                    'sql:/etc/ipsec.d/', '-n', name],
> > >> +                                    stdout=subprocess.PIPE,
> > >> +                                    stderr=subprocess.PIPE)
> > >> +            proc.wait()
> > >> +            if proc.returncode:
> > >> +                raise Exception(proc.stderr.read())
> > >> +
> > >> +            if cacert:
> > >> +                proc = subprocess.Popen(['certutil', '-D', '-d',
> > >> +                                        'sql:/etc/ipsec.d/', '-n', 'cacert'],
> > >> +                                        stdout=subprocess.PIPE,
> > >> +                                        stderr=subprocess.PIPE)
> > >> +                proc.wait()
> > > Sorry, may have forgotten python, but if in the next line you invoke
> > > .wait(), is there a reason you can't merge this into .call()? Same
> > > elsewhere.
> >
> > .call() only returns returncode of the process. I need to read stderr as well.
> >
> > >
> > > Also, can these commands block for long time?
> >
> > I checked the certutil manual and didn't find any information about
> > blocking. Do you have any suggestions on how to test this blocking
> > property?
>
> To make educated guess about likelihood of certutil blocking I would
> recommend to run it under strace. If it does any IPC over sockets (ie
> you should see connect(), send(), recv() calls), then likelyhood that
> it can block for longer times is guaranteed.
> Nevertheless there are other ways for process to block and there (e.g.
> if you are operating on network filesystem, then read() can block).
>

The strace log shows that certutil operations open *.db file and then
read/write the file. So if the file is not in a network file system,
the command won't block.

>
>
> >
> > >
> > >> +                if proc.returncode:
> > >> +                    raise Exception(proc.stderr.read())
> > >> +        except Exception as e:
> > >> +            vlog.err("Delete certs and key failed.\n" + str(e))
> > >> +
> > >> +    def _import_local_certs_and_key(self, pki):
> > >> +        """Import certs and key to the NSS database."""
> > >> +        cert = pki["certificate"]
> > >> +        key = pki["private_key"]
> > >> +        name = pki["local_name"]
> > >> +        cacert = pki["ca_cert"]
> > >> +
> > >> +        try:
> > >> +            # Create p12 file from pem files
> > >> +            proc = subprocess.Popen(['openssl', 'pkcs12', '-export',
> > >> +                                    '-in', cert, '-inkey', key, '-out',
> > >> +                                    '/tmp/%s.p12' % name, '-name', name,
> > >> +                                    '-passout', 'pass:'],
> > >> +                                    stdout=subprocess.PIPE,
> > >> +                                    stderr=subprocess.PIPE)
> > >> +            proc.wait()
> > >> +            if proc.returncode:
> > >> +                raise Exception(proc.stderr.read())
> > >> +
> > >> +            # Load p12 file to the database
> > >> +            proc = subprocess.Popen(['pk12util', '-i', '/tmp/%s.p12' % name,
> > >> +                                    '-d', 'sql:/etc/ipsec.d/', '-W', ''],
> > >> +                                    stdout=subprocess.PIPE,
> > >> +                                    stderr=subprocess.PIPE)
> > >> +            proc.wait()
> > >> +            if proc.returncode:
> > >> +                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.wait()
> > >> +                if proc.returncode:
> > >> +                    raise Exception(proc.stderr.read())
> > >> +        except Exception as e:
> > >> +            vlog.err("Import cert and key failed.\n" + str(e))
> > >> +        subprocess.call(['rm', '-f', '/tmp/%s.p12' % name])
> > > I would use os.remove().
> > >
> > > Also is it possible for name to contain "../.." that would allow to
> > > navigate outside /tmp and delete other files?
> >
> > I plan to use `if name.startswith('../'):` to filter illegal name. Do
> > you think this is a good solution?
>
> I don't think it is safe enought, because escape from /tmp path could
> start not only with "../", but possibly with "<some other dir in
> tmp>/../..". to achieve the same effect.
>
> You could generate path and then make sure that os.dirname() is still in "/tmp".
>
> But I would prefer if there was a way to do this without creating
> files on the filesystem. Especially since the file you create in /tmp
> could be overwritten by another daemon that is hacked and then
> libreswan could be fooled to connect to someone else (unlikely attack,
> but still).
>
> Do openssl and pk12util allow to pass the file through STDIN and
> STDOUT? Haven't looked into man pages.

I agree this is a potential security risk. I checked pk12util and
didn't see it accepts input from STDIN. In my new revision, I use
os.path.abspath to generate path to make sure the file is under /tmp/.
Maybe we can create a directory in /tmp/ and make sure only
ovs-monitor-ipsec can access it?

>
> >
> > >> +
> > >> +
> > >> +class IPsecTunnel(object):
> > >> +    """This is the base class for IPsec tunnel."""
> > >> +
> > >> +    unixctl_config_tmpl = Template("""\
> > >> +  Tunnel Type:    $tunnel_type
> > >> +  Local IP:       $local_ip
> > >> +  Remote IP:      $remote_ip
> > >> +  SKB mark:       $skb_mark
> > >> +  Local cert:     $certificate
> > >> +  Local name:     $local_name
> > >> +  Local key:      $private_key
> > >> +  Remote cert:    $remote_cert
> > >> +  Remote name:    $remote_name
> > >> +  CA cert:        $ca_cert
> > >> +  PSK:            $psk
> > >> +""")
> > >> +
> > >> +    unixctl_status_tmpl = Template("""\
> > >> +  Ofport:         $ofport
> > >> +  CFM state:      $cfm_state
> > >> +""")
> > >> +
> > >> +    def __init__(self, name, row):
> > >> +        self.name = name  # 'name' will not change because it is key in OVSDB
> > >> +        self.version = 0  # 'version' is increased on configuration changes
> > >> +        self.last_refreshed_version = -1
> > >> +        self.state = "INIT"
> > >> +        self.conf = {}
> > >> +        self.status = {}
> > >> +        self.update_conf(row)
> > >> +
> > >> +    def update_conf(self, row):
> > >> +        """This function updates IPsec tunnel configuration by using 'row'
> > >> +        from OVSDB interface table.  If configuration was actually changed
> > >> +        in OVSDB then this function returns True.  Otherwise, it returns
> > >> +        False."""
> > >> +        ret = False
> > >> +        options = row.options
> > >> +        remote_cert = options.get("remote_cert")
> > >> +        remote_name = options.get("remote_name")
> > >> +        if remote_cert:
> > >> +            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")}
> > >> +
> > >> +        if self.conf != new_conf:
> > >> +            # Configuration was updated in OVSDB.  Validate it and figure
> > >> +            # out what to do next with this IPsec tunnel.  Also, increment
> > >> +            # version number of this IPsec tunnel so that we could tell
> > >> +            # apart old and new tunnels in "ipsec status" output.
> > >> +            self.version += 1
> > >> +            ret = True
> > >> +            self.conf = new_conf
> > >> +
> > >> +            if self._is_valid_tunnel_conf():
> > >> +                self.state = "CONFIGURED"
> > >> +            else:
> > >> +                vlog.warn("%s contains invalid configuration%s" %
> > >> +                          (self.name, self.invalid_reason))
> > >> +                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]}
> > >> +
> > >> +        if self.status != new_status:
> > >> +            # Tunnel has become unhealthy or ofport changed.  Simply log this.
> > >> +            vlog.dbg("%s changed status from %s to %s" %
> > >> +                     (self.name, str(self.status), str(new_status)))
> > >> +            self.status = new_status
> > >> +        return ret
> > >> +
> > >> +    def mark_for_removal(self):
> > >> +        """This function marks tunnel for removal."""
> > >> +        self.version += 1
> > >> +        self.state = "REMOVED"
> > >> +
> > >> +    def show(self, policies, securities, conns):
> > >> +        state = self.state
> > >> +        if self.state == "INVALID":
> > >> +            state += self.invalid_reason
> > >> +        header = "Interface name: %s v%u (%s)\n" % (self.name, self.version,
> > >> +                                                    state)
> > >> +        conf = self.unixctl_config_tmpl.substitute(self.conf)
> > >> +        status = self.unixctl_status_tmpl.substitute(self.status)
> > >> +        spds = "Kernel policies installed:\n"
> > >> +        remote_ip = self.conf["remote_ip"]
> > >> +        if remote_ip in policies:
> > >> +            for line in policies[remote_ip]:
> > >> +                spds += "  " + line + "\n"
> > >> +        sas = "Kernel security associations installed:\n"
> > >> +        if remote_ip in securities:
> > >> +            for line in securities[remote_ip]:
> > >> +                sas += "  " + line + "\n"
> > >> +        cons = "IPsec connections that are active:\n"
> > >> +        if self.name in conns:
> > >> +            for tname in conns[self.name]:
> > >> +                cons += "  " + conns[self.name][tname] + "\n"
> > >> +
> > >> +        return header + conf + status + spds + sas + cons + "\n"
> > >> +
> > >> +    def _is_valid_tunnel_conf(self):
> > >> +        """This function verifies if IPsec tunnel has valid configuration
> > >> +        set in 'conf'.  If it is valid, then it returns True.  Otherwise,
> > >> +        it returns False and sets the reason why configuration was considered
> > >> +        as invalid.
> > >> +
> > >> +        This function could be improved in future to also verify validness
> > >> +        of certificates themselves so that ovs-monitor-ipsec would not
> > >> +        pass malformed configuration to IKE daemon."""
> > >> +
> > >> +        self.invalid_reason = None
> > >> +
> > >> +        if not self.conf["remote_ip"]:
> > >> +            self.invalid_reason = ": 'remote_ip' is not set"
> > >> +            return False
> > >> +
> > >> +        if not self.conf["local_ip"]:
> > > Can we make local_ip wilcardable again?
> >
> > Yes.
> >
> > >
> > >> +            self.invalid_reason = ": 'local_ip' is not set"
> > >> +            return False
> > >> +
> > >> +        if self.conf["psk"]:
> > >> +            if self.conf["certificate"] or self.conf["private_key"] \
> > >> +                    or self.conf["ca_cert"] or self.conf["remote_cert"] \
> > >> +                    or self.conf["remote_name"]:
> > >> +                self.invalid_reason = ": 'certificate', 'private_key', "\
> > >> +                                      "'ca_cert', 'remote_cert', and "\
> > >> +                                      "'remote_name' must be unset with PSK"
> > >> +                return False
> > >> +        elif self.conf["remote_name"]:
> > > I am a little bit confused about this remote_name check that must
> > > evaluate to true here. Does this mean that no authentication method
> > > was specified or that remote_name could not be retrieved (at least to
> > > me the invalid_reason seems incorrectly set without giving hint to
> > > user what is missing)?
> >
> > There are only three valid authentication configurations -- pre-shared
> > key, self-signed certificate, CA-signed certificate. Both self-signed
> > certificate and CA-signed certificate have remote_name set for the
> > tunnel. When using self-signed certificate, the remote_name is
> > extracted from the certificate. When using CA-signed certificate, the
> > user needs to explicitly specify the remote_name. I will add more
> > comments and explanations on the invalid_reason.
> Thanks.
> >
> > >
> > >> +            if not self.conf["certificate"]:
> > >> +                self.invalid_reason = ": must set 'certificate' with PKI"
> > >> +                return False
> > >> +            elif not self.conf["private_key"]:
> > >> +                self.invalid_reason = ": must set 'private_key' with PKI"
> > >> +                return False
> > >> +            if not self.conf["remote_cert"] and not self.conf["ca_cert"]:
> > >> +                self.invalid_reason = ": must set 'remote_cert' or 'ca_cert'"
> > >> +                return False
> > >> +        else:
> > >> +            self.invalid_reason = ": must choose a authentication method"
> > >> +            return False
> > >> +
> > >> +        return True
> > >> +
> > >> +
> > >> +class IPsecMonitor(object):
> > >> +    """This class monitors and configures IPsec tunnels"""
> > >> +
> > >> +    def __init__(self, root_prefix):
> > >> +        self.IPSEC = root_prefix + "/usr/sbin/ipsec"
> > >> +        self.tunnels = {}
> > >> +
> > >> +        # Global configuration shared by all tunnels
> > >> +        self.conf = {
> > >> +            "pki": {
> > >> +                "private_key": None,
> > >> +                "certificate": None,
> > >> +                "ca_cert": None,
> > >> +                "local_name": None
> > >> +            },
> > >> +            "skb_mark": None
> > >> +        }
> > >> +        self.conf_in_use = copy.deepcopy(self.conf)
> > >> +
> > >> +        # Choose to either use StrongSwan or LibreSwan as IKE daemon
> > > An easier method to pick right IKE driver would be to pass a specifc,
> > > hardcoded argument to ovs-monitor-ipsec daemon from systemd unit file
> > > or debian init.d script.
> > >
> > > Althugh, that would prevent to easily switch between libreswan on
> > > Ubuntu or strongSwan on Fedora.
> > >
> > > Here are the concerns I have with the current method you are proposing:
> > > 1. on Ubuntu you terminate ovs-monitor-ipsec if it could not find
> > > ipsec utility; and the would let the monitor process to restart it in
> > > loop
> > > 2. on Fedora you simply give up (inconsistent wrt Ubuntu)
> > > 3. if `ipsec` changes STDOUT format then ovs-monitor-ipsec may not
> > > work due to this small detail.
> >
> > I think both LibreSwan and StrongSwan call `sys.exit(1)` if no `ipsec`
> > utility is detected. Their behaviors are consistent but might bring
> > the monitor process in a loop. Do you have any suggestions on how to
> > avoid such behavior? How to kill the current process and also the
> > monitor process?
> One way to solve this would be to have ovs-monitor-ipsec monitoring on
> Fedora. Can you take a look at how it is done for ovs-vswitchd
> process?
> >
> > I agree with 3. Actually the StrongSwan interface in Fedora is very
> > different from Ubuntu. And currently ovs-monitor-ipsec doesn't support
> > StrongSwan in Fedora. I might just change the init script to use
> > LibreSwan in Fedora and StrongSwan in Ubuntu.
>
> This is good with me. Would keep python code simpler.
>
> >
> > >
> > >> +        try:
> > >> +            proc = subprocess.Popen([self.IPSEC, "--version"],
> > >> +                                    stdout=subprocess.PIPE)
> > >> +            line = proc.stdout.readline().strip().split(" ")
> > >> +
> > >> +            if len(line) >= 2 and line[1] in ["strongSwan", "Libreswan"]:
> > >> +                if line[1] == "strongSwan":
> > >> +                    self.ike_helper = StrongSwanHelper(root_prefix)
> > >> +                else:
> > >> +                    self.ike_helper = LibreSwanHelper(root_prefix)
> > >> +            else:
> > >> +                raise Exception("IKE daemon is not installed. Please install "
> > >> +                            "the supported daemon (StrongSwan or LibreSwan).")
> > >> +        except Exception as e:
> > >> +            vlog.err(str(e))
> > > I think in few cases this would cause unhandled exceptions to be
> > > dumped in log file, no?
> >
> > Can you explain this a little bit more? In this exception case,
> > ovs-monitor-ipsec log the exception reason and exit.
>
> the /var/log/openvswitch/ovs-monitor-ipsec.log file is supposed to be
> readable by administrator.
>
> I believe vlog.err(str(e)) will stringify exception containing
> stacktrace and dump it into the log file. This is not helpful to
> administrator to figure out what actually went wrong unless he looks
> into python code. He is not supposed to do that.
>
> The only exception to this rule is the top level exception handler
> that is supposed to catch unhandled exceptions that developers forgot
> to handle by mistake.
>

Thanks for the explanation. This makes a lot of sense. I added more
info in the vlog.err() message.

>
> >
> > >
> > >> +            sys.exit(1)
> > >> +
> > >> +        self.ike_helper.start_ike_daemon()
> > >> +
> > >> +    def is_tunneling_type_supported(self, tunnel_type):
> > >> +        """Returns True if we know how to configure IPsec for these
> > >> +        types of tunnels.  Otherwise, returns False."""
> > >> +        return tunnel_type in ["gre", "geneve", "vxlan", "stt"]
> > >> +
> > >> +    def is_ipsec_required(self, options_column):
> > >> +        """Return True if tunnel needs to be encrypted.  Otherwise,
> > >> +        returns False."""
> > >> +        return "psk" in options_column or \
> > >> +                "remote_name" in options_column or \
> > >> +                "remote_cert" in options_column
> > >> +
> > >> +    def add_tunnel(self, name, row):
> > >> +        """Adds a new tunnel that monitor will provision with 'name'."""
> > >> +        vlog.info("Tunnel %s appeared in OVSDB" % (name))
> > >> +        self.tunnels[name] = IPsecTunnel(name, row)
> > >> +
> > >> +    def update_tunnel(self, name, row):
> > >> +        """Updates configuration of already existing tunnel with 'name'."""
> > >> +        tunnel = self.tunnels[name]
> > >> +        if tunnel.update_conf(row):
> > >> +            vlog.info("Tunnel's '%s' configuration changed in OVSDB to %u" %
> > >> +                      (tunnel.name, tunnel.version))
> > >> +
> > >> +    def del_tunnel(self, name):
> > >> +        """Deletes tunnel by 'name'."""
> > >> +        vlog.info("Tunnel %s disappeared from OVSDB" % (name))
> > >> +        self.tunnels[name].mark_for_removal()
> > >> +
> > >> +    def update_conf(self, pki, skb_mark):
> > >> +        """Update the global configuration for IPsec tunnels"""
> > >> +        # Update PKI certs and key
> > > I think the # comment is redundant
> > >> +        self.conf["pki"]["certificate"] = pki[0]
> > >> +        self.conf["pki"]["private_key"] = pki[1]
> > >> +        self.conf["pki"]["ca_cert"] = pki[2]
> > >> +        self.conf["pki"]["local_name"] = pki[3]
> > >> +
> > >> +        # Update skb_mark used in IPsec policies.
> > >> +        self.conf["skb_mark"] = skb_mark
> > >> +
> > >> +    def read_ovsdb_open_vswitch_table(self, data):
> > >> +        """This functions reads IPsec relevant configuration from Open_vSwitch
> > >> +        table."""
> > >> +        pki = [None, None, None, None]
> > >> +        skb_mark = None
> > >> +        is_valid = False
> > >> +
> > >> +        for row in data["Open_vSwitch"].rows.itervalues():
> > >> +            pki[0] = row.other_config.get("certificate")
> > >> +            pki[1] = row.other_config.get("private_key")
> > >> +            pki[2] = row.other_config.get("ca_cert")
> > >> +            skb_mark = row.other_config.get("ipsec_skb_mark")
> > >> +
> > >> +        # Test whether it's a valid configration
> > >> +        if pki[0] and pki[1]:
> > >> +            pki[3] = self._get_cn_from_cert(pki[0])
> > >> +            if pki[3]:
> > >> +                is_valid = True
> > >> +        elif not pki[0] and not pki[1] and not pki[2]:
> > >> +            is_valid = True
> > >> +
> > >> +        if not is_valid:
> > >> +            vlog.warn("The cert and key configuration is not valid. "
> > >> +                "The valid configuations are 1): certificate, private_key "
> > >> +                "and ca_cert are not set; or 2): certificate and "
> > >> +                "private_key are all set.")
> > >> +        else:
> > >> +            self.update_conf(pki, skb_mark)
> > >> +
> > >> +    def read_ovsdb_interface_table(self, data):
> > >> +        """This function reads the IPsec relevant configuration from Interface
> > >> +        table."""
> > >> +        ifaces = set()
> > >> +
> > >> +        for row in data["Interface"].rows.itervalues():
> > >> +            if not self.is_tunneling_type_supported(row.type):
> > >> +                continue
> > >> +            if not self.is_ipsec_required(row.options):
> > >> +                continue
> > >> +            if row.name in self.tunnels:
> > >> +                self.update_tunnel(row.name, row)
> > >> +            else:
> > >> +                self.add_tunnel(row.name, row)
> > >> +            ifaces.add(row.name)
> > >> +
> > >> +        # Mark for removal those tunnels that just disappeared from OVSDB
> > >> +        for tunnel in self.tunnels.keys():
> > >> +            if tunnel not in ifaces:
> > >> +                self.del_tunnel(tunnel)
> > >> +
> > >> +    def read_ovsdb(self, data):
> > >> +        """This function reads all configuration from OVSDB that
> > >> +        ovs-monitor-ipsec is interested in."""
> > >> +        self.read_ovsdb_open_vswitch_table(data)
> > >> +        self.read_ovsdb_interface_table(data)
> > >> +
> > >> +    def show(self, unix_conn, policies, securities):
> > >> +        """This function prints all tunnel state in 'unix_conn'.
> > >> +        It uses 'policies' and securities' received from Linux Kernel
> > >> +        to show if tunnels were actually configured by the IKE deamon."""
> > >> +        if not self.tunnels:
> > >> +            unix_conn.reply("No tunnels configured with IPsec")
> > >> +            return
> > >> +        s = ""
> > >> +        conns


More information about the dev mailing list