Re: stable/12: jail(2) failures after ca9ab8ea1774

On 10/11/2021 06:46, Fabian Keil wrote:
> ElectroBSD contains patches for ggatec and ggated to
> drop privileges using jail(2).
> 
> After rebasing from stable/11 to stable/12 this stopped working
> for ggated but not for ggatec.
> 
> The problem seems to be caused by:
> 
> | commit ca9ab8ea17748a1758701fde262cb272fb757989
> | Author: Jamie Gritton <jamie@FreeBSD.org>
> | Date:   Fri Feb 19 14:13:35 2021 -0800
> |
> |     MFC jail: Change both root and working directories in jail_attach(2)
> |
> |     jail_attach(2) performs an internal chroot operation, leaving it up to
> |     the calling process to assure the working directory is inside the jail.
> |
> |     Add a matching internal chdir operation to the jail's root.  Also
> |     ignore kern.chroot_allow_open_directories, and always disallow the
> |     operation if there are any directory descriptors open.
> |
> |    Reported by:    mjg
> |    Approved by:    markj, kib
> 
> One of the differences between ggated and ggatec that is probably
> relevant is that ggated uses pidfile_open().
> 
> Is jail(2) expected to work for applications using the
> pidfile family of functions with pid files located outside
> the jail directory?
> 
> Currently the struct returned by pidfile_open() contains
> a file descriptor to the directory containing the pid file
> which I suspect is the problem.
> 
> ElectroBSD's ggated tries to jail into /var/empty while
> the pid file is located in /var/run.
> 
> Shouldn't pidfile_open()'s use of cap_rights_limit() make
> this acceptable?
> 
> ElectroBSD's ggate[cd]-related patches are available at:
> <https://www.fabiankeil.de/sourcecode/electrobsd/ElectroBSD-12-0b7b773f3ef4-2021.11.10-ggate.diff>
> They should apply cleanly on stable/12 e644c87aa.

Lots of great stuff in that patch file! Thanks for sharing!
Which ones of those should we back-port to fbsd?
I've got patches open on ggate too, see
https://reviews.freebsd.org/D31722
https://reviews.freebsd.org/D31727
https://reviews.freebsd.org/D31709
(and duplicated some of your work)

> For testing purposes I added a -j option and a sysctl
> to toggle the behaviour added in ca9ab8ea1774:
> 
> fk@t520 ~ $sudo ggated -v -j
> info: Listen on port: 3080.
> error: Unable to jail process in directory /var/empty
> error: Exiting.
> fk@t520 ~ $sudo sysctl kern.pwd_chroot_chdir_check_open_directories=0
> kern.pwd_chroot_chdir_check_open_directories: 1 -> 0
> fk@t520 ~ $sudo ggated -v -j
> info: Listen on port: 3080.
> debug: Privileges successfully dropped using jail+setgid+setuid.
> 
> Fabian
>