Re: emacsclient startup messages

[Jim Porter <jporterbugs@gmail.com>]

On 11/5/2021 2:40 AM, Ulrich Mueller wrote:
>>>>>> On Fri, 05 Nov 2021, Eli Zaretskii wrote:
> 
>>> I have been applying the patch to my Ubuntu 20.04 builds for a couple
>>> of days and everything as worked well. Wouldn't it be a good time to
>>> put it into emacs-28?
> 
>> Not yet.
> 
>> Ulrich, any comments on this?  Including its importance for Emacs 28?
> 
> I've just tested with current master.
> 
> The patch breaks the workflow used with Gentoo's app-emacs/emacs-daemon
> package. That is, a persistent Emacs daemon started with its socket in
> ${TMPDIR}/emacs${UID}/server, and emacsclient started with XDG_*
> variables in its environment:
> 
>     $ emacsclient -t --alternate-editor=
>     emacsclient: can't find socket; have you started the server?
>     emacsclient: To start the server in Emacs, type "M-x server-start".
>     Another Emacs daemon is already running at process id 6084
>     Error: server did not start correctly
>     Error: Could not start the Emacs daemon
>     $

Why are you (or the Gentoo package) running with `--alternate-editor=' 
if a daemon was already started? If it's feasible to stop doing that, it 
would be very helpful. The compromise that I suggested was that if the 
alternate editor is unset, emacsclient will try both XDG_RUNTIME_DIR and 
TMPDIR, so this use-case should work with the patch, provided you can 
remove `--alternate-editor='.

> I haven't looked into the details of the code yet, but shouldn't the
> client first try to connect to an existing socket at both possible
> locations? And only start a new daemon when both attempts have failed?

No, this behavior opens users to symlink attacks if they call 
emacsclient without an Emacs daemon already running, which is why Emacs 
uses XDG_RUNTIME_DIR in the first place. Even the compromise in my patch 
isn't truly ideal: a user might normally run `emacs --daemon' on startup 
(with XDG_RUNTIME_DIR already set) and then call emacsclient *without* 
setting the alternate editor; if the daemon had been killed somehow, 
emacsclient will currently check TMPDIR, opening the user to a symlink 
attack.

I'm not 100% clear on how dangerous the symlink attack is, but if Emacs 
is going to try and prevent it, I think it should at least ensure that 
it's easy for users to set things up so that they're not vulnerable.

Strictly from a security perspective, Emacs 27 is better here: if 
XDG_RUNTIME_DIR is set when you call emacsclient, Emacs 27 will *only* 
look in the XDG_RUNTIME_DIR for a socket, not in TMPDIR. Thus, users 
with XDG_RUNTIME_DIR are never vulnerable. However, for the Gentoo case 
above, that requires unsetting XDG_RUNTIME_DIR before calling 
emacsclient. Doing it this way maintains Emacs' security against symlink 
attacks, since a user has to explicitly opt into looking in TMPDIR for a 
socket.

My compromise patch is the best I could come up with for a solution that 
a) protects users who run `emacs --daemon' on-demand via the "alternate 
editor" setting and b) allows the Gentoo case to work without having to 
unset XDG_RUNTIME_DIR. As mentioned, there's still a vulnerability here, 
but the patch should shrink the number of users who hit the vuln.

As for my personal opinion on this, I think it would be best to revert 
bug#33847 (which supports the Gentoo use-case). If necessary, we could 
provide a way to explicitly opt into the (insecure) bug#33847 behavior, 
e.g. by adding a --fall-back-to-tmpdir-sockets flag. I'm also ok with my 
compromise patch, even though it's not perfect.

I think something should be done here for Emacs 28, but given how close 
it is to release, I'm not sure what's the safest move. Hopefully the 
above explains the pros and cons of our options fairly clearly.

- Jim