all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Re: sudo:: method in tramp possible security issue
       [not found]                     ` <87o9ajvost.fsf@gmx.de>
@ 2018-11-20 14:13                       ` João Távora
  2018-11-20 21:14                         ` Paul Eggert
  2018-11-20 22:30                         ` Michael Albinus
  0 siblings, 2 replies; 21+ messages in thread
From: João Távora @ 2018-11-20 14:13 UTC (permalink / raw)
  To: Michael Albinus, emacs-devel; +Cc: Eli Zaretskii, Stefan Monnier

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

Hello emacs-devel,

The off-list discussion below is about TRAMP's usage of
the /sudo:: method, which surprised me very much recently
because I discovered that it lets any elisp run arbitrary shell
commands with root permissions while the buffer editing a file
with /sudo:: is live.

So in theory you could write malicious elisp code to lay there
hoping to hijack a users system on their first file of
/sudo::/etc/apt/sources.list, for example. Supposing all the user
wanted is to edit that file, starting a full "elisp sudo server" for
the duration of the buffer is clearly overkill and unnecessarily
dangerous for most users.

For me this is a very serious security hole, but apparently
it's part of the contract of the /sudo:: method.

I am arguing for:

1. A sudoedit method that works like `sudo -e`

2. A one-time stern warning the first time that the user uses /sudo::
to explain the security implications to new users.

Michael and I are converging on some possibilities, but I
think it's a good idea to have the rest of emacs-devel speak
their mind.

[Michael you have my new in-line replies below, too]

Thanks,
João


On Tue, Nov 20, 2018 at 1:53 PM Michael Albinus <michael.albinus@gmx.de>
wrote:

> João Távora <joaotavora@gmail.com> writes:
>
> >     For the time being, the Tramp manual says:
> >
> >     --8<---------------cut here---------------start------------->8---
> >     ‘sudo’
> >
> >          Similar to ‘su’ method, ‘sudo’ uses ‘sudo’.  ‘sudo’ must have
> >          sufficient rights to start a shell.
> >     --8<---------------cut here---------------end--------------->8---
> >
> >     It says already that a shell is used, but perhaps we shall
> >     emphasize it more. As I said: manuals are unread ...
> >
> > Very true, and this is why I am suggesting an in-your-face warning.
>
> That would be bossy. People would start to modify the code in order to
> bypass this.
>

Perhaps I didn't explain myself. I'm talking about a one-time thing, like
the warnings for a disabled command. Something that you can answer
"I understand the risks: never ask for this again".

Once sudoedit is offered by Tramp, I expect a respective recommendation
> for Emacs.
>

OK. I believe this is not quite enough. But I'm talking for myself. I think
we should indeed raise the matter publicly.


> > And I don't agree it's like sudo -i, because I don't think there is an
> > easy way for non-root code running before the sudo -i session to
> > inject itself into the root session, or is there? Certainly not as
> > easy as writing some trivial elisp.
>
> Everybody is able to write such code. A simple `shell-command' with
> "sudo" might suffice to run any command with root permissions. Or use
> the interactive `shell', and call "sudo -i" there. Nothing you need
> Tramp for.
>

I was just arguing that the comparison between "sudo -i", meant to be
run in a shell and tramp's launching of a "sudo shell" isn't adequate,
because
there's no easy way for some unprivileged code to stay around
and wait for an opportunity to hijack your interactive "sudo -i"


>
> > /Sudoedit:: would be a really good addition. It would replace all the
> > usage I and quite possibly many others have ever had of /sudo::.
>
> Agreed. If it is possible to implement as proposed.
>
> (Please write the bug report about, in order to discuss it with greater
> audience. Or start a discussion at the emacs-devel ML.)
>

I just did that.

