[ovs-dev] [PATCH 2/4] ipsec: Allow custom file locations

Mark Gray mark.d.gray at redhat.com
Wed Mar 31 07:49:15 UTC 2021


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 <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,
> 

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
>>>> dev at openvswitch.org
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
> 



More information about the dev mailing list