On 10/02/2013 03:49:21 AM, Ashwini Sharma wrote:
Hi list,
Attached are the implementations for groupadd, useradd and mkpasswd
commands.
Ok, I've caught up to this...
Patches are as follows
1. __lib.patch__ : this includes changes made to lib/lib.h and
__lib/password.c__.
lib/passowrd.c is modified to share the function __update_password()__
among groupadd, useradd commands.
This also has the factored out code, common to both, for __passwd__
and
__mkpasswd__.
Also has the cosmetic cleanup changes.
Alas, lib/password.c is one of the pieces that would be in pending if
login.c didn't predate it. I never got this properly cleaned up. Still,
adding this doesn't change that, so... first patch applied, and lemme
at least look at the new bits.
We add three #defines to the header, which are never used. They should
be in the patch that actually uses them. (MAX_SALT_LEN of 20 is wrong,
the largest actual salt is 16, that's the buffer size needed to
process... something. Right...)
The first function in there, random_number_generator(), just reads an
int from a filehandle. It's only ever called from one place, and that
caller is what opens and closes /dev/urandom, so having the read in the
same place doesn't add any new portability constraints. (I.E. let's
just inline this function at its only callsite.)
The return value is fed into inttoc(), which is also the only caller of
that function. inttoc() discards all but 10 bits of it, so we read 32
bits of entropy, and discard 21 of those bits? Not idea on devices with
limited entropy.
Um, hang on:
i &= 0x3f; // masking for using 10 bits only
You can't use 10 bits out of an 8 bit byte. And 0x3f is 00111111 which
is 6 bits. So the comment is both impossible and wrong.
Let's see, max salt is actually 16, times 6 is 96 bits which is an even
12 bytes, so we can read in a buffer of that... this logic is largely
the same as uuencode by the way.
All the users are in get_salt(), which is a strange function. Each
algorithm has a unique digit indicated in the $#$ signature in the
hash, but we don't use that digit to indicate the algorithm, instead we
pass in a string which we check and then _set_ the digit. Why do we do
that? If it was an int we could just use an array to set the salt
length. Also, the function has an error return value for matching none
of the strings, but didn't we pass _in_ that string? Not sure of the
design goal here, who is currently calling this...
Um, there's a second copy in toys/lsb/passwd.c...? Ah, I see, the
second patch in the series removes the function. so it's a move spread
across two patches. Lemme redo that checkin to do both halves...
(Sigh. My bad. If I don't clean up code, people extend the
non-cleaned-up code...)
I need to set up a test environment for this. Ok, I've inlined
random_number_generator() and inttoc() and it compiles, but I can't
test it. I'll check it in anyway and try to come up with a test
chroot...
2. __passwd.c.patch__ : this file is modified due to the common code
factoring out and cosmetic cleanup changes.
I checked in this and the previous patch together as one logical unit,
because it's moving code from one file to another and the result
doesn't make sense without both of them.
Rob
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