[-- Attachment #2: Type: text/html, Size: 5127 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 14:13                       ` sudo:: method in tramp possible security issue João Távora
@ 2018-11-20 21:14                         ` Paul Eggert
  2018-11-20 21:18                           ` Stefan Monnier
  2018-11-20 22:16                           ` Michael Albinus
  2018-11-20 22:30                         ` Michael Albinus
  1 sibling, 2 replies; 21+ messages in thread
From: Paul Eggert @ 2018-11-20 21:14 UTC (permalink / raw)
  To: João Távora, Michael Albinus, emacs-devel
  Cc: Eli Zaretskii, Stefan Monnier

Is there a simple way to configure Emacs so that it does not use this 
sudo (or sudoedit) feature of Tramp? If not, perhaps there should be one.

I long ago put (setq tramp-mode nil) into my ~/.emacs file because of 
security concerns, so I wouldn't need to selectively disable sudo 
myself. But perhaps others who are less concerned about security (but 
still somewhat concerned) might want it.

Come to think of it, if 'emacs -Q' enables Tramp by default then perhaps 
I should stop using 'emacs -Q'....




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 21:14                         ` Paul Eggert
@ 2018-11-20 21:18                           ` Stefan Monnier
  2018-11-20 21:25                             ` Paul Eggert
  2018-11-20 22:16                           ` Michael Albinus
  1 sibling, 1 reply; 21+ messages in thread
From: Stefan Monnier @ 2018-11-20 21:18 UTC (permalink / raw)
  To: Paul Eggert
  Cc: Eli Zaretskii, Michael Albinus, João Távora,
	emacs-devel

> Is there a simple way to configure Emacs so that it does not use this sudo
> (or sudoedit) feature of Tramp?

Yes: use the default config and just don't use Tramp's sudo method.

> I long ago put (setq tramp-mode nil) into my ~/.emacs file because of
> security concerns, so I wouldn't need to selectively disable sudo
> myself.

Tramp is not magical: it can do no more nor less than what an attacker
could do.  So disabling it doesn't buy you anything in this respect, AFAICT.


        Stefan



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 21:18                           ` Stefan Monnier
@ 2018-11-20 21:25                             ` Paul Eggert
  2018-11-20 21:44                               ` Stefan Monnier
  2018-11-20 22:06                               ` Michael Albinus
  0 siblings, 2 replies; 21+ messages in thread
From: Paul Eggert @ 2018-11-20 21:25 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Eli Zaretskii, Michael Albinus, João Távora,
	emacs-devel

On 11/20/18 1:18 PM, Stefan Monnier wrote:
> Tramp is not magical: it can do no more nor less than what an attacker
> could do.

Sure, if the attacker has control over my keyboard, or over my display, 
or over the Lisp code that I load and execute. That being said, Tramp 
does make attacks easier, so it has been an easy call for me to disable it.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 21:25                             ` Paul Eggert
@ 2018-11-20 21:44                               ` Stefan Monnier
  2018-11-20 22:06                               ` Michael Albinus
  1 sibling, 0 replies; 21+ messages in thread
From: Stefan Monnier @ 2018-11-20 21:44 UTC (permalink / raw)
  To: emacs-devel

>> Tramp is not magical: it can do no more nor less than what an attacker
>> could do.
> Sure, if the attacker has control over my keyboard, or over my display, or
> over the Lisp code that I load and execute.  That being said, Tramp does make
> attacks easier, so it has been an easy call for me to disable it.

I don't see in which way you think it makes attacks easier.
Are you thinking if things like file-local variables which may point
to a file like "/sudo:..."?

I'd expect that in most such cases such vars pointing to arbitrary files
would be a risk even without the sudo method, so I'd hope we'd plug
those quickly enough (and yes, the sudo method would make such attacks
worse, indeed).


        Stefan




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 21:25                             ` Paul Eggert
  2018-11-20 21:44                               ` Stefan Monnier
@ 2018-11-20 22:06                               ` Michael Albinus
  2018-11-20 22:27                                 ` João Távora
  1 sibling, 1 reply; 21+ messages in thread
From: Michael Albinus @ 2018-11-20 22:06 UTC (permalink / raw)
  To: Paul Eggert
  Cc: Eli Zaretskii, emacs-devel, Stefan Monnier, João Távora

Paul Eggert <eggert@cs.ucla.edu> writes:

> On 11/20/18 1:18 PM, Stefan Monnier wrote:
>> Tramp is not magical: it can do no more nor less than what an attacker
>> could do.
>
> Sure, if the attacker has control over my keyboard, or over my
> display, or over the Lisp code that I load and execute. That being
> said, Tramp does make attacks easier, so it has been an easy call for
> me to disable it.

Tramp's sudo method needs your credentials. If you don't provide them,
Tramp cannot do anything.

Like calling sudo in a terminal.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 21:14                         ` Paul Eggert
  2018-11-20 21:18                           ` Stefan Monnier
@ 2018-11-20 22:16                           ` Michael Albinus
  1 sibling, 0 replies; 21+ messages in thread
From: Michael Albinus @ 2018-11-20 22:16 UTC (permalink / raw)
  To: Paul Eggert
  Cc: Eli Zaretskii, João Távora, Stefan Monnier, emacs-devel

Paul Eggert <eggert@cs.ucla.edu> writes:

> Is there a simple way to configure Emacs so that it does not use this
> sudo (or sudoedit) feature of Tramp? If not, perhaps there should be
> one.

Remove the corresponding entries from tramp-methods. Something like

(setq tramp-methods (delete (assoc "sudo" tramp-methods) tramp-methods))

Same for "su", and "sudoedit" (once it is added to Tramp). However,
"sudoedit" is intented to just read a file, and save the respective
buffer. No interactive shell under the hood, no backup files, no remote
processes - nothing else but file reading and buffer saving. The idea is
to use a proper emacsclient call for this.

> I long ago put (setq tramp-mode nil) into my ~/.emacs file because of
> security concerns, so I wouldn't need to selectively disable sudo
> myself. But perhaps others who are less concerned about security (but
> still somewhat concerned) might want it.
>
> Come to think of it, if 'emacs -Q' enables Tramp by default then
> perhaps I should stop using 'emacs -Q'....

Tramp manifests itself via autoloads. They are still active when running
'emacs -Q'.

Tramp does already some actions when it detects that emacs has started
with "-Q". For example, it doesn't read the persistency file
"~/.emacs.d/tramp". We could extend this mechanism.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 22:06                               ` Michael Albinus
@ 2018-11-20 22:27                                 ` João Távora
  2018-11-20 23:12                                   ` Stefan Monnier
  2018-11-21  7:41                                   ` Michael Albinus
  0 siblings, 2 replies; 21+ messages in thread
From: João Távora @ 2018-11-20 22:27 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Eli Zaretskii, Paul Eggert, Stefan Monnier, emacs-devel

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

On Tue, Nov 20, 2018 at 10:06 PM Michael Albinus <michael.albinus@gmx.de>
wrote:

> Paul Eggert <eggert@cs.ucla.edu> writes:
>
> > On 11/20/18 1:18 PM, Stefan Monnier wrote:
> >> Tramp is not magical: it can do no more nor less than what an attacker
> >> could do.
> >
> > Sure, if the attacker has control over my keyboard, or over my
> > display, or over the Lisp code that I load and execute. That being
> > said, Tramp does make attacks easier, so it has been an easy call for
> > me to disable it.
>
> Tramp's sudo method needs your credentials. If you don't provide them,
> Tramp cannot do anything.
>
> Like calling sudo in a terminal.


It's not exactly like calling sudo in a terminal, because when you
use sudo you generally:

1. perform a one time action and are back at a non-sudo prompt; OR
2. start an interactive superuser session that easy to identify visually
   and for which there isn't a programmatic way for other programs
   to interfere

In other words, what bothers me the most about the sudo:: method is
the persistent sudo session that makes me vulnerable to attackers, and
to my elisp developing mistakes.  This is why I think a warning makes
sense, or some visual way to identify this vulnerable state.

In contrast, using sudoedit:: should not bring about this vulnerable state.

That being said, if your non-elevated user has already been compromised,
entering sudo credentials into Emacs, where elisp can do whatever, is
probably a very bad idea, regardless of Tramp.

João

[-- Attachment #2: Type: text/html, Size: 2189 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 14:13                       ` sudo:: method in tramp possible security issue João Távora
  2018-11-20 21:14                         ` Paul Eggert
@ 2018-11-20 22:30                         ` Michael Albinus
  2018-11-20 22:54                           ` João Távora
  1 sibling, 1 reply; 21+ messages in thread
From: Michael Albinus @ 2018-11-20 22:30 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel

João Távora <joaotavora@gmail.com> writes:

> Hello emacs-devel, 

Hi João,

> The off-list discussion below is about TRAMP's usage of 
> the /sudo:: method, which surprised me very much recently 
> because I discovered that it lets any elisp run arbitrary shell 
> commands with root permissions while the buffer editing a file 
> with /sudo:: is live.  
>
> So in theory you could write malicious elisp code to lay there
> hoping to hijack a users system on their first file of 
> /sudo::/etc/apt/sources.list, for example. Supposing all the user 
> wanted is to edit that file, starting a full "elisp sudo server" for 
> the duration of the buffer is clearly overkill and unnecessarily 
> dangerous for most users.

It isn't overkill. The implementation in Tramp depends on the file name
handler concept, which requires to implement 70 basic functions. How
would it be possible to implement `file-attributes', for example, w/o an
interactive shell with root permissions?

> For me this is a very serious security hole, but apparently
> it's part of the contract of the /sudo:: method.
>
> I am arguing for:
>
> 1. A sudoedit method that works like `sudo -e`

Agreed. It shall basically implement just `insert-file-contents' and
`write-region'. (If possible, I haven't started to investigate in detail).

> 2. A one-time stern warning the first time that the user uses /sudo:: 
> to explain the security implications to new users.

Here I'm not convinced. I agree that it must be said more prominent in
the Tramp manual, that an interactive session with root permissions is
running in the background, but I believe it would be too bossy to tell
users they shall not use "/sudo::". It is like telling something like
this to users, who call sudo in a terminal. Are there such warnings,
somewhere?

> Michael and I are converging on some possibilities, but I
> think it's a good idea to have the rest of emacs-devel speak
> their mind.
>
> Thanks,
> João

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 22:30                         ` Michael Albinus
@ 2018-11-20 22:54                           ` João Távora
  2018-11-21  7:49                             ` Michael Albinus
  0 siblings, 1 reply; 21+ messages in thread
From: João Távora @ 2018-11-20 22:54 UTC (permalink / raw)
  To: Michael Albinus; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel

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

On Tue, Nov 20, 2018 at 10:30 PM Michael Albinus <michael.albinus@gmx.de>
wrote:

>
> It isn't overkill. The implementation in Tramp depends on the file name
> handler concept, which requires to implement 70 basic functions. How
> would it be possible to implement `file-attributes', for example, w/o an
> interactive shell with root permissions?
>

Sorry, I meant overkill for most **uses**.  By which I mean 100%
of uses that **I** have ever had for /sudo::, which is to quickly
edit a system file. Of course there are other uses that other
users might want.


> Here I'm not convinced. I agree that it must be said more prominent in
> the Tramp manual, that an interactive session with root permissions is
> running in the background, but I believe it would be too bossy to tell
> users they shall not use "/sudo::". It is like telling something like
> this to users, who call sudo in a terminal. Are there such warnings,
> somewhere?
>

I've explained that I don't think it is the same to run sudo in a
terminal and inside emacs.  And I'm not suggesting "/sudo::
considered dangerous, thou shalt not use it!", just a one time
summary explanation of what is about to happen.

Alternatively, how would you feel about adding something to
the mode-line clearly showing that there is an ongoing
superuser session going on?

For me, mentioning it in the manual isn't very useful, because
few people read the manual, fine as it may be :-)

