[PATCH] correct_passwd and sulogin size reduction

Bernhard Fischer rep.nop at aon.at
Wed Feb 1 08:32:40 UTC 2006


On Sat, Jan 21, 2006 at 10:21:21PM +0100, Tito wrote:
>Hi,
>this patch set contains a new GPL'ed  version of correct_password.c
>with reduced size and more flexibility so that it can be used in more apps.

I'd prefer to wait for Rob to tag 1.1.1 and only afterwards check this
in, unless someone else ack's this for 1.1.1.

A few mere cosmetic comments below.

>correct_passwd.patch:
>
>Main differences vs. previous version are:
>a) interface change from:
>     extern int correct_password ( const struct passwd *pw );
>     extern int correct_password (struct passwd *pw, int timeout, int skip);
>
>b) return value change:
>    Return 0 if the user gives the correct password or the password is empty.
>    Return 1 on authentication failure.
>    Return 2 if bb_askpass returns NULL   (needed for sulogin).
>c) permit to skip empty password check (needed for sulogin,  and vlock (just cosmetical))
>d) do not exit on error and do not display error messages if getspnam
>    fails, but let the calling app do what is needed on authentication error.
>e) size reduction:
>   text    data     bss     dec     hex filename
>    199       0       0     199      c7 busybox/libbb/correct_password.o.orig
>    160       0       0     160      a0 libbb/correct_password.o
> 
>compatibility.patch:
>
>Changes login, su and vlock to the new correct_password interface
> (plus some minor size saving changes in su).
>   text    data     bss     dec     hex filename
>    623       0       0     623     26f loginutils/su.o.orig
>    594       0       0     594     252 loginutils/su.o
>
>sulogin.patch:
>
>Changes sulogin to use correct_password (with major size savings):
>   text    data     bss     dec     hex filename
>   1209       0       0    1209     4b9 busybox/loginutils/sulogin.o.orig
>     975       0       0       975     3cf busybox/loginutils/sulogin.o
>
>Overall size reduction with same .config:
>
>   text    data     bss     dec     hex filename
>  18833     636    8804   28273    6e71 busybox.orig
>  18689     628     584   19901    4dbd busybox
>
>Patches are against svn revision: 13480.
>
>Please apply if you think it is ok to do so.
>
>Ciao,
>Tito
>
>

