Bug 277210 - jail(8): exec.clean retrieves PWD from user info (can cause services to crash on jail start-up)
Summary: jail(8): exec.clean retrieves PWD from user info (can cause services to crash...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: --- Affects Many People
Assignee: Jamie Gritton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-21 11:55 UTC by johannes.kunde
Modified: 2024-08-23 09:08 UTC (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description johannes.kunde 2024-02-21 11:55:25 UTC
I recently ran into a problem where one of my services, configured via rc(8), 
failed to start, but only if the corresponding jail was (re)started as a whole.
The same service (re)started without problems when running 'service foo start'.
Digging into the problem, I found out that in case 'exec.clean' has been set in 
jail.conf(5), PWD is set to '/root' (only root:wheel can read here) when (re)starting the jail, 
but is set to '/' when running 'service foo start' from within the running jail. 
I think this behavior is caused by the following lines:
https://github.com/freebsd/freebsd-src/blob/main/usr.sbin/jail/command.c#L727-L797.

It is also not documented in the manpage that a chdir(2) happens with 'exec.clean'.

So far, I've found two ways to overcome this behavior. One is to omit 'exec.clean' in 
jail.conf(5), the other is to set '${name}_chdir="/"' in the corresponding rc(8) script.

However, I wanted to file this bug and discuss the problem, as I'm uncertain if this behavior 
is intended. When running the very same service on a FreeBSD host system (not in a jail) 
it starts up normally, because PWD will be "/" in any case. It's also quite hard to trace 
that issue down to the above lines, as there is no obvious error message other than what 
is printed out by the affected service itself.

The problem can be easily reproduced with the below C program and rc(8) script:

Compile the program with 'cc -o getcwd getcwd.c' and copy it to '/usr/local/sbin'.



#include <unistd.h>
#include <limits.h>
#include <syslog.h>

int main() {
	char cwd[PATH_MAX];
		if (getcwd(cwd, sizeof(cwd)) != NULL) {
			syslog(LOG_DEBUG, "Current working dir: %s\n", cwd);
		} else {
			syslog(LOG_DEBUG, "getcwd() error");
			return 1;
		}
	return 0;
}



Copy the following script to '/usr/local/etc/rc.d/getcwd' and run 'chmod +x /usr/local/etc/rc.d/getcwd'.



#!/bin/sh
#
# PROVIDE: getcwd

. /etc/rc.subr

name="getcwd"
rcvar="getcwd_enable"
command="/usr/local/sbin/${name}"

load_rc_config $name
: ${getcwd_enable:=NO}

run_rc_command "$1"



Run 'sysrc getcwd_enable="YES"'

Now do the following:
1. Restart the jail
2. Run 'service getcwd start'
3. Take a look at '/var/log/debug.log'


NOTE: This behavior is not bound to or related to any specific Jail-Manager.
Comment 1 Michael Osipov freebsd_committer freebsd_triage 2024-02-21 12:04:42 UTC
Please paste the output of syslog.
Comment 2 johannes.kunde 2024-02-21 12:22:05 UTC
That's how a 'service getcwd start' looks like:
Feb 21 10:55:07 testjail getcwd[4421]: Current working dir: /
That's how a (re)start of the jail looks like:
Feb 21 10:55:25 testjail getcwd[5256]: Current working dir: /root
Comment 3 Michael Osipov freebsd_committer freebsd_triage 2024-02-21 12:54:17 UTC
(In reply to johannes.kunde from comment #2)

The second case is problematic because if you do a su(1) or daemon(8) it will fail and depending on the code the executable might exit.
Comment 4 johannes.kunde 2024-02-21 13:05:13 UTC
(In reply to Michael Osipov from comment #3)

Correct, and that's exactly where I'm coming from. I've a GO webservice controlled by daemon(8) with '-u www' flag set and on jail start-up the program exits, because it trys to retrieve an absolute path at /root.
Comment 5 Jamie Gritton freebsd_committer freebsd_triage 2024-02-22 02:12:17 UTC
While it does make sense to root from the jail's root under exec.clean, it unfortunately clashes with a decade of current practice.

I'll consider a change, and see what people think, but I can't guarantee it'll happen given the long history.  It seems reasonable that if a user is specified (even directly like -U root), then it would still chdir to its home, but in the absence of a user, starting in "/" would in fact be the most expected outcome.
Comment 6 johannes.kunde 2024-02-22 07:09:44 UTC
(In reply to Jamie Gritton from comment #5)

> While it does make sense to root from the jail's root under exec.clean, it unfortunately clashes with a decade of current practice.

I get the idea. I believe it runs contrary to what most people expect to be the outcome, which would be a behavior consistent with the host system. A service running inside a jail with a user other than root most likely doesn't want to start at /root.

> It seems reasonable that if a user is specified (even directly like -U root), then it would still chdir to its home, but in the absence of a user, starting in "/" would in fact be the most expected outcome.

That sounds reasonable.

I think one of the main problems is also the lack of information when the problem occurs unexpectedly in the wild. All I got printed out from an external library, which caused the issue in my case, was this line:

> Feb 12 20:20:39 myjail myservice[9364]: 2024/02/12 20:20:39 stat .: permission denied

Starting from there, it's almost impossible to have a clue what's going on.
I don't know if there is any good chance to generate helpful output at runtime, but some hints in the manpage would help as well.
Comment 7 Michael Osipov freebsd_committer freebsd_triage 2024-02-22 08:06:45 UTC
From jail(8):


       exec.clean
	       Run commands in a clean environment.  The environment  is  dis-
	       carded  except  for HOME, SHELL,	TERM and USER.	HOME and SHELL
	       are set to the target login's default values.  USER is  set  to
	       the  target  login.  TERM is imported from the current environ-
	       ment.  The environment variables	from the login class  capabil-
	       ity database for	the target login are also set.

I completely miss the chdir(2) here with PWD. That is the first one to be fixed even if it gets reverted because it is not documented.
Comment 8 Frank Behrens 2024-03-14 16:39:49 UTC
I discovered the same problem, even without an exec.clean.

In  the script, called from the service start script is a
d=`pwd`
..
cd $d

which fails, after the effective user for the service has been changed.
Comment 9 johannes.kunde 2024-03-14 17:35:04 UTC
(In reply to Frank Behrens from comment #8)

> In  the script, called from the service
> start script is a
> d=`pwd`
> ..
> cd $d

Can you please provide the source? Would be great to have it tracked here.
Comment 10 Siva Mahadevan 2024-07-09 21:29:19 UTC
I'm running into the same issue while trying to run the www/py-gunicorn rc service. It took me a while to debug this issue due to the fact that the error would only pop up once in a while.

Since the behaviour of the working directory being $HOME is not documented anywhere, I think it should be fine to remove JUST the chdir block here https://github.com/freebsd/freebsd-src/blob/5f75cd390a67cbec06993c4c66f784f0f777c854/usr.sbin/jail/command.c#L791-L795.

I agree with the suggestion of running exec.clean jail commands rooted at '/'.
Comment 11 johannes.kunde 2024-07-12 10:08:02 UTC
Has there already been any feedback from the core team on how to deal with the problem?
Comment 12 Michael Osipov freebsd_committer freebsd_triage 2024-07-12 10:10:51 UTC
jrm,

who's our expect to ask? Especially looking at comment #10.
Comment 13 Joseph Mingrone freebsd_committer freebsd_triage 2024-07-12 14:07:44 UTC
Jamie, as the author of that code, could you please have another look and share your thoughts, given the new information?
Comment 14 Jamie Gritton freebsd_committer freebsd_triage 2024-07-16 14:10:21 UTC
It will be a couple weeks until I can take a closer look, but I'm still inclined toward setting it to "/" unless the user is explicitly specified.
Comment 15 Jamie Gritton freebsd_committer freebsd_triage 2024-08-05 05:43:31 UTC
I've presented a simple fix in https://reviews.freebsd.org/D46226.  It's not complete yet, but won't be much more complex than that one-liner.  jexec(8) is also patched in the same manner.
Comment 16 commit-hook freebsd_committer freebsd_triage 2024-08-12 22:36:43 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=5cf705491727dd963485f9911ee3d52c3bf148db

commit 5cf705491727dd963485f9911ee3d52c3bf148db
Author:     Jamie Gritton <jamie@FreeBSD.org>
AuthorDate: 2024-08-12 22:23:28 +0000
Commit:     Jamie Gritton <jamie@FreeBSD.org>
CommitDate: 2024-08-12 22:23:28 +0000

    jail: only chdir to user's home directory when user is specified

    jail(8) with the "exec.clean" parameter not only cleans the enviromnent
    variables before running commands, but also changes to the user's home
    directory.  While this makes sense when auser is specified (via one of
    the exec.*_user parameters), it leads to all commands being run in the
    jail's /root directory even in the absence of an explicitly specified
    user.  This can lead to problems when e.g. rc scripts are run from that
    non-world-readable directory, and run counter to expectations that jail
    startup is analogous to system startup.

    Restrict this behvaiour to only users exlicitly specified, either via
    the command line or jail parameters, but not the implicit root user.
    While this changes long-stand practice, it's the more intuitive action.

    jexec(8) has the same problem, and the same fix.

    PR:             277210
    Reported by:    johannes.kunde at gmail
    Differential Revision:  https://reviews.freebsd.org/D46226

 usr.sbin/jail/command.c | 2 +-
 usr.sbin/jail/jail.8    | 7 ++++++-
 usr.sbin/jexec/jexec.8  | 7 ++++++-
 usr.sbin/jexec/jexec.c  | 2 +-
 4 files changed, 14 insertions(+), 4 deletions(-)
Comment 17 Michael Osipov 2024-08-14 16:19:53 UTC
Johannes, does this fix your issue?

Is this something we can easily MFC?
Comment 18 Jamie Gritton freebsd_committer freebsd_triage 2024-08-14 17:36:12 UTC
It can be MFC'd with no problem, as is probably fine for 14.2, but I don't think it should go into 13.4.  Since it does change long-standing behavior, I want it to sit around and let people react to it for a while.  13.4 is practically out the door.
Comment 19 Michael Osipov freebsd_committer freebsd_triage 2024-08-15 10:35:26 UTC
(In reply to Jamie Gritton from comment #18)

That is fine, MFC to 14 in a month?
Comment 20 Jamie Gritton freebsd_committer freebsd_triage 2024-08-15 16:39:05 UTC
I'd rather get it in 14 sooner actually, giving those who use STABLE a chance to try it out.  If it turns out to be a problem, there's ample time to reverse it before 14.2.
Comment 21 Michael Osipov freebsd_committer freebsd_triage 2024-08-16 08:28:55 UTC
(In reply to Jamie Gritton from comment #20)

Make sense, having in 14.2-RELEASE would be awesome.
Comment 22 commit-hook freebsd_committer freebsd_triage 2024-08-16 17:13:41 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=1ff3118d72b15fb4c02ee156a5073e3e40528587

commit 1ff3118d72b15fb4c02ee156a5073e3e40528587
Author:     Jamie Gritton <jamie@FreeBSD.org>
AuthorDate: 2024-08-12 22:23:28 +0000
Commit:     Jamie Gritton <jamie@FreeBSD.org>
CommitDate: 2024-08-16 17:12:24 +0000

    MFC jail: only chdir to user's home directory when user is specified

    jail(8) with the "exec.clean" parameter not only cleans the enviromnent
    variables before running commands, but also changes to the user's home
    directory.  While this makes sense when auser is specified (via one of
    the exec.*_user parameters), it leads to all commands being run in the
    jail's /root directory even in the absence of an explicitly specified
    user.  This can lead to problems when e.g. rc scripts are run from that
    non-world-readable directory, and run counter to expectations that jail
    startup is analogous to system startup.

    Restrict this behvaiour to only users exlicitly specified, either via
    the command line or jail parameters, but not the implicit root user.
    While this changes long-stand practice, it's the more intuitive action.

    jexec(8) has the same problem, and the same fix.

    PR:             277210
    Reported by:    johannes.kunde at gmail
    Differential Revision:  https://reviews.freebsd.org/D46226

    (cherry picked from commit 5cf705491727dd963485f9911ee3d52c3bf148db)

 usr.sbin/jail/command.c | 2 +-
 usr.sbin/jail/jail.8    | 7 ++++++-
 usr.sbin/jexec/jexec.8  | 7 ++++++-
 usr.sbin/jexec/jexec.c  | 2 +-
 4 files changed, 14 insertions(+), 4 deletions(-)
Comment 23 johannes.kunde 2024-08-17 07:29:05 UTC
(In reply to Michael Osipov from comment #17)

I've currently no access to my computer. I'll test it in the course of next week.
Comment 24 johannes.kunde 2024-08-23 08:56:45 UTC
Today, I've tested commit 1ff3118d72b15fb4c02ee156a5073e3e40528587.
Works as expected (first line: jail start, second line: service start):

Aug 23 08:51:03 testjail getcwd[23042]: Current working dir: /
Aug 23 08:51:15 testjail getcwd[23078]: Current working dir: /

Thank you very much Jamie and everybody involved!