João Távora

[-- Attachment #2: Type: text/html, Size: 2196 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 22:27                                 ` João Távora
@ 2018-11-20 23:12                                   ` Stefan Monnier
  2018-11-21  7:41                                   ` Michael Albinus
  1 sibling, 0 replies; 21+ messages in thread
From: Stefan Monnier @ 2018-11-20 23:12 UTC (permalink / raw)
  To: João Távora
  Cc: Eli Zaretskii, Paul Eggert, Michael Albinus, emacs-devel

> In other words, what bothers me the most about the sudo:: method is
> the persistent sudo session that makes me vulnerable to attackers, and
> to my elisp developing mistakes.  This is why I think a warning makes
> sense, or some visual way to identify this vulnerable state.

I guess it all depends on the sudo setup:

Can you run a shell via sudo?  On those machines where I can do that,
I typically do "sudo zsh" and then live happily in my root shell.
But even you don't, after you've used sudo, there's a time window
during which sudo won't ask for your password and during which an
attacker could run "sudo sh" via start-process, regardless of Tramp.

If you can't run a shell via sudo, then Tramp's sudo method won't work
anyway.


        Stefan



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 22:27                                 ` João Távora
  2018-11-20 23:12                                   ` Stefan Monnier
@ 2018-11-21  7:41                                   ` Michael Albinus
  2018-11-21 12:26                                     ` Michael Albinus
                                                       ` (2 more replies)
  1 sibling, 3 replies; 21+ messages in thread
From: Michael Albinus @ 2018-11-21  7:41 UTC (permalink / raw)
  To: João Távora
  Cc: Eli Zaretskii, Paul Eggert, Stefan Monnier, emacs-devel

João Távora <joaotavora@gmail.com> writes:

>     Tramp's sudo method needs your credentials. If you don't provide
>     them, Tramp cannot do anything.
>
>     Like calling sudo in a terminal.
>
> It's not exactly like calling sudo in a terminal, because when you
> use sudo you generally:
>
> 1. perform a one time action and are back at a non-sudo prompt; OR
> 2. start an interactive superuser session that easy to identify
> visually 
>    and for which there isn't a programmatic way for other programs 
>    to interfere
>
> In other words, what bothers me the most about the sudo:: method is 
> the persistent sudo session that makes me vulnerable to attackers, and
> to my elisp developing mistakes.  This is why I think a warning makes 
> sense, or some visual way to identify this vulnerable state.

There is already a "visual way to identify this state". It is called
tramp-theme, a GNU ELPA package.

This is documented in the Tramp manual, see (info "(tramp) Frequently Asked Questions")
Again, nobody reads the manual :-(

The command `tramp-cleanup-connection' closes any background session for
a Tramp connection, including removing cached passwords. Maybe we shall
call this for sudo/su methods automatically after a given timeout, like
the password expiration for sudo in a terminal. 5 minutes seem to be a
sensible value to me.

> João

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-20 22:54                           ` João Távora
@ 2018-11-21  7:49                             ` Michael Albinus
  0 siblings, 0 replies; 21+ messages in thread
From: Michael Albinus @ 2018-11-21  7:49 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel

João Távora <joaotavora@gmail.com> writes:

> Alternatively, how would you feel about adding something to 
> the mode-line clearly showing that there is an ongoing 
> superuser session going on?

As said the other mail, this is provided by the GNU ELPA package
tramp-theme. When writing this package I decided against making it the
default, because people are very idiosyncratic about the modeline.

> For me, mentioning it in the manual isn't very useful, because
> few people read the manual, fine as it may be :-)

