On Thu, Dec 6, 2018 at 9:53 AM Rob Landley <[email protected]> wrote: > > On 12/5/18 7:34 PM, enh via Toybox wrote: > > In particular this reuses the password.c code for random ASCII bytes. > > --- > > lib/lib.h | 1 + > > lib/password.c | 35 +++++++++++++++++++++-------------- > > tests/mktemp.test | 6 ++++++ > > toys/lsb/mktemp.c | 24 ++++++++++++++++++------ > > 4 files changed, 46 insertions(+), 20 deletions(-) > > Sigh. lib/password.c was an external contribution that I've cleaned up a bit > but > that whole pile of commands (passwd, su, sudo, useradd, userdel, groupadd, > groupdel...) are still kinda on the todo list until I get mkroot to the point > it > can use/test them. > > > diff --git a/lib/lib.h b/lib/lib.h > > index 14bb7cf6..d51bad4a 100644 > > --- a/lib/lib.h > > +++ b/lib/lib.h > > @@ -306,6 +306,7 @@ int pollinate(int in1, int in2, int out1, int > > out2, int timeout, int shutdown_ti > > char *ntop(struct sockaddr *sa); > > > > // password.c > > +void get_random_ascii(char *buf, int buflen); > > int get_salt(char *salt, char * algo); > > > > // commas.c > > diff --git a/lib/password.c b/lib/password.c > > index b9cc1346..e3598989 100644 > > --- a/lib/password.c > > +++ b/lib/password.c > > @@ -8,6 +8,26 @@ > > #include "toys.h" > > #include <time.h> > > > > +void get_random_ascii(char *buf, int buflen) > > +{ > > + int i; > > + > > + // Read appropriate number of random bytes for salt > > + xgetrandom(libbuf, ((buflen*6)+7)/8, 0); > > + > > + // Grab 6 bit chunks and convert to characters in ./0-9a-zA-Z > > + for (i=0; i<buflen; i++) { > > + int bitpos = i*6, bits = bitpos/8; > > + > > + bits = ((libbuf[i]+(libbuf[i+1]<<8)) >> (bitpos&7)) & 0x3f; > > + bits += 46; > > + if (bits > 57) bits += 7; > > + if (bits > 90) bits += 6; > > That can return / (ascii 47) as part of the acceptable character set, which > you > do not want in mktemp's filename. (I was adjusting so all 64 entries were in > the > old "posix portable filename character set", which is "a-z", "A-Z", "0-9", and > the punctuation ".", "-", and "_". The one I excluded was _ because it's off > by > itself in the ascii table and -. are adjacent.)
yes, i've already sent a replacement for the second patch. we should still get the first patch in though. > > + buf[i] = bits; > > + } > > +} > > My concern is that if xgetrandom() fails (called early in the boot, on an > older > kernel before /proc is mounted, etc) mktemp should still return something > reasonable. (The mktemp out of the library code will, and if mktemp exits with > no stdout output a calling script doing VAL=$(mktemp -u) won't necessarily > catch > the error.) So I added WARN_ONLY support to xgetrandom() and fallback code > loosely modeled on what musl was doing in its mktemp(). > > This is different from what passwd() should do: that's persistent key data > that > needs to be made with strong entropy, and when it can't do that it should exit > with an error. this still seems like something that should be fixed in xgetrandom though. it already takes flags, and a "low quality is fine" flag sounds like a better answer. (works for uuidgen too, for example.) > Using libbuf does solve the length problem, although it seems unlikely that > would ever come up for either use case? (64/6 is 10 characters of entropy, and > an 11th with reduced range... Still, if it's in lib/ why not use libbuf...) that's actually a bug in my latest patch: if you have more Xes than fit in toybuf. i can send a third patch to xgetrandom in-place in `template`. (or another amend of the second patch if you prefer.) but can we at least get the first of the patches in? > > diff --git a/tests/mktemp.test b/tests/mktemp.test > > index ee023d6b..0c235469 100755 > > --- a/tests/mktemp.test > > +++ b/tests/mktemp.test > > @@ -37,3 +37,9 @@ testing "-p DIR -t TEMPLATE but no TMPDIR" "TMPDIR= > > mktemp -u -p DIR -t hello.XX > > > > # mktemp -u doesn't need to be able to write to the directory. > > testing "-u" "mktemp -u -p /proc | grep -q '^/proc/tmp\...........$' > > && echo yes" "yes\n" "" "" > > + > > +# mktemp needs at least XX in the template. > > +testing "bad template" "mktemp -u helloX || echo error" "error\n" "" "" > > + > > +# mktemp -q shouldn't print the path. > > +testing "-q" "mktemp -p /proc -q || echo only-failure" "only-failure\n" "" > > "" > > diff --git a/toys/lsb/mktemp.c b/toys/lsb/mktemp.c > > index 57d1d118..b9e144dc 100644 > > Tests are good. Another patch with a mix of things I have to cherry pick out > of. > Probably over the weekend a this point... > > Rob _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
