exec* vs. initially-closed file descriptors in a set-ID context

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

exec* vs. initially-closed file descriptors in a set-ID context

Jim Meyering
Hi Uli,

As I know you already know :-)
POSIX allows exec-family functions to ensure that the first
three file descriptors are open for programs that are run set-ID:

  http://www.opengroup.org/susv3xsh/execl.html

glibc implements this in sysdeps/generic/check_fds.c with this code:

  check_one_fd (STDIN_FILENO, O_RDONLY | O_NOFOLLOW);
  check_one_fd (STDOUT_FILENO, O_RDWR | O_NOFOLLOW);
  check_one_fd (STDERR_FILENO, O_RDWR | O_NOFOLLOW);

Can you think of a reason *not* to change it to look like this?

  check_one_fd (STDIN_FILENO, O_WRONLY | O_NOFOLLOW);
  check_one_fd (STDOUT_FILENO, O_RDONLY | O_NOFOLLOW);
  check_one_fd (STDERR_FILENO, O_RDONLY | O_NOFOLLOW);

Note that that would make the reopened stdin *write-only*,
and the other two would be *read-only*.

Here's the motivation:

I'd like my robust hello-world program to fail when its stdout
is redirected to a closed file descriptor, and it usually does:

  $ ./hello >&-
  ./hello: write error: Bad file descriptor

The problem is that when I make my program set-ID root, it no
longer fails, because the C library opens /dev/null on stdout
*for writing*.  So the actual write syscall succeeds (the output
goes to /dev/null), and the set-ID program has no chance of
detecting the conceptual failure.

If instead glibc opened /dev/null read-only, this particular program
would fail (as I think it should) even when set-ID root.

However, it'd be even better if it could open some special
device for which *any* operation on the file descriptor would
fail.  Otherwise, a set-ID program that calls e.g., lseek on
an initially-closed standard file descriptor would mistakenly
succeed in this unusual case.  If we could find/use such a special
device, a set-ID program run with initially-closed standard file
descriptors that does something contrary like reading stdout or
writing stdin would fail (as it should), too.

Jim
Reply | Threaded
Open this post in threaded view
|

Re: exec* vs. initially-closed file descriptors in a set-ID context

Ulrich Drepper
I've changed the code to use the proposed modes.  There is one
additional step in the new code: for the writable descriptor 0 we're now
using /dev/full which causes the operation to fail even if 0 is
invalidly is used for writing.

--
? Ulrich Drepper ? Red Hat, Inc. ? 444 Castro St ? Mountain View, CA ?
Reply | Threaded
Open this post in threaded view
|

Re: exec* vs. initially-closed file descriptors in a set-ID context

Jim Meyering
Ulrich Drepper <[hidden email]> wrote:
> I've changed the code to use the proposed modes.  There is one
> additional step in the new code: for the writable descriptor 0 we're now
> using /dev/full which causes the operation to fail even if 0 is
> invalidly is used for writing.

Thanks for the quick fix.
I like the idea of using /dev/full for descriptor 0.

This seems to be the first use of /dev/full in glibc.
Do all of glibc's target systems provide a usable /dev/full?
Reply | Threaded
Open this post in threaded view
|

Re: exec* vs. initially-closed file descriptors in a set-ID context

Roland McGrath
> This seems to be the first use of /dev/full in glibc.
> Do all of glibc's target systems provide a usable /dev/full?

GNU/Hurd and GNU/Linux do.  If others don't and have good reason, they can
contribute code to do something different in libc for their configurations.
Reply | Threaded
Open this post in threaded view
|

Re: exec* vs. initially-closed file descriptors in a set-ID context

Roland McGrath
In reply to this post by Jim Meyering
However, it may be an issue that e.g. chroot setups don't have /dev/full.
Perhaps it should fall back to /dev/null if opening /dev/full fails.
Reply | Threaded
Open this post in threaded view
|

one more openat-style function required: fchmodat

Jim Meyering
In reply to this post by Jim Meyering
Please consider adding an fchmodat function, analogous to fchownat.

I've rewritten coreutils' fts.c not to change the current
working directory.  That included adjusting the clients
that use fts: du, chmod, chown, chgrp.

In the process, I found that neither Solaris nor glibc
provides a directory-fd-relative chmod function, which I needed
for src/chmod.c.  It is straightforward to simulate the
function using /proc/self/fd/<dir_fd_num>/<file_name> or
save_cwd/fchdir/chmod/restore_cwd, but it'd be better to have a
native (and glibc-supported) implementation to go along with all
of the other ones.

