/etc/ppp/ip-up.d/000resolvconf doesn't check if envvar USEPEERDNS is set

First of all, thanks for the report.

> but manpage clearly indicates it will set this envvar regardless

I don't see where the manpage clearly states this. Its formulation is a material implication:

     The addresses supplied by the peer (if any) are passed
     to the /etc/ppp/ip-up script in the environment variables
    DNS1 and DNS2, and the environment variable
    USEPEERDNS will be set to 1.

That these variables are *only* set, and only set to these values, under the described condition, is not explicitly said. Nor is it said that any of the variables is set, failing the described condition. Thus, so far as this text clearly states, it could be the case that if no addresses are supplied by the peer then any or all of the variables are unset, left untouched, or set to the same or other values.

If there is another passage that makes the semantics clearer then I'd be happy for you to point it out so that we can, in addition to addressing the resolvconf problem, also decide whether or not we need to improve one or more manpages.

Now, having said this, it does seems sensible for /etc/ppp/ip-up.d/000resolvconf to test for USEPEERDNS having the value 1. Assuming the manpage is correct, this is safe to do. The advantage is that it prevents resolvconf from inappropriately processing DNS1 and DNS2 if they are merely passed through from the environment, for example. The fact that 0000usepeerdns itself checks the value of USEPEERDNS is also a very strong indication that this is a good idea. :)

This issue affects Debian too. I will attach a patch here which will be applied for the next Debian release.

The relevant code in /etc/ppp/ip-up.d/0000usepeerdns (ppp version 2.4.5-5ubuntu2) is the following.

    [ "$USEPEERDNS" ] || exit 0

That is, it only checks that the variable has some value, not that the value is 1. Interesting.

Should 000resolvconf do the same, or the following?

    [ "$USEPEERDNS" = 1 ] || exit 0

I can see the ambiguity of that section, but the quoted section provided in the report does not guarantee that the var is NOT set failing "usepeerdns" and instead offers a second definition without reference to "usepeerdns"

I've reached out to the ppp devs to get their thoughts on updating the manpage. If this bug isn't closed out before it is clarified, I'll update it with the results (and open a new bug in the ppp package to update the manpage).

By the way, how did this bug manifest itself in practice?

Is it the case that you disabled usepeerdns mode and found that nameserver addresses were still submitted to resolvconf? What were those addresses and where to you think they came from?

I had configured the peer without usepeerdns and the addresses were negotiated during IPCP via ms-dns1 and ms-dns2 TLVs.

I will fix this in Debian with

    [ "$USEPEERDNS" ] || exit 0

for consistency with /etc/ppp/ip-up.d/0000usepeerdns. This fix will appear in 1.69 which will be released well before the 13.04 merge-pulse.

I just stumbled across Debian bug report #367715 which has the following title.

    ppp: overwrites /etc/resolv.conf despite "usepeerdns" being unset

The report says about Debian ppp 2.4.4b1-1 that

    pppd sets the environment variable USEPEERDNS regardless
    of the option 'usepeerdns' being set or unset in a peer's
    configuration file.

In that package, /etc/ppp/ip-up.d/0000usepeerdns already contained '[ "$USEPEERDNS" ] || exit 0'.

If the ppp package hasn't been fixed since then, then to check that usepeerdns is set it won't suffice to check for "$USEPEERDNS" being nonempty.

William McCall: Can you please test with the current version of the ppp package? If you put

    [ "$USEPEERDNS" ] || exit 0

near the top of /etc/ppp/ip-up.d/000resolvconf does that suffice to prevent the malfunction that you reported? Or does it have to be

    [ "$USEPEERDNS" = 1 ] || exit 0

@William McCall: Can you please test with the current version of the ppp package? If you put the following

    [ "$USEPEERDNS" ] || exit 0

near the top of /etc/ppp/ip-up.d/000resolvconf does that suffice to prevent the malfunction that you reported? Or does it have to be the following?

    [ "$USEPEERDNS" = 1 ] || exit 0

Thomas--

I have implemented this with " [ "$USEPEERDNS" ] || exit 0" and found that the behavior is now appropriate.

William wrote:
> I have implemented this with " [ "$USEPEERDNS" ] || exit 0"
> and found that the behavior is now appropriate.

Thanks, I'll go ahead and release Debian resolvconf 1.69 with that change then.

Resolvconf 1.69 has been released.

This bug was fixed in the package resolvconf - 1.69ubuntu1

---------------
resolvconf (1.69ubuntu1) raring; urgency=low

  * Merge from Debian. Remaining changes: (LP: #1085756)
    - Make /etc/resolv.conf a relative symlink so that it works when
      setting up chroots.
    - Instead of throwing an error that aborts the upgrade when
      /etc/resolv.conf is immutable, pop a debconf error message to let the
      user know what's happening, then clear the immutable flag and continue
      with the installation. LP: #989585.
    - debian/config, debian/templates, debian/postinst: if we don't know that
      /etc/resolv.conf was being dynamically managed before install (in at
      least some cases), link the original contents of /etc/resolv.conf to
      /etc/resolvconf/resolv.conf.d/tail so that any statically configured
      nameservers aren't lost. LP: #923685.
    - Use an upstart job instead of sysvinit script.
    - Pre-Depend on the /run-providing version of initscripts instead of
      depending, so that the preinst does the right thing when creating
      /run/resolvconf/interfaces instead of getting clobbered later by a bind
      mount on initscripts upgrade. LP: #922491.
    - Completely drop the standard_run_subdirs_created helper function from
      debian/postinst, since it serves no purpose here.
    - postinst: Set resolvconf/linkify-resolvconf to false after initial
      conversion, ensuring upgrades won't convert /etc/resolv.conf to
      a symlink if the user manually converted it back to plain text.
      (LP: #922677)
    - Move the #DEBHELPER# token in debian/postinst to after the resolv.conf
      symlink is set, so the init script can actually start (since it expects
      /etc/resolv.conf to be a symlink).
    - Eliminate all references to /etc/resolvconf/run. This should all be done
      directly in /run, there is no reason to support making any of this
      configurable with a symlink since we already have a versioned dependency
      on the version of initscripts that introduces the /run transition.
    - Also remove debian/triggers to avoid needlessly calling resolvconf's
      postinst. (LP: #931335)
  * Fix previous mis-merge of debian/postinst as well as the few other comments
    from the bug report. (LP: #1085862)

resolvconf (1.69) unstable; urgency=low

  * [276fc24] Bump Standards-Version; no changes required
  * [6982da6] README: Name packages that clobber /etc/resolv.conf
  * [4cb082a] README: Update
  * [044e9a3] ip-up.d/000resolvconf: Do nothing if USEPEERDNS not set
  * [d988a9e] if-up.d/000resolvconf: Silently (for now) accept option
    name 'dns-nameserver' as a synonym for 'dns-nameservers' in
    anticipation of support in ifupdown (no sooner than wheezy+1) for
    duplicate options in a stanza, after which it will make sense to
    specify multiple nameserver addresses on equally many
    "dns-nameserver" lines
  * [e0db2cb] Deal with obsolete conf files (Closes: #687507, #681623)
 -- Stephane Graber <email address hidden> Fri, 18 Jan 2013 11:52:10 -0500