[bug#68675] [PATCH v3 2/2] services: dhcp: Support the dhcpcd implementation.

["Sören Tempel" <soeren@soeren-tempel.net>]

Hi,

Ludovic Courtès <ludo@gnu.org> wrote:
> Apologies for the late reply.

No worries, I understand that you have a lot of other things to do :)

> I would very much like to avoid the ‘open-input-pipe’ dance here.  Maybe
> there’s a way to ask it to not fork, in which case we don’t need to wait
> for a PID file?

When using a PID file, AFAIK the only thing we can do is replicate the C
code that I referenced in my prior email for determining the PID file
name [1]. Is there a specific reason why you want to avoid using
open-input-pipe?

> What do others do?  I guess it’s not possible to have a systemd .service
> file given those PID file semantics, is it?

systemd and other service supervisor do not rely on PID files for
services supervision as they are inherently racy. Therefore, as
mentioned in my prior email, I also think it would be preferable to not
use a PID file for supervising either dhclient or dhcpcd.

I only ended up using a PID file here as dhclient already used one and I
wanted to keep the service changes as minimal as possible to ensure a
swift review since I believe this to be a security-critical issue (see
Guix issue #68619).

> Two notes: I would find it clearer to call ‘fork+exec-command’ above
> instead of constructing ‘exec-config’.
> 
> Ideally, we’d use:
> 
>   (start #~(if (file-exists? (string-append #$package "/bin/dhclient"))
>                (make-forkexec-constructor …)   ;ISC version, with #:pid-file
>                (make-forkexec-constructor …))) ;dhcpcd, maybe without #:pid-file

If we separate the dhclient and the dhcpcd code like this, then it may
be worthwhile to have entirely separated services instead of combining
them both in a single service. This would also ease providing a
configuration for these services.

> Maybe that is too naive, but at least we should get as close as possible
> to that, for instance by avoiding direct calls to ‘fork+exec-command’ +
> ‘waitpid’ + ‘read-pid-file’ as much as possible.

Ideally, I would not want to refactor existing service code as much in
this patchset. As mentioned above, I believe Guix using dhclient by
default (which has been EOL for 2 years) has security implications.
Therefore, I would want to migrate dhcp-client-service-type away from
dhclient to dhcpcd as soon as possible. As part of this migration, I
would strongly suggest a deprecation and removal of dhclient support
from dhcp-client-service-type. As soon as dhclient support is removed,
refactorings of dhcp-client-service-type can be conducted. Including a
removal of PID files for supervision, instead supervising dhcpcd by
spawning it as non-double-forking child process [2].

Greetings
Sören

[1]: https://github.com/NetworkConfiguration/dhcpcd/blob/v10.0.6/src/dhcpcd.c#L2144
[2]: https://github.com/nmeum/guix-channel/blob/b1b80697a9d35ca015ce56ccf3031f91f2bf554f/src/nmeum/services/networking.scm#L209-L211