Skip to content

Commit

Permalink
Partially revert 3b54df0, make its code optional
Browse files Browse the repository at this point in the history
With 3b54df0, macOS users massively report this issue:
	WARN:   Could not get current default route (Parsing /proc/net/route failed).
	WARN:   Protecting tunnel route has failed. But this can be working except for some cases.
See #1118.

With 3b54df0, NetworkManager users on Linux report routing problems.
See #1120.

Without 3b54df0, users on Linux with PPP 2.5.0 report this issue:
	Peer refused to agree to his IP address.
See #1114.

Therefore, make the code optional, to cover all cases. Of course, this
is a short term solution. We need to understand what happens and provide
a real fix that doesn't involve options.
  • Loading branch information
DimitriPapadopoulos committed Jun 22, 2023
1 parent 0141147 commit 87b3103
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 5 deletions.
3 changes: 3 additions & 0 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const struct vpn_config invalid_cfg = {
.pppd_ipparam = NULL,
.pppd_ifname = NULL,
.pppd_call = NULL,
.pppd_accept_remote = -1,
#endif
#if HAVE_USR_SBIN_PPP
.ppp_system = NULL,
Expand Down Expand Up @@ -563,6 +564,8 @@ void merge_config(struct vpn_config *dst, struct vpn_config *src)
free(dst->pppd_call);
dst->pppd_call = src->pppd_call;
}
if (src->pppd_accept_remote != invalid_cfg.pppd_accept_remote)
dst->pppd_accept_remote = src->pppd_accept_remote;
#endif
#if HAVE_USR_SBIN_PPP
if (src->ppp_system) {
Expand Down
1 change: 1 addition & 0 deletions src/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ struct vpn_config {
char *pppd_ipparam;
char *pppd_ifname;
char *pppd_call;
int pppd_accept_remote;
#endif
#if HAVE_USR_SBIN_PPP
char *ppp_system;
Expand Down
9 changes: 7 additions & 2 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
#define PPPD_USAGE \
" [--pppd-use-peerdns=<0|1>] [--pppd-log=<file>]\n" \
" [--pppd-ifname=<string>] [--pppd-ipparam=<string>]\n" \
" [--pppd-call=<name>] [--pppd-plugin=<file>]\n"
" [--pppd-call=<name>] [--pppd-plugin=<file>]\n" \
" [--pppd-accept-remote]\n"

#define PPPD_HELP \
" --pppd-use-peerdns=[01] Whether to ask peer ppp server for DNS server\n" \
Expand All @@ -52,7 +53,9 @@
" and ip-down scripts. See man (8) pppd.\n" \
" --pppd-call=<name> Move most pppd options from pppd cmdline to\n" \
" /etc/ppp/peers/<name> and invoke pppd with\n" \
" 'call <name>'.\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"
#elif HAVE_USR_SBIN_PPP
#define PPPD_USAGE \
" [--ppp-system=<system>]\n"
Expand Down Expand Up @@ -243,6 +246,7 @@ int main(int argc, char **argv)
.pppd_ipparam = NULL,
.pppd_ifname = NULL,
.pppd_call = NULL,
.pppd_accept_remote = 0,
#endif
#if HAVE_USR_SBIN_PPP
.ppp_system = NULL,
Expand Down Expand Up @@ -305,6 +309,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},
{"plugin", required_argument, NULL, 0}, // deprecated
#endif
#if HAVE_USR_SBIN_PPP
Expand Down
65 changes: 62 additions & 3 deletions src/tunnel.c
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,51 @@ static int pppd_run(struct tunnel *tunnel)
} else {
static const char *const v[] = {
ppp_path,
"230400", // speed
":169.254.2.1", // <local_IP_address>:<remote_IP_address>
/*
* On systems such as 4.4BSD and NetBSD, any speed can
* be specified. Other systems (e.g. Linux, SunOS) only
* support the commonly-used baud rates.
*/
"230400",
/*
* Set the local and/or remote interface IP addresses.
* Either one may be omitted. The IP addresses can be
* specified with a host name or in decimal dot notation
* (e.g. 150.234.56.78). The default local address is
* the (first) IP address of the system (unless the
* noipdefault option is given). The remote address will
* be obtained from the peer if not specified in any
* option.
* Thus, in simple cases, this option is not required.
* If a local and/or remote IP address is specified with
* this option, pppd will not accept a different value
* from the peer in the IPCP negotiation, unless the
* ipcp-accept-local and/or ipcp-accept-remote options
* are given, respectively.
*/
":169.254.2.1",
/*
* Disables the default behaviour when no local
* IP address is specified, which is to determine
* (if possible) the local IP address from the hostname.
* With this option, the peer will have to supply the
* local IP address during IPCP negotiation (unless
* it specified explicitly on the command line or in
* an options file).
*/
"noipdefault",
/*
* With this option, pppd will accept the peer's idea
* of our local IP address, even if the local IP address
* was specified in an option.
*
* This option attempts to fix this:
* Peer refused to agree to our IP address
*
* Yet, this doesn't make sense: we do not specify
* a local IP address, and we use noipdefault.
*/
"ipcp-accept-local",
"ipcp-accept-remote",
"noaccomp",
"noauth",
"default-asyncmap",
Expand Down Expand Up @@ -315,6 +355,25 @@ 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) {
Expand Down

0 comments on commit 87b3103

Please sign in to comment.