[Toybox] [PATCH] mktemp: yet more tests, and yet more fixes.

[Rob Landley]

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.)

> +    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.

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...)

> 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