Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PPP_PATH_RESOLV should be moved #414

Open
rfc1036 opened this issue Apr 18, 2023 · 18 comments
Open

PPP_PATH_RESOLV should be moved #414

rfc1036 opened this issue Apr 18, 2023 · 18 comments
Assignees

Comments

@rfc1036
Copy link
Contributor

rfc1036 commented Apr 18, 2023

Nowadays it is not acceptable anymore to have daemons write state files to /etc/, which could as well be read-only, so PPP_PATH_RESOLV should be moved to a more appropriate place. I.e. somewhere in /run/.

The obvious options are:

  • /run/ppp-resolv.conf is the easiest, but do we want to keep polluting the top level directory?
  • /run/ppp/resolv.conf is tidier and could also be garbage collected by systemd, but then for consistency also the PID and TDB files should be moved there: would this cause compatibility issues with other software?

More data points:

  • embedded distributions generally move it to /tmp/, which is wrong too (and also vulnerable to TOCTOU symlink attacks)
  • Debian has been silencing since forever the error message emitted when the file cannot be created
@enaess
Copy link
Contributor

enaess commented Apr 23, 2023

Seems like a reasonable change to make, but changing paths have repercussions for all packagers ...

@enaess
Copy link
Contributor

enaess commented Apr 23, 2023

The way pppd was designed was to update /etc/resolv.conf with the new nameserver information. While changing this behavior to write it to /var/run or /var/run/ppp dependent on the --localstatedir=/run/ppp (to configure), I fear it may break the originally intended behavior.

Question for you (and other package maintainers) @yarda @rfc1036, is if better integration with e.g. systemd-resolvd (or similar package) is needed. I know ubuntu does some work-around on this issue, but having multiple processes trying to maintain /etc/resolv.conf seems like a bad choice to me.

@paulusmack any input from you here would also be valuable.

@rfc1036
Copy link
Contributor Author

rfc1036 commented Apr 23, 2023

This is not about what ppp or other packages do with the file: the current behavior already is writing it to a path which is not /etc/resolv.conf. The point is that daemons must not update at run time files in /etc/.

@enaess
Copy link
Contributor

enaess commented Apr 23, 2023

@rfc1036 I get that. Changing it to e.g. /run/resolve.conf changes the original behavior /intended design. Question for you and other distro maintainers are, what's the better integration here? E.g. integrate with systemd/resolvd, which may be available on certain distros. What about embedded?

@rfc1036
Copy link
Contributor Author

rfc1036 commented Apr 24, 2023

I still see no behavior change.

At least Debian does not have any integration with systemd-resolved, but it could be added by using the DBus interface and the $DNS1 and $DNS2 variables: ppp's own resolv.conf does not matter here.

I think that embedded distributions just symlink /etc/resolv.conf to wherever ppp's own resolv.conf is.

@enaess
Copy link
Contributor

enaess commented Apr 24, 2023

For most distros, no. Network Manager does a pretty good job at it. The typical case is to use a plugin and propagate this information.

When this software was written 20 years ago, then none of this existed. The original behavior was to replace /etc/resolve.conf to update the nameserver information.

I am not sure who uses this now and will be impacted by moving it (or completely whacking this code entirely). Maybe some embedded system, initrd image, etc? Moving it to /run/pppd seems "safe" as users of this file could use a symlink to it ... The question is who would be impacted by this?

IIRC Ubuntu have some plumbing in place to account for this replacement. It would be nice to unwrangle that ...

@yarda
Copy link
Contributor

yarda commented Apr 25, 2023

I am afraid there is no universal solution. Maybe at the moment the simplest and cleanest would be to use the resolvconf package/helper, but unfortunately it's not installed/enabled everywhere. E.g. we do not have it on Fedora. The systemd has by default emulation for it, but the emulation only correctly works if you use systemd-resolved. For Fedoras where user disabled systemd-resolved the resolvconf calls succeed, but no resolver changes are performed. But this could be covered in the documentation.

Currently I am downstream patching ppp and using change_resolv_conf script from the network-scripts, but this is just for the downstream backward compatibility and really not the way to go upstream.

@rfc1036
Copy link
Contributor Author

rfc1036 commented May 1, 2023

Again: there are no significant compatibility issues because most integration with this feature is inside the ppp package distributions will adjust whatever else is needed. There also can be no doubts that this needs to be changed, because writing files to /etc/ is not acceptable anymore.

The only question here is if this (and the PID file and the TDB file) should go to /run/ or /run/pppd/, but I think that this is settled now that 2.5.0 has moved them (Debian has not been following yet, but I believe that we will at some point).

@enaess
Copy link
Contributor

enaess commented May 2, 2023

I checked with the network manager guys. Like any VPN, pppd will communicate the DNS information to network-manager via a plugin, and thus modifying /etc/resolv.conf is bad (we knew that). What they don't want is for pppd to also tell systemd-resolved to change the dns information in that case (otherwise, you'll end up with duplicate information). Network-Manager also writes a /run/NetworkManager/resolv.conf and may swap it with /etc/resolv.conf if configured to do so.

