subversion changes file permissions on commit

[Attila Nagy]

Hi,

I store OS images in svn, so I need to record file permissions and
ownership. For this, I use properties.
But svn changes real file permissions:
# ls -l fstab
-rw-r--r-- 1 root wheel 230 Oct 22 2011 fstab
# svn pg file:permissions fstab
mode=33188 user=(0) group=(0)
# chmod 600 fstab
# svn ps file:permissions 'mode=33152 user=(0) group=(0)' fstab
property 'file:permissions' set on 'fstab'
# ls -l fstab
-rw------- 1 root wheel 230 Oct 22 2011 fstab
# svn commit -m test fstab
Sending fstab

Committed revision 29956.
# ls -l fstab
-rw-r--r-- 1 root wheel 230 Oct 22 2011 fstab

Why svn works like this and what can I do to disable/work around this
feature?

BTW, this is the trace where the chmod happens:
56718 svn CALL unlink(0x804c93ba0)
56718 svn NAMI "/tmp/svn-e21669fe.tmp"
56718 svn RET unlink 0
56718 svn CALL fstat(0x7,0x7fffffffcce0)
56718 svn STRU struct stat {dev=795533270, ino=6620,
mode=-rw------- , nlink=1, uid=0, gid=0, rdev=4294967295,
atime=1319311412.576898307, stime=1319311412.576898307,
ctime=1381814933.936353362, birthtime=1319311412.576898307, size=230,
blksize=4096, blocks=2, flags=0x0 }
56718 svn RET fstat 0
56718 svn CALL close(0x7)
56718 svn RET close 0
56718 svn CALL
chmod(0x804c927c8,0x1a4<S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH>)
56718 svn NAMI "/data/bootsrv/conf/test/etc/fstab"
56718 svn RET chmod 0

# svn --version
svn, version 1.8.3 (r1516576)
# apr-1-config --version
1.4.8

ps: please keep me on CC.

[Attila Nagy]

On 10/15/2013 08:09 AM, Attila Nagy wrote:

I store OS images in svn, so I need to record file permissions and ownership. For this, I use properties.
But svn changes real file permissions:

OK, long story short. Isn't this a security issue?

$ ls -li zzz
4262518 -rw-------  1 bra  bra  0 Oct 21 18:06 zzz
$ svn add zzz
A         zzz
$ ls -li zzz
4262518 -rw-------  1 bra  bra  0 Oct 21 18:06 zzz
$ svn commit -m tst zzz
Adding         zzz
Transmitting file data .
Committed revision 30074.
$ ls -li zzz
4262518 -rw-r--r--  1 bra  bra  0 Oct 21 18:06 zzz
$ svn --version
svn, version 1.8.3 (r1516576)
   compiled Oct 20 2013, 10:32:56 on amd64-portbld-freebsd9.2

The file is the same, it's not recreated, so "svn doesn't handles permissions, so it uses your umask to create files" doesn't stand here. svn does handle permissions. :(

Why svn does this?
Because it doesn't preserve file permissions on updates, so it chmods the files to umask bits on commit?

[Branko Čibej]

On 21.10.2013 18:16, Attila Nagy wrote:
> On 10/15/2013 08:09 AM, Attila Nagy wrote:
>>
>> I store OS images in svn, so I need to record file permissions and
>> ownership. For this, I use properties.
>> But svn changes real file permissions:
> OK, long story short. Isn't this a security issue?

No, because Subversion does not promise to restore original file
permissions, and therefore you shouldn't rely on it to do so.

There used to be a Unix-specific patch for storing and restoring the
permissions bits, but it does not appear to be mantained.

-- Brane

--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. br...@wandisco.com

[Thorsten Schöning]

Guten Tag Branko Čibej,
am Dienstag, 22. Oktober 2013 um 07:13 schrieben Sie:

> No, because Subversion does not promise to restore original file
> permissions, and therefore you shouldn't rely on it to do so.

It's not about restoring, but not changing them during/after a commit.

Mit freundlichen Grüßen,

Thorsten Schöning

--
Thorsten Schöning E-Mail:Thorsten....@AM-SoFT.de
AM-SoFT IT-Systeme http://www.AM-SoFT.de/

Telefon...........05151- 9468- 55
Fax...............05151- 9468- 88
Mobil..............0178-8 9468- 04

AM-SoFT GmbH IT-Systeme, Brandenburger Str. 7c, 31789 Hameln
AG Hannover HRB 207 694 - Geschäftsführer: Andreas Muchow

[be...@qqmail.nl]

Does the file have any specific properties, such as svn:eol-style or svn:keywords?

Bert

[Branko Čibej]

On 22.10.2013 07:13, Branko Čibej wrote:

On 21.10.2013 18:16, Attila Nagy wrote:

On 10/15/2013 08:09 AM, Attila Nagy wrote:

I store OS images in svn, so I need to record file permissions and
ownership. For this, I use properties.
But svn changes real file permissions:

OK, long story short. Isn't this a security issue?

No, because Subversion does not promise to restore original file
permissions, and therefore you shouldn't rely on it to do so.

There used to be a Unix-specific patch for storing and restoring the
permissions bits, but it does not appear to be mantained.

And moreover the working copy keeps a pristine version of every (committed) file; those pristine versions are also created with the default permissions dictated by umask.

[Branko Čibej]

On 24.10.2013 14:08, Attila Nagy wrote:
> On 10/22/13 09:56, Branko Čibej wrote:

>> On 22.10.2013 07:13, Branko ?ibej wrote:
>>> On 21.10.2013 18:16, Attila Nagy wrote:
>>>> On 10/15/2013 08:09 AM, Attila Nagy wrote:
>>>>> I store OS images in svn, so I need to record file permissions and
>>>>> ownership. For this, I use properties.
>>>>> But svn changes real file permissions:
>>>> OK, long story short. Isn't this a security issue?
>>> No, because Subversion does not promise to restore original file
>>> permissions, and therefore you shouldn't rely on it to do so.
>>>
>>> There used to be a Unix-specific patch for storing and restoring the
>>> permissions bits, but it does not appear to be mantained.

> As Thorsten has pointed out, this is a different case. BTW, I have a
> similar solution, like contrib/asvn, but the current operation of svn
> makes it impossible/very hard to make it work, because it screws up
> real file permissions on each commits.

Yes, that's a valid point. Subversion will possibly write the file
and/or change permissions on commit in two cases, I think:

* The file has the svn:needs-lock property set, and has to be made
read-only after commit; or,
* The file contains keywords, which have to be updated (e.g., author
and revision change after a commit).

[Attila Nagy]

Hi,

On 10/24/13 22:05, Branko Čibej wrote:
>>
>> As Thorsten has pointed out, this is a different case. BTW, I have a
>> similar solution, like contrib/asvn, but the current operation of svn
>> makes it impossible/very hard to make it work, because it screws up
>> real file permissions on each commits.
> Yes, that's a valid point. Subversion will possibly write the file
> and/or change permissions on commit in two cases, I think:
>
> * The file has the svn:needs-lock property set, and has to be made
> read-only after commit; or,
> * The file contains keywords, which have to be updated (e.g., author
> and revision change after a commit).

I'm not aware of these.
Subversion doesn't rewrite the file, because it has the same inode
before and after the commit. And according to a trace, it does a direct
chmod on it.
So if neither of these are standing, is this a bug?

[Stefan Sperling]

I believe it's the stupid code replaced below, which I wrote in r880420.
Because of it we end up setting perms based on umask upon every commit,
and end up expanding restrictive file permissions.

This patch should fix the problem.

Note that Subversion users cannot and couldn't ever rely on per-file
permission flags to keep files in a working copy hidden from other
users. This is because Subversion often resets permission bits when
it re-creates working files from temporary files. There is no way to
set bits of those files to something other than the current umask.
If you have secret files in your working copy, the only way to keep
them secret is to set restrictive permissions on the top-level directory
of the working copy, or set a restrictive umask. (It might be nice to
have a user-configurable default umask for the svn process, but that's
future work.)

[[[
* subversion/libsvn_subr/io.c
(io_set_file_perms): Set the user read/write bits to make a file read/write,
instead of merging in the default bits from the current umask.
Merging bits from the current umask is a bad idea if the file's
permissions are more restrictive than the umask implies.
]]]

Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1536349)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -1591,14 +1591,9 @@ io_set_file_perms(const char *path,
{
if (enable_write) /* Make read-write. */
{
- apr_file_t *fd;
-
- /* Get the perms for the original file so we'll have any other bits
- * that were already set (like the execute bits, for example). */
- SVN_ERR(svn_io_file_open(&fd, path, APR_READ,
- APR_OS_DEFAULT, pool));
- SVN_ERR(merge_default_file_perms(fd, &perms_to_set, pool));
- SVN_ERR(svn_io_file_close(fd, pool));
+ /* Tweak the owner bits only. The group/other bits aren't safe to
+ * touch because we may end up setting them in undesired ways. */
+ perms_to_set |= (APR_UREAD|APR_UWRITE);
}
else
{

[Attila Nagy]

On 10/30/13 09:56, Stefan Sperling wrote:
> I believe it's the stupid code replaced below, which I wrote in r880420.
> Because of it we end up setting perms based on umask upon every commit,
> and end up expanding restrictive file permissions.
>
> This patch should fix the problem.

Indeed, the file remains at 600 after commit. Can I expect it in the
next release?

>
> Note that Subversion users cannot and couldn't ever rely on per-file
> permission flags to keep files in a working copy hidden from other
> users. This is because Subversion often resets permission bits when
> it re-creates working files from temporary files. There is no way to
> set bits of those files to something other than the current umask.

If the original file is there, subversion could copy its permissions.
Of course, if it's lost, it can't do it.

Thanks,

[Stefan Sperling]

On Wed, Oct 30, 2013 at 10:18:35AM +0100, Attila Nagy wrote:
> On 10/30/13 09:56, Stefan Sperling wrote:
> >I believe it's the stupid code replaced below, which I wrote in r880420.
> >Because of it we end up setting perms based on umask upon every commit,
> >and end up expanding restrictive file permissions.
> >
> >This patch should fix the problem.
> Indeed, the file remains at 600 after commit. Can I expect it in the next
> release?

Yes. I'll commit it now and nominate it for backport to 1.8.5.

> >Note that Subversion users cannot and couldn't ever rely on per-file
> >permission flags to keep files in a working copy hidden from other
> >users. This is because Subversion often resets permission bits when
> >it re-creates working files from temporary files. There is no way to
> >set bits of those files to something other than the current umask.
> If the original file is there, subversion could copy its permissions.
> Of course, if it's lost, it can't do it.

That's what Subversion tries to do and what it does in most cases.
But in some cases that's impossible (I don't recall which in detail).

Thanks for your report! If you find more problems let us know.