Suggested enhancement: allow emacsclient to send arbitrary LISP expressions

Suggested enhancement: allow emacsclient to send arbitrary LISP expressions

[tytso]

This patch allows the emacs client to pass in an arbitrary lisp
expression, which will be executed by the emacs server process.  

For example: emacsclient '(make-frame-on-display "localhost:12")'

The patch is fully backwards compatible with older versions of
emacsclient, since emacsclient always sends absolute pathnames to the
emacs server.  The leading open parenthesis is used to indicate that
string is a lisp expression which should be evalulated.

On the emacsclient side, currently we simply use a leading open
parenthesis to indicate that the argument is a LISP expression.  This
means that attempts to edit a file that begins with an open expression
will now be interpreted as a LISP expression.  In practice I don't
expect this to be a problem, although if this is a major concern, a
option such as --lisp could be added to emacsclient to inhibit adding
the absolute pathname to the argument, instead of always treating
arguments that begin with an open parenthesis as LISP expressions.

Note: If I were designing the emacsclient/emacsserver protocol all
over again from scratch, emacsclient would always pass s-expressions
which would be intepreted as lisp expressions by the emacs process.
This would eliminate the current kludgy, ad-hoc quoting that
emacsclient uses.

						- Ted


--- emacsclient.c	2003/01/02 19:33:41	1.2
+++ emacsclient.c	2003/01/02 19:34:43
@@ -326,7 +326,7 @@
 	  if (*p != 0)
 	    fprintf (out, "%s/", quote_file_name (cwd));
 	}
-      else if (*argv[i] != '/')
+      else if (*argv[i] != '/' && *argv[i] != '(')
 	fprintf (out, "%s/", quote_file_name (cwd));
 
       fprintf (out, "%s ", quote_file_name (argv[i]));
--- /home/tytso/em/server.el	2003-01-02 15:21:03.000000000 -0500
+++ /usr/share/emacs/20.7/lisp/server.el	1999-03-12 19:20:25.000000000 -0500
@@ -213,8 +213,7 @@
 	  (coding-system (and default-enable-multibyte-characters
 			      (or file-name-coding-system
 				  default-file-name-coding-system)))
-	  client nowait 
-	  (result "error in lisp expression")
+	  client nowait
 	  (files nil)
 	  (lineno 1))
       ;; Remove this line from STRING.
@@ -226,24 +225,6 @@
 	      (setq request (substring request (match-end 0)))
 	      (setq client (list (substring request 0 (string-match " " request))))
 	      (setq request (substring request (match-end 0)))
-	      (if (string-match "^(" request)
-		  (let ((q (read-from-string request))
-			arg
-			(pos 0))
-		    (setq arg (substring request 0 (cdr q)))
-		    (setq request (substring request (cdr q)))
-		    (while (string-match "&." arg pos)
-		      (setq pos (1+ (match-beginning 0)))
-		      (let ((nextchar (aref arg pos)))
-			(cond ((= nextchar ?&)
-			       (setq arg (replace-match "&" t t arg)))
-			      ((= nextchar ?-)
-			       (setq arg (replace-match "-" t t arg)))
-			      (t
-			       (setq arg (replace-match " " t t arg))))))
-		    (unwind-protect
-			(setq result (eval (car (read-from-string arg)))))
-		    ))
 	      (while (string-match "[^ ]+ " request)
 		(let ((arg
 		       (substring request (match-beginning 0) (1- (match-end 0))))
@@ -275,20 +256,14 @@
 			    (cons (list arg lineno)
 				  files))
 		      (setq lineno 1)))))
-	      (if files
-		  (progn 
-		    (server-visit-files files client nowait)
-		    ;; CLIENT is now a list (CLIENTNUM BUFFERS...)
-		    (or nowait
-			(setq server-clients (cons client server-clients)))
-		    (server-switch-buffer (nth 1 client))
-		    (run-hooks 'server-switch-hook)
-		    (message (substitute-command-keys
-			      "When done with a buffer, type \\[server-edit]")))
-		(let ((logmsg (format "Close: %s %s\n" (car client) 
-				      (prin1-to-string result))))
-		  (send-string server-process logmsg)
-		  (server-log logmsg))))))))
+	      (server-visit-files files client nowait)
+	      ;; CLIENT is now a list (CLIENTNUM BUFFERS...)
+	      (or nowait
+		  (setq server-clients (cons client server-clients)))
+	      (server-switch-buffer (nth 1 client))
+	      (run-hooks 'server-switch-hook)
+	      (message (substitute-command-keys
+			"When done with a buffer, type \\[server-edit]")))))))
   ;; Save for later any partial line that remains.
   (setq server-previous-string string))

Re: expressions

[Richard Stallman]

This is a useful feature.  I have a security question for the people
on emacs-devel who support non-GNU-like platforms.  server.el sets up
mode 0700 for the socket.  Is that sufficient security, on all
platforms, to make it safe to install this feature?

      In practice I don't
    expect this to be a problem, although if this is a major concern, a
    option such as --lisp could be added to emacsclient to inhibit adding
    the absolute pathname to the argument, instead of always treating
    arguments that begin with an open parenthesis as LISP expressions.

Perhaps there should be an option --file that forces an argument to be
treated as a file even if it starts with open-paren.  Scripts could
use that; meanwhile, interactive use would remain maximally convenient.

Re: expressions

[Bob Halley]

Richard Stallman <rms@gnu.org> writes:

> This is a useful feature.  I have a security question for the people
> on emacs-devel who support non-GNU-like platforms.  server.el sets up
> mode 0700 for the socket.  Is that sufficient security, on all
> platforms, to make it safe to install this feature?

No, it is not safe.  On Solaris, at least as recent as Solaris 8, the
file permissions on UNIX domain sockets are totally ignored.  This may
be true on other platforms as well, though many platforms "do the
right thing" and check file permissions.

The standard way of dealing with this portably is to create a
directory for the socket filename to live in, and control access via
permissions on the directory.

/Bob

Re: expressions

[Kim F. Storm]

Bob Halley <halley@play-bow.org> writes:

> Richard Stallman <rms@gnu.org> writes:
> 
> > This is a useful feature.  I have a security question for the people
> > on emacs-devel who support non-GNU-like platforms.  server.el sets up
> > mode 0700 for the socket.  Is that sufficient security, on all
> > platforms, to make it safe to install this feature?
> 
> No, it is not safe.  On Solaris, at least as recent as Solaris 8, the
> file permissions on UNIX domain sockets are totally ignored.  This may
> be true on other platforms as well, though many platforms "do the
> right thing" and check file permissions.

Currently, server.el places the socket in /tmp/esrv<UID>-<HOST>

I've always wondered why the socket wasn't placed in $HOME.

> 
> The standard way of dealing with this portably is to create a
> directory for the socket filename to live in, and control access via
> permissions on the directory.


Maybe we should create a directory "~/.esrv" (if it doesn't exist),
check that it is owned by the current user (fail to start server if
not), make it 700, and then create the server socket (e.g. named
`server') in that directory.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: expressions

[Bob Halley]

storm@cua.dk (Kim F. Storm) writes:

> Maybe we should create a directory "~/.esrv" (if it doesn't exist),
> check that it is owned by the current user (fail to start server if
> not), make it 700, and then create the server socket (e.g. named
> `server') in that directory.

This sounds like a good idea to me, though I'd keep the filename
something like server-<hostname>, in case the user shares their home
directory over NFS (or some other network filesystem).

/Bob

Re: expressions

[Richard Stallman]

    The standard way of dealing with this portably is to create a
    directory for the socket filename to live in, and control access via
    permissions on the directory.

We could change both server.el and emacsclient to do this, I guess.
It would be an incompatible change, and some users who try to use
an older emacsclient with a newer Emacs would lose.

We could change emacsclient to try the new method first and then
try the old method.  Then it would work with old Emacs versions.
Is there a way to make server.el support both methods?

    Currently, server.el places the socket in /tmp/esrv<UID>-<HOST>

This way, different user names with the same UID can interoperate.

    I've always wondered why the socket wasn't placed in $HOME.

That way, they could not interoperate.

The technique of making a directory to regulate access can be used
under /tmp, I guess.  Would these directories in /tmp get cleared out
when /tmp is cleared?  If not, that would be a problem.

Re: expressions

[chad]

Bob Halley <halley@play-bow.org> wrote:

> > Maybe we should create a directory "~/.esrv" (if it doesn't exist),
> > check that it is owned by the current user (fail to start server if
> > not), make it 700, and then create the server socket (e.g. named
> > `server') in that directory.
> 
> This sounds like a good idea to me, though I'd keep the filename
> something like server-<hostname>, in case the user shares their home
> directory over NFS (or some other network filesystem).

I would expect that the reason /tmp was chosen was the inability of some
systems to create sockets in NFS/AFS/SFS/Coda/whatever remote filesystem
that the suer might be using for $HOME.  I suspect that this might still
be a problem.  The ssh/openssh technique involves creating a directory
in $TMPDIR named ssh-$unique (with other refinements that I think do not
apply).  Perhaps this would be a safer strategy?

chad

Re: expressions

[Bob Halley]

chad <y@mit.edu> writes:

> I would expect that the reason /tmp was chosen was the inability of some
> systems to create sockets in NFS/AFS/SFS/Coda/whatever remote filesystem
> that the suer might be using for $HOME.  I suspect that this might still
> be a problem.  The ssh/openssh technique involves creating a directory
> in $TMPDIR named ssh-$unique (with other refinements that I think do not
> apply).  Perhaps this would be a safer strategy?

Good point, though we don't want a unique name like SSH since we're
trying to rendezvous on the name.  I tested creating sockets on an NFS
mounted directory with Solaris and it worked, but I have no idea what
happens with other platforms or filesystem types.

So, if we're using /tmp, and want to keep the UID in there for reasons
RMS explained in another message, maybe we want something like

        /tmp/.eserv-<UID>-<HOSTNAME>/server

which should cover us against pretty much everything :).

/Bob

Re: expressions

[Bob Halley]

Richard Stallman <rms@gnu.org> writes:

> The technique of making a directory to regulate access can be used
> under /tmp, I guess.  Would these directories in /tmp get cleared out
> when /tmp is cleared?  If not, that would be a problem.

Depends on the OS.  Many other applications use this "directory in
tmp" approach, so we don't have to worry about being an unusual case.

Re: expressions

[Kim F. Storm]

Bob Halley <halley@play-bow.org> writes:

> chad <y@mit.edu> writes:
> 
> > I would expect that the reason /tmp was chosen was the inability of some
> > systems to create sockets in NFS/AFS/SFS/Coda/whatever remote filesystem
> > that the suer might be using for $HOME.  

On a disk-less work station, even /tmp may be on a remote filesystem...

>                                            I suspect that this might still
> > be a problem.  The ssh/openssh technique involves creating a directory
> > in $TMPDIR named ssh-$unique (with other refinements that I think do not
> > apply).  Perhaps this would be a safer strategy?
> 
> Good point, though we don't want a unique name like SSH since we're
> trying to rendezvous on the name.  I tested creating sockets on an NFS
> mounted directory with Solaris and it worked, but I have no idea what
> happens with other platforms or filesystem types.
> 
> So, if we're using /tmp, and want to keep the UID in there for reasons
> RMS explained in another message, 

I didn't see that message...  Was it copied to the list?


>                                   maybe we want something like
> 
>         /tmp/.eserv-<UID>-<HOSTNAME>/server
> 
> which should cover us against pretty much everything :).

In that case, I would suggest naming it something like:

        /tmp/.emacs-UID/server-HOSTNAME

This would give us a standard place in /tmp/.emacs-UID for other
per-user stuff that we may need in the future.  The directory name
should be put in a new lisp variable, e.g. user-private-directory,
which is bound on start-up.

(defcustom user-private-directory (expand-file-name (format "/tmp/.emacs-%d" (user-uid)))
  "*Directory for user private files."
  :set (lambda (...)
        ... create & set modes on the directory if it doesn't exist ...)
  ...)


server.el could then use that variable to create the name of the socket.

(defvar server-socket-name
  (format "%s/server-%s" user-private-directory
	  (substring (system-name) 0 (string-match "\\." (system-name)))))

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: expressions

[Kai Großjohann]

storm@cua.dk (Kim F. Storm) writes:

> Maybe we should create a directory "~/.esrv" (if it doesn't exist),
> check that it is owned by the current user (fail to start server if
> not), make it 700, and then create the server socket (e.g. named
> `server') in that directory.

How about using ~/.emacs.d?  How about doing chmod go-rwx on that
directory?  (I think go-rwx are good permissions for the directory
anyway.)
-- 
Ambibibentists unite!

Re: expressions

[Theodore Ts'o]

On Sun, Jan 05, 2003 at 11:55:49AM -0800, Bob Halley wrote:
> No, it is not safe.  On Solaris, at least as recent as Solaris 8, the
> file permissions on UNIX domain sockets are totally ignored.  This may
> be true on other platforms as well, though many platforms "do the
> right thing" and check file permissions.

Wow.  I didn't realize Solaris was that broken/stupid.  

Although granted adding the ability to server.el to evaluate arbitrary
LISP expression becomes a security disaster, even without that, users
who have enable-local-variables set to t and who run server.el are in
trouble --- since an attacker can ask emacs to open an arbitrary file
created by the attacker, and the local variables in the file can
contain arbitrary lisp expressions.

> The standard way of dealing with this portably is to create a
> directory for the socket filename to live in, and control access via
> permissions on the directory.

Yes, although this causes the backwards compatibility problems
mentioned in other mail messages.  One solution would be to allow the
location to be configurable via an emacs-server variable to be one of
the following possibilities, with emacsclient searching all of these
locations:

	/tmp/esrv<uid>-hostname		
		(for backwards compatibility with old emacsclient binaries)

	/tmp/esrv-<uid>/hostname	
		(for homedirs using filesystems that don't support sockets)

	~/.eserv/<hostname>
		(to avoid tmp cleaners)

(BTW, Note that one way of dealing with the temp cleaner problem would
be to have the emacsserver binary periodically wake up every so often,
and try touching the containing directory and socket.  If they have
disappeared, the binary could recreate them.)


Another thought --- if we're willing to bag backwards compatibility
altogether, a friend recently pointed me at the existence of another
package, gnuserv/gnuclient, which does most of what I had wanted.  It
is completely non-backwards compatible with emacslcleint --- indeed,
because it uses the same variable and function names as server.el, it
cannot be loaded at the same time as server.el.  However, it uses
s-expressions for evaluating lisp expressions as well as for passing
filenames to be edited, so it is cleaner than the existing
emacsclient/emacsserver.  It also has security features built in, and
can listen on TCP ports, so you use it across the network.

						- Ted

Re: expressions

[Luc Teirlinck]

Kim Storm wrote:
   
   I didn't see that message...  Was it copied to the list?

There seems to be a substantial delay (more than a day), because the
archives seem to be in the process of being updated.  (That is what I
guess.)  Some of yesterday morning's messages have not been posted
yet.

Sincerely,

Luc.

Re: expressions

[Richard Stallman]

    Although granted adding the ability to server.el to evaluate arbitrary
    LISP expression becomes a security disaster, even without that, users
    who have enable-local-variables set to t and who run server.el are in
    trouble --- since an attacker can ask emacs to open an arbitrary file
    created by the attacker, and the local variables in the file can
    contain arbitrary lisp expressions.

That's true, but enable-local-variables is nil by default.
So this change would really make the problem considerably more
likely.

    (BTW, Note that one way of dealing with the temp cleaner problem would
    be to have the emacsserver binary periodically wake up every so often,
    and try touching the containing directory and socket.  If they have
    disappeared, the binary could recreate them.)

I thin the current code has the same problem--it makes no attempt
to cope if you clean /tmp.  I guess it would be nice to add this
facility to try to cope.

I was worried about whether clearing /tmp might fail to delete
subdirs.  ISTR there was such a problem in the past.  Maybe not now.

    Another thought --- if we're willing to bag backwards compatibility
    altogether, a friend recently pointed me at the existence of another
    package, gnuserv/gnuclient, which does most of what I had wanted.

I think there are problems getting papers for that.
I don't remember the details, though.  Maybe we could implement
whatever security feature it has.  How does that work?

Re: Suggested enhancement: allow emacsclient to send arbitrary LISP expressions

[Kim F. Storm]

tytso@mit.edu writes:

> This patch allows the emacs client to pass in an arbitrary lisp
> expression, which will be executed by the emacs server process.  
> 
> For example: emacsclient '(make-frame-on-display "localhost:12")'


Stepping back to the beginning of this thread, this is actually
already implemented in CVS emacs:

        emacsclient --eval '(make-frame-on-display "localhost:12")'

There's also a new --display option, so you may even write:

        emacsclient --eval '(make-frame)' --display "localhost:12"


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: expressions

[Andreas Schwab]

Richard Stallman <rms@gnu.org> writes:

|>     The standard way of dealing with this portably is to create a
|>     directory for the socket filename to live in, and control access via
|>     permissions on the directory.
|> 
|> We could change both server.el and emacsclient to do this, I guess.
|> It would be an incompatible change, and some users who try to use
|> an older emacsclient with a newer Emacs would lose.

I don't think this is a problem, since emacsclient and emacs should
always be in sync.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Suggested enhancement: allow emacsclient to send arbitrary LISP expressions

[Richard Stallman]

    Stepping back to the beginning of this thread, this is actually
    already implemented in CVS emacs:

	    emacsclient --eval '(make-frame-on-display "localhost:12")'

Then it would appear we have a serious security problem already,
on some systems.

I will comment out this feature; when someone makes the changes
necessary to make it safe, we can turn it back on.

Re: expressions

[Stefan Monnier]

> 	/tmp/esrv<uid>-hostname
> 		(for backwards compatibility with old emacsclient binaries)

I actually have no idea why the hostname should be included.
After all, /tmp is not supposed to be shared among several machines
(i.e. it might be NFS-mounted but it should be dedicated to one
particular host).

> (BTW, Note that one way of dealing with the temp cleaner problem would
> be to have the emacsserver binary periodically wake up every so often,
> and try touching the containing directory and socket.  If they have
> disappeared, the binary could recreate them.)

I don't see why /tmp cleaning should be a problem.
It isn't for SSH and X11 and for Emacs until now.


	Stefan

Re: expressions

[Stefan Monnier]

> > > I would expect that the reason /tmp was chosen was the inability of some
> > > systems to create sockets in NFS/AFS/SFS/Coda/whatever remote filesystem
> > > that the suer might be using for $HOME.  
> 
> On a disk-less work station, even /tmp may be on a remote filesystem...

So what ?  The point is that if $HOME works, then /tmp works
as well, whereas the opposite is not true.


	Stefan

Re: Suggested enhancement: allow emacsclient to send arbitrary LISP expressions

[Theodore Ts'o]

On Thu, Jan 09, 2003 at 06:13:16PM -0500, Richard Stallman wrote:
>     Stepping back to the beginning of this thread, this is actually
>     already implemented in CVS emacs:
> 
> 	    emacsclient --eval '(make-frame-on-display "localhost:12")'

Great!  So it sounds like I don't need to send legal papers to the
FSF, since somoene else has implemented this rather obvious idea.

> Then it would appear we have a serious security problem already,
> on some systems.
> 
> I will comment out this feature; when someone makes the changes
> necessary to make it safe, we can turn it back on.

How about it making it configurable via a variable, which is nil by
default.  That way those of us who are running non-broken systems
where the permissions on unix-domain sockets are actually respected
can enjoy the functionality right away.

						- Ted

Re: Suggested enhancement: allow emacsclient to send arbitrary LISP expressions

[Richard Stallman]

    How about it making it configurable via a variable, which is nil by
    default.  That way those of us who are running non-broken systems
    where the permissions on unix-domain sockets are actually respected
    can enjoy the functionality right away.

I expect someone will fix the security problem pretty soon, by using a
subdirectory.  Once that is done, we can simply turn it back on.

I turned it off now just as insurance against forgetting about it.