On Sat, May 11, 2013 at 01:50:50PM +0200, Branko Čibej wrote:
> On 11.05.2013 13:31, Stefan Sperling wrote:
> > DST_TMP has more restrictive permissions than that. It is created with
> > the mkstemp() function under the hood. So we can assume that the perms
> > of DST_TMP are configured such that only the user running svn can
> > access the file. So we always start off with something like 600 on
> > DST_TMP. And we need to preserve the permission bits of the existing
> > DST (whatever they are) when renaming DST_TMP on top of it.
>
> So, on Unix, you chmod DST_TMP to DST's permissions before renaming.
Let me reiterate...
In this case, chmod DST_TMP usually means *expanding* DST_TMP's permissions
from 600 to something like 644, potentially giving other people read access
to DST_TMP for a brief amount of time (after chmod and before the rename).
If that is done with DST_TMP living in the system temp dir, this opens
the possibility for other users to read the file, which might contain
some secret. Hence I'd prefer to rename first, and then chmod.
If we rename first the working file DST temporarily has more restrictive
permissions but there is no risk of exposing secrets to other users.
The location of DST is assumed to be safely protected, e.g. via perms
on a parent directory of DST.
This is not a practical issue for the case at hand, because we know that
the tempfile is in .svn/tmp, i.e. it is protected in the same way as DST
already is. So I'm willing to chmod/rename in any order people prefer,
but will add a comment in case we chmod first, to alert people tempted to
reuse this code in a different case where DST_TMP lives in the system tempdir.
Does that make sense?
Received on 2013-05-11 14:10:56 CEST