>--- busybox.orig/loginutils/login.c	2006-01-06 22:12:11.000000000 +0100
>+++ busybox/loginutils/login.c	2006-01-21 21:28:40.000000000 +0100
>@@ -189,7 +189,7 @@
> 			goto auth_ok;
> 
> 		/* authorization takes place here */
>-		if ( correct_password ( pw ))
>+		if ( correct_password ( pw, 0, 1) == 0)
> 			goto auth_ok;
> 
> 		failed = 1;
>--- busybox.orig/loginutils/su.c	2006-01-20 08:26:42.000000000 +0100
>+++ busybox/loginutils/su.c	2006-01-21 21:27:44.000000000 +0100
>@@ -117,9 +117,8 @@
> 	openlog ( bb_applet_name, 0, LOG_AUTH );
> #endif
> 
>-	pw = getpwnam ( opt_username );
>-	if ( !pw )
>-		bb_error_msg_and_die ( "user %s does not exist", opt_username );
>+	/* Exit if user is not in /etc/passwd */
>+	pw = getpwuid(bb_xgetpwnam(opt_username));
> 
> 	/* Make sure pw->pw_shell is non-NULL.  It may be NULL when NEW_USER
> 	   is a username that is retrieved via NIS (YP), but that doesn't have
>@@ -127,7 +126,7 @@
> 	if ( !pw->pw_shell || !pw->pw_shell [0] )
> 		pw->pw_shell = (char *) DEFAULT_SHELL;
> 
>-	if ((( cur_uid == 0 ) || correct_password ( pw ))) {
>+	if ((( cur_uid == 0 ) || correct_password ( pw, 0, 1) == 0)) {
> 		log_su_successful(pw->pw_uid, old_user, tty );
> 	} else {
> 		log_su_failure (pw->pw_uid, old_user, tty );
>--- busybox.orig/loginutils/vlock.c	2006-01-06 22:12:11.000000000 +0100
>+++ busybox/loginutils/vlock.c	2006-01-21 21:43:56.000000000 +0100
>@@ -132,7 +132,7 @@
> 	do {
> 		printf("Virtual Console%s locked.\n%s's ", (o_lock_all) ? "s" : "", pw->pw_name);
> 		fflush(stdout);
>-		if (correct_password (pw)) {
>+		if (correct_password (pw, 0, 0) == 0) {
> 			break;
> 		}
> 		bb_do_delay(FAIL_DELAY);

>--- busybox.orig/include/libbb.h	2006-01-15 15:17:02.000000000 +0100
>+++ busybox/include/libbb.h	2006-01-21 21:20:21.000000000 +0100
>@@ -430,7 +430,7 @@
> extern int run_parts(char **args, const unsigned char test_mode, char **env);
> extern int restricted_shell ( const char *shell );
> extern void setup_environment ( const char *shell, int loginshell, int changeenv, const struct passwd *pw );
>-extern int correct_password ( const struct passwd *pw );
>+extern int correct_password (struct passwd *pw, int timeout, int skip);
> extern char *pw_encrypt(const char *clear, const char *salt);
> extern struct spwd *pwd_to_spwd(const struct passwd *pw);
> extern int obscure(const char *old, const char *newval, const struct passwd *pwdp);
>--- busybox.orig/libbb/correct_password.c	2006-01-09 23:40:00.000000000 +0100
>+++ busybox/libbb/correct_password.c	2006-01-21 21:30:21.000000000 +0100
>@@ -1,77 +1,59 @@
> /* vi: set sw=4 ts=4: */
> /*
>- * Copyright 1989 - 1991, Julianne Frances Haugh <jockgrrl at austin.rr.com>
>- * All rights reserved.
>+ * Utility function for busybox
>  *
>- * Redistribution and use in source and binary forms, with or without
>- * modification, are permitted provided that the following conditions
>- * are met:
>- * 1. Redistributions of source code must retain the above copyright
>- *    notice, this list of conditions and the following disclaimer.
>- * 2. Redistributions in binary form must reproduce the above copyright
>- *    notice, this list of conditions and the following disclaimer in the
>- *    documentation and/or other materials provided with the distribution.
>- * 3. Neither the name of Julianne F. Haugh nor the names of its contributors
>- *    may be used to endorse or promote products derived from this software
>- *    without specific prior written permission.
>+ * Copyright (C) 2006 Tito Ragusa <farmatito at tiscali.it>
>  *
>- * THIS SOFTWARE IS PROVIDED BY JULIE HAUGH AND CONTRIBUTORS ``AS IS'' AND
>- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>- * ARE DISCLAIMED.  IN NO EVENT SHALL JULIE HAUGH OR CONTRIBUTORS BE LIABLE
>- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
>- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
>- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
>- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>- * SUCH DAMAGE.
>+ * Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
>  */
> 
>-#include <stdio.h>
>-#include <errno.h>
>-#include <unistd.h>
> #include <string.h>
>-#include <stdlib.h>
>-#include <syslog.h>
>-#include <ctype.h>
>-#include <crypt.h>
> 
> #include "libbb.h"
> 
> 
>+/*  Ask the user for a password.
>+ *  If skip == 0 do not check for empty passwords.
>+ *  Return 0 if the user gives the correct password or the password is empty.
>+ *  Return 1 on authentication failure.
>+ *  Return 2 if bb_askpass returns NULL;
>+ */
> 
>-/* Ask the user for a password.
>-   Return 1 if the user gives the correct password for entry PW,
>-   0 if not.  Return 1 without asking for a password if run by UID 0
>-   or if PW has an empty password.  */
>-
>-int correct_password ( const struct passwd *pw )
>+int correct_password (struct passwd *pw, int timeout, int skip)
> {
>-	char *unencrypted, *encrypted, *correct;
>-
>+	char *clear;
>+	const char *crypt_pw;
-> #ifdef CONFIG_FEATURE_SHADOWPASSWDS

#if ENABLE_FEATURE_SHADOWPASSWDS

>-	if (( strcmp ( pw-> pw_passwd, "x" ) == 0 ) || ( strcmp ( pw-> pw_passwd, "*" ) == 0 )) {
>-		struct spwd *sp = getspnam ( pw-> pw_name );
>-
>-		if ( !sp )
>-			bb_error_msg_and_die ( "\nno valid shadow password" );
>-
>-		correct = sp-> sp_pwdp;
>+	struct spwd *sp;
>+	
>+	/* Use /etc/shadow if needed */
>+	if (pw-> pw_passwd[0] == 'x' || pw-> pw_passwd[0] == '*') {

No space after "->"

>+		/* Do not exit on error and do not display error messages,
>+		 * the calling app will do what is needed on authentication error.
>+		 */
>+		if((sp = getspnam(pw->pw_name))) {

missing space; if space bracket

>+			/* use shadow password */
>+			pw->pw_passwd = sp->sp_pwdp;
>+		}
> 	}
>-	else
> #endif
>-    	correct = pw-> pw_passwd;
>-
>-	if ( correct == 0 || correct[0] == '\0' )
>+	/* No password or empty password */
>+	if (skip && (!pw->pw_passwd || pw->pw_passwd[0] == '\0')) {
>+		return 0;
>+	}
>+	if ((!(clear = bb_askpass(timeout, "Password:")))) {
>+		/* This is for sulogin */
>+		return 2;
>+	}
>+	/* pw_encrypt allows the use of SHA1_PASSWORDS */
>+	if (!(crypt_pw = pw_encrypt(clear, pw->pw_passwd))) {
> 		return 1;
>-
>-	unencrypted = bb_askpass ( 0, "Password: " );
>-	if ( !unencrypted )
>-	{
>+	}
>+	/* Clean up */
>+	memset(clear, 0, strlen(clear));
>+	/* Do authentication */
>+	if(strcmp(crypt_pw, pw->pw_passwd) == 0) {
> 		return 0;
> 	}
>-	encrypted = crypt ( unencrypted, correct );
>-	memset ( unencrypted, 0, bb_strlen ( unencrypted ));
>-	return ( strcmp ( encrypted, correct ) == 0 ) ? 1 : 0;
>+	return 1;
> }

>--- busybox.orig/loginutils/sulogin.c	2006-01-06 22:12:11.000000000 +0100
>+++ busybox/loginutils/sulogin.c	2006-01-21 21:24:42.000000000 +0100
>@@ -17,9 +17,8 @@
> #include "busybox.h"
> 
> 
>-// sulogin defines
> #define SULOGIN_PROMPT "\nGive root password for system maintenance\n" \
>-	"(or type Control-D for normal startup):"
>+	"(or type Control-D for normal startup).\n"
> 
> static const char * const forbid[] = {
> 	"ENV",
>@@ -46,52 +45,33 @@
> 	exit(EXIT_FAILURE);
> }
> 
>-
> extern int sulogin_main(int argc, char **argv)
> {
>-	char *cp;
>-	char *device = (char *) 0;
>-	const char *name = "root";
>+	char *timeout_arg;
> 	int timeout = 0;
>-	
>-#define pass bb_common_bufsiz1
>-	
>-	struct passwd pwent;
> 	struct passwd *pwd;
> 	const char * const *p;
>-#ifdef CONFIG_FEATURE_SHADOWPASSWDS
>-	struct spwd *spwd = NULL;
>-#endif							/* CONFIG_FEATURE_SHADOWPASSWDS */
>-
>-	openlog("sulogin", LOG_PID | LOG_CONS | LOG_NOWAIT, LOG_AUTH);
>-	if (argc > 1) {
>-		if (strncmp(argv[1], "-t", 2) == 0) {
>-			if (strcmp(argv[1], "-t") == 0) {
>-				if (argc > 2) {
>-					timeout = atoi(argv[2]);
>-					if (argc > 3) {
>-						device = argv[3];
>-					}
>-				}
>-			} else {
>-				if (argc > 2) {
>-					device = argv[2];
>-				}
>-			}
>-		} else {
>-			device = argv[1];
>+
>+	openlog(bb_applet_name, LOG_PID | LOG_CONS | LOG_NOWAIT, LOG_AUTH);
>+	if (bb_getopt_ulflags (argc, argv, "t:", &timeout_arg)) {
>+		if(safe_strtoi(timeout_arg, &timeout)) {
>+			timeout = 0;
> 		}
>-		if (device) {
>-			close(0);
>-			close(1);
>-			close(2);
>-			if (open(device, O_RDWR) >= 0) {
>-				dup(0);
>-				dup(0);
>-			} else {
>-				syslog(LOG_WARNING, "cannot open %s\n", device);
>-				exit(EXIT_FAILURE);
>-			}
>+	}
>+
>+	argc -= optind;
>+	argv += optind;
>+	
>+	if (argc) {
>+		close(0);
>+		close(1);
>+		close(2);
>+		if (open(argv[0], O_RDWR) >= 0) {
>+			dup(0);
>+			dup(0);
>+		} else {
>+			syslog(LOG_WARNING, "cannot open %s\n", argv[0]);
>+			exit(EXIT_FAILURE);
> 		}
> 	}
> 	if (access(bb_path_passwd_file, 0) == -1) {

Personally i prefer to test against !(access()) in these situations,
fwiw.

>@@ -109,44 +89,32 @@
> 
> 
> 	signal(SIGALRM, catchalarm);
>-	if (!(pwd = getpwnam(name))) {
>+	if (!(pwd = getpwuid(0))) {
> 		syslog(LOG_WARNING, "No password entry for `root'\n");
> 		bb_error_msg_and_die("No password entry for `root'\n");
> 	}
>-	pwent = *pwd;
>-#ifdef CONFIG_FEATURE_SHADOWPASSWDS
>-	spwd = NULL;
>-	if (pwd && ((strcmp(pwd->pw_passwd, "x") == 0)
>-				|| (strcmp(pwd->pw_passwd, "*") == 0))) {
>-		endspent();
>-		spwd = getspnam(name);
>-		if (spwd) {
>-			pwent.pw_passwd = spwd->sp_pwdp;
>-		}
>-	}
>-#endif							/* CONFIG_FEATURE_SHADOWPASSWDS */
>+
> 	while (1) {
>-		cp = bb_askpass(timeout, SULOGIN_PROMPT);
>-		if (!cp || !*cp) {
>-			puts("\n");
>-			fflush(stdout);
>-			syslog(LOG_INFO, "Normal startup\n");
>-			exit(EXIT_SUCCESS);
>-		} else {
>-			safe_strncpy(pass, cp, sizeof(pass));
>-			bzero(cp, strlen(cp));
>-		}
>-		if (strcmp(pw_encrypt(pass, pwent.pw_passwd), pwent.pw_passwd) == 0) {
>-			break;
>-		}
>-		bb_do_delay(FAIL_DELAY);
>-		puts("Login incorrect");
>+		printf("%s", SULOGIN_PROMPT);
> 		fflush(stdout);

perhaps bb_xfflush_stdout()

>-		syslog(LOG_WARNING, "Incorrect root password\n");
>+		switch(correct_password(pwd, timeout, 0)) {
>+			case 2:
>+				puts("\n");
>+				syslog(LOG_INFO, "Normal startup\n");
>+				bb_fflush_stdout_and_exit(EXIT_SUCCESS);
>+			case 1:
>+				bb_do_delay(FAIL_DELAY);
>+				puts("Login incorrect");
>+				fflush(stdout);
>+				syslog(LOG_WARNING, "Incorrect root password\n");
>+				break;
>+			case 0:
>+				goto MAINTENANCE_MODE;
>+		}

is the switch smaller than a temporary var and if else here?
(perhaps reuse "timeout" ...)

> 	}
>-	bzero(pass, strlen(pass));
>+MAINTENANCE_MODE:
> 	signal(SIGALRM, SIG_DFL);

perhaps mv syslog() before the output to the console.

>-	puts("Entering System Maintenance Mode\n");
>+	puts("System Maintenance Mode\n");
> 	fflush(stdout);
> 	syslog(LOG_INFO, "System Maintenance Mode\n");
> 
>@@ -154,7 +122,7 @@
> 	renew_current_security_context();
> #endif
> 
>-	run_shell(pwent.pw_shell, 1, 0, 0);
>+	run_shell(pwd->pw_shell, 1, 0, 0);
> 
> 	return (0);
> }



More information about the busybox mailing list