[icedtea-web] RFC: reduce permissions on created files

[Dr Andrew John Hughes]

On 13:56 Wed 24 Nov     , Deepak Bhole wrote:
> * Omair Majid <omajid at redhat.com> [2010-11-24 13:49]:
> > On 11/24/2010 01:41 PM, Deepak Bhole wrote:
> > >* Omair Majid<omajid at redhat.com>  [2010-11-23 14:54]:
> > >>On 11/22/2010 03:31 PM, Deepak Bhole wrote:
> > >>>* Omair Majid<omajid at redhat.com>   [2010-11-12 09:46]:
> > >>>>On 11/12/2010 06:54 AM, Dr Andrew John Hughes wrote:
> > >>>>>On 20:14 Thu 11 Nov     , Omair Majid wrote:
> > >>>>>>Hi,
> > >>>>>>
> > >>>>>>The attached patch tries to make files created by netx/plugin more
> > >>>>>>secure by removing unnecessary permissions. IcedTea6 used to carry a
> > >>>>>>patch to change the umask used by the javaws process which IcedTea-Web
> > >>>>>>does not. This patch tries to make netx/plugin behave like they would in
> > >>>>>>the presence of such a patch.
> > >>>>>>
> > >>>>>>This patch does not change the file permissions on files that are cached
> > >>>>>>(mostly because I dont see why they should be protected), but does
> > >>>>>>change permissions on KeyStores, native directories created under /tmp/,
> > >>>>>>lock files, files created through the JNLP api and log files.
> > >>>>>>
> > >>>>>
> > >>>>>I think this is a better approach, though we should be aware that the permissions
> > >>>>>are now changed after the file has been created, whereas before (I presume) they
> > >>>>>were created with the correct permissions to begin with.
> > >>>>>
> > >>>>
> > >>>>Hm... Does creating a new (blank) file, setting appropriate
> > >>>>permissions on it and then writing content to it ensure that the
> > >>>>content can not be seen by others? This was the original intent with
> > >>>>this patch (I have since noticed one mistake in the patch). If
> > >>>>creating a blank file, setting permissions on it and then writing
> > >>>>actual content does not ensure the confidentiality of the data, then
> > >>>>I would like to find another way to accomplish this.
> > >>>>
> > >>>>Thanks for looking over the patch.
> > >>>>
> > >>>
> > >>>Good point Andrew! This patch needs to be modified. If a file is created
> > >>>with og+[r|w|x] and then restricted, anything that opened the handle
> > >>>before the change will have the permissions it did when the handle was
> > >>>opened. We need to create the file under a different (temp) name, change
> > >>>permissions, and then move it. AFAIK that shouldn't allow access.
> > >>>
> > >>>Access bypass can be verified as follows:
> > >>>1. Create a file as one user (A), allow a+r
> > >>>2. tail -f that file as another user (B)
> > >>>3. As A, og-rwx the file
> > >>>4. As A, write to the file
> > >>>
> > >>>The tail -f command from #2 will show the output, thus confirming that
> > >>>user B has access when they shouldn't.
> > >>>
> > >>>tail -f no longer gets the info if the file is renamed to something else
> > >>>between step 3 and 4.
> > >>>
> > >>
> > >>Thanks for the name change idea. The attached patch implements it. Thoughts?
> > >>
> > >
> > >Looks good! Once suggestion though -- it might be helpful for
> > >debugging/catching errors more easily if the exceptions thrown in
> > >createRestrictedFile() printed the file name along with the message.
> > >Also, the message should be in the message properties file imo, as users
> > >might encounter this more frequently if they set the wrong values.
> > >
> > 
> > Sure. Do we have a policy on what goes in the Messages.properties
> > file? I know all user-visible GUI text belongs in there, but should
> > we add (some? all?) exception messages in there too?
> 
> I think error messages that are only useful to devs are fine in code,
> but messages that users can see and may need to understand (i.e. fixable
> on their end), should be in Messages.properties..
> 

The message of the exception should be user-readable.  I'm sure we can still
work out where it came from by the backtrace.

> Cheers,
> Deepak

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint = F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8