Maintaining Tramp for ~15 years, I'm often feeling that writing
something into the manual is superfluous. For almost every explanation
there, I still get request to repeat the description in the mailing list
/ on stackexchange / you name it.

> João Távora

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-21  7:41                                   ` Michael Albinus
@ 2018-11-21 12:26                                     ` Michael Albinus
  2018-11-21 12:44                                     ` Filipp Gunbin
  2018-11-21 14:44                                     ` John Shahid
  2 siblings, 0 replies; 21+ messages in thread
From: Michael Albinus @ 2018-11-21 12:26 UTC (permalink / raw)
  To: João Távora
  Cc: Eli Zaretskii, Paul Eggert, Stefan Monnier, emacs-devel

Michael Albinus <michael.albinus@gmx.de> writes:

> The command `tramp-cleanup-connection' closes any background session for
> a Tramp connection, including removing cached passwords. Maybe we shall
> call this for sudo/su methods automatically after a given timeout, like
> the password expiration for sudo in a terminal. 5 minutes seem to be a
> sensible value to me.

I give it a try. I've pushed a change to the master branch, which
expires sudo and doas background sessions after 5 minutes. Check the
Tramp manual for details.

Let's see how it goes.

>> João

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-21  7:41                                   ` Michael Albinus
  2018-11-21 12:26                                     ` Michael Albinus
@ 2018-11-21 12:44                                     ` Filipp Gunbin
  2018-11-21 12:51                                       ` Michael Albinus
  2018-11-21 14:44                                     ` John Shahid
  2 siblings, 1 reply; 21+ messages in thread
