bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Teemu Likonen]

When method /su: or /sudo: is used to _create_ a file the file's
permission will be set to -rwxrwxrwx (777), that is, allow everything
for everyone. Obviously this is serious security bug. Steps to
reproduce:

 1. Start Emacs as a normal user:

        emacs -Q

 2. Create a file in a directory to which the user who launched this
    Emacs session doesn't have write access.

        C-x C-f /su::/root/test.txt

 3. Write some content to the file and save it with "C-x C-s".

 4. Check file's permissions. It has 777 permission bits:

        $ ls -l /root/test.txt
        -rwxrwxrwx 1 root root 5 2009-06-29 17:58 /root/test.txt

For some reason, if I create similar file to the same user's home
directory who launched this Emacs session (/su::$HOME/test.txt) then it
gets 644 permissions (probably honoring umask settings).


In GNU Emacs 23.1.50.4 (i686-pc-linux-gnu, GTK+ Version 2.12.12)
 of 2009-06-29 on mithlond
Windowing system distributor `The X.Org Foundation', version 11.0.10402000
configured using `configure  '--prefix=/home/dtw/local''

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Teemu Likonen]

On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:

> When method /su: or /sudo: is used to _create_ a file the file's
> permission will be set to -rwxrwxrwx (777), [...]

This also happens when _editing_ an existing file because "backup by
renaming" will move the old file aside and the new version of file is
really creating a new file.

So, if you want to give your /etc/passwd and /etc/shadow the -rwxrwxrwx
permissions just edit the files with tramp's /su or /sudo method while
having backup by renaming enabled.

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Michael Albinus]

Teemu Likonen <tlikonen@iki.fi> writes:

> On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:
>
>> When method /su: or /sudo: is used to _create_ a file the file's
>> permission will be set to -rwxrwxrwx (777), [...]

I've committed a fix, to both the trunk and the 23.1 branch.

> This also happens when _editing_ an existing file because "backup by
> renaming" will move the old file aside and the new version of file is
> really creating a new file.
>
> So, if you want to give your /etc/passwd and /etc/shadow the -rwxrwxrwx
> permissions just edit the files with tramp's /su or /sudo method while
> having backup by renaming enabled.

