GNU bug report logs - #31112
Patching the default PATH of `su`

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31112 in the body.
You can then email your comments to 31112 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: guix-patches <at> gnu.org
Subject: Re: Patching the default PATH of `su`
Date: Mon, 9 Apr 2018 12:17:12 -0400

[Message part 1 (text/plain, inline)]

On Fri, Apr 06, 2018 at 10:01:57AM +0200, Ludovic Courtès wrote:
> Probably, yes.  It would be good to check how this affects
> mingetty/login, sshd, etc.

I've attached a patch that sets the variables in /etc/login.defs.

The resulting PATH only includes "system" packages, which are found in
/run/current-system and /run/setuid-programs.

It would be better if it also included the user-specific programs in
~/.guix-profile, but I don't think this is possible without patching
`su` to look up usernames.

Shell variables are not expanded by `su`, so using $HOME doesn't work.
And I'm not sure how to use /var/guix/profiles/per-user without making
`su` look up usernames.

Nevertheless, I think it's an improvement, although maybe it's less
confusing for the PATH to be totally wrong than merely missing the
user's packages. WDYT?

With the attached patch, I tested mingetty and agetty's login, as well
as OpenSSH sshd login, and everything seemed to work — this changes
should have no effect in those cases because it affects non-login shells
only.

> Note that libc also has its own default PATH value in <paths.h>:
> 
>   /* Default search path. */
>   #define	_PATH_DEFPATH	"/usr/bin:/bin"
>   /* All standard utilities path. */
>   #define	_PATH_STDPATH \
>           "/usr/bin:/bin:/usr/sbin:/sbin"
> 
> Does ‘su’ rely on this?  In a future rebuild cycle we could change these
> values, but /run/current-system/bin wouldn’t work on foreign distros, so
> it’s not clear there’s much to gain.

AFAICT, `su` doesn't use this. The relevant code is below; it hard-codes
the fall-back PATHs rather than refer to libc.

------
	/*
	 * Create the PATH environmental variable and export it.
	 */

	cp = getdef_str ((info->pw_uid == 0) ? "ENV_SUPATH" : "ENV_PATH");

	if (NULL == cp) {
		/* not specified, use a minimal default */
		addenv ((info->pw_uid == 0) ? "PATH=/sbin:/bin:/usr/sbin:/usr/bin" : "PATH=/bin:/usr/bin", NULL);
	} else if (strchr (cp, '=')) {
		/* specified as name=value (PATH=...) */
		addenv (cp, NULL);
	} else {
		/* only value specified without "PATH=" */
		addenv ("PATH", cp);
}
------

https://github.com/shadow-maint/shadow/blob/15be89f89d553c06d52453721ee8e9a8433cfdfd/libmisc/setupenv.c#L263

[0001-system-Provide-a-fall-back-PATH-for-non-login-shells.patch (text/plain, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Leo Famulari <leo <at> famulari.name>
Cc: guix-patches <at> gnu.org
Subject: Re: Patching the default PATH of `su`
Date: Mon, 09 Apr 2018 22:47:30 +0200

Hey Leo,

Leo Famulari <leo <at> famulari.name> skribis:

> I've attached a patch that sets the variables in /etc/login.defs.
>
> The resulting PATH only includes "system" packages, which are found in
> /run/current-system and /run/setuid-programs.
>
> It would be better if it also included the user-specific programs in
> ~/.guix-profile, but I don't think this is possible without patching
> `su` to look up usernames.
>
> Shell variables are not expanded by `su`, so using $HOME doesn't work.
> And I'm not sure how to use /var/guix/profiles/per-user without making
> `su` look up usernames.
>
> Nevertheless, I think it's an improvement, although maybe it's less
> confusing for the PATH to be totally wrong than merely missing the
> user's packages. WDYT?

I think the patch is an improvement.

I don’t see how we could add the user ~/.guix-profile as well,
especially since there’s no central place that parses ‘login.defs’
(Shadow has its own parser in lib/getdef.c).

BTW, for a similar reason, OpenSSH’s ‘scp’ doesn’t work when the remote
machine runs GuixSD, even if OpenSSH is in the user’s profile.

> With the attached patch, I tested mingetty and agetty's login, as well
> as OpenSSH sshd login, and everything seemed to work — this changes
> should have no effect in those cases because it affects non-login shells
> only.

Great.

> AFAICT, `su` doesn't use this. The relevant code is below; it hard-codes
> the fall-back PATHs rather than refer to libc.

OK.

> From dc5098edd98c85ad45d2e22ca1824a9445dcc36d Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo <at> famulari.name>
> Date: Sun, 8 Apr 2018 14:30:05 -0400
> Subject: [PATCH] system: Provide a fall-back PATH for non-login shells started
>  with su(1).
>
> * gnu/system.scm (operating-system-etc-service): Provide values for
> ENV_PATH and ENV_SUPATH in '/etc/login.defs'.

So LGTM, thank you!

Ludo’.

Message #13 received at 31112-done <at> debbugs.gnu.org (full text, mbox):

From: Leo Famulari <leo <at> famulari.name>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 31112-done <at> debbugs.gnu.org
Subject: Re: Patching the default PATH of `su`
Date: Fri, 13 Apr 2018 11:52:00 -0400

[Message part 1 (text/plain, inline)]

On Mon, Apr 09, 2018 at 10:47:30PM +0200, Ludovic Courtès wrote:
> Leo Famulari <leo <at> famulari.name> skribis:
> > From dc5098edd98c85ad45d2e22ca1824a9445dcc36d Mon Sep 17 00:00:00 2001
> > From: Leo Famulari <leo <at> famulari.name>
> > Date: Sun, 8 Apr 2018 14:30:05 -0400
> > Subject: [PATCH] system: Provide a fall-back PATH for non-login shells started
> >  with su(1).
> >
> > * gnu/system.scm (operating-system-etc-service): Provide values for
> > ENV_PATH and ENV_SUPATH in '/etc/login.defs'.
> 
> So LGTM, thank you!

Alright, pushed as e453da132a3482540d2166b23554ef693b2c0c0d

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.