On 11.05.2013 14:10, Stefan Sperling wrote:
> 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.
Yes, that's safer in general. It leaves the other race (e.g., editors
not being able to read the file), but that's indeed secondary.
> 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?
+1
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Received on 2013-05-11 14:26:26 CEST