From: Filipp Gunbin @ 2018-11-21 12:44 UTC (permalink / raw)
  To: Michael Albinus
  Cc: Eli Zaretskii, Paul Eggert, emacs-devel, João Távora,
	Stefan Monnier

On 21/11/2018 08:41 +0100, Michael Albinus wrote:

> Maybe we shall call this for sudo/su methods automatically after a
> given timeout, like the password expiration for sudo in a terminal. 5
> minutes seem to be a sensible value to me.

Yes, that'd be nice (for root).

Filipp



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-21 12:44                                     ` Filipp Gunbin
@ 2018-11-21 12:51                                       ` Michael Albinus
  0 siblings, 0 replies; 21+ messages in thread
From: Michael Albinus @ 2018-11-21 12:51 UTC (permalink / raw)
  To: Filipp Gunbin
  Cc: Eli Zaretskii, Paul Eggert, emacs-devel, João Távora,
	Stefan Monnier

Filipp Gunbin <fgunbin@fastmail.fm> writes:

Hi Filipp,

>> Maybe we shall call this for sudo/su methods automatically after a
>> given timeout, like the password expiration for sudo in a terminal. 5
>> minutes seem to be a sensible value to me.
>
> Yes, that'd be nice (for root).

I have just committed a patch to master, which does it for all "sudo"
connections. If you want it only for root, you could configure it as
such. See the Tramp manual.

