On Tue, 2018-04-03 at 20:03 -0400, Max Rees wrote:
> 
> +               case OPT_SERVER:
> +                       if (openconnect_parse_url(vpninfo, config_arg))
> +                               exit(1);
> +                       break;

Here you *only* accept a bare URL, not the name of one of the servers
which is found in the XML config file (see config_lookup_host). So the
--server option isn't the same as the bare command line argument.


>                 case OPT_JUNIPER:
>                         fprintf(stderr, "WARNING: Juniper Network Connect 
> support is experimental.\n");
>                         fprintf(stderr, "It will probably be superseded by 
> Junos Pulse support.\n");
> @@ -1457,7 +1463,7 @@ int main(int argc, char **argv)
>         if (optind < argc - 1) {
>                 fprintf(stderr, _("Too many arguments on command line\n"));
>                 usage();
> -       } else if (optind > argc - 1) {
> +       } else if (optind > argc - 1 && !vpninfo->hostname) {
>                 fprintf(stderr, _("No server specified\n"));
>                 usage();
>         }

OK, except...

> @@ -1513,7 +1519,10 @@ int main(int argc, char **argv)
>         if (config_lookup_host(vpninfo, argv[optind]))
>                 exit(1);

Isn't that going to crash, since it's using argv[optind]?
 
> -       if (!vpninfo->hostname) {
> +       /* The last argument without a corresponding --option is taken
> +        * to be the server URL and overrides any --server option on the
> +        * command line or from a --config */
> +       if (!vpninfo->hostname || optind < argc) {

.. and isn't that redundant? 
>                 char *url = strdup(argv[optind]);
>  
>                 if (openconnect_parse_url(vpninfo, url))

I'd try something like this... and it needs the help text in usage()
and the man page updated too.

From ef819ed362ec0d95c4011edafa00950e465ded34 Mon Sep 17 00:00:00 2001
From: Max Rees <[email protected]>
Date: Tue, 3 Apr 2018 20:03:51 -0400
Subject: [PATCH] Allow specifying server in configuration file

This allows the configuration file to have an entry of the form:
server https://server[:port][/group]
similar to the CLI invocation.

Signed-off-by: Max Rees <[email protected]>
---
 main.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/main.c b/main.c
index d09efd5..0c507ba 100644
--- a/main.c
+++ b/main.c
@@ -187,6 +187,7 @@ enum {
 	OPT_HTTP_AUTH,
 	OPT_LOCAL_HOSTNAME,
 	OPT_PROTOCOL,
+	OPT_SERVER,
 	OPT_PASSTOS,
 };
 
@@ -269,6 +270,7 @@ static const struct option long_options[] = {
 	OPTION("dump-http-traffic", 0, OPT_DUMP_HTTP),
 	OPTION("no-system-trust", 0, OPT_NO_SYSTEM_TRUST),
 	OPTION("protocol", 1, OPT_PROTOCOL),
+	OPTION("server", 1, OPT_SERVER),
 #ifdef OPENCONNECT_GNUTLS
 	OPTION("gnutls-debug", 1, OPT_GNUTLS_DEBUG),
 #endif
@@ -1062,6 +1064,7 @@ int main(int argc, char **argv)
 	char *ip;
 	const char *compr = "";
 	char *proxy = getenv("https_proxy");
+	char *server = NULL;
 	char *vpnc_script = NULL;
 	const struct oc_ip_info *ip_info;
 	int autoproxy = 0;
@@ -1164,6 +1167,9 @@ int main(int argc, char **argv)
 			if (openconnect_set_protocol(vpninfo, config_arg))
 				exit(1);
 			break;
+		case OPT_SERVER:
+			server = keep_config_arg();
+			break;
 		case OPT_JUNIPER:
 			fprintf(stderr, "WARNING: Juniper Network Connect support is experimental.\n");
 			fprintf(stderr, "It will probably be superseded by Junos Pulse support.\n");
@@ -1454,10 +1460,14 @@ int main(int argc, char **argv)
 	if (gai_overrides)
 		openconnect_override_getaddrinfo(vpninfo, gai_override_cb);
 
+	/* We allow precisely one non-option argument, which is optional
+	 * if the --server option has already been provided */
 	if (optind < argc - 1) {
 		fprintf(stderr, _("Too many arguments on command line\n"));
 		usage();
-	} else if (optind > argc - 1) {
+	} else if (optind == argc - 1) {
+		server = argv[optind];
+	} else if (!server) {
 		fprintf(stderr, _("No server specified\n"));
 		usage();
 	}
@@ -1510,11 +1520,14 @@ int main(int argc, char **argv)
 	if (vpninfo->sslkey && do_passphrase_from_fsid)
 		openconnect_passphrase_from_fsid(vpninfo);
 
-	if (config_lookup_host(vpninfo, argv[optind]))
+	if (config_lookup_host(vpninfo, server))
 		exit(1);
 
+	/* The last argument without a corresponding --option is taken
+	 * to be the server URL and overrides any --server option on the
+	 * command line or from a --config */
 	if (!vpninfo->hostname) {
-		char *url = strdup(argv[optind]);
+		char *url = strdup(server);
 
 		if (openconnect_parse_url(vpninfo, url))
 			exit(1);
-- 
2.7.4

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
openconnect-devel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/openconnect-devel

Reply via email to