flimflam: Add L2TP/IPsec VPN plugin

test/connect-vpn

 #!/usr/bin/python
 # Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import optparse
 import pprint, sys
 import dbus, flimflam
 
+def show_usage(parser, vpn_type):
+    parser.error("Incorrect number of parameters provided for %s" % vpn_type)
+
 def main(argv):
-    parser = optparse.OptionParser('%prog [options]... (VPN)\n\n'
-                                   '   VPN := openvpn <vpn-name> '
-                                   '<remote-host-ip> <vpn-domain>\n            '
-                                   '      <ca-file> <client-cert-file> '
-                                   '<client-key-file>')
-    parser.add_option('--verbose',
-                      dest    = 'verbose',
-                      action  = 'store_true',
+    parser = optparse.OptionParser(
+        "%prog [options]... (VPN)\n"
+        "\n"
+        "   VPN := openvpn <vpn-name> <remote-host-ip> <vpn-domain>\n"
+        "             <ca-file> <client-cert-file> <client-key-file>\n"
+        "        | l2tpipsec <vpn-name> <remote-host-ip> <vpn-domain>\n"
+        "             <ca-file> <client-cert-file> <client-key-file>\n"
+        "             <ipsec-psk> <username> <password>")
+    parser.add_option("--verbose",
+                      dest    = "verbose",
+                      action  = "store_true",
                       default = False,
                       help    = "Output diagnostic information during run.")
-    parser.add_option('--complzo',
-                      dest    = 'complzo',
-                      action  = 'store_true',
+    parser.add_option("--complzo",
+                      dest    = "complzo",
+                      action  = "store_true",
                       default = True,
-                      help    = "Enables the OpenVPN option 'complzo' (default).")
-    parser.add_option('--no-complzo',
-                      dest    = 'complzo',
-                      action  = 'store_false',
+                      help    = ("Enables the OpenVPN option 'complzo' "
+                                 "(default)"))
+    parser.add_option("--no-complzo",
+                      dest    = "complzo",
+                      action  = "store_false",
                       help    = "Disables the OpenVPN option 'complzo'.")
-    parser.add_option('--remote-cert-tls',
-                      dest    = 'remote_cert_tls',
-                      action  = 'store',
-                      default = 'server',
-                      type    = 'string',
-                      metavar = '(server | client | none)',
-                      help    = "This is passed through to OpenVPN when "
-                                "not 'none'.")
+    parser.add_option("--remote-cert-tls",
+                      dest    = "remote_cert_tls",
+                      action  = "store",
+                      default = "server",
+                      type    = "string",
+                      metavar = "(server | client | none)",
+                      help    = ("This is passed through to OpenVPN when "
+                                 "not 'none'."))
 
     (options, args) = parser.parse_args(argv[1:])
 
     if (len(args) > 1):
         params = { "Type" : args[0] }
 
         if params["Type"] == "openvpn":
             if (len(args) == 7):
                 params["Name"]           = args[1]
                 params["Host"]           = args[2]
                 params["VPN.Domain"]     = args[3]
                 params["OpenVPN.CACert"] = args[4]
                 params["OpenVPN.Cert"]   = args[5]
                 params["OpenVPN.Key"]    = args[6]
 
-                if options.complzo: # 'complzo' can only be enabled.
-                    params["OpenVPN.CompLZO"] = 'true'
+                if options.complzo: # "complzo" can only be enabled.

[thutt, 2011/03/17 15:02:21]

I think the single quotes on complzo here are ok, as it's just a comment.  No
problem if you don't want to change it, though.

+                    params["OpenVPN.CompLZO"] = "true"
 
-                if options.remote_cert_tls != 'server' and \
-                   options.remote_cert_tls != 'client' and \
-                   options.remote_cert_tls != 'none':
-                    print "\n--remote-cert-tls argument ('%s') " \
-                        "is invalid.\n" % options.remote_cert_tls
+                if options.remote_cert_tls != "server" and \
+                   options.remote_cert_tls != "client" and \
+                   options.remote_cert_tls != "none":
+                    print ("\n--remote-cert-tls argument ('%s') "

[thutt, 2011/03/17 15:02:21]

Would it be better to remove the space preceding '(', since the rest of the file
does it that way?

+                           "is invalid.\n" % options.remote_cert_tls)
                     sys.exit(1)
 
                 params["OpenVPN.RemoteCertTLS"] = options.remote_cert_tls
             else:
-                parser.error("Incorrect number of parameters "
-                             "provided for 'openvpn'")
+                show_usage(parser, params["Type"])

[thutt, 2011/03/17 15:02:21]

I look forward to seeing what affect this has on the output in this case, as I
didn't know about this (and practically anything Python).

+        elif params["Type"] == "l2tpipsec":

[thutt, 2011/03/17 15:02:21]

Just noticed that it looks like your editor is putting tabs in the file.  Can
you please make them spaces?

Also, as I've aligned the multiple assignments, would it be objectionable to
align these too (or unalign mine in 53..58), for consistency.

+            if len(args) == 10:
+                params["Name"] = args[1]
+                params["Host"] = args[2]
+                params["VPN.Domain"] = args[3]
+                params["L2TPIPsec.CACert"] = args[4]
+                params["L2TPIPsec.Cert"] = args[5]
+                params["L2TPIPsec.Key"] = args[6]
+                params["L2TPIPsec.PSK"] = args[7]
+                params["L2TPIPsec.User"] = args[8]
+                params["L2TPIPsec.Password"] = args[9]
+            else:
+                show_usage(parser, params["Type"])
         else:
             print "Unknown VPN type: '%s'" % params["Type"]

[thutt, 2011/03/17 15:02:21]

Should this be replaced with a show_usage() like at line 72?
If so, and if that function exits, then line 88 can also be removed.

             sys.exit(1)
 
         if options.verbose:
             print "\nVPN Startup Parameters:\n"
             for k, v in params.iteritems():
                 print "  %25s: '%s'" % (k, v)
             print ""
 
         flim    = flimflam.FlimFlam(dbus.SystemBus())
         service = flim.GetVPNService(params)
 
-        if options.verbose == 'true':
+        if options.verbose == "true":
             print "VPN is %s, connecting..." % service.object_path
 
-        (success, diagnostics) = flim.ConnectService(service_type = 'vpn',
+        (success, diagnostics) = flim.ConnectService(service_type = "vpn",
                                                      service = service)
         if not success or options.verbose:
-            print 'Success:', success
+            print "Success:", success
             pprint.pprint(diagnostics)
     else:
         parser.print_help()
         sys.exit(1)
 
 if __name__ == '__main__':
   main(sys.argv)