> Filipp

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-21  7:41                                   ` Michael Albinus
  2018-11-21 12:26                                     ` Michael Albinus
  2018-11-21 12:44                                     ` Filipp Gunbin
@ 2018-11-21 14:44                                     ` John Shahid
  2018-11-21 14:52                                       ` Michael Albinus
  2 siblings, 1 reply; 21+ messages in thread
From: John Shahid @ 2018-11-21 14:44 UTC (permalink / raw)
  To: Michael Albinus
  Cc: Eli Zaretskii, Paul Eggert, emacs-devel, João Távora,
	Stefan Monnier


Michael Albinus <michael.albinus@gmx.de> writes:

> The command `tramp-cleanup-connection' closes any background session for
> a Tramp connection, including removing cached passwords. Maybe we shall
> call this for sudo/su methods automatically after a given timeout, like
> the password expiration for sudo in a terminal. 5 minutes seem to be a
> sensible value to me.

Warning, I am not familiar with the internals of tramp, so please
forgive my following comment if it doesn't make sense.

Is there a reason for doing a manual expiration instead of relying on
the default sudo behavior.  If tramp start a new sudo shell for example
to get file attributes, then sudo can take care of caching the password
or asking for it after the configured timeout.  That would consolidate
the configuration in one place (i.e. /etc/sudoers for the timeout) as
well as let users manage the cache (e.g. sudo -k when the user logs out)
the same way they do today.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-21 14:44                                     ` John Shahid
@ 2018-11-21 14:52                                       ` Michael Albinus
  2018-11-21 14:55                                         ` John Shahid
  0 siblings, 1 reply; 21+ messages in thread
From: Michael Albinus @ 2018-11-21 14:52 UTC (permalink / raw)
  To: John Shahid
  Cc: Eli Zaretskii, Paul Eggert, emacs-devel, João Távora,
	Stefan Monnier

John Shahid <jvshahid@gmail.com> writes:

Hi John,

> Is there a reason for doing a manual expiration instead of relying on
> the default sudo behavior.  If tramp start a new sudo shell for example
> to get file attributes, then sudo can take care of caching the password
> or asking for it after the configured timeout.  That would consolidate
> the configuration in one place (i.e. /etc/sudoers for the timeout) as
> well as let users manage the cache (e.g. sudo -k when the user logs out)
> the same way they do today.

The point is that Tramp (until now) keeps a session open forever. Tramp
doesn't "start a new sudo shell for example to get file attributes".
Therefore, there's no chance that sudo could ask for a password,
again. That's why the new mechanism interrupts the session after the
session timeout, and opening a new one depends on sudo's mechanism for
cached passwords.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-21 14:52                                       ` Michael Albinus
@ 2018-11-21 14:55                                         ` John Shahid
  2018-11-21 15:07                                           ` Michael Albinus
  0 siblings, 1 reply; 21+ messages in thread
