[PATCH 3/3] su: FEATURE_SU_NULLOK_SECURE

[Natanael Copa]

On Thu,  5 Nov 2015 16:27:36 +0200
Kaarle Ritvanen <kaarle.ritvanen at datakunkku.fi> wrote:

> When this feature is enabled, blank passwords are not accepted by su
> unless the user is on a secure TTY defined in /etc/securetty. This
> resembles the default PAM configuration of some Linux distros which
> specify the nullok_secure option for pam_unix.so.

Denys,

Those 3 patches would be the optimal solution for my blank root password
problem.

- It allows me to create containers with blank root password so i can
  log in via console from host.
- It allows containers run services as non-root without intruders being
  able to elevate privileges with su.
- It makes the configuration for the end user very similar to
  traditional GNU linux using PAM, without depending on the extra PAM
  bloat.
- It adds the functionality in harmony how it was solved in busybox
  'login'. Consistency is good.

I'd be very happy if you could apply those patches.

bloatcheck (x86_64):

function                                             old     new   delta
check_securetty                                        -     160    +160
su_main                                              581     611     +30
ask_and_check_password_extended                      142     147      +5
ask_and_check_password                                14      19      +5
login_main                                          1431    1299    -132
------------------------------------------------------------------------------
(add/remove: 2/0 grow/shrink: 3/1 up/down: 200/-132)           Total: 68 bytes
   text	   data	    bss	    dec	    hex	filename
 127235	   3691	   2800	 133726	  20a5e	busybox_old
 127303	   3691	   2800	 133794
 20aa2	busybox_unstripped


-nc

> ---
>  loginutils/su.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/loginutils/su.c b/loginutils/su.c
> index 3c0e8c1..85d8e11 100644
> --- a/loginutils/su.c
> +++ b/loginutils/su.c
> @@ -24,6 +24,11 @@
>  //config:	bool "Enable su to check user's shell to be listed in /etc/shells"
>  //config:	depends on SU
>  //config:	default y
> +//config:
> +//config:config FEATURE_SU_NULLOK_SECURE
> +//config:      bool "Disallow blank passwords from TTYs other than specified in /etc/securetty"
> +//config:      depends on SU
> +//config:      default n
>  
>  //applet:/* Needs to be run by root or be suid root - needs to change uid and gid: */
>  //applet:IF_SU(APPLET(su, BB_DIR_BIN, BB_SUID_REQUIRE))
> @@ -76,6 +81,7 @@ int su_main(int argc UNUSED_PARAM, char **argv)
>  	struct passwd *pw;
>  	uid_t cur_uid = getuid();
>  	const char *tty;
> +	int allow_blank = 1;
>  #if ENABLE_FEATURE_UTMP
>  	char user_buf[64];
>  #endif
> @@ -96,6 +102,12 @@ int su_main(int argc UNUSED_PARAM, char **argv)
>  		argv++;
>  	}
>  
> +	tty = xmalloc_ttyname(STDIN_FILENO);
> +	if (!tty) tty = "none";
> +	tty = skip_dev_pfx(tty);
> +
> +	if (ENABLE_FEATURE_SU_NULLOK_SECURE) allow_blank = check_securetty(tty);
> +
>  	if (ENABLE_FEATURE_SU_SYSLOG) {
>  		/* The utmp entry (via getlogin) is probably the best way to
>  		 * identify the user, especially if someone su's from a su-shell.
> @@ -109,16 +121,12 @@ int su_main(int argc UNUSED_PARAM, char **argv)
>  			pw = getpwuid(cur_uid);
>  			old_user = pw ? xstrdup(pw->pw_name) : "";
>  		}
> -		tty = xmalloc_ttyname(2);
> -		if (!tty) {
> -			tty = "none";
> -		}
>  		openlog(applet_name, 0, LOG_AUTH);
>  	}
>  
>  	pw = xgetpwnam(opt_username);
>  
> -	if (cur_uid == 0 || ask_and_check_password(pw) > 0) {
> +	if (cur_uid == 0 || ask_and_check_password_extended(pw, 0, allow_blank, "Password: ") > 0) {
>  		if (ENABLE_FEATURE_SU_SYSLOG)
>  			syslog(LOG_NOTICE, "%c %s %s:%s",
>  				'+', tty, old_user, opt_username);