Add "ipcp-accept-remote" pppd option

[DimitriPapadopoulos]

PPP ≥ 2.5.0 requires this option to avoid this error:

	Peer refused to agree to his IP address

It makes sense to let the remote decide for the remote IP address.

Unfortunately, PPP < 2.5.0 does not like this option. Of course, it doesn't make any sense.

Fixes #1076.

[DimitriPapadopoulos]

@adrienverge Would you mind having a look? PPP ≥ 2.5.0 requires "ipcp-accept-remote", which makes sense. Unfortunately, it breaks PPP < 2.5.0. I don't have much time to write code to detect the version of pppd at run-time, so I came up with this solution of a build-time option for targets with PPP < 2.5.0.

[adrienverge]

Thanks @DimitriPapadopoulos, it looks like a good solution.

However, the commit message and the in-code comments contradict themselves: it's pppd ≥ 2.5.0 that requires this option to avoid this error, right? Not pppd < 2.5.0.

PS: Also I think we should write "pppd" (the software, with "d" as daemon), not "PPP" (the protocol).

[DimitriPapadopoulos]

Changed PPP → pppd, thank you for that.

I slightly reworded the code comments and the commit message for clarity (?):

• I see the Peer refused to agree to our IP address error for pppd < 2.5.0 and when "ipcp-accept-local" is missing, which doesn't make sense to me but so be it.
• I see the Peer refused to agree to his IP address error for pppd ≥ 2.5.0 and when "ipcp-accept-remote" is missing, which does make sense.
• I see an error (cannot recall which exact one right now) for pppd < 2.5.0 when "ipcp-accept-remote" is added, which really doesn't make sense.

[DimitriPapadopoulos]

Also, I could try to detect macOS and disable "ipcp-accept-remote" by default, instead of enabling it by default as on Linux. The rationale is that current macOS systems do not ship pppd ≥ 2.5.0, while many recent Linux distributions do ship pppd ≥ 2.5.0. However, that seems fragile over time, and I don't really have time for this.

I'll try to reach to the Homebrew packagers before the next release, so that they can add configure option --enable-legacy-pppd in their formula.

[adrienverge]

OK I understand clearly now! Thanks, the new code comments are crystal-clear.

I'll try to reach to the Homebrew packagers before the next release, so that they can add configure option --enable-legacy-pppd in their formula.

Good idea. Better let the packagers handle system-specific tweaks (while waiting for pppd ≥ 2.5.0 to be packaged on MacOS).

[DimitriPapadopoulos]

@adrienverge After thinking a little bit more about it, I have modified this change in #1153:

• Passing option "ipcp-accept-remote" to pppd, or not, is now a runtime choice.
• By default, openfortivpn will pass option "ipcp-accept-remote" to pppd by default. Build-time option --disable-pppd-default changes that: with this build-time option, openfortivpn will not pass option "ipcp-accept-remote" to pppd by deafult.
• Runtime openfortivpn option --pppd-accept-remote used to take no argument. I have changed it to accept a boolean argument (--pppd-accept-remote=<0|1>) so that end-users can modify the above default. The argument is optional for backwards compatibility.

This leaves the choice to packagers to build openfortivpn with --disable-pppd-default when targetting platforms with pppd < 2.5.0, and without --disable-pppd-default when targetting platforms with pppd ≥ 2.5.0.

End-users are free to change the default chosen by packagers, using runtime option --pppd-accept-remote=1 or --pppd-accept-remote=0 respectively.

[adrienverge]

Thanks for the clear explanation @DimitriPapadopoulos. It would have been nice to keep the code base simpler with only a choice at compilation-time (for systems that have pppd version x vs. y), but being able to change the default choice at runtime is indeed more flexible.

[DimitriPapadopoulos]

I would also prefer the simpler solution, but I fear it will break platforms such as macOS where packagers may target multiple versions of macOS, past ones with pppd < 2.5.0 and possibly new ones with pppd ≥ 2.5.0 in the future. This way, end-users will be to work around either default setting chosen by the packagers.