The most prudent way to move forward on this would be to add a runtime configuration option to swap out /etc/resolv.conf for anyone still dependent on that functionality and only update the /etc/resolv.conf when configured. Then write the information to /resolv.conf by default.

@enaess
Copy link
Contributor

enaess commented May 2, 2023

Their response:

NM does not want that ppp reconfigures the system (/etc/resolv.conf or systemd-resolved). Instead -- when pppd is run by NM -- pppd should report the information back to NM. NM then decides what to do. We have a pppd plugin, which reports information back to NM. pppd needs to expose the necessary information to the plugin.

if you run pppd outside of NM, then it can make sense that pppd does certain things (like writing /etc/resolv.conf (or not); or talking to systemd-resolved). Those things optimally are configurable. The issue requests a new way for (not) writing /etc/resolv.conf. That sounds sensible.

NM always (also) writes /run/NetworkManager/resolv.conf -- and depending on configuration it may or may not write /etc/resolv.conf. Something like that seems to be requested (and sounds sensible)

@enaess
Copy link
Contributor

enaess commented May 2, 2023

I'd favor the suggestion of using /run/ppp/resolv.conf, and also move the pppd2.tdb and pidfiles there. Any objections, @paulusmack

/run/ppp/resolv.conf is tidier and could also be garbage collected by systemd, but then for consistency also the PID and TDB files should be moved there: would this cause compatibility issues with other software?

@Neustradamus
Copy link
Member

@paulusmack: Have you seen comments?

@paulusmack
Copy link
Collaborator

I'd favor the suggestion of using /run/ppp/resolv.conf, and also move the pppd2.tdb and pidfiles there. Any objections, @paulusmack

/run/ppp/resolv.conf is tidier and could also be garbage collected by systemd, but then for consistency also the PID and TDB files should be moved there: would this cause compatibility issues with other software?

No objection as long as it can still be configured to use the previous location.

@rma-x
Copy link

rma-x commented Jan 10, 2024

@enaess I think you and @rfc1036 have talked past each other.

You said that pppd had been designed to modify /etc/resolv.conf directly, but AFAICS it always wrote its own /etc/ppp/resolv.conf since the usepeerdns option was added 26 years ago (168c1d1 and 6c9ad30) and left it up to the up/down scripts to do the actual file swapping or content merging.

So the change proposed here is not about stopping ppp from touching the original file, but just about moving its own generated file from /etc/ppp to a (nowadays) preferable location in the /run hierarchy.

As for where to put it under /run, I (speaking as the ppp maintainer for SUSE) would prefer everything ppp related to be under /run/ppp, for orthogonality with /etc/ppp and to reduce the cluttering of the /run directory.

It would be nice if we could settle this before the 2.5.1 release.

@paulusmack
Copy link
Collaborator

At the moment we have PPP_PATH_CONFDIR and PPP_RUNTIME_DIR. The latter gets log files and the connection database (pppd2.tdb). The former is mostly things that pppd only reads (e.g., options, pap-secrets, etc.). The resolv.conf file is the one exception. However, I don't want to just move it (resolv.conf) to the runtime directory unilaterally since that would break existing setups.

Two possible options are (a) add another configure option to specify where it should go, and default to PPP_PATH_CONFDIR if not set; or (b) add another boolean configure option to tell it to put resolv.conf in the runtime directory rather than the config directory.

Opinions? (Or even better, suggested patches?)

@rfc1036
Copy link
Contributor Author

rfc1036 commented Apr 29, 2024

I still believe that writing to /etc/ by default is a bug which should be fixed, and that distributions will adapt trivially to the new location in PPPD_RUNTIME_DIR.
If you do not want to change the default then just do whatever is simpler.

@paulusmack
Copy link
Collaborator

I guess we can change the default as long as there is a way to get back to the old behaviour and we call out the change in the README.

@enaess
Copy link
Contributor

enaess commented Apr 29, 2024

I am with @rfc1036 on this, pppd should not write to the /etc/resolve.conf (or the /etc directory in general). It's the distribution maintainers responsibility to configure this correctly. On my installation of Ubuntu w/Systemd installed,

/etc/resolv.conf -> ../run/systemd/resolve/stub-resolv.conf

And systemd manages the resolve.conf, pppd should integrate with systemd by telling it what the name servers for the new link should be.

Using configure, and leaving the default behavior to the existing is fine with me. Allowing distribution maintainers to override this, and set the path of the resolv.conf to that of the runtime directory is also fine. I believe the sd_notify option is simply to tell systemd that the link came up and is ready (e.g. starting pppd from a service level script). I am sure there are APIs in systemd to allow us to tell it the link name, ip, dns and other details. That'll be a different ticket / issue alltogether.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants