<br><br><div class="gmail_quote">On Thu, Oct 17, 2013 at 6:46 AM, Rob Landley <span dir="ltr"><<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<div class="im">On 10/02/2013 03:49:21 AM, Ashwini Sharma wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
Hi list,<br>
<br>
Attached are the implementations for groupadd, useradd and mkpasswd<br>
commands.<br>
</blockquote>
<br></div>
Ok, I've caught up to this...<div class="im"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
Patches are as follows<br>
<br>
1. __lib.patch__ : this includes changes made to lib/lib.h and<br>
__lib/password.c__.<br>
lib/passowrd.c is modified to share the function __update_password()__<br>
among groupadd, useradd commands.<br>
This also has the factored out code, common to both, for __passwd__ and<br>
__mkpasswd__.<br>
Also has the cosmetic cleanup changes.<br>
</blockquote>
<br></div>
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.<br>
<br>
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...)<br>
</blockquote><div> </div><div>20 is broken into 16 bytes for salt, 3 for $#$, 1 for _nul_ termination of string. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
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.)<br>
<br>
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.<br>
<br>
Um, hang on:<br>
<br>
i &= 0x3f; // masking for using 10 bits only<br>
<br>
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.<br> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
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.<br>
<br>
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...<br>
</blockquote><div> </div><div>Algorithm to be used can be given at command line, which is verified in get_salt(). That's why an error return is there at end.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<br>
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...<br>
<br>
(Sigh. My bad. If I don't clean up code, people extend the non-cleaned-up code...)<br>
<br>
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...<div class="im">
<br></div></blockquote><div><pre><font face="verdana,sans-serif"> </font><font face="arial,helvetica,sans-serif"><span class="plusline">+ // Grab 6 bit chunks and convert to characters in ./0-9a-zA-Z
</span><span class="plusline"> + for (i=0; i<len; i++) {
</span> <span class="plusline">+ int bitpos = i*6, bits = bitpos/8;
</span> <span class="plusline">+
</span> <span class="plusline">+ bits = ((buf[i]+(buf[i+1]<<8)) >> (bitpos&7)) & 0x3f;
</span> <span class="plusline">+ bits += 46;
</span> <span class="plusline">+ if (bits > 57) bits += 8;
</span> <span class="plusline">+ if (bits > 90) bits += 7;
</span> <span class="plusline">+
</span> <span class="plusline">+ salt[i] = bits;
</span> <span class="plusline">+ }</span></font></pre><pre><span class="plusline"><font face="arial,helvetica,sans-serif">In this snippet, bits is added with 8 or 7 when it is >57 or >90, doing this makes it jump to 66 or 98 respectively.</font></span></pre>
<pre><span class="plusline"><font face="arial,helvetica,sans-serif">which leaves _A_ or _a_, instead this can be</font></span></pre><pre><span class="plusline"><span class="plusline"><font face="arial,helvetica,sans-serif">+ if (bits > 57) bits += 7; To jump to _A_
</font></span><span class="plusline"><font face="arial,helvetica,sans-serif">+ if (bits > 90) bits += 6; To jump to _a_</font> </span></span> </pre></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
<div class="im">
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid">
2. __passwd.c.patch__ : this file is modified due to the common code<br>
factoring out and cosmetic cleanup changes.<br>
</blockquote>
<br></div>
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.<span class="HOEnZb"><font color="#888888"><br>
<br>
Rob</font></span></blockquote></div><div> </div><div>Ashwini<br></div>