From: John Shahid @ 2018-11-21 14:55 UTC (permalink / raw)
  To: Michael Albinus
  Cc: Eli Zaretskii, Paul Eggert, emacs-devel, João Távora,
	Stefan Monnier


Michael Albinus <michael.albinus@gmx.de> writes:

> John Shahid <jvshahid@gmail.com> writes:
>
> Hi John,
>
>> Is there a reason for doing a manual expiration instead of relying on
>> the default sudo behavior.  If tramp start a new sudo shell for example
>> to get file attributes, then sudo can take care of caching the password
>> or asking for it after the configured timeout.  That would consolidate
>> the configuration in one place (i.e. /etc/sudoers for the timeout) as
>> well as let users manage the cache (e.g. sudo -k when the user logs out)
>> the same way they do today.
>
> The point is that Tramp (until now) keeps a session open forever. Tramp
> doesn't "start a new sudo shell for example to get file attributes".
> Therefore, there's no chance that sudo could ask for a password,
> again. That's why the new mechanism interrupts the session after the
> session timeout, and opening a new one depends on sudo's mechanism for
> cached passwords.

That was the essence of my question.  What is stopping us from starting
a new session as needed instead of keeping one around forever ?



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: sudo:: method in tramp possible security issue
  2018-11-21 14:55                                         ` John Shahid
@ 2018-11-21 15:07                                           ` Michael Albinus
  0 siblings, 0 replies; 21+ messages in thread
From: Michael Albinus @ 2018-11-21 15:07 UTC (permalink / raw)
  To: John Shahid
  Cc: Eli Zaretskii, Paul Eggert, emacs-devel, João Távora,
	Stefan Monnier

John Shahid <jvshahid@gmail.com> writes:

Hi John,

> That was the essence of my question.  What is stopping us from starting
> a new session as needed instead of keeping one around forever ?

Performance. Opening a new sudo session every single information bit
would be awful slow.

We could set a command timeout for the sudo invocation, but nobody
guarantees that it happens while data are transferred.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Tramp sudoedit method (was: sudo:: method in tramp possible security issue)
       [not found]           ` <87ftvwyxh7.fsf@gmx.de>
@ 2018-12-16 15:24             ` Michael Albinus
  0 siblings, 0 replies; 21+ messages in thread
From: Michael Albinus @ 2018-12-16 15:24 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, João Távora, emacs-devel

Michael Albinus <michael.albinus@gmx.de> writes:

Hi,

