[ovs-dev] [PATCH 2/3] ipsec: add CA-cert based authentication
Qiuyu Xiao
qiuyu.xiao.qyx at gmail.com
Wed Jun 27 20:52:04 UTC 2018
Hi Aaron,
Thanks for your comments!
On Wed, Jun 27, 2018 at 1:12 PM, Aaron Conole <aconole at redhat.com> wrote:
>
> Qiuyu Xiao <qiuyu.xiao.qyx at gmail.com> writes:
>
> > This patch adds CA-cert based authentication to the ovs-monitor-ipsec
> > daemon. With CA-cert based authentication enabled, OVS approves IPsec
> > tunnel if the peer has a cert signed by a trusted CA and the identity of
> > the peer cert is as expected. Belows are the major changes and the
> > reasons:
> >
> > 1) Added CA-cert based authentication. Compared with peer-cert based
> > authentication, this one doesn't need to import peer cert to the local
> > host to do configuration. This is especially beneficial if host has
> > mutiple peers and peers frequently update their certs. This feature is
> > required for the upcoming OVN IPsec support.
> >
> > 2) Changed the host cert and private key configuration interface.
> > Previously, the host's cert and private key can be configured in either
> > Open_vSwitch's SSL column or the SSL table. Now, the host certificate
> > and private key can be only configured in the Open_vSwitch table's
> > other_config column. Since it is not SSL cert and key, we'd better not
> > to confuse users by saying so.
> >
> > 3) Changed the peer cert configuration interface. Previously, the peer
> > cert is configured by setting the interface's options column as the
> > content of the peer cert. It's changed to setting the column as the path
> > of the peer cert. This is easier to be configured by the command line
> > tool, and is consistent with other cert and key configuration interface
> > which is better from a usability point of view.
> >
> > Signed-off-by: Qiuyu Xiao <qiuyu.xiao.qyx at gmail.com>
> > ---
> > Documentation/howto/ipsec.rst | 78 ++++++++++++++++---
> > ipsec/ovs-monitor-ipsec | 138 +++++++++++++++++++++-------------
> > 2 files changed, 156 insertions(+), 60 deletions(-)
> >
> > diff --git a/Documentation/howto/ipsec.rst b/Documentation/howto/ipsec.rst
> > index 4e4f4d211..b42312da5 100644
> > --- a/Documentation/howto/ipsec.rst
> > +++ b/Documentation/howto/ipsec.rst
> > @@ -21,9 +21,9 @@
> >
> > Avoid deeper levels because they do not render well.
> >
> > -==============================================
> > -How to Encrypt Open vSwitch Tunnels with IPsec
> > -==============================================
> > +=======================================
> > +Encrypt Open vSwitch Tunnels with IPsec
> > +=======================================
>
> It seems odd to introduce something and then cut it in the very next
> patch. Please don't do this. Most of the diffs in this file can be
> folded into patch 1/3 - please do that instead of fixing things later in
> the series.
Patch 1/3 is from Ansis. I separated the patches so he will know the
changes I made.
But it makes sense to make 1/3 and 2/3 a single patch. I will change
that in the next version.
>
>
> > This document describes how to use Open vSwitch integration with strongSwan
> > 5.1 or later to provide IPsec security for STT, GENEVE, GRE and VXLAN tunnels.
> > @@ -77,6 +77,67 @@ Install strongSwan and openvswitch-ipsec debian packages::
> > Configuration
> > -------------
> >
> > +The IPsec configuration is done by setting options of the tunnel interface.
> > +ovs-monitor-ipsec configures strongSwan accordingly based on the tunnel options.
> > +
> > +Authentication Methods
> > +~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Hosts of the IPsec tunnel need to authenticate each other to build a secure
> > +channel. There are three authentication methods:
> > +
> > +1) You can set a pre-shared key in both hosts to do authentication. This
> > + method is easier to use but less secure::
> > +
> > + % ovs-vsctl add-port br0 ipsec_gre0 -- \
> > + set interface ipsec_gre0 type=gre \
> > + options:remote_ip=1.2.3.4 \
> > + options:psk=swordfish])
> > +
> > +2) You can use peer certificates to do authentication. First, generate
> > + certificate and private key in each host. The certificate could be
> > + self-signed. Refer to the ovs-pki(8) man page for more information
> > + regarding certificate and key generation. Then, copy the peer certificate
> > + to the local host and type::
> > +
> > + % ovs-vsctl set Open_vSwitch . \
> > + other_config:certificate=/path/to/local_cert.pem \
> > + other_config:private_key=/path/to/priv_key.pem
> > + % ovs-vsctl add-port br0 ipsec_gre0 -- \
> > + set interface ipsec_gre0 type=gre \
> > + options:remote_ip=1.2.3.4 \
> > + options:pki=peer_auth \
> > + options:peer_cert=/path/to/peer_cert.pem
> > +
> > + `local_cert.pem` is the certificate of the local host. `priv_key.pem`
> > + is the private key of the local host. `priv_key.pem` needs to be stored in
> > + a secure location. `peer_cert.pem` is the certificate of the remote host.
> > +
> > +3) You can also use CA certificate to do authentication. First, you need to
> > + establish your public key infrastructure. The certificate of each host
> > + needs to be signed by the CA certificate. Refer to the ovs-pki(8) man page
> > + for more information regarding PKI establishment. Then, copy the CA
> > + certificate to the local host and type::
> > +
> > + % ovs-vsctl set Open_vSwitch . \
> > + other_config:certificate=/path/to/local_cert.pem \
> > + other_config:private_key=/path/to/priv_key.pem \
> > + other_config:ca_cert=/path/to/ca_cert.pem
> > + % ovs-vsctl add-port br0 ipsec_gre0 -- \
> > + set interface ipsec_gre0 type=gre \
> > + options:remote_ip=1.2.3.4 \
> > + options:pki=ca_auth \
> > + options:local_name=local_cn \
> > + options:remote_name=remote_cn
> > +
> > + strongSwan extracts identity from the certificate's `subjectAltName` field,
> > + so you have to use x509 v3 certificate. `local_cn` is the hostname from the
> > + `subjectAltName` field of the local host certificate. `remote_cn` is the
> > + hostname from the `subjectAltName` field of the peer host certificate.
> > +
> > +Forwarding Modes
> > +~~~~~~~~~~~~~~~~
> > +
> > IPsec with ovs-monitor-ipsec daemon can be configured in three
> > different forwarding policy modes:
> >
> > @@ -116,7 +177,7 @@ different forwarding policy modes:
> > ovs-monitor-ipsec installed IPsec policies.
> > However, assumption here is that OpenFlow controller was careful
> > and installed OpenFlow rule with set SKB mark action specified in
> > - OVSDB Open_vSwitch table before the first packets were able to leave
> > + OVSDB Open_vSwitch table before the first packet was able to leave
> > the OVS tunnel.
> >
> > 3) ovs-monitor-ipsec assumes that packets coming from all OVS tunnels
> > @@ -126,7 +187,7 @@ different forwarding policy modes:
> >
> > % ovs-vsctl set Open_vSwitch . other_config:default_ipsec_skb_mark=0/1
> > % ovs-vsctl add-port br0 ipsec_gre0 -- \
> > - set interface gre0 type=gre \
> > + set interface ipsec_gre0 type=gre \
> > options:remote_ip=1.2.3.4 \
> > options:psk=swordfish])
> >
> > @@ -189,11 +250,10 @@ If you think you may have found a bug with security implications, like
> > 1) IPsec protected tunnel accepted packets that came unencrypted; OR
> > 2) IPsec protected tunnel allowed packets to leave unencrypted;
> >
> > -Then report such bugs according to `security guidelines
> > -<Documentation/internals/security.rst>`__.
> > +Then report such bugs according to :doc:`/internals/security`.
> > If bug does not have security implications, then report it according to
> > -instructions in `<REPORTING-bugs.rst>`__.
> > +instructions in :doc:`/internals/bugs`.
> >
> > There is also a possibility that there is a bug in strongSwan. In that
> > -case report it to strongSwan mailing list.
> > \ No newline at end of file
> > +case report it to strongSwan mailing list.
> > diff --git a/ipsec/ovs-monitor-ipsec b/ipsec/ovs-monitor-ipsec
> > index 322a7045a..65f360a80 100755
> > --- a/ipsec/ovs-monitor-ipsec
> > +++ b/ipsec/ovs-monitor-ipsec
> > @@ -179,21 +179,30 @@ $auth_section
> > left=$local_ip
> > right=$remote_ip
> > authby=psk"""),
> > - "rsa" : Template("""\
> > + "pki_peer" : Template("""\
> > left=$local_ip
> > right=$remote_ip
> > - rightcert=ovs-$remote_ip.pem
> > - leftcert=$certificate""")}
> > + leftcert=$certificate
> > + rightcert=$peer_cert"""),
> > + "pki_ca" : Template("""\
> > + left=$local_ip
> > + right=$remote_ip
> > + leftcert=$certificate
> > + leftid=$local_name
> > + rightid=$remote_name""")}
>
> This diff block is difficult to follow. There is a re-order of the
> options mixed with a change in values. Can you separate those changes?
Will do it.
> > unixctl_config_tmpl = Template("""\
> > - Remote IP: $remote_ip
> > Tunnel Type: $tunnel_type
> > + Remote IP: $remote_ip
>
> Why is this change made?
umm, there is not much need to change this.
>
> > Local IP: $local_ip
> > SKB mark: $skb_mark
> > - Use SSL cert: $use_ssl_cert
> > - My cert: $certificate
> > - My key: $private_key
> > - His cert: $peer_cert
> > + Local cert: $certificate
> > + Local key: $private_key
> > + CA cert: $ca_cert
> > + Remote cert: $peer_cert
> > + Remote name: $remote_name
> > + Local name: $local_name
> > + PKI: $pki
> > PSK: $psk
> > """)
> >
> > @@ -209,7 +218,6 @@ $auth_section
> > self.state = "INIT"
> > self.conf = {}
> > self.status = {}
> > - self.cert_file = None
> > self.update_conf(row)
> >
> > def update_conf(self, row):
> > @@ -219,11 +227,7 @@ $auth_section
> > False."""
> > ret = False
> > options = row.options
> > - use_ssl_cert = options.get("use_ssl_cert") == "true"
> > - cert = options.get("certificate")
> > - key = options.get("private_key")
> > - if use_ssl_cert: # Override with SSL certs if told so
> > - (cert, key) = (keyer.public_cert, keyer.private_key)
> > + (cert, key, ca_cert) = (keyer.public_cert, keyer.private_key, keyer.ca_cert)
> >
> > new_conf = {
> > "ifname" : self.name,
> > @@ -233,8 +237,11 @@ $auth_section
> > "skb_mark" : keyer.wanted_skb_mark,
> > "certificate" : cert,
> > "private_key" : key,
> > - "use_ssl_cert" : use_ssl_cert,
> > + "ca_cert" : ca_cert,
> > "peer_cert" : options.get("peer_cert"),
> > + "remote_name" : options.get("remote_name"),
> > + "local_name" : options.get("local_name"),
> > + "pki" : options.get("pki"),
> > "psk" : options.get("psk")}
> > if self.conf != new_conf:
> > # Configuration was updated in OVSDB. Validate it and figure
> > @@ -303,7 +310,10 @@ $auth_section
> > else:
> > secrets.write("%s : RSA %s\n" % (self.conf["remote_ip"],
> > self.conf["private_key"]))
> > - auth_section = self.auth_tmpl["rsa"].substitute(self.conf)
> > + if self.conf["pki"] == "ca_auth":
> > + auth_section = self.auth_tmpl["pki_ca"].substitute(self.conf)
> > + else:
> > + auth_section = self.auth_tmpl["pki_peer"].substitute(self.conf)
> > vals = self.conf.copy()
> > vals["auth_section"] = auth_section
> > vals["version"] = self.version
> > @@ -344,45 +354,54 @@ $auth_section
> > pass malformed configuration to strongSwan."""
> >
> > self.invalid_reason = None
> > +
> > if not self.conf["remote_ip"]:
> > self.invalid_reason = ": 'remote_ip' is not set"
> > - elif self.conf["peer_cert"]:
> > +
> > + if self.conf["pki"]:
> > if self.conf["psk"]:
> > self.invalid_reason = ": 'psk' must be unset with PKI"
> > elif not self.conf["certificate"]:
> > self.invalid_reason = ": must set 'certificate' with PKI"
> > elif not self.conf["private_key"]:
> > self.invalid_reason = ": must set 'private_key' with PKI"
> > +
> > + if self.conf["pki"] == "ca_auth":
> > + if not self.conf["ca_cert"]:
> > + self.invalid_reason = ": must set 'ca_cert' "\
> > + "for ca_cert-based authentication"
> > + elif not self.conf["local_name"]:
> > + self.invalid_reason = ": must set 'local_name' "\
> > + "for ca_cert-based authentication"
> > + elif not self.conf["remote_name"]:
> > + self.invalid_reason = ": must set 'remote_name' "\
> > + "for ca_cert-based authentication"
> > + elif self.conf["pki"] == "peer_auth":
> > + if not self.conf["peer_cert"]:
> > + self.invalid_reason = ": must set 'peer_cert' "\
> > + "for peer_cert-based authentication"
> > + else:
> > + self.invalid_reason = ": must set 'pki' "\
> > + "as 'ca_auth' or 'peer_auth'."
> > elif self.conf["psk"]:
> > - if self.conf["certificate"] or self.conf["private_key"]:
> > - self.invalid_reason = ": 'certificate', 'private_key' and "\
> > - "'use_ssl_cert' must be unset with PSK"
> > + if self.conf["certificate"] or self.conf["private_key"] \
> > + or self.conf["ca_cert"] or self.conf["peer_cert"]:
> > + self.invalid_reason = ": 'certificate', 'private_key', "\
> > + "'ca_cert' and 'peer_cert' must be unset with PSK"
> > + else:
> > + self.invalid_reason = ": must use 'pki' or 'psk' option "\
> > + "for authentication"
> > +
> > if self.invalid_reason:
> > return False
> > +
> > return True
> >
> > def _cleanup_old_state(self):
> > - if self.cert_file:
> > - try:
> > - os.remove(self.cert_file)
> > - except OSError:
> > - vlog.warn("could not remove '%s'" % (self.cert_file))
> > - self.cert_file = None
> > + vlog.info("Clean old state.")
> >
> > def _push_new_state(self):
> > - if self.conf["peer_cert"]:
> > - fn = keyer.CERT_DIR + "/ovs-%s.pem" % (self.name)
> > - try:
> > - cert = open(fn, "w")
> > - try:
> > - cert.write(self.conf["peer_cert"])
> > - finally:
> > - cert.close()
> > - vlog.info("created peer certificate %s" % (fn))
> > - self.cert_file = fn
> > - except (OSError, IOError) as e:
> > - vlog.err("could not create certificate '%s'" % (fn))
> > -
> > + vlog.info("Push new state.")
>
> These two functions don't seem useful anymore. Wouldn't it be better to
> remove them?
>
Yes, I will remove it.
> > class StrongSwanKeyer(object):
> > """This class is a wrapper around strongSwan."""
> > @@ -407,18 +426,33 @@ conn %%default
> > SHUNT_POLICY = """conn prevent_unencrypted_gre
> > type=drop
> > leftprotoport=gre
> > - mark=%s
> > + mark={0}
> > +
> > +conn prevent_unencrypted_geneve
> > + type=drop
> > + leftprotoport=udp/6081
> > + mark={0}
> >
> > conn prevent_unencrypted_stt
> > type=drop
> > leftprotoport=tcp/7471
> > - mark=%s
> > + mark={0}
> > +
> > +conn prevent_unencrypted_vxlan
> > + type=drop
> > + leftprotoport=udp/4789
> > + mark={0}
> > +
> > +"""
> > + CA_SECTION = """ca ca_auth
> > + cacert=%s
> >
> > """
> >
> > def __init__(self, strongswan_root_prefix):
> > self.private_key = None
> > self.public_cert = None
> > + self.ca_cert = None
> > self.wanted_skb_mark = None
> > self.in_use_skb_mark = None
> > self.tunnels = {}
> > @@ -436,7 +470,7 @@ conn prevent_unencrypted_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 "peer_cert" in options_column
> > + return "psk" in options_column or "pki" in options_column
> >
> > def restart(self):
> > """This function restarts strongSwan."""
> > @@ -465,6 +499,7 @@ conn prevent_unencrypted_stt
> > """Updates already existing """
> > self.public_cert = credentials[0]
> > self.private_key = credentials[1]
> > + self.ca_cert = credentials[2]
> >
> > def update_skb_mark(self, skb_mark):
> > """Update skb_mark used in IPsec policies."""
> > @@ -499,8 +534,12 @@ conn prevent_unencrypted_stt
> > needs_refresh = True
> >
> > if self.in_use_skb_mark:
> > - ipsec_conf.write(" mark_out=%s\n" % self.in_use_skb_mark)
> > - ipsec_conf.write(self.SHUNT_POLICY % (self.in_use_skb_mark, self.in_use_skb_mark))
> > + ipsec_conf.write(self.SHUNT_POLICY.format(self.in_use_skb_mark))
> > +
> > + # No need to set needs_refresh here if ca_cert is changed. The tunnel
> > + # iteration will set needs_refresh.
> > + if self.ca_cert:
> > + ipsec_conf.write(self.CA_SECTION % self.ca_cert)
> >
> > for name, tunnel in self.tunnels.iteritems():
> > if tunnel.run():
> > @@ -565,7 +604,6 @@ conn prevent_unencrypted_stt
> > vlog.info("%s is outdated %u" % (conn, ver))
> > subprocess.call([self.IPSEC, "stroke", "down-nb", conn])
> >
> > -
> > def _get_strongswan_conns(self):
> > """This function parses output from 'ipsec status' command.
> > It returns dictionary where <key> is interface name (as in OVSDB)
> > @@ -612,11 +650,12 @@ conn prevent_unencrypted_stt
> > def read_ovsdb_open_vswitch_table(data):
> > """This functions reads IPsec relevant configuration from Open_vSwitch
> > table."""
> > - ssl = (None, None)
> > + ssl = (None, None, None)
> > global_ipsec_skb_mark = None
> > for row in data["Open_vSwitch"].rows.itervalues():
> > - if row.ssl:
> > - ssl = (row.ssl[0].certificate, row.ssl[0].private_key)
> > + ssl = (row.other_config.get("certificate"), \
> > + row.other_config.get("private_key"), \
> > + row.other_config.get("ca_cert"))
> > global_ipsec_skb_mark = row.other_config.get("default_ipsec_skb_mark")
> > keyer.update_ssl_credentials(ssl)
> > keyer.update_skb_mark(global_ipsec_skb_mark)
> > @@ -646,7 +685,6 @@ def read_ovsdb(data):
> > read_ovsdb_open_vswitch_table(data)
> > read_ovsdb_interface_table(data)
> >
> > -
>
> This will cause a flake8 violation.
>
I will fix this.
> > def main():
> > parser = argparse.ArgumentParser()
> > parser.add_argument("database", metavar="DATABASE",
> > @@ -674,7 +712,6 @@ def main():
> > ["name", "type", "options", "cfm_fault",
> > "ofport"])
> > schema_helper.register_columns("Open_vSwitch", ["ssl", "other_config"])
> > - schema_helper.register_columns("SSL", ["certificate", "private_key"])
> > idl = ovs.db.idl.Idl(remote, schema_helper)
> >
> > ovs.daemon.daemonize()
> > @@ -715,7 +752,6 @@ def main():
> > unixctl_server.close()
> > idl.close()
> >
> > -
>
> This will cause a flake8 violation.
>
> > if __name__ == '__main__':
> > try:
> > main()
More information about the dev
mailing list