[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Moving bash from essential/required to important?



Package: login
Version: 1:4.1.4.2+svn3283-3
Severity: wishlist
Tags: patch

Hi!

On Mon, 2011-04-04 at 10:16:35 -0700, Steve Langasek wrote:
> On Mon, Apr 04, 2011 at 06:04:20PM +0200, Luk Claes wrote:
> > What do others think of moving bash to important (required and important
> > are part of the base system)?

I also think this would be great!

> Consider that 'base-passwd' and 'login' are also part of the essential set.
> Why?  Because being able to log in as root is part of the "minimal set of
> functionality that must be available and usable on the system at all times".
> 
> So if we drop bash from essential, how do we guarantee that root can log in? 
> Do we set root's default shell to /bin/sh instead?  I don't think anyone
> would be happy with that except those people who already change it to zsh
> anyway.  :-)

Well, we can always fix login to behave more robustly, no? :)

> If login worked consistently in the face of the configured shell going
> missing (automatically falling back to /bin/sh for root), then I think it
> would be worthwhile to do the work necessary to remove bash from the
> essential set.  But until then, the primary purpose of Essential, to me, is
> the "minimal set guaranteed to be usable" aspect, not the "you don't have to
> depend on it" aspect.

That's more or less what the attached patch does. It could certainly be
improved, as the knowledge of when to fallback is spread all over the
place, but that's an existing problem in the code anyway.

The SHELL variable in configure.in is changed to an explicit "/bin/sh"
because relying on $SHELL might change depending on the shell used for
configure, and the existing code expects /bin/sh for fallback and script
invokation cases, this could be considered a bug on its own though. The
only fishy point is when calling shell() with a second argument, which
will get preserved, and might not quite match what was invoked
afterwards, but probably not worth worrying.

The code could also warn that it needed to fallback to a POSIX shell,
but I'm not sure what's the policy from the shadow code PoV here.

Tested with:

  # chsh root -s /bin/csh
  chsh: Warning: /bin/csh does not exist
  # su
  # echo $SHELL
  /bin/sh
  # exit
  # su -
  # echo $SHELL
  /bin/sh
  # exit
  # login -f root
  Last login: Tue Apr  5 01:36:13 CEST 2011 on pts/10
  # echo $SHELL
  /bin/sh

And on a virtual console.

regards,
guillem
diff --git a/configure.in b/configure.in
index 4021479..585bdb1 100644
--- a/configure.in
+++ b/configure.in
@@ -574,7 +574,7 @@ if test "$enable_utmpx" = "yes"; then
 	          [Define if utmpx should be used])
 fi
 
-AC_DEFINE_UNQUOTED(SHELL, ["$SHELL"], [The default shell.])
+AC_DEFINE_UNQUOTED(SHELL, ["/bin/sh"], [The default shell.])
 
 AM_GNU_GETTEXT_VERSION(0.16)
 AM_GNU_GETTEXT([external], [need-ngettext])
diff --git a/libmisc/setupenv.c b/libmisc/setupenv.c
index 666b1c7..601430f 100644
--- a/libmisc/setupenv.c
+++ b/libmisc/setupenv.c
@@ -241,7 +241,8 @@ void setup_env (struct passwd *info)
 	 * Create the SHELL environmental variable and export it.
 	 */
 
-	if ((NULL == info->pw_shell) || ('\0' == *info->pw_shell)) {
+	if ((NULL == info->pw_shell) || ('\0' == *info->pw_shell) ||
+	    access (info->pw_shell, R_OK|X_OK) != 0) {
 		static char temp_pw_shell[] = SHELL;
 
 		info->pw_shell = temp_pw_shell;
diff --git a/libmisc/shell.c b/libmisc/shell.c
index d815f2d..5634580 100644
--- a/libmisc/shell.c
+++ b/libmisc/shell.c
@@ -53,6 +53,7 @@ extern size_t newenvc;
 
 int shell (const char *file, /*@null@*/const char *arg, char *const envp[])
 {
+	const char *fallback_arg;
 	char arg0[1024];
 	int err;
 
@@ -71,6 +72,10 @@ int shell (const char *file, /*@null@*/const char *arg, char *const envp[])
 		(void) snprintf (arg0, sizeof arg0, "-%s", Basename ((char *) file));
 		arg0[sizeof arg0 - 1] = '\0';
 		arg = arg0;
+
+		fallback_arg = "-sh";
+	} else {
+		fallback_arg = arg;
 	}
 
 	/*
@@ -81,7 +86,14 @@ int shell (const char *file, /*@null@*/const char *arg, char *const envp[])
 	(void) execle (file, arg, (char *) 0, envp);
 	err = errno;
 
-	if (access (file, R_OK|X_OK) == 0) {
+	if (err == ENOENT) {
+		/*
+		 * The requested shell does not seem to be present,
+		 * fallback to a POSIX shell.
+		 */
+		(void) execle (SHELL, fallback_arg, (char *)0, envp);
+		err = errno;
+	} else if (access (file, R_OK|X_OK) == 0) {
 		/*
 		 * Assume this is a shell script (with no shebang).
 		 * Interpret it with /bin/sh
diff --git a/src/su.c b/src/su.c
index f3ff666..12bd03b 100644
--- a/src/su.c
+++ b/src/su.c
@@ -759,7 +759,8 @@ int main (int argc, char **argv)
 	/*
 	 * Set the default shell.
 	 */
-	if ((NULL == shellstr) || ('\0' == shellstr[0])) {
+	if ((NULL == shellstr) || ('\0' == shellstr[0]) ||
+	    access (pwent.pw_shell, R_OK|X_OK) != 0) {
 		shellstr = SHELL;
 	}
 

Reply to: