Clean up 1a6d2e9

Runtime option `pppd-accept-remote` will remain available at all time,
but will now accept a boolean argument:
* `--pppd-accept-remote=1`: invoke pppd with the `ipcp-accept-remote`
  option, which is required by pppd ≥ 2.5.0.
* `--pppd-accept-remote=0`: invoke pppd without the `ipcp-accept-remote`
  option, which is required by pppd < 2.5.0.
* `--pppd-accept-remote` is kept as a deprecated, undocumented synonym
  of `--pppd-accept-remote=1`.

Build-time option `--enable-legacy-pppd` will change the default from
`--pppd-accept-remote=1` to `--pppd-accept-remote=0`. This might be
preferable on older platforms shipping pppd < 2.5.0, such as macOS.

configure.ac

@@ -228,11 +228,11 @@ AC_ARG_WITH([pppd],
 		with_ppp="no"
 	])
 )
-# this is specifically for pppd < 2.5.0
+# support pppd < 2.5.0 by default instead of pppd >= 2.5.0
 AC_ARG_ENABLE([legacy_pppd],
 	AS_HELP_STRING([--enable-legacy-pppd],
-	               [work around pppd < 2.5.0 issues]))
-# and this is for the ppp user space client on FreeBSD
+	               [support pppd < 2.5.0 by default instead of pppd >= 2.5.0]))
+# this is for the ppp user space client on FreeBSD
 AC_ARG_WITH([ppp],
 	AS_HELP_STRING([--with-ppp],
 		       [set the path to the ppp userspace client on FreeBSD]),

doc/openfortivpn.1.in

@@ -39,6 +39,7 @@ openfortivpn \- Client for PPP+SSL VPN tunnel services
 [\fB\-\-pppd\-ipparam=\fI<string>\fR]
 [\fB\-\-pppd\-ifname=\fI<string>\fR]
 [\fB\-\-pppd\-call=\fI<name>\fR]
+[\fB\-\-pppd\-accept\-remote=\fI<bool>\fR]
 [\fB\-\-ppp\-system=\fI<string>\fR]
 [\fB\-\-use\-resolvconf=\fI<bool>\fR]
 [\fB\-\-persistent=\fI<interval>\fR]
@@ -227,6 +228,10 @@ This can be useful on Debian and Ubuntu, where unprivileged users in
 group `dip' can invoke `pppd call <name>' to make pppd read and apply
 options from /etc/ppp/peers/<name> (including privileged ones).
 .TP
+\fB\-\-pppd\-accept\-remote=\fI<bool>\fR
+Whether to invoke pppd with `ipcp-accept-remote'. Enabling this option breaks
+pppd < 2.5.0 but is required by newer pppd versions.
+.TP
 \fB\-\-ppp\-system=\fI<string>\fR
 Only available if compiled for ppp user space client (e.g. on FreeBSD).
 Connect to the specified system as defined in /etc/ppp/ppp.conf

src/config.c

@@ -350,6 +350,15 @@ int load_config(struct vpn_config *cfg, const char *filename)
 		} else if (strcmp(key, "pppd-call") == 0) {
 			free(cfg->pppd_call);
 			cfg->pppd_call = strdup(val);
+		} else if (strcmp(key, "pppd-accept-remote") == 0) {
+			int pppd_accept_remote = strtob(val);
+
+			if (pppd_accept_remote < 0) {
+				log_warn("Bad pppd-accept-remote in configuration file: \"%s\".\n",
+				         val);
+				continue;
+			}
+			cfg->pppd_accept_remote = pppd_accept_remote;
 #else
 		} else if (strcmp(key, "pppd") == 0) {
 			log_warn("Ignoring pppd option \"%s\" in the config file.\n",

src/main.c

@@ -37,8 +37,7 @@
 "                    [--pppd-use-peerdns=<0|1>] [--pppd-log=<file>]\n" \
 "                    [--pppd-ifname=<string>] [--pppd-ipparam=<string>]\n" \
 "                    [--pppd-call=<name>] [--pppd-plugin=<file>]\n" \
-"                    [--pppd-accept-remote]\n"
-
+"                    [--pppd-accept-remote=<0|1>]\n"
 #define PPPD_HELP \
 "  --pppd-use-peerdns=[01]       Whether to ask peer ppp server for DNS server\n" \
 "                                addresses and make pppd rewrite /etc/resolv.conf.\n" \
@@ -54,8 +53,8 @@
 "  --pppd-call=<name>            Move most pppd options from pppd cmdline to\n" \
 "                                /etc/ppp/peers/<name> and invoke pppd with\n" \
 "                                'call <name>'.\n" \
-"  --pppd-accept-remote          Invoke pppd with option 'ipcp-accept-remote'." \
-"                                It might help avoid errors with PPP 2.5.0.\n"
+"  --pppd-accept-remote=[01]     Whether to invoke pppd with 'ipcp-accept-remote'.\n" \
+"                                Disable for pppd < 2.5.0.\n"
 #elif HAVE_USR_SBIN_PPP
 #define PPPD_USAGE \
 "                    [--ppp-system=<system>]\n"
@@ -246,7 +245,11 @@ int main(int argc, char **argv)
 		.pppd_ipparam = NULL,
 		.pppd_ifname = NULL,
 		.pppd_call = NULL,
+#if LEGACY_PPPD
 		.pppd_accept_remote = 0,
+#else
+		.pppd_accept_remote = 1,
+#endif
 #endif
 #if HAVE_USR_SBIN_PPP
 		.ppp_system = NULL,
@@ -309,7 +312,7 @@ int main(int argc, char **argv)
 		{"pppd-ipparam",         required_argument, NULL, 0},
 		{"pppd-ifname",          required_argument, NULL, 0},
 		{"pppd-call",            required_argument, NULL, 0},
-		{"pppd-accept-remote",   no_argument, &cli_cfg.pppd_accept_remote, 1},
+		{"pppd-accept-remote",   optional_argument, NULL, 0},
 		{"plugin",               required_argument, NULL, 0}, // deprecated
 #endif
 #if HAVE_USR_SBIN_PPP
@@ -390,6 +393,22 @@ int main(int argc, char **argv)
 				cli_cfg.pppd_call = strdup(optarg);
 				break;
 			}
+			if (strcmp(long_options[option_index].name,
+			           "pppd-accept-remote") == 0) {
+				if (optarg) {
+					int pppd_accept_remote = strtob(optarg);
+
+					if (pppd_accept_remote < 0) {
+						log_warn("Bad pppd-accept-remote option: \"%s\"\n",
+						         optarg);
+						break;
+					}
+					cli_cfg.pppd_accept_remote = pppd_accept_remote;
+				} else {
+					cli_cfg.pppd_accept_remote = 1;
+				}
+				break;
+			}
 			// --plugin is deprecated, use --pppd-plugin
 			if (cli_cfg.pppd_plugin == NULL &&
 			    strcmp(long_options[option_index].name,

src/tunnel.c

@@ -277,26 +277,8 @@ static int pppd_run(struct tunnel *tunnel)
 				 * 1. we do not specify a local IP address,
 				 * 2. we use option noipdefault to specifically ask the
 				 *    peer to supply the local IP address.
-				 *
-				 * pppd ≥ 2.5.0 might not require it, but I don't dare
-				 * removing it.
 				 */
 				"ipcp-accept-local",
-#ifndef LEGACY_PPPD
-				/*
-				 * With this option, pppd accepts the peer's idea of its
-				 * (remote) IP address, even if the remote IP address was
-				 * specified in an option.
-				 *
-				 * pppd ≥ 2.5.0 requires this option to avoid this error:
-				 *     Peer refused to agree to his IP address
-				 * This makes perfect sense.
-				 *
-				 * Unfortunately, pppd < 2.5.0 does not like this option.
-				 * Again, this doesn't make sense to me.
-				 */
-				"ipcp-accept-remote",
-#endif
 				"noaccomp",
 				"noauth",
 				"default-asyncmap",
@@ -313,6 +295,23 @@ static int pppd_run(struct tunnel *tunnel)
 					return 1;
 				}
 		}
+		if (tunnel->config->pppd_accept_remote)
+			/*
+			 * With this option, pppd will accept the peer's idea of its
+			 * (remote) IP address, even if the remote IP address was
+			 * specified in an option.
+			 *
+			 * pppd ≥ 2.5.0 requires this option to avoid this error:
+			 *     Peer refused to agree to his IP address
+			 * This makes sense.
+			 *
+			 * Unfortunately, pppd < 2.5.0 does not like this option.
+			 * Again, this doesn't make sense to me.
+			 */
+			if (ofv_append_varr(&pppd_args, "ipcp-accept-remote")) {
+				free(pppd_args.data);
+				return 1;
+			}
 		if (tunnel->config->pppd_use_peerdns)
 			if (ofv_append_varr(&pppd_args, "usepeerdns")) {
 				free(pppd_args.data);
@@ -375,25 +374,6 @@ static int pppd_run(struct tunnel *tunnel)
 				return 1;
 			}
 		}
-		if (tunnel->config->pppd_accept_remote)
-			/*
-			 * With this option, pppd will accept the peer's idea of
-			 * its (remote) IP address, even if the remote IP address
-			 * was specified in an option.
-			 *
-			 * This option attempts to fix this with PPP 2.5.0:
-			 *     Peer refused to agree to his IP address
-			 *
-			 * Currently (always?) breaks on macOS with:
-			 *     Could not get current default route
-			 *     (Parsing /proc/net/route failed).
-			 *     Protecting tunnel route has failed.
-			 *     But this can be working except for some cases.
-			 */
-			if (ofv_append_varr(&pppd_args, "ipcp-accept-remote")) {
-				free(pppd_args.data);
-				return 1;
-			}
 #endif
 #if HAVE_USR_SBIN_PPP
 		if (tunnel->config->ppp_system) {