[RFC, PATCH] passwd size reduction

Bernhard Fischer rep.nop at aon.at
Mon Nov 27 10:00:19 PST 2006


Hi,

I'm attaching a (minor, non-intrusive) shrinkage of passwd. Note that
it was only compile-tested, so don't check this in as-is!

Tito, vda, i'd be glad if you'd give it a whirl, if you find the time.
You need to apply all patches in ascending order.

thanks!

passwd.o.orig:	current svn
passwd.o.01b:	remove get_algo. the helptext needs updating to remove
sha1 since this was not supported anyway, AFAICS.
passwd.o.01c:	move some _main state vars into 'opt'; includes
non-existing 01b-patch, sorry.
passwd.o.01d:	move some more _main uid-related state vars into 'opt'
passwd.o.01e:	sanitize error pathes
passwd.o.01f:	sanitize some more error pathes and reuse more of libbb

$ size passwd.o*
   text    data     bss     dec     hex filename
   2984       0     135    3119     c2f passwd.o.orig
   2984       0     135    3119     c2f passwd.o.01b
   2895       0     135    3030     bd6 passwd.o.01c
   2887       0     135    3022     bce passwd.o.01d
   2824       0     135    2959     b8f passwd.o.01e
   2810       0     135    2945     b81 passwd.o.01f

-------------- next part --------------
$ size passwd.o*
   text    data     bss     dec     hex filename
   2984       0     135    3119     c2f passwd.o.orig
   2984       0     135    3119     c2f passwd.o.01b
   2895       0     135    3030     bd6 passwd.o.01c
   2887       0     135    3022     bce passwd.o.01d
   2824       0     135    2959     b8f passwd.o.01e
   2810       0     135    2945     b81 passwd.o.01f

-------------- next part --------------
--- loginutils/passwd.c.orig	2006-10-04 09:29:10.000000000 +0200
+++ loginutils/passwd.c	2006-11-27 17:42:52.000000000 +0100
@@ -12,16 +12,16 @@
 static int new_password(const struct passwd *pw, int amroot, int algo);
 static void set_filesize_limit(int blocks);
 
-
+#if 0
 static int get_algo(char *a)
 {
-	int x = 1;					/* standard: MD5 */
-
+	/* standard: MD5 */
+	int x = 1;
 	if (strcasecmp(a, "des") == 0)
 		x = 0;
 	return x;
 }
-
+#endif
 
 static int update_passwd(const struct passwd *pw, const char *crypt_pw)
 {
@@ -133,6 +133,9 @@
 		OPT_unlock = 0x4, /* -u - unlock account */
 		OPT_delete = 0x8, /* -d - delete password */
 		OPT_lud = 0xe,
+		STATE_ALGO_md5 = 0x10,
+		STATE_ALGO_des = 0x20,
+		STATE_AMROOT   = 0x40,
 	};
 	unsigned opt;
 	char *opt_a;
@@ -141,7 +144,6 @@
 	char *np;
 	char *name;
 	char *myname;
-	int algo = 1;
 	const struct passwd *pw;
 
 	amroot = (getuid() == 0);
@@ -149,7 +151,11 @@
 	opt = getopt32(argc, argv, "a:lud", &opt_a);
 	argc -= optind;
 	argv += optind;
-	if (opt & OPT_algo) algo = get_algo(opt_a); // -a
+	
+	if ((opt & OPT_algo) && (strcasecmp(opt_a, "des") == 0)) /* -a */
+		opt |= STATE_ALGO_des;
+	else
+		opt |= STATE_ALGO_md5;
 	if ((opt & OPT_lud) && (!argc || !amroot))
 		bb_show_usage();
 
@@ -181,7 +187,7 @@
 			}
 		}
 		printf("Changing password for %s\n", name);