>>> Right, but would it run emacs-lisp code with elevated privileges?
>>> Can't we just make a sudoedit method that just invokes the sudoedit
>>> program or "sudo -e"?  This is what I thought sudo:: did in the first place.
>>
>> All of Tramp basically works by starting an `sh` process (via ssh, sudo,
>> you name it) and then sending it commands to retrieve meta-info about
>> files, contents of file, etc...
>
> Right. But maybe we could offer a Tramp method "sudoedit", which behaves
> differently. If somebody opens "C-x C-f /sudoedit::/etc/passwd", Tramp
> starts in the background the process "env EDITOR=emacsclient sudoedit
> /etc/passwd" (plus fiddling with `server-start', password handling, and
> alike). Nothing else but just editing the file, and saving it, would be
> possible.
>
> Don't know what it means for implementation, because visiting a file is
> more than just calling `insert-file-contents'. Handlers for all the
> other magic file operations should return proper results, w/o running a
> shell under root permissions.

I gave it a try. I could make `insert-file-contents' and `write-region'
run based on emacsclient, but it wasn't mature enough. So I have
implemented the "sudoedit" method by applying a respective sudo call for
everything. It isn't using the sudoedit command any longer, but I kept
the method name, because it behaves similar. If it confuses people too
much, we could change the name.

Contrary to the "sudo" method, it is applicable only to the local host,
and it cannot be used in multi-hop methods. Furthermore, it has a worse
performance than "sudo", especially for large directories like "/etc",
visited with `dired'. Maybe this could be improved by a native
implementation of `directory-files-and-attributes' and `insert-directory'.

But it keeps the security promise, not to run a shell in the background
with other user credentials, which could be attacked by malicious code.

Pushed to master. What do people think?

>>         Stefan

Best regards, Michael.



^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-12-16 15:24 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CALDnm52oYOV_kPWZH62ub8seM4uug-GH68ywgXGJfDAbG7_4Xg@mail.gmail.com>
     [not found] ` <87ftvwdcdw.fsf@gmx.de>
     [not found]   ` <CALDnm51fhzD48-40-t04KU9S19Lz8_sf8Y=pKZbnSHR+tbCgTQ@mail.gmail.com>
     [not found]     ` <87bm6kdb68.fsf@gmx.de>
     [not found]       ` <CALDnm53tX4vFz4CH=P27_dUt=9dXWMrE7xdTz3iZuqybsndygg@mail.gmail.com>
     [not found]         ` <jwvmuq4zqsz.fsf-monnier+emacs@gnu.org>
     [not found]           ` <CALDnm52moQthtMvSEw2ELWjZg3yJ8jypG=TBLgSsdr6R8ru0Aw@mail.gmail.com>
     [not found]             ` <87bm6kyxc3.fsf@gmx.de>
     [not found]               ` <CALDnm5211UDT_pW-HzgnRb5dQtnCZSgN+GRGHYM1hPVAjBWavA@mail.gmail.com>
     [not found]                 ` <87k1l83yd3.fsf@gmx.de>
     [not found]                   ` <CALDnm52KU0wNd3Sd-7m78JrPcLsiNuZ8hQxcTs3PgDNG7+0a0g@mail.gmail.com>
     [not found]                     ` <87o9ajvost.fsf@gmx.de>
2018-11-20 14:13                       ` sudo:: method in tramp possible security issue João Távora
2018-11-20 21:14                         ` Paul Eggert
2018-11-20 21:18                           ` Stefan Monnier
2018-11-20 21:25                             ` Paul Eggert
2018-11-20 21:44                               ` Stefan Monnier
2018-11-20 22:06                               ` Michael Albinus
2018-11-20 22:27                                 ` João Távora
2018-11-20 23:12                                   ` Stefan Monnier
2018-11-21  7:41                                   ` Michael Albinus
2018-11-21 12:26                                     ` Michael Albinus
2018-11-21 12:44                                     ` Filipp Gunbin
2018-11-21 12:51                                       ` Michael Albinus
2018-11-21 14:44                                     ` John Shahid
2018-11-21 14:52                                       ` Michael Albinus
2018-11-21 14:55                                         ` John Shahid
2018-11-21 15:07                                           ` Michael Albinus
2018-11-20 22:16                           ` Michael Albinus
2018-11-20 22:30                         ` Michael Albinus
2018-11-20 22:54                           ` João Távora
2018-11-21  7:49                             ` Michael Albinus
     [not found]           ` <87ftvwyxh7.fsf@gmx.de>
2018-12-16 15:24             ` Tramp sudoedit method (was: sudo:: method in tramp possible security issue) Michael Albinus

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.