Re: Possible bug in NFSv4 with krb5p security?

Andrey Simonenko wrote:
> On Tue, Feb 19, 2013 at 05:35:50PM +0800, Elias Martenson wrote:
> > On 19 February 2013 17:31, Andrey Simonenko
> > <simon_at_comsys.ntu-kpi.kiev.ua>wrote:
> >
> > It can require bigger buffer, since root can get the pw_password
> > field
> > > in the struct passwd{}.
> > >
> > > Since sysconf(_SC_GETPW_R_SIZE_MAX) does not work on FreeBSD, the
> > > buffer
> > > for getpwnam_r() call should have at least (2 * MAXLOGNAME + 2 *
> > > MAXPATHLEN +
> > > _PASSWORD_LEN + 1) bytes (it is unclear how much is required for
> > > pw_gecos).
> > >
> > > This buffer can be dynamically reallocated until getpwnam_r() is
> > > not
> > > return ERANGE error (the following code has not been compiled and
> > > verified):
> > >
> >
> > Is this really a better solution than to aim high right away? A
> > series of
> > malloc() calls should certainly have much higher overhead than the
> > previous
> > stack-allocated solution.
> >
> > A better compromise would be to do the lookup in a separate
> > function, that
> > allocates the buffer using alloca() instead, yes?
> 
> I cannot find how to get information about maximum buffer size for
> the getpwnam_r() function. This information should be returned by
> sysconf(_SC_GETPW_R_SIZE_MAX), but since it does not work on FreeBSD
> it is necessary to guess its size. Original value is 128 and it works
> for somebody, 1024 works for your environment, but it can fail for
> another environment.
> 
> SUSv4 specifies "Storage referenced by the structure is allocated from
> the memory provided with the buffer parameter", but then tells about
> groups
> in EXAMPLE for getpwnam_r() "Note that sysconf(_SC_GETPW_R_SIZE_MAX)
> may
> return -1 if there is no hard limit on the size of the buffer needed
> to
> store all the groups returned".
> 
> malloc() can give overhead, but that function can try to call
> getpwnam_r()
> with buffer allocated from stack and if getpwnam_r() failed with
> ERANGE
> use dynamically allocated buffer.
> 
> #define PWBUF_SIZE_INI (2 * MAXLOGNAME + 2 * MAXPATHLEN +
> _PASSWORD_LEN + 1)
> #define PWBUF_SIZE_INC 128
> 
> char bufs[2 * MAXLOGNAME + MAXPATHLEN + PASSWORD_LEN + 1 + 32];
> 
> error = getpwnam_r(lname, &pwd, bufs, sizeof(bufs), &pw);
> if (pw != NULL) {
> *uidp = pw->pw_uid;
> return (GSS_S_COMPLETE);
> } else if (error != ERANGE)
> return (GSS_S_FAILURE);
> 
> size = PWBUF_SIZE_INI;
> for (;;) {
> size += PWBUF_SIZE_INC;
> buf = malloc(size);
> if (buf == NULL)
> return (GSS_S_FAILURE);
> error = getpwnam_r(lname, &pwd, buf, size, &pw);
> free(buf);
> if (pw != NULL) {
> *uidp = pw->pw_uid;
> return (GSS_S_COMPLETE);
> } else {
> if (error == ERANGE &&
> size <= SIZE_MAX - PWBUF_SIZE_INC)
> continue;
> return (GSS_S_FAILURE);
> }
> }

Just my opinion, but I think the above is a good approach.
(ie. First trying a fairly large buffer on the stack that
 will succeed 99.99% of the time, but check for ERANGE and
 loop trying progressively larger malloc'd buffers until
 it stops reporting ERANGE.)

I suspect the overheads behind getpwnam_r() are larger than
the difference between using a buffer on the stack vs malloc,
so I think it should use a fairly large buffer the first time.

Personally, I might have coded it as a single do { } while(),
with the first attempt in it, but that's just personal stylistic
taste. (You can check for buf != bufs before doing a free() of it.)
And, if you wanted to be clever, the code could use a static "bufsiz_hint",
which is set to the largest size needed sofar and that is used as
the initial malloc size. That way it wouldn't loop as much for a
site with huge passwd entries. (An entire bio in the GECOS field or ???)

Btw, the same fix is needed in gssd.c, where it calls
getpwuid_r(). { Interesting that for Elias's case, it
must work for 128, although the getpwnam_r() didn't quite fit
in 128. }

Also, FYI, kuserok.c uses a 2048 byte buffer and doesn't check
for ERANGE.

rick

> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to
> "freebsd-current-unsubscribe_at_freebsd.org"