-		if (new_password(pw, amroot, algo)) {
+		if (new_password(pw, amroot, opt & STATE_ALGO_md5)) {
 			bb_error_msg_and_die("the password for %s is unchanged", name);
 		}
 	} else if (opt & OPT_lock) {
@@ -352,7 +358,7 @@
 	memset(orig, 0, sizeof(orig));
 	memset(salt, 0, sizeof(salt));
 
-	if (algo == 1) {
+	if (algo == 1) { /* MD5 */
 		strcpy(salt, "$1$");
 		strcat(salt, crypt_make_salt());
 		strcat(salt, crypt_make_salt());
-------------- next part --------------
--- loginutils/passwd.c.01c	2006-11-27 17:42:52.000000000 +0100
+++ loginutils/passwd.c 2006-11-27 17:46:41.000000000 +0100
@@ -135,28 +135,30 @@
 		OPT_lud = 0xe,
 		STATE_ALGO_md5 = 0x10,
 		STATE_ALGO_des = 0x20,
-		STATE_AMROOT   = 0x40,
+		STATE_amroot   = 0x40,
 	};
 	unsigned opt;
 	char *opt_a;
-	int amroot;
+	uid_t myuid;
 	char *cp;
 	char *np;
 	char *name;
 	char *myname;
 	const struct passwd *pw;
 
-	amroot = (getuid() == 0);
 	openlog("passwd", LOG_PID | LOG_CONS | LOG_NOWAIT, LOG_AUTH);
 	opt = getopt32(argc, argv, "a:lud", &opt_a);
 	argc -= optind;
 	argv += optind;
-	
+
 	if ((opt & OPT_algo) && (strcasecmp(opt_a, "des") == 0)) /* -a */
 		opt |= STATE_ALGO_des;
 	else
 		opt |= STATE_ALGO_md5;
-	if ((opt & OPT_lud) && (!argc || !amroot))
+	myuid = getuid();
+	if (!myuid)
+		opt |= STATE_amroot;
+	if ((opt & OPT_lud) && (!argc || !(opt & STATE_amroot)))
 		bb_show_usage();
 
 	myname = xstrdup(bb_getpwuid(NULL, getuid(), -1));
@@ -167,7 +169,7 @@
 	if (!pw) {
 		bb_error_msg_and_die("unknown user %s", name);
 	}
-	if (!amroot && pw->pw_uid != getuid()) {
+	if (!(opt & STATE_amroot) && pw->pw_uid != getuid()) {
 		syslog(LOG_WARNING, "can't change pwd for '%s'", name);
 		bb_error_msg_and_die("permission denied");
 	}
@@ -180,14 +182,14 @@
 	np = name;
 	safe_strncpy(crypt_passwd, cp, sizeof(crypt_passwd));
 	if (!(opt & OPT_lud)) {
-		if (!amroot) {
+		if (!(opt & STATE_amroot)) {
 			if (cp[0] == '!') {
 				syslog(LOG_WARNING, "password locked for '%s'", np);
 				bb_error_msg_and_die("the password for %s cannot be changed", np);
 			}
 		}
 		printf("Changing password for %s\n", name);
-		if (new_password(pw, amroot, opt & STATE_ALGO_md5)) {
+		if (new_password(pw, opt & STATE_amroot, opt & STATE_ALGO_md5)) {
 			bb_error_msg_and_die("the password for %s is unchanged", name);
 		}
 	} else if (opt & OPT_lock) {
-------------- next part --------------
--- loginutils/passwd.c.01d	2006-11-27 17:46:41.000000000 +0100
+++ loginutils/passwd.c	2006-11-27 18:00:52.000000000 +0100
@@ -65,9 +65,7 @@
 
 	snprintf(buf, sizeof buf, "%s-", filename);
 	if (create_backup(buf, fp)) {
-		fcntl(fileno(fp), F_SETLK, &lock);
-		fclose(fp);
-		return 1;
+		goto cleanup_fp;
 	}
 	snprintf(buf, sizeof buf, "%s+", filename);
 	mask = umask(0777);
@@ -75,10 +73,8 @@
 	umask(mask);
 	if ((!out_fp) || (fchmod(fileno(out_fp), sb.st_mode & 0777))
 		|| (fchown(fileno(out_fp), sb.st_uid, sb.st_gid))) {
-		fcntl(fileno(fp), F_SETLK, &lock);
-		fclose(fp);
 		fclose(out_fp);
-		return 1;
+		goto cleanup_fp;
 	}
 
 	continued = 0;
@@ -109,19 +105,19 @@
 
 	if (fflush(out_fp) || fsync(fileno(out_fp)) || fclose(out_fp)) {
 		unlink(buf);
-		fcntl(fileno(fp), F_SETLK, &lock);
-		fclose(fp);
-		return 1;
+		goto cleanup_fp;
 	}
 	if (rename(buf, filename) < 0) {
-		fcntl(fileno(fp), F_SETLK, &lock);
-		fclose(fp);
-		return 1;
+		goto cleanup_fp;
 	} else {
 		fcntl(fileno(fp), F_SETLK, &lock);
 		fclose(fp);
 		return 0;
 	}
+cleanup_fp:
+	fcntl(fileno(fp), F_SETLK, &lock);
+	fclose(fp);
+	return 1;
 }
 
 
-------------- next part --------------
--- loginutils/passwd.c.01e	2006-11-27 18:00:52.000000000 +0100
+++ loginutils/passwd.c	2006-11-27 18:44:02.000000000 +0100
@@ -25,13 +25,12 @@
 
 static int update_passwd(const struct passwd *pw, const char *crypt_pw)
 {
-	char filename[1024];
-	char buf[1025];
+	char *filename;
+	char *buf;
 	char buffer[80];
 	char username[32];
 	char *pw_rest;
-	int mask;
-	int continued;
+	int continued = 0;
 	FILE *fp;
 	FILE *out_fp;
 	struct stat sb;
@@ -39,17 +38,17 @@
 
 #if ENABLE_FEATURE_SHADOWPASSWDS
 	if (access(bb_path_shadow_file, F_OK) == 0) {
-		snprintf(filename, sizeof filename, "%s", bb_path_shadow_file);
+		filename = concat_path_file(NULL, bb_path_shadow_file);
 	} else
 #endif
 	{
-		snprintf(filename, sizeof filename, "%s", bb_path_passwd_file);
+		filename = concat_path_file(NULL, bb_path_passwd_file);
 	}
 
 	fp = fopen(filename, "r+");
 	if (fp == 0 || fstat(fileno(fp), &sb)) {
 		/* return 0; */
-		return 1;
+		goto cleanup_file;
 	}
 
 	/* Lock the password file before updating */
@@ -59,18 +58,18 @@
 	lock.l_len = 0;
 	if (fcntl(fileno(fp), F_SETLK, &lock) < 0) {
 		bb_perror_msg("%s", filename);
-		return 1;
+		goto cleanup_file;
 	}
 	lock.l_type = F_UNLCK;
 
-	snprintf(buf, sizeof buf, "%s-", filename);
+	buf = xasprintf("%s-", filename);
 	if (create_backup(buf, fp)) {
 		goto cleanup_fp;
 	}
-	snprintf(buf, sizeof buf, "%s+", filename);
-	mask = umask(0777);
+	sprintf(buf, "%s+", filename);
+	continued = umask(0777);
 	out_fp = fopen(buf, "w");
-	umask(mask);
+	umask(continued);
 	if ((!out_fp) || (fchmod(fileno(out_fp), sb.st_mode & 0777))
 		|| (fchown(fileno(out_fp), sb.st_uid, sb.st_gid))) {
 		fclose(out_fp);
@@ -95,6 +94,7 @@
 		} else {
 			fputs(buffer, out_fp);
 		}
+//XXX: perhaps last_char_is(buffer, '\n') ?
 		if (buffer[strlen(buffer) - 1] == '\n') {
 			continued = 0;
 		} else {
@@ -110,14 +110,19 @@
 	if (rename(buf, filename) < 0) {
 		goto cleanup_fp;
 	} else {
-		fcntl(fileno(fp), F_SETLK, &lock);
-		fclose(fp);
-		return 0;
+		continued = 0;
+		goto exit_ok;
 	}
+
 cleanup_fp:
+	continued = 1; /* reuse 'continued' var to denote retval */
+exit_ok:
+	free(buf);
 	fcntl(fileno(fp), F_SETLK, &lock);
 	fclose(fp);
-	return 1;
+cleanup_file:
+	free(filename);
+	return continued;
 }
 
 


More information about the busybox mailing list