[fedfs-utils] [PATCH] libnsdb: set file mode on open(..., O_CREAT)

[Jeff Layton]

On Wed, 27 Jul 2011 10:53:47 -0400
Chuck Lever <chucklever at gmail.com> wrote:

> On Mon, Jul 11, 2011 at 7:57 AM, Jeff Layton <jlayton at poochiereds.net> wrote:
> > On Sun, 10 Jul 2011 22:13:37 +0100
> > Chuck Lever <chucklever at gmail.com> wrote:
> >
> >> On Thu, Jul 7, 2011 at 9:33 PM, Jeff Layton <jlayton at redhat.com> wrote:
> >> > open(2) with O_CREAT requires the caller to provide a mode.
> >>
> >> Interesting.  I assumed that the gcc system headers would flag
> >> something like this and warn, but I haven't seen such a warning.
> >>
> >
> > It does warn, but you may have to set a -D compiler flag. This bug
> > caused the build to fail via rpmbuild.
> 
> I'm applying this and releasing 0.6.5 soon, to enable RPM packaging
> work to continue.
> 
> Do you perhaps have the compiler flag that exposes this?  I'd like to
> consider including it in the default compiler options for fedfs-utils.
>  Also, can you provide the compiler error listing?
> 
> >> > (Chuck: Does 0666 make sense here?)
> >>
> >> /me pages in just enough to answer...
> >>
> >> I've confirmed with an RH-local LDAP expert that we should drop cert
> >> files into a directory under /var/lib/fedfs.  Such a directory would
> >> be owned by the FEDFS user and group.
> >>
> >> Right now this function is invoked by the nsdbparams command, running
> >> as root; and the rpc.fedfsd server, running with dropped privileges as
> >> the FEDFS user and group.   These programs can create, replace, or
> >> delete these cert files.
> >>
> >> Cert files will be read by user processes that handle junction
> >> resolution, and by NSDB administrative programs... these programs run
> >> as normal users, no guarantee that they will have any kind of special
> >> privileges.
> >>
> >> Thus I think cert files should be owned by the FEDFS user and group
> >> whether this function is invoked by a root caller or a caller running
> >> as the FEDFS user and group.  A cert file can be readable, but should
> >> not be writeable, by the world.
> >>
> >> I doubt the function works quite this way right at the moment.  This
> >> is one of the areas that still needs to be completed.
> >>
> >> Another concern is how to tag these files with the correct SELinux
> >> contexts as they are created...
> >>
> >> > Signed-off-by: Jeff Layton <jlayton at redhat.com>
> >> > ---
> >> >  src/libnsdb/nsdb.c |    2 +-
> >> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >> >
> >> > diff --git a/src/libnsdb/nsdb.c b/src/libnsdb/nsdb.c
> >> > index cad92eb..00be913 100644
> >> > --- a/src/libnsdb/nsdb.c
> >> > +++ b/src/libnsdb/nsdb.c
> >> > @@ -532,7 +532,7 @@ nsdb_new_certfile(const char *certdata, const unsigned int certlen,
> >> >                goto out;
> >> >        }
> >> >
> >> > -       fd = open(pathbuf, O_WRONLY | O_SYNC | O_CREAT | O_EXCL);
> >> > +       fd = open(pathbuf, O_WRONLY | O_SYNC | O_CREAT | O_EXCL, 0666);
> >> >        if (fd == -1) {
> >> >                xlog(D_GENERAL, "%s: Failed to open %s: %m",
> >> >                        __func__, pathbuf);
> >>
> >
> >
> > --
> > Jeff Layton <jlayton at poochiereds.net>
> >

Here's an excerpt from the build with RPM:

gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../..  -I. -I../../src/include  -ggdb -fstrict-aliasing -fpie -Wall -Wextra -pedantic -Wformat=2 -Wstrict-aliasing=2 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -c nsdb.c
gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../..  -I. -I../../src/include  -ggdb -fstrict-aliasing -fpie -Wall -Wextra -pedantic -Wformat=2 -Wstrict-aliasing=2 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -c sqlite.c
In file included from /usr/include/fcntl.h:267:0,
                 from nsdb.c:39:
In function 'open',
    inlined from 'nsdb_new_certfile' at nsdb.c:535:5:
/usr/include/bits/fcntl2.h:51:24: error: call to '__open_missing_mode' declared with attribute error: open with O_CREAT in second argument needs 3 arguments

I think the -D_FORTIFY_SOURCE=2 is what makes this check happen.

-- 
Jeff Layton <jlayton at poochiereds.net>