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

[Chuck Lever]

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



-- 
"Blast this Christmas music!  It's joyful _and_ triumphant!"
 -- The Grinch