On 31/03/2021 08:31, Eelco Chaudron wrote:
>
>
> 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 <[email protected]>
>>>> ---
>>>> 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,
>
Oh, I see what you mean. I think I was using the '+' operator to
concatenate strings at one stage and that remained. I will fix it. Thanks.
>>>> + "--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
>>>> [email protected]
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