Since Solaris didn't add that interface[*], I'm wondering if I'm missing
something.  Can it be simulated some other way?  Using openat and
fchmod is not adequate, because openat fails for inaccessible files.
If a file were chmod'd to 0 (or even to u-rw), we'd be unable to
adjust permissions without first using chdir.

A google search for chmodat found nothing interesting, but there
is a single, tantalizing reference to a Solaris function named fchmodat.
It's an entry in the _Index for Solaris Systems Programming_:

  fchmodat function, 597-599

If anyone knows more about that, please share.

In case you're wondering how/why chmod -R now needs fchmodat,
it's because this new version of fts traverses the hierarchy via
a virtual working directory.  As a result, the driver chmod.c is
no longer fchdir/chdir'd to each directory, but instead gets a file
descriptor, FD, open on each.  So rather than chmod (file, mode),
it now needs to do something like fchmodat (FD, file, mode, 0).

------
[*] but Solaris didn't add mkdirat either, so maybe the lack of
fchmodat means nothing.

Reply | Threaded
Open this post in threaded view
|

Re: one more openat-style function required: fchmodat

Roland McGrath
It's a natural, obvious, and useful addition, no matter what Solaris does
or doesn't have.  I've put it in.  If there are any more *at additions that
might be useful, right now is the time to bring them up.  So rack your brain.


Thanks,
Roland

Reply | Threaded
Open this post in threaded view
|

