Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Date: Mon, 12 Jun 2000 15:52:02 -0400
Message-Id: <200006121952.PAA29904@k5.localdomain>
From: Jeff Trawick <trawickj@bellsouth.net>
To: new-httpd@apache.org
In-reply-to: <Pine.LNX.4.21.0006121042300.18430-100000@koj.rkbloom.net>
	(rbb@covalent.net)
Subject: Re: cvs commit: apache-2.0/src/main http_log.c
Reply-to: trawickj@bellsouth.net
References: <Pine.LNX.4.21.0006121042300.18430-100000@koj.rkbloom.net>

> From: rbb@covalent.net
> Date: Mon, 12 Jun 2000 10:45:47 -0700 (PDT)
> 
> > I think ap_set_default_fperms() is a misleading function name.  It
> > doesn't directly set file permissions.  It actually does just the
> > opposite, depending on how you look at it.  If I call
> > ap_set_default_fperms(APR_UREAD|APRUWRITE|APR_GREAD|APR_GWRITE), it
> > would seem that a file I then create with APR_OS_DEFAULT should be
> > rw-rw----, but instead I am assured that those flags are *not* on
> > because ap_set_default_fperms() passed 0660 to umask().
> 
> Yeah, you may be right about the function name.  I needed something so I
> chose this.  If you hate the name, feel free to change it.

Thinking about it more, I don't just disagree with the function name;
I don't see how the whole idea fits into APR as it seems to be mired
in Unixisms.  I noticed that you left #ifndef WIN32 around the
ap_set_default_fperms() call :)

> > If I look at the function name and I read your documentation, I don't
> > see any mention of the umask behavior.  Also, unless on non-Unix you
> > want to simulate umask behavior in setting the permissions of other
> > files created via APR, I don't think setting-the-umask belongs in APR
> > anyway.
> 
> Non-unix systems will need to simulate umask in this instance.  It is
> absolutely important that APR has this type of feature, because it allows
> us to provide some form of security.

Hmmm...  I'd be surprised if this can be properly matched on the
non-Unix platforms.  Even if it could, there would always be
differences based on the fact that on Unix we set the process umask
and thus affect everything the process does whereas on non-Unix at
best we could only simulate it in APR. 

> 
> > I'm also a little disturbed about setting the process umask, as it
> > will affect everything done by the process, not just the APR file I/O
> > functions. 
> 
> So maybe this needs to be in the process stuff, so that people really
> understand the implications of what they are doing.  Regardless, this
> feature is necessary.
> 
> > What do we really want to accomplish?  It seems that with the
> > http_log.c you committed, we want to ensure that log files are only
> > writable by the owner.  Maybe what we instead need is a flag like
> > APR_PERM_ABSOLUTE which when passed to ap_open() along with
> > permissions says that APR must do whatever it takes (including
> > temporarily subverting the process's umask on Unix) to ensure that
> > they are exactly as specified and not subject to any twiddling
> > according to any environmental settings (e.g., umask).
> 
> I would have no problem with that.  If you like that approach more, feel
> free to change it.

On further thought, I don't think we need APR_PERM_ABSOLUTE.  As far
as security goes, we just have to make sure that we don't end up with
a file with looser permissions than we specify (not a problem on any
known system).  

When the umask code was added some months back, I think a simpler
change would have been to specify APR_UREAD | APR_UWRITE | APR_GREAD |
APR_WREAD instead of APR_OS_DEFAULT.  If on Unix the umask of the
process turns off any of those bits, so be it.

My change would be to back out your change, back out the old umask
change (revision 1.27), and then specify APR_UREAD | APR_UWRITE |
APR_GREAD | APR_WREAD instead of APR_OS_DEFAULT.  That cleans up that
piece of code and punts on the umask issue until we have to figure out
what will make sense cross-platform (hopefully never).

O.k.?

-- 
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
     http://www.geocities.com/SiliconValley/Park/9289/
          Born in Roswell... married an alien...