[Toybox] mkpasswd.c

[scsijon]

> Message: 1
> Date: Wed, 25 Jun 2014 23:02:03 -0500
> From: Rob Landley <rob at landley.net>
> To: toybox at lists.landley.net
> Subject: [Toybox] [CLEANUP] mkpasswd.c
> Message-ID: <53AB9B3B.7020806 at landley.net>
> Content-Type: text/plain; charset=ISO-8859-1
>
> Commit 1363: http://landley.net/hg/toybox/rev/1363
>
> Ok, starting with toys/pending/mkpasswd.c:
>
> The is_salt_valid() function is only called from one place, and using a
> regex to check isalnum() or two punctuation characters is a bit
> overkill. (Since we did not use TOYFLAG_LOCALE we're in the C locale,
> and thus isalnum() has predictable behavior defined by the C99 spec,
> sections 7.4.1 and 5.2.1.1.) So zap that function and replace it with a
> for loop at the call site.
>
> We don't need "-m help" when we can put the list of supported types in
> the help text itself. (Unless this is used programmatically to
> autodetect support? The ubuntu version outputs a lot of extra verbiage
> that would make parsing hard, and the busybox-1.19.0 I have lying around
> doesn't support -m help at all?) I've yanked it for now, I can put -m
> help back if anybody's actually using it...
>

um, as I read it the -m allows you to SET which encription method type 
IS to be used and the -m help allows the reading script to pick which 
one it is set for from those available or failover, the only one I have 
found across my testing systems that uses it is selinux, although it 
could be used by HLFS as that tends to use this sort of stuff (I don't 
have that installed at the moment).

scsijon

> This calls get_salt() which is a function in lib/password.c, a pending
> library function. So let's clean that up.
>
> get_salt:
>
>    - Use libbuf instead of a local buf[]
>    - The bit at the beginning with the if/else staircase can use a table.
>    - Why is offset always 3? Even in the xase of algo="des" where we
>      only added one $, we return an offset of 3. I'm going to assume
>      that was a mistake.
>    - We don't need to track "offset", just retain the initial salt
>      starting point and use pointer subtraction. So new "char *s = salt;"
>    - loop on ARRAY_LEN(), do the work in the loop, and return -1 if the
>      loop exits.
>
> So, move the prototype from lib/portability.h to lib/lib.h to track
> which functions have been cleaned up.
>
> Back in mkpasswd.c, right after calling get_salt() we overwrite the
> returned salt with command line argument salt if we have it, so we read
> entropy from /dev/urandom but didn't use it. Given how often that's a
> finite resource on embedded devices this makes me uncomfortable, but
> I'll leave it for now...
>
> Why do we dup2() TT.pfd instead of using that as what we read from? If
> it was never set it'll be 0, which is stdin anyway. (Also, if you fed
> the old code -P 0 it would dup it, then close stdin. Oops?) Ah, I see:
> it's because read_password() doesn't let us control the fd it reads
> from, and we want -p to be able to specify a tty. (And here's _another_
> pending library function I need to clean up... Eh, leave it for now.)
>
> Ok, change the FLAG_P test to if (TT.pfd) and then we don't close stdin
> if they go -P 0. Don't bother saying STDIN_FILENO, just use 0.
>
> You know, int offset and int i don't overlap in scope, just have the top
> one be int i. (And it doesn't need to be initialized because get_salt()
> assigns to offset as the first use.
>
> We don't need to strcpy the first argument to toybuf in the else case,
> just put an ? : in the call to crypt.
>
> So that's the cleanup on mkpasswd itself. Still have read_password() to
> do in lib/password.c, but other than that the command's ready to be
> promoted. (To "other" I guess, since it says "No standard" and thus
> presumably is not in LSB? Yeah, not in posix or LSB. Apparently it comes
> from the "whois" package, for no readily apparent reason...?)
>
> Rob
>
>