pppd: Do not overwrite /etc/resolv.conf

With option usepeerdns, pppd by default writes to /etc/ppp/resolv.conf,
which is not very useful and might be impossible. However, when more
than one interface can be used for DNS resolution, pppd will overwrite
any existing entries when /etc/resolv.conf is used instead, which is
bad. Fix this by setting the path to a place we assume is always
writeable, /tmp/ppp-resolv.conf, and which does not interfere with other
mechanisms that write DNS resolver configurations.
---
 package/pppd/pppd.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On 28-08-17 17:56, Jeroen Roovers wrote:
> With option usepeerdns, pppd by default writes to /etc/ppp/resolv.conf,
> which is not very useful and might be impossible. However, when more
> than one interface can be used for DNS resolution, pppd will overwrite
> any existing entries when /etc/resolv.conf is used instead, which is
> bad. 

 I agree that this is bad.

> Fix this by setting the path to a place we assume is always
> writeable, /tmp/ppp-resolv.conf, and which does not interfere with other
> mechanisms that write DNS resolver configurations.

 However, this is a regression. People who currently have ppp as their only
interface, or who never have ppp and ethernet running at the same time,
currently have a fully working solution. With this change, DNS doesn't work
anymore over ppp unless you add some script to merge /tmp/ppp-resolv.conf with
/tmp/resolv.conf

 So I think this patch should be combined with a /etc/ppp/ip-up (and ip-down)
script that updates /etc/resolv.conf, similar like how udhcpc.script does it.

 I guess you must have something like that anyway, otherwise you would never
have DNS over ppp, right?

 Regards,
 Arnout

> ---
>  package/pppd/pppd.mk | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pppd/pppd.mk b/package/pppd/pppd.mk
> index 6139c5b085..47937730d5 100644
> --- a/package/pppd/pppd.mk
> +++ b/package/pppd/pppd.mk
> @@ -36,10 +36,10 @@ PPPD_POST_EXTRACT_HOOKS += PPPD_DROP_INTERNAL_IF_PPOL2TP_H
>  
>  # pppd defaults to /etc/ppp/resolv.conf, which not be writable and is
>  # definitely not useful since the C library only uses
> -# /etc/resolv.conf. Therefore, we change pppd to use /etc/resolv.conf
> +# /etc/resolv.conf. Therefore, we change pppd to use /tmp/ppp-resolv.conf
>  # instead.
>  define PPPD_SET_RESOLV_CONF
> -	$(SED) 's,ppp/resolv.conf,resolv.conf,' $(@D)/pppd/pathnames.h
> +	$(SED) 's,/etc/ppp/resolv.conf,/tmp/ppp-resolv.conf,' $(@D)/pppd/pathnames.h
>  endef
>  PPPD_POST_EXTRACT_HOOKS += PPPD_SET_RESOLV_CONF
>  
>

On 28 August 2017 at 22:28, Arnout Vandecappelle <arnout@mind.be> wrote:

>> pppd will overwrite any existing entries when /etc/resolv.conf is used instead, which is
>> bad.
>
>  I agree that this is bad.

Thanks.

>  However, this is a regression.

1. dnsmasq(8) mentions using /etc/dhcp/resolv.conf as well as
/etc/ppp/resolv.conf.
2. On the greater Internet you can easily find plenty of examples of
people getting confused because /etc/resolv.conf does not reflect what
usepeerdns has written to the wrong file in /etc/ppp/
3. usepeerdns is not enable by default.

> People who currently have ppp as their only
> interface, or who never have ppp and ethernet running at the same time

So it's a regression for what I can only assume must be a tiny
minority of system builders who don't configure their own systems and
are powerless but to rely on a second-guessed set of defaults that you
would like to support.

Additionally, the current pppd.mk suggests /etc/ppp/resolv.conf is not
writeable (in some configurations) but you say adding an ip-up script
would solve the issue, when that ip-up script is then supposed to
write to /etc/resolv.conf, which supposedly *is* writeable.

Please enlighten me.


Kind regards,
     jer

On 11-09-17 09:58, Jeroen Roovers wrote:
> On 28 August 2017 at 22:28, Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>> pppd will overwrite any existing entries when /etc/resolv.conf is used instead, which is
>>> bad.
>>
>>  I agree that this is bad.
> 
> Thanks.
> 
>>  However, this is a regression.
> 
> 1. dnsmasq(8) mentions using /etc/dhcp/resolv.conf as well as
> /etc/ppp/resolv.conf.
> 2. On the greater Internet you can easily find plenty of examples of
> people getting confused because /etc/resolv.conf does not reflect what
> usepeerdns has written to the wrong file in /etc/ppp/
> 3. usepeerdns is not enable by default.
> 
>> People who currently have ppp as their only
>> interface, or who never have ppp and ethernet running at the same time
> 
> So it's a regression for what I can only assume must be a tiny
> minority of system builders who don't configure their own systems and
> are powerless but to rely on a second-guessed set of defaults that you
> would like to support.

 My point is: people who currently use pppd and another network interface
simultaneously either have something that doesn't really work (unlikely) or have
fixed it locally (more likely). However, people who currently have a working
ppp-only setup would no longer have a working setup after updating Buildroot.
That's something we prefer to avoid.

 If there is no way to avoid breaking existing configs, we probably indeed
should still accept this patch because as you say, it does fix things for a
(probably) more common use case. Well, except it doesn't really fix things,
because you still need an additional script to actually use the DNS information.

 Your point 1. is interesting. It suggests that a better solution would be to
not change pathnames.h at all, but instead symlink /etc/ppp/resolv.conf to
either /etc/resolv.conf (keeping the current behaviour) or /tmp/ppp-resolv.conf
(providing the behaviour you want).


 We recently added a "Migrating" section in the Buildroot manual. So what we
could do is:
1. ln -s ../../tmp/ppp-resolv.conf $(TARGET_DIR)/etc/ppp/resolv.conf
2. Add a paragraph to the Migrating section explaining that a configuration that
was using pppd with usepeerdns needs to either enable dnsmasq or to symlink
$(TARGET_DIR)/etc/ppp/resolv.conf to /tmp/resolv.conf or to create a
/etc/ppp/ip-up script.

 Does that sound reasonable?


> 
> Additionally, the current pppd.mk suggests /etc/ppp/resolv.conf is not
> writeable (in some configurations) but you say adding an ip-up script
> would solve the issue, when that ip-up script is then supposed to
> write to /etc/resolv.conf, which supposedly *is* writeable.

 Indeed, /etc/resolv.conf is a symlink to /tmp/resolv.conf.


 Regards,
 Arnout

> 
> Please enlighten me.
> 
> 
> Kind regards,
>      jer
>

Hi Jeroen,

On 12-09-17 01:04, Arnout Vandecappelle wrote:
> 
> 
> On 11-09-17 09:58, Jeroen Roovers wrote:
>> On 28 August 2017 at 22:28, Arnout Vandecappelle <arnout@mind.be> wrote:
>  My point is: people who currently use pppd and another network interface
> simultaneously either have something that doesn't really work (unlikely) or have
> fixed it locally (more likely). However, people who currently have a working
> ppp-only setup would no longer have a working setup after updating Buildroot.
> That's something we prefer to avoid.
> 
>  If there is no way to avoid breaking existing configs, we probably indeed
> should still accept this patch because as you say, it does fix things for a
> (probably) more common use case. Well, except it doesn't really fix things,
> because you still need an additional script to actually use the DNS information.
> 
>  Your point 1. is interesting. It suggests that a better solution would be to
> not change pathnames.h at all, but instead symlink /etc/ppp/resolv.conf to
> either /etc/resolv.conf (keeping the current behaviour) or /tmp/ppp-resolv.conf
> (providing the behaviour you want).
> 
> 
>  We recently added a "Migrating" section in the Buildroot manual. So what we
> could do is:
> 1. ln -s ../../tmp/ppp-resolv.conf $(TARGET_DIR)/etc/ppp/resolv.conf
> 2. Add a paragraph to the Migrating section explaining that a configuration that
> was using pppd with usepeerdns needs to either enable dnsmasq or to symlink
> $(TARGET_DIR)/etc/ppp/resolv.conf to /tmp/resolv.conf or to create a
> /etc/ppp/ip-up script.
> 
>  Does that sound reasonable?

 I imagine that my proposed solution is way too complicated. So instead, I
applied the patch from Maksim Salau [1]. It is also very simple, like yours, but
in addition it keeps backward compatibility.

 A more extended solution can still be added in the future.

 Regards,
 Arnout


[1] http://patchwork.ozlabs.org/patch/826889/