This I cannot reproduce. I have set `backup-by-copying' to nil. Backups
of files under /sudo::... have the same permissions as the original
file.

Best regards, Michael.

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Teemu Likonen]

On 2009-06-29 23:15 (+0200), Michael Albinus wrote:

>> On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:
>>> When method /su: or /sudo: is used to _create_ a file the file's
>>> permission will be set to -rwxrwxrwx (777), [...]
>
> I've committed a fix, to both the trunk and the 23.1 branch.

Thanks. Otherwise OK but I don't like the fact that it gives executable
bits (-rwxr-xr-x) by default. Normal behavior for new files is to drop
umask bits _and_ executable bits. Executable must be added explicitly.

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Teemu Likonen]

On 2009-06-30 01:01 (+0300), Teemu Likonen wrote:

> On 2009-06-29 23:15 (+0200), Michael Albinus wrote:
>
>>> On 2009-06-29 18:16 (+0300), Teemu Likonen wrote:
>>>> When method /su: or /sudo: is used to _create_ a file the file's
>>>> permission will be set to -rwxrwxrwx (777), [...]
>>
>> I've committed a fix, to both the trunk and the 23.1 branch.
>
> Thanks. Otherwise OK but I don't like the fact that it gives executable
> bits (-rwxr-xr-x) by default. Normal behavior for new files is to drop
> umask bits _and_ executable bits. Executable must be added explicitly.

And when editing existing files it should obviously respect the bits
that the file already has. Currently -- even with this fix -- tramp is
adding "x" bits at some point because "backup by rename" moves old
version out of the way and new is created with -rwxr-xr-x bits.

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Michael Albinus]

Teemu Likonen <tlikonen@iki.fi> writes:

>> Thanks. Otherwise OK but I don't like the fact that it gives executable
>> bits (-rwxr-xr-x) by default. Normal behavior for new files is to drop
>> umask bits _and_ executable bits. Executable must be added explicitly.

When creating a new file, Tramp uses Emacs' default file modes. You can
check them with "M-: (default-file-modes)".

If you want to change them, you could apply for example
"M-: (set-default-file-modes #o0400)". The value is used then for all
newly created files, also for local ones.

> And when editing existing files it should obviously respect the bits
> that the file already has. Currently -- even with this fix -- tramp is
> adding "x" bits at some point because "backup by rename" moves old
> version out of the way and new is created with -rwxr-xr-x bits.

As I said already, I cannot reproduce it. However, there seems to be a
small annoyance in special cases. I've fixed this. Could you, please,
check, whether it is OK now for you?

Best regards, Michael.

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Teemu Likonen]

On 2009-06-30 14:21 (+0200), Michael Albinus wrote:

> When creating a new file, Tramp uses Emacs' default file modes. You
> can check them with "M-: (default-file-modes)".
>
> If you want to change them, you could apply for example "M-:
> (set-default-file-modes #o0400)". The value is used then for all newly
> created files, also for local ones.

Hmm, I didn't know about those functions, thanks. And I compiled my
Emacs with your recent changes too.

I still don't like the default difference between creating a file as a
normal user or through /su: or /sudo:. Here's again an example starting
from command

    umask 0022; emacs -Q

When I create a file without Tramp (C-x C-f ~/test.txt RET) to my home
directory it gets bits 0644. When I create a file through Tramp to
/sudo::/root/test.txt it gets bits 0755 (i.e. with executable bits). In
both cases Emacs's default-file-modes is the same, the untouched default
which is #o755. In fact, all the settings are the same.

I'm not sure where this difference should be fixed but from user's point
of view the Tramp part brings the unexpected end result. It's unexpected
because no other programs create new executable files by default, even
when umask doesn't mask executable bits.

I appreciate your hint about set-default-file-modes, and I'll use it if
necessary, but in my opinion user shouldn't need to run

    (set-default-file-modes #o0644)

in her ~/.emacs just because she wants Tramp to behave similarly to her
umask=0022 settings. Instead, the similar behavior should be the
default.

>> And when editing existing files it should obviously respect the bits
>> that the file already has. Currently -- even with this fix -- tramp
>> is adding "x" bits at some point because "backup by rename" moves old
>> version out of the way and new is created with -rwxr-xr-x bits.
>
> As I said already, I cannot reproduce it. However, there seems to be a
> small annoyance in special cases. I've fixed this. Could you, please,
> check, whether it is OK now for you?

I could reproduce it before but it seems that not anymore with your
newest changes. If you want clear steps how to reproduce it I can
inspect the issue more closely.

Anyway, thanks for your work on Tramp and Emacs! :-)

bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method

[Michael Albinus]

Teemu Likonen <tlikonen@iki.fi> writes:

> I'm not sure where this difference should be fixed but from user's point
> of view the Tramp part brings the unexpected end result. It's unexpected
> because no other programs create new executable files by default, even
> when umask doesn't mask executable bits.

OK, you've convinced me. Execution bits are removed now for newly
created remote files.

>> As I said already, I cannot reproduce it. However, there seems to be a
>> small annoyance in special cases. I've fixed this. Could you, please,
>> check, whether it is OK now for you?
>
> I could reproduce it before but it seems that not anymore with your
> newest changes. If you want clear steps how to reproduce it I can
> inspect the issue more closely.

If it works also for you it is OK for me.

> Anyway, thanks for your work on Tramp and Emacs! :-)

Best regards, Michael.

bug#3712: marked as done (23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]


Your message dated Tue, 30 Jun 2009 19:36:32 +0300
with message-id <878wj9fzlb.fsf@iki.fi>
and subject line Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
has caused the Emacs bug report #3712,
regarding 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@emacsbugs.donarmstrong.com
immediately.)


-- 
3712: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=3712
Emacs Bug Tracking System
Contact owner@emacsbugs.donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 3570 bytes --]

From: Teemu Likonen <tlikonen@iki.fi>
To: emacs-pretest-bug@gnu.org
Subject: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Mon, 29 Jun 2009 18:16:30 +0300
Message-ID: <87ljnbax4h.fsf@iki.fi>

When method /su: or /sudo: is used to _create_ a file the file's
permission will be set to -rwxrwxrwx (777), that is, allow everything
for everyone. Obviously this is serious security bug. Steps to
reproduce:

 1. Start Emacs as a normal user:

        emacs -Q

 2. Create a file in a directory to which the user who launched this
    Emacs session doesn't have write access.

        C-x C-f /su::/root/test.txt

 3. Write some content to the file and save it with "C-x C-s".

 4. Check file's permissions. It has 777 permission bits:

        $ ls -l /root/test.txt
        -rwxrwxrwx 1 root root 5 2009-06-29 17:58 /root/test.txt

For some reason, if I create similar file to the same user's home
directory who launched this Emacs session (/su::$HOME/test.txt) then it
gets 644 permissions (probably honoring umask settings).


In GNU Emacs 23.1.50.4 (i686-pc-linux-gnu, GTK+ Version 2.12.12)
 of 2009-06-29 on mithlond
Windowing system distributor `The X.Org Foundation', version 11.0.10402000
configured using `configure  '--prefix=/home/dtw/local''


[-- Attachment #3: Type: message/rfc822, Size: 2107 bytes --]

From: Teemu Likonen <tlikonen@iki.fi>
To: Michael Albinus <michael.albinus@gmx.de>
Cc: 3712-done@emacsbugs.donarmstrong.com
Subject: Re: bug#3712: 23.1.50; SECURITY: Tramp creates -rwxrwxrwx permission files with /su and /sudo method
Date: Tue, 30 Jun 2009 19:36:32 +0300
Message-ID: <878wj9fzlb.fsf@iki.fi>

On 2009-06-30 17:34 (+0200), Michael Albinus wrote:

> OK, you've convinced me. Execution bits are removed now for newly
> created remote files.

> If it works also for you it is OK for me.

It seems to work perfectly now. Huge thanks! I'm happy to close this
bug.