[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] passwd size reduction take 6
From: Tito <farmatito () tiscali ! it>
Date: 2006-01-30 13:19:40
Message-ID: 200601301419.41168.farmatito () tiscali ! it
[Download RAW message or body]
On Saturday 28 January 2006 23:01, Rob Landley wrote:
> On Wednesday 25 January 2006 14:08, Tito wrote:
> > Hi,
> > This is take 6 of passwd size reduction and this time
> > I think I've got it right..
> >
> > The program now acts more like the real passwd on my linux box:
> > 1) allow 3 retries as long as a good (not easy to guess) new password is
> > typed (for normal user)
>
> You need to type a good password in order to be allowed to retry?
>
> I'm confused: passwd changes the password, right? So if they type a good, not
> easy to guess password, why is that not simply accepted?
Sorry, just bad English.
1) allow 3 retries until a not easy to guess new password is typed.
> > 2) override obscure password checking for root
> > 3) don't ask for old password again if obscure test fails.
> > 4) exits on password retype mismatch.
> > 5) use bb_do_delay in all errors related to asking for the old password in
> > new_password() .
>
> Ok.
>
> > 6) rip out -a switch (non standard: on my system it is -a
> > -all), hope nobody complains.....
>
> Hmmm, glance at 1.1.0 tarball... It selects algorithm (des or md5). Yeah,
> allowing a user to specify that on an individual password makes very little
> sense.
>
> How is this specified now? If CONFIG_FEATURE_SHA1_PASSWORDS is configured in,
> shouldn't it _always_ generate SHA1 passwords? (in which case the salt[] bit
> in new_password, and hence the whole bb_read_random64_string() stuff, should
> #ifdef out? Hmmm...)
Yes i was wondering about this as CONFIG_FEATURE_SHA1_PASSWORDS
is not in Config.in, .config or bb_config.h, so there is no way to select this
option (maybe only manually!?) and probably It could just be removed.
It is only in:
busybox.13608/include/libbb.h
busybox.13608/include/usage.h
busybox.13608/libbb/pw_encrypt.c
> Looking closer, I'm not quite sure what libbb/pw_encrypt.c is for. It has
> three users: httpd.c, sulogin.c, and passwd.c. We know what passwd.c is
> doing, so let's look at the other two:
snip
>
> > 7) default to use salted passwords (more secure?!).
> > 8) generate random salt if /dev/random could be opened
>
> Coolness, that closes a bug in the bug generator.
>
> > 9) use /etc/shadow when CONFIG_FEATURE_SHADOWPASSWDS is enabled
> > only if the /etc/shadow exists else default to /etc/passwd.
>
> Falling back to /etc/passwd probably isn't a good idea. If you don't want to
> create /etc/shadow automatically, I'd prefer to fail noisily if it doesn't
> exist.
This is what real passwd does, i've tested it.
There are 3 cases:
1) user is not in /etc/passwd or /etc/passwd is missing: exit with unknown user
2) user is in /etc/passwd but not in /etc/shadow: exit with unknown user
3) user is in /etc/passwd and /etc/shadow is missing: password is changed in /etc/passwd.
Feel free to change it, size will be reduced a lot.
> > 10) fix the license as suggested on the list.
> > 11) code cleanup
> >
> > Size reduction is:
> >
> > text data bss dec hex filename
> > 3166 0 160 3326 cfe passwd.o.orig
> > 2009 136 140 2285 8ed passwd.o
>
> Nice.
>
snip
>
> I twiddled copy_passwd_file() a bit. It would leak out_fp if the chmod() or
> chown() failed,
I couldn't imagine why they should fail on a path we was able to open
(maybe only if path is deleted?)
> it was recalculating strlen() repeatedly, etc.
I've tried to use a variable to store this value but size increased.
> Using copy_passwd_file() to create a backup copy of the password file is a bit
> silly. You already have the old file, and you really shouldn't be modifying
> its contents (that leads to all sorts of fun race conditions with other
> processes trying to read /etc/passwd while you diddle it), so why not just
> hardlink it to the backup using link(), make the new file off to one side,
> and then do an atomic rename() to put it in the right place (which deletes
> the file being squashed for you, atomically?) That way you not only have
> better multitasking behavior but you get the backup copy for free without
> having to diddle with stat() and utime() to manually propogate the timestamp
> to the backup copy...
>
> I can sort of see doing the file locking stuff so that if two passwd()
> processes try to update at the same time they don't stomp each other, but
> wouldn't it be better to use F_SETLKW to wait, rather than failing
> immediately? (Is there any way to specify a timeout on this? Or do we have
> to use alarm()? In which case this should _definitely_ be wrapped in libbb
> somewhere.)
This was the behaviour of the old passwd applet.
> Sorry, old OS/2 habits kicking in when file locking comes up...
>
> read_random64_string() is discarding entropy from /dev/random. This generally
> isn't something you want to do on embedded devices that have limited
> quantities of real entropy available and will potentially block indefinitely
> if they run out.
>
> And reading in a whole file into a char * given a path to the file is
> something mdev does, and probably other things. That seems like a candidate
> for libbb.
>
> Also, there's already base64 handling code in networking/wget.c function
> base64enc(), coreutils/uudecode.c function read_base64(),
> coreutils/uuencode.c table tbl_base64[], networking/httpd.c function
> decodeBase64(), and probably elsewhere. That _really_ needs to be in libbb,
> but I'll leave it alone for now. (TODO item added.)
Yes, I've seen them also and thought about unifying them, but
don't understand well this stuff......
>
> In new_password, why are ret and retry static?
Ops....
> > /* Do not use bb_error_msg() as the output should not
> > contain the program name for the sake of scripting utilties.*/
>
> What scripting utilities? (I tried to check the relevant standard, but this
> is yet another utility that the Open Group Base Standards version 6 doesn't
> even _mention_, which kinda sucks. Is there a larger standard that
> incorporates this thing somewhere? LSB, perhaps?)
Vodz dixit:
The Passwd utility is unlike nonterminal. For hack this many people
using "expect". bb_error_msg("eror_msg") produce "utility_name: error_msg"
and unexpect for "expect"
>
> Question; this fprintf + expect stuff applies only to this two messages?
Yes.
> I've modified the passwd.c file you sent me, and if I can find another 2 hours
> to work on it today I'll check it in.
>
> Rob
OK, thanks and Ciao,
Tito.
BTW: I'll be offline for the next two weeks for vacation,
so to avoid that you run out of patches to audit ;-)
I've fixed some minor issues and modified a little the previous
obscure stuff patch and added a config option:
config CONFIG_RELAXED_PASSWD_CHECK
bool " Support for relaxed password checking"
default n
depends on CONFIG_PASSWD
help
Permit passwords that not contain a mix of four different
types of characters (upper case letters, lower case letters,
numbers and special characters) but request an increase in
length for each missing type.
With this option enabled the behaviour is as before
else non mixed passwords are considered weak.
I also moved the check for root to obscure.c so it now warns about
weak root passwords but doesn't return an error.
If this patch is checked in, this line in passwd.c new_password()
if (obscure(bb_common_bufsiz1, cp, pw) && getuid()) {
should be changed to;
if (obscure(bb_common_bufsiz1, cp, pw)) {
Bye.
:-P
["obscure02.patch" (text/x-diff)]
--- libbb/obscure.c.orig 2006-01-14 01:56:44.000000000 +0100
+++ libbb/obscure.c 2006-01-23 22:50:24.000000000 +0100
@@ -1,256 +1,201 @@
/* vi: set sw=4 ts=4: */
/*
- * Copyright 1989 - 1994, Julianne Frances Haugh <jockgrrl@austin.rr.com>
- * Copyright 2006, Bernhard Fischer <busybox@busybox.net>
- * All rights reserved.
+ * Mini weak password checker implementation 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@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.
*/
-/*
- * This version of obscure.c contains modifications to support "cracklib"
- * by Alec Muffet (alec.muffett@uk.sun.com). You must obtain the Cracklib
- * library source code for this function to operate.
- */
+/* A good password:
+ 1) should contain at least six characters (man passwd) so empty passwords are not permitted
+ 2) should contain a mix of four different types of characters
+ upper case letters,
+ lower case letters,
+ numbers,
+ special characters such as !@#$%^&*,;"
+ so this password types should not be permitted:
+ a) pure numbers: birthdates,
+ social security number,
+ license plate,
+ phone numbers;
+ b) words and all letters only passwords (uppercase, lowercase or mixed)
+ as palindromes, consecutive or repetitive letters
+ or adjacent letters on your keyboard;
+
+ If you enable CONFIG_RELAXED_PASSWD_CHECK for each missing type of
+ characters an increase of password length is requested rather than
+ returning an error.
+
+ Also not permitted passwords are:
+ c) username, real name, company name or (e-mail?) address
+ in any form (as-is, reversed, capitalized, doubled, etc.).
+ (we can check only against username, gecos and hostname)
+
+ CAVEAT: we cannot check for this without the use of dictionaries:
+ d) common and obvious letter-number replacements
+ (e.g. replace the letter O with number 0)
+ such as "M1cr0$0ft" or "P@ssw0rd".
+*/
-#include <stdlib.h>
-#include <stdio.h>
-#include <string.h>
#include <ctype.h>
-#include "libbb.h"
-
-/*
- * can't be a palindrome - like `R A D A R' or `M A D A M'
- */
-
-static int palindrome(const char *newval)
-{
- int i, j;
+#include <unistd.h>
+#include <string.h>
- i = strlen(newval);
+#include "libbb.h"
- for (j = 0; j < i; j++)
- if (newval[i - j - 1] != newval[j])
- return 0;
+#define MINLEN 6
- return 1;
-}
+static int string_checker_helper(const char *p1, const char *p2) __attribute__ ((__pure__));
-/*
- * more than half of the characters are different ones.
- */
-
-static int similiar(const char *old, const char *newval)
+static int string_checker_helper(const char *p1, const char *p2)
{
- int i, j;
-
- for (i = j = 0; newval[i] && old[i]; i++)
- if (strchr(newval, old[i]))
- j++;
-
- if (i >= j * 2)
- return 0;
-
- return 1;
+ /* as-is or capitalized */
+ if (strcasecmp(p1, p2) == 0
+ /* as sub-string */
+ || strcasestr(p2, p1) != NULL
+ /* invert in case haystack is shorter than needle */
+ || strcasestr(p1, p2) != NULL)
+ return 1;
+ return 0;
}
-/*
- * a nice mix of characters.
- */
-
-static int simple(const char *newval)
+static int string_checker(const char *p1, const char *p2)
{
-#define digits 1
-#define uppers 2
-#define lowers 4
-#define others 8
- int c, is_simple = 0;
int size;
- int i;
-
- for (i = 0; (c = *newval++) != 0; i++) {
- if (isdigit(c))
- is_simple |= digits;
- else if (isupper(c))
- is_simple |= uppers;
- else if (islower(c))
- is_simple |= lowers;
- else
- is_simple |= others;
+ /* check string */
+ int ret = string_checker_helper(p1, p2);
+ /* Make our own copy */
+ char *p = bb_xstrdup(p1);
+ /* reverse string */
+ size = strlen(p);
+
+ while (size--) {
+ *p = p1[size];
+ p++;
}
+ /* restore pointer */
+ p -= strlen(p1);
+ /* check reversed string */
+ ret |= string_checker_helper(p, p2);
+ /* clean up */
+ memset(p, 0, strlen(p1));
+ free(p);
+ return ret;
+}
+
+#define LOWERCASE 1
+#define UPPERCASE 2
+#define NUMBERS 4
+#define SPECIAL 8
- /*
- * The scam is this - a password of only one character type
- * must be 8 letters long. Two types, 7, and so on.
- */
-
- size = 9;
- if (is_simple & digits)
- size--;
- if (is_simple & uppers)
- size--;
- if (is_simple & lowers)
- size--;
- if (is_simple & others)
- size--;
-
- if (size <= i)
- return 0;
-
- return 1;
-#undef digits
-#undef uppers
-#undef lowers
-#undef others
-}
-
-static char *str_lower(char *string)
-{
- char *cp;
-
- for (cp = string; *cp; cp++)
- *cp = tolower(*cp);
- return string;
-}
-
-static const char *
-password_check(const char *old, const char *newval, const struct passwd *pwdp)
+static const char *obscure_msg(const char *old_p, const char *new_p, const struct passwd *pw)
{
- const char *msg;
- char *newmono, *wrapped;
- int lenwrap;
-
- if (strcmp(newval, old) == 0)
- return "no change";
- if (simple(newval))
- return "too simple";
-
- msg = NULL;
- newmono = str_lower(bb_xstrdup(newval));
- lenwrap = strlen(old);
- wrapped = (char *) xmalloc(lenwrap * 2 + 1);
- str_lower(strcpy(wrapped, old));
-
- if (palindrome(newmono))
- msg = "a palindrome";
-
- else if (strcmp(wrapped, newmono) == 0)
- msg = "case changes only";
-
- else if (similiar(wrapped, newmono))
- msg = "too similiar";
-
- else {
- safe_strncpy(wrapped + lenwrap, wrapped, lenwrap + 1);
- if (strstr(wrapped, newmono))
- msg = "rotated";
+ int i;
+ int c;
+ int length;
+ int mixed = 0;
+ const char *p;
+ char hostname[255];
+
+ /* size */
+ if (!new_p || (length = strlen(new_p)) < MINLEN)
+ return("too short");
+
+ /* no username as-is, as sub-string, reversed, capitalized, doubled */
+ if (string_checker(new_p, pw->pw_name)) {
+ return "similar to username";
+ }
+ /* no gecos as-is, as sub-string, reversed, capitalized, doubled */
+ if (string_checker(new_p, pw->pw_gecos)) {
+ return "similar to gecos";
+ }
+ /* hostname as-is, as sub-string, reversed, capitalized, doubled */
+ if(gethostname(hostname, 255) == 0) {
+ hostname[254] = '\0';
+ if (string_checker(new_p, hostname)) {
+ return "similar to hostname";
+ }
}
- memset(newmono, 0, strlen(newmono));
- memset(wrapped, 0, lenwrap * 2);
- free(newmono);
- free(wrapped);
-
- return msg;
-}
-
-static const char *
-obscure_msg(const char *old, const char *newval, const struct passwd *pwdp)
-{
- int maxlen, oldlen, newlen;
- char *new1, *old1;
- const char *msg;
-
- oldlen = strlen(old);
- newlen = strlen(newval);
-
-#if 0 /* why not check the password when set for the first time? --marekm */
- if (old[0] == '\0')
- /* return (1); */
- return NULL;
-#endif
-
- if (newlen < 5)
- return "too short";
-
- /*
- * Remaining checks are optional.
- */
- /* Not for us -- Sean
- *if (!getdef_bool("OBSCURE_CHECKS_ENAB"))
- * return NULL;
- */
- msg = password_check(old, newval, pwdp);
- if (msg)
- return msg;
-
- /* The traditional crypt() truncates passwords to 8 chars. It is
- possible to circumvent the above checks by choosing an easy
- 8-char password and adding some random characters to it...
- Example: "password$%^&*123". So check it again, this time
- truncated to the maximum length. Idea from npasswd. --marekm */
-
- maxlen = 8;
- if (oldlen <= maxlen && newlen <= maxlen)
- return NULL;
-
- new1 = (char *) bb_xstrdup(newval);
- old1 = (char *) bb_xstrdup(old);
- if (newlen > maxlen)
- new1[maxlen] = '\0';
- if (oldlen > maxlen)
- old1[maxlen] = '\0';
-
- msg = password_check(old1, new1, pwdp);
-
- memset(new1, 0, newlen);
- memset(old1, 0, oldlen);
- free(new1);
- free(old1);
+ /* Should / Must contain a mix of: */
+ for (i = 0; i < length; i++) {
+ if (islower(new_p[i])) { /* a-z */
+ mixed |= LOWERCASE;
+ } else if (isupper(new_p[i])) { /* A-Z */
+ mixed |= UPPERCASE;
+ } else if (isdigit(new_p[i])) { /* 0-9 */
+ mixed |= NUMBERS;
+ } else { /* special characters */
+ mixed |= SPECIAL;
+ }
+ /* More than 50% similar characters? */
+ c = 0;
+ p = new_p;
+ while (1) {
+ if ((p = strchr(p, new_p[i])) == NULL) {
+ break;
+ }
+ c++;
+ if (!++p) {
+ break; /* move past the matched char if possible */
+ }
+ }
+
+ if (c >= (length / 2)) {
+ return "too many similar characters";
+ }
+ }
+ if (ENABLE_RELAXED_PASSWD_CHECK) {
+ int size = 9;
+ if (mixed & LOWERCASE) {
+ size--;
+ }
+ if (mixed & UPPERCASE) {
+ size--;
+ }
+ if (mixed & NUMBERS) {
+ size--;
+ }
+ if (mixed & SPECIAL) {
+ size--;
+ }
+ if (length < size)
+ return "too weak";
+ } else {
+ if (mixed < 15) {
+ return "too weak";
+ }
+ }
+
+ if (old_p && old_p[0] != '\0') {
+ /* check vs. old password */
+ if (string_checker(new_p, old_p)) {
+ return "similar to old password";
+ }
+ }
- return msg;
+ return NULL;
}
-/*
- * Obscure - see if password is obscure enough.
- *
- * The programmer is encouraged to add as much complexity to this
- * routine as desired. Included are some of my favorite ways to
- * check passwords.
- */
-
extern int obscure(const char *old, const char *newval, const struct passwd *pwdp)
{
const char *msg = obscure_msg(old, newval, pwdp);
- /* if (msg) { */
if (msg != NULL) {
printf("Bad password: %s.\n", msg);
- /* return 0; */
- return 1;
+ /* If user is root warn only */
+ return (getuid())? 1 : 0;
}
- /* return 1; */
return 0;
}
+
+
+/* END CODE */
+/*
+Local Variables:
+c-file-style: "linux"
+c-basic-offset: 4
+tab-width: 4
+End:
+*/
--- loginutils/Config.in.orig 2005-12-21 20:00:40.000000000 +0100
+++ loginutils/Config.in 2006-01-23 14:07:09.000000000 +0100
@@ -131,6 +131,16 @@
Note that Busybox binary must be setuid root for this applet to
work properly.
+config CONFIG_RELAXED_PASSWD_CHECK
+ bool " Support for relaxed password checking"
+ default n
+ depends on CONFIG_PASSWD
+ help
+ Permit passwords that not contain a mix of four different
+ types of characters (upper case letters, lower case letters,
+ numbers and special characters) but request an increase in
+ length for each missing type.
+
config CONFIG_SU
bool "su"
default n
_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic