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

[enh]

On Fri, Dec 7, 2018 at 7:41 AM Rob Landley <rob at landley.net> wrote:

> On 12/6/18 10:52 AM, enh via Toybox wrote:
> > We can't reuse the password.c code for random ASCII salts because that
> > allows '/' (plus it seems to generate sequences of trailing '.'s for
> > some reason). Do the simplest thing that could possibly work instead.
>
> I need a week sometime to properly put the user account management stuff
> into
> mkroot and get it all promoted.
>
> Part of the reason it's further down my todo list is it's no use to Android
> because PIDs mean something else there and login data would go in your
> version
> of the registry instead of being unix-style text files anyway...
>
> > ---
> >  tests/mktemp.test |  6 ++++++
> >  toys/lsb/mktemp.c | 34 ++++++++++++++++++++++++++++------
> >  2 files changed, 34 insertions(+), 6 deletions(-)
> >
> > 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" "" ""
>
> The one on ubuntu 14.04 required three XXX, so that's what I checked for...
>

yeah, they seem to have relaxed that to just XX in newer versions, but i
don't think it matters.


> > +# 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..55ab1aff 100644
> > --- a/toys/lsb/mktemp.c
> > +++ b/toys/lsb/mktemp.c
> > @@ -37,6 +37,8 @@ void mktemp_main(void)
> >    int template_dir = template && !!strchr(template, '/');
> >    int flags_dir = (toys.optflags & (FLAG_p|FLAG_t));
> >    int use_dir = flags_dir && !template_dir;
> > +  char *s, *e;
> > +  int len;
> >
> >    if (template_dir && flags_dir) error_exit("conflicting directories
> given");
> >
> > @@ -61,12 +63,32 @@ void mktemp_main(void)
> >    // TODO: coreutils cleans paths, so -p /t/// would result in /t/xxx...
> >    template = use_dir ? xmprintf("%s/%s", TT.p, template) :
> xstrdup(template);
> >
> > -  if (toys.optflags & FLAG_u) {
> > -    template = mktemp(template);
> > -  } else if (toys.optflags & FLAG_d ? !mkdtemp(template) :
> > mkstemp(template) == -1) {
> > -    if (toys.optflags & FLAG_q) toys.exitval = 1;
> > -    else perror_exit("Failed to create %s %s/%s",
> > -        toys.optflags & FLAG_d ? "directory" : "file", TT.p, template);
> > +  // Point `s` and `e` to the start and end of the last region of
> XXXXXXes.
> > +  s = e = strrchr(template, 'X');
> > +  if (!e || e == template || *(e-1) != 'X') error_exit("need XX in
> template");
> > +  while (s >= template && *(s-1) == 'X') --s;
> > +  len = (e-s+1);
> > +
> > +  while (1) {
> > +    struct stat sb;
> > +    int i;
> > +
> > +    xgetrandom(toybuf, len, 0);
> > +    for (i = 0; i < len; ++i) {
> > +      // mktemp randomness is only from "A-Za-z0-9".
> > +      s[i] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
> > +      "abcdefghijklmnopqrstuvwxyz"
> > +      "0123456789"[toybuf[i] % (26*2+10)];
> > +    }
>
> I wanted to avoid a "long long" division on 32 bit systems pulling in the
> function unnecessarily, and you only need 2 more chars for 64, and the
> "posix
> portable file character set" thing has 3 more (- . and _).
>
> > +    if ((FLAG(u) && lstat(template, &sb) == -1 && errno == ENOENT) ||
> > +        (FLAG(d) && mkdir(template, 0700) != -1) ||
> > +        (open(template, O_CREAT|O_CLOEXEC, 0500) != -1)) break;
> > +    if (errno == EEXIST) continue;
> > +    if (FLAG(q)) {
> > +      toys.exitval = 1;
> > +      return;
> > +    } else perror_exit("%s", template);
>
> I didn't see this until just now (see "list mass unsubscribe again"), but
> I'll
> try to take a proper look this weekend and see what I missed.
>

i think you missed an entire change? toybox ToT doesn't currently build.
mktemp assumes that xgetrandom returns bool and has a new WARN_ONLY flag,
but xgetrandom is void and doesn't have a special case for both getrandom
and /dev not being available...


> Rob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20181207/246dfec5/attachment-0001.htm>