[ovs-dev] [PATCH 2/4] ipsec: Allow custom file locations
Eelco Chaudron
echaudro at redhat.com
Wed Mar 31 07:31:24 UTC 2021
On 30 Mar 2021, at 18:15, Mark Gray wrote:
> On 30/03/2021 12:15, Eelco Chaudron wrote:
>>
>>
>> On 8 Mar 2021, at 18:57, Mark Gray wrote:
>>
>>> "ovs_monitor_ipsec" assumes certain file locations for a number
>>> of Libreswan objects. This patch allows these locations to be
>>> configurable at startup in the Libreswan case.
>>>
>>> This additional flexibility enables system testing for
>>> OVS IPsec.
>>>
>>> Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
>>> ---
>>> ipsec/ovs-monitor-ipsec.in | 106
>>> +++++++++++++++++++++++++++++--------
>>> 1 file changed, 83 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in
>>> index 64111768b33a..9f412aaaf25a 100755
>>> --- a/ipsec/ovs-monitor-ipsec.in
>>> +++ b/ipsec/ovs-monitor-ipsec.in
>>> @@ -439,12 +439,20 @@ conn prevent_unencrypted_vxlan
>>> CERT_PREFIX = "ovs_cert_"
>>> CERTKEY_PREFIX = "ovs_certkey_"
>>>
>>> - def __init__(self, libreswan_root_prefix):
>>> + def __init__(self, libreswan_root_prefix, ipsec_conf, ipsec_d,
>>> + ipsec_secrets, ipsec_ctl):
>>> 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.IPSEC_CONF = libreswan_root_prefix + ipsec_conf
>>> + self.IPSEC_SECRETS = libreswan_root_prefix + ipsec_secrets
>>> + self.IPSEC_D = "sql:" + libreswan_root_prefix + ipsec_d
>>> + self.IPSEC_CTL = libreswan_root_prefix + ipsec_ctl
>>> self.conf_file = None
>>> self.secrets_file = None
>>> + vlog.dbg("Using: " + self.IPSEC)
>>> + vlog.dbg("Configuration file: " + self.IPSEC_CONF)
>>> + vlog.dbg("Secrets file: " + self.IPSEC_SECRETS)
>>> + vlog.dbg("ipsec.d: " + self.IPSEC_D)
>>> + vlog.dbg("Pluto socket: " + self.IPSEC_CTL)
>>>
>>> def restart_ike_daemon(self):
>>> """This function restarts LibreSwan."""
>>> @@ -539,7 +547,8 @@ conn prevent_unencrypted_vxlan
>>>
>>> def refresh(self, monitor):
>>> vlog.info("Refreshing LibreSwan configuration")
>>> - subprocess.call([self.IPSEC, "auto", "--rereadsecrets"])
>>> + subprocess.call([self.IPSEC, "auto", "--ctlsocket",
>>> self.IPSEC_CTL,
>>> + "--config", self.IPSEC_CONF,
>>> "--rereadsecrets"])
>>> tunnels = set(monitor.tunnels.keys())
>>>
>>> # Delete old connections
>>> @@ -566,7 +575,9 @@ conn prevent_unencrypted_vxlan
>>>
>>> if not tunnel or tunnel.version != ver:
>>> vlog.info("%s is outdated %u" % (conn, ver))
>>> - subprocess.call([self.IPSEC, "auto", "--delete",
>>> conn])
>>> + subprocess.call([self.IPSEC, "auto",
>>> "--ctlsocket",
>>> + self.IPSEC_CTL, "--config",
>>> self.IPSEC_CONF,
>>> + "--delete", conn])
>>> elif ifname in tunnels:
>>> tunnels.remove(ifname)
>>>
>>> @@ -586,22 +597,46 @@ conn prevent_unencrypted_vxlan
>>> # Update shunt policy if changed
>>> if monitor.conf_in_use["skb_mark"] !=
>>> monitor.conf["skb_mark"]:
>>> if monitor.conf["skb_mark"]:
>>> - subprocess.call([self.IPSEC, "auto", "--add",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>
>> Guess the +’s above (and the below ones) are cut/paste leftovers from
>> the debug logs that need removal?
>
> I don't understand? These should be valid changes.
Too many +’s on one line, I meant:
+ "--config", + self.IPSEC_CONF,
+ "--ctlsocket", + self.IPSEC_CTL,
to
+ "--config", self.IPSEC_CONF,
+ "--ctlsocket", self.IPSEC_CTL,
>>> + "--add",
>>> "--asynchronous",
>>> "prevent_unencrypted_gre"])
>>> - subprocess.call([self.IPSEC, "auto", "--add",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>> + "--add",
>>> "--asynchronous",
>>> "prevent_unencrypted_geneve"])
>>> - subprocess.call([self.IPSEC, "auto", "--add",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>> + "--add",
>>> "--asynchronous",
>>> "prevent_unencrypted_stt"])
>>> - subprocess.call([self.IPSEC, "auto", "--add",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>> + "--add",
>>> "--asynchronous",
>>> "prevent_unencrypted_vxlan"])
>>> else:
>>> - subprocess.call([self.IPSEC, "auto", "--delete",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>> + "--delete",
>>> "--asynchronous",
>>> "prevent_unencrypted_gre"])
>>> - subprocess.call([self.IPSEC, "auto", "--delete",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>> + "--delete",
>>> "--asynchronous",
>>> "prevent_unencrypted_geneve"])
>>> - subprocess.call([self.IPSEC, "auto", "--delete",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>> + "--delete",
>>> "--asynchronous",
>>> "prevent_unencrypted_stt"])
>>> - subprocess.call([self.IPSEC, "auto", "--delete",
>>> + subprocess.call([self.IPSEC, "auto",
>>> + "--config", + self.IPSEC_CONF,
>>> + "--ctlsocket", + self.IPSEC_CTL,
>>> + "--delete",
>>> "--asynchronous",
>>> "prevent_unencrypted_vxlan"])
>>> monitor.conf_in_use["skb_mark"] =
>>> monitor.conf["skb_mark"]
>>>
>>> @@ -613,7 +648,8 @@ conn prevent_unencrypted_vxlan
>>> sample line from the parsed outpus as <value>. """
>>>
>>> conns = {}
>>> - proc = subprocess.Popen([self.IPSEC, 'status'],
>>> stdout=subprocess.PIPE)
>>> + proc = subprocess.Popen([self.IPSEC, 'status', '--ctlsocket',
>>> + self.IPSEC_CTL],
>>> stdout=subprocess.PIPE)
>>>
>>> while True:
>>> line = proc.stdout.readline().strip().decode()
>>> @@ -644,7 +680,10 @@ conn prevent_unencrypted_vxlan
>>> # 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",
>>> + proc = subprocess.Popen([self.IPSEC, "auto",
>>> + "--config", self.IPSEC_CONF,
>>> + "--ctlsocket", self.IPSEC_CTL,
>>> + "--start",
>>> "--asynchronous", conn],
>>> stdout=subprocess.PIPE,
>>> stderr=subprocess.PIPE)
>>> @@ -658,7 +697,7 @@ conn prevent_unencrypted_vxlan
>>> """Remove all OVS IPsec related state from the NSS
>>> database"""
>>> try:
>>> proc = subprocess.Popen(['certutil', '-L', '-d',
>>> - 'sql:/etc/ipsec.d/'],
>>> + self.IPSEC_D],
>>> stdout=subprocess.PIPE,
>>> stderr=subprocess.PIPE,
>>> universal_newlines=True)
>>> @@ -682,7 +721,7 @@ conn prevent_unencrypted_vxlan
>>> normal certificate."""
>>> try:
>>> proc = subprocess.Popen(['certutil', '-A', '-a', '-i',
>>> cert,
>>> - '-d', 'sql:/etc/ipsec.d/', '-n',
>>> + '-d', self.IPSEC_D, '-n',
>>> name, '-t', cert_type],
>>> stdout=subprocess.PIPE,
>>> stderr=subprocess.PIPE)
>>> @@ -695,7 +734,7 @@ conn prevent_unencrypted_vxlan
>>> def _nss_delete_cert(self, name):
>>> try:
>>> proc = subprocess.Popen(['certutil', '-D', '-d',
>>> - 'sql:/etc/ipsec.d/', '-n', name],
>>> + self.IPSEC_D, '-n', name],
>>> stdout=subprocess.PIPE,
>>> stderr=subprocess.PIPE)
>>> proc.wait()
>>> @@ -723,7 +762,7 @@ conn prevent_unencrypted_vxlan
>>>
>>> # Load p12 file to the database
>>> proc = subprocess.Popen(['pk12util', '-i', path, '-d',
>>> - 'sql:/etc/ipsec.d/', '-W', ''],
>>> + self.IPSEC_D, '-W', ''],
>>> stdout=subprocess.PIPE,
>>> stderr=subprocess.PIPE)
>>> proc.wait()
>>> @@ -738,7 +777,7 @@ conn prevent_unencrypted_vxlan
>>> try:
>>> # Delete certificate and private key
>>> proc = subprocess.Popen(['certutil', '-F', '-d',
>>> - 'sql:/etc/ipsec.d/', '-n', name],
>>> + self.IPSEC_D, '-n', name],
>>> stdout=subprocess.PIPE,
>>> stderr=subprocess.PIPE)
>>> proc.wait()
>>> @@ -925,7 +964,8 @@ class IPsecTunnel(object):
>>> class IPsecMonitor(object):
>>> """This class monitors and configures IPsec tunnels"""
>>>
>>> - def __init__(self, root_prefix, ike_daemon, restart):
>>> + def __init__(self, root_prefix, ike_daemon, ipsec_conf, ipsec_d,
>>> + ipsec_secrets, ipsec_ctl, restart):
>>> self.IPSEC = root_prefix + "/usr/sbin/ipsec"
>>> self.tunnels = {}
>>>
>>> @@ -945,7 +985,8 @@ class IPsecMonitor(object):
>>> if ike_daemon == "strongswan":
>>> self.ike_helper = StrongSwanHelper(root_prefix)
>>> elif ike_daemon == "libreswan":
>>> - self.ike_helper = LibreSwanHelper(root_prefix)
>>> + self.ike_helper = LibreSwanHelper(root_prefix,
>>> ipsec_conf, ipsec_d,
>>> + ipsec_secrets,
>>> ipsec_ctl)
>>> else:
>>> vlog.err("The IKE daemon should be strongswan or
>>> libreswan.")
>>> sys.exit(1)
>>> @@ -1190,6 +1231,19 @@ def main():
>>> " (either libreswan or strongswan).")
>>> parser.add_argument("--no-restart-ike-daemon",
>>> action='store_true',
>>> help="Don't restart the IKE daemon on
>>> startup.")
>>> + parser.add_argument("--ipsec-conf", metavar="IPSEC-CONF",
>>> + help="Use DIR/IPSEC-CONF as location for "
>>> + " ipsec.conf (libreswan only).")
>>> + parser.add_argument("--ipsec-d", metavar="IPSEC-D",
>>> + help="Use DIR/IPSEC-D as location for "
>>> + " ipsec.d (libreswan only).")
>>> + parser.add_argument("--ipsec-secrets", metavar="IPSEC-SECRETS",
>>> + help="Use DIR/IPSEC-SECRETS as location for "
>>> + " ipsec.secrets (libreswan only).")
>>> + parser.add_argument("--ipsec-ctl", metavar="IPSEC-CTL",
>>> + help="Use DIR/IPSEC-CTL as location for "
>>> + " pluto ctl socket (libreswan only).")
>>> +
>>>
>>> ovs.vlog.add_args(parser)
>>> ovs.daemon.add_args(parser)
>>> @@ -1200,9 +1254,15 @@ def main():
>>> global monitor
>>> global xfrm
>>>
>>> + ipsec_conf = args.ipsec_conf if args.ipsec_conf else
>>> "/etc/ipsec.conf"
>>> + ipsec_d = args.ipsec_d if args.ipsec_d else "/etc/ipsec.d"
>>> + ipsec_secrets = (args.ipsec_secrets if args.ipsec_secrets
>>> + else "/etc/ipsec.secrets")
>>> + ipsec_ctl = args.ipsec_ctl if args.ipsec_ctl else
>>> "/run/pluto/pluto.ctl"
>>
>> Would it not be nicer to just pass in None to the IPsecMonitor if not
>> configured, i.e., not doing the above 5 lines, and have the
>> LibreSwanHelper object configure the defaults if none was given?
>
> Yeah, this makes more sense as this is quite specific to libreswan. In
> fact, I should just pass 'args' to the helper so it can parse any
> helper-specific 'args'
>>
>>> root_prefix = args.root_prefix if args.root_prefix else ""
>>> xfrm = XFRM(root_prefix)
>>> - monitor = IPsecMonitor(root_prefix, args.ike_daemon,
>>> + monitor = IPsecMonitor(root_prefix, args.ike_daemon, ipsec_conf,
>>> ipsec_d,
>>> + ipsec_secrets, ipsec_ctl,
>>> not args.no_restart_ike_daemon)
>>>
>>> remote = args.database
>>> --
>>> 2.27.0
>>>
>>> _______________________________________________
>>> dev mailing list
>>> dev at openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
More information about the dev
mailing list