euidaccessat [Re: one more openat-style function required: fchmodat

Jim Meyering
Roland McGrath <[hidden email]> wrote:
> It's a natural, obvious, and useful addition, no matter what Solaris does
> or doesn't have.  I've put it in.

Thanks for the speedy addition!

> If there are any more *at additions that
> might be useful, right now is the time to bring them up.  So rack your brain.

In case I didn't mention it on this list before, euidaccessat is one
function that is required to implement a POSIX-conforming rm(1) based
on a chdir-free recursive-removal function.

Here is the relevant function from coreutils/src/remove.c.
If you can see a way to do this without a function like euidaccessat,
POSIX-conformance weenies using rm on deep ACL/xattr-protected trees
will thank you.  FYI, this function serves only to determine whether
rm (without -f) should prompt the user before removing the file or
directory in question, so it's not a big deal either way.


/* Return true if FILE is determined to be an unwritable non-symlink.
   Otherwise, return false (including when lstat'ing it fails).
   If lstat (aka fstatat) succeeds, set *BUF_P to BUF.
   This is to avoid calling euidaccess when FILE is a symlink.  */
static bool
write_protected_non_symlink (int fd_cwd,
                             char const *file,
                             Dirstack_state const *ds,
                             struct stat **buf_p,
                             struct stat *buf)
{
  if (fstatat (fd_cwd, file, buf, AT_SYMLINK_NOFOLLOW) != 0)
    return false;
  *buf_p = buf;
  if (S_ISLNK (buf->st_mode))
    return false;
  /* Here, we know FILE is not a symbolic link.  */

  /* In order to be reentrant -- i.e., to avoid changing the working
     directory, and at the same time to be able to deal with alternate
     access control mechanisms (ACLs, xattr-style attributes) and
     arbitrarily deep trees -- we need a function like eaccessat, i.e.,
     like Solaris' eaccess, but fd-relative, in the spirit of openat.  */

  /* In the absence of a native eaccessat function, here are some of
     the implementation choices [#4 and #5 were suggested by Paul Eggert]:
     1) call openat with O_WRONLY|O_NOCTTY
        Disadvantage: may create the file and doesn't work for directory,
        may mistakenly report `unwritable' for EROFS or ACLs even though
        perm bits say the file is writable.

     2) fake eaccessat (save_cwd, fchdir, call euidaccess, restore_cwd)
        Disadvantage: changes working directory (not reentrant) and can't
        work if save_cwd fails.

     3) if (euidaccess (full_filename (file), W_OK) == 0)
        Disadvantage: doesn't work if full_filename is too long.
        Inefficient for very deep trees (O(Depth^2)).

     4) If the full pathname is sufficiently short (say, less than
        PATH_MAX or 8192 bytes, whichever is shorter):
        use method (3) (i.e., euidaccess (full_filename (file), W_OK));
        Otherwise: vfork, fchdir in the child, run euidaccess in the
        child, then the child exits with a status that tells the parent
        whether euidaccess succeeded.

        This avoids the O(N**2) algorithm of method (3), and it also avoids
        the failure-due-to-too-long-file-names of method (3), but it's fast
        in the normal shallow case.  It also avoids the lack-of-reentrancy
        and the save_cwd problems.
        Disadvantage; it uses a process slot for very-long file names,
        and would be very slow for hierarchies with many such files.

     5) If the full file name is sufficiently short (say, less than
        PATH_MAX or 8192 bytes, whichever is shorter):
        use method (3) (i.e., euidaccess (full_filename (file), W_OK));
        Otherwise: look just at the file bits.  Perhaps issue a warning
        the first time this occurs.

        This is like (4), except for the "Otherwise" case where it isn't as
        "perfect" as (4) but is considerably faster.  It conforms to current
        POSIX, and is uniformly better than what Solaris and FreeBSD do (they
        mess up with long file names). */

  {
    /* This implements #5: */
    size_t file_name_len
      = obstack_object_size (&ds->dir_stack) + strlen (file);

    return (file_name_len < MIN (PATH_MAX, 8192)
            ? euidaccess (full_filename (file), W_OK) != 0 && errno == EACCES
            : euidaccess_stat (buf, W_OK) != 0);
  }
}

Reply | Threaded
Open this post in threaded view
|

Re: euidaccessat [Re: one more openat-style function required: fchmodat

Roland McGrath
Is Solaris eaccess the same as euidaccess?  Should glibc provide eaccess as
an alias for euidaccess?

Except on the Hurd, euidaccess actually either just uses access (when r==e)
or uses stat and the usual st_mode rules assuming those are what the
filesystem will actually use, which you can do yourself.  rm et al I expect
are never setuid and so can always use the method of calling access, which
is superior in telling truth about permission, but lacks the *at features.

In keeping with the other interfaces, it should be faccessat and use a new
AT_* flag bit to distinguish real-user from effective-user access checking.

        int faccessat (int fd, const char *file, int type, int flag);

Does that sound reasonable?


Thanks,
Roland

Reply | Threaded
Open this post in threaded view
|

Re: euidaccessat [Re: one more openat-style function required: fchmodat

Jim Meyering
Roland McGrath <[hidden email]> wrote:
> Is Solaris eaccess the same as euidaccess?

Yes.  Irix and FreeBSD have eaccess, too.

> Should glibc provide eaccess as
> an alias for euidaccess?

Good idea.

> Except on the Hurd, euidaccess actually either just uses access (when r==e)
> or uses stat and the usual st_mode rules assuming those are what the
> filesystem will actually use, which you can do yourself.  rm et al I expect
> are never setuid and so can always use the method of calling access, which

I admit it is very unlikely that anyone will ever find
a use for an rm binary with the set-UID bit set.
However, rm might easily be invoked from a set-UID
program or script, and using access(2) in that context
would be wrong.

> is superior in telling truth about permission, but lacks the *at features.
>
> In keeping with the other interfaces, it should be faccessat and use a new
> AT_* flag bit to distinguish real-user from effective-user access checking.
>
> int faccessat (int fd, const char *file, int type, int flag);
>
> Does that sound reasonable?

That sounds fine.
Thanks!

Reply | Threaded
Open this post in threaded view
|

Re: one more openat-style function required: fchmodat

Ulrich Drepper
In reply to this post by Roland McGrath
Roland McGrath wrote:
> It's a natural, obvious, and useful addition, no matter what Solaris does
> or doesn't have.  I've put it in.

I'm not at all convinced this is a good think.  It can be achieved using
openat + fchmod.  Yes, other functions we already added fall into that
category, too, but they are either in Solaris' code.  Plus, fstatat, for
instance, if far more performance critical than fchmodat.  Except for
chmod -R no code uses it in mass and chmod is well served by fchmodat.
So, provide a good reason to keep the code or I'll revert the patch.

--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


signature.asc (259 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: one more openat-style function required: fchmodat

Jim Meyering
Ulrich Drepper <[hidden email]> wrote:
> Roland McGrath wrote:
>> It's a natural, obvious, and useful addition, no matter what Solaris does
>> or doesn't have.  I've put it in.
>
> I'm not at all convinced this is a good think.  It can be achieved using
> openat + fchmod.

No, it can't, as I said in the original message.
Is you can't open a file, then you can't use fchmod on it.

Reply | Threaded
Open this post in threaded view
|

Re: one more openat-style function required: fchmodat

Roland McGrath
In reply to this post by Ulrich Drepper
> I'm not at all convinced this is a good think.  It can be achieved using
> openat + fchmod.  

If it has mode 000 or suchlike, this won't work.  If it is a device where
opening and closing have magic effects, you don't want to do this.