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

Mark Gray mark.d.gray at redhat.com
Tue Mar 30 16:15:36 UTC 2021


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.
> 
>> +                            "--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