unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: [Emacs-diffs] scratch/allow-custom-load-paths-in-elisp-flymake 4ef9711: Allow custom load paths in elisp's byte-compilation Flymake
       [not found] ` <20181204233601.273DD209DC@vcs0.savannah.gnu.org>
@ 2018-12-05  4:34   ` Stefan Monnier
  2018-12-05 15:14     ` João Távora
  2018-12-05 20:00     ` Glenn Morris
  0 siblings, 2 replies; 18+ messages in thread
From: Stefan Monnier @ 2018-12-05  4:34 UTC (permalink / raw)
  To: emacs-devel; +Cc: João Távora

> +(put 'elisp-flymake-byte-compile-load-path 'safe-local-variable
> +     (lambda (x) (and (listp x) (catch 'tag
> +                                  (dolist (path x t) (unless (stringp path)
> +                                                       (throw 'tag nil)))))))

Just reminded me: we have a serious problem w.r.t security of
flymake-mode in Elisp buffers: if someone enables flymake-mode for all
elisp-mode buffers (i.e. what I'd like to do by default), the mere act
of visiting an Elisp file means that file will be passed to byte-compile
which will happily execute some of the code within, running arbitrarily
dangerous code.


        Stefan



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

* Re: [Emacs-diffs] scratch/allow-custom-load-paths-in-elisp-flymake 4ef9711: Allow custom load paths in elisp's byte-compilation Flymake
  2018-12-05  4:34   ` [Emacs-diffs] scratch/allow-custom-load-paths-in-elisp-flymake 4ef9711: Allow custom load paths in elisp's byte-compilation Flymake Stefan Monnier
@ 2018-12-05 15:14     ` João Távora
  2018-12-05 20:00     ` Glenn Morris
  1 sibling, 0 replies; 18+ messages in thread
From: João Távora @ 2018-12-05 15:14 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> +(put 'elisp-flymake-byte-compile-load-path 'safe-local-variable
>> +     (lambda (x) (and (listp x) (catch 'tag
>> +                                  (dolist (path x t) (unless (stringp path)
>> +                                                       (throw 'tag nil)))))))
>
> Just reminded me: we have a serious problem w.r.t security of
> flymake-mode in Elisp buffers: if someone enables flymake-mode for all
> elisp-mode buffers (i.e. what I'd like to do by default), the mere act
> of visiting an Elisp file means that file will be passed to byte-compile
> which will happily execute some of the code within, running arbitrarily
> dangerous code.

True.  And that's why we don't enable flymake-mode automatically, and
possibly never will (unless we come up with a solution).  The fact that
it is so inherently dangerous is the reason that I considered adding a
safety spec to elisp-flymake-byte-compile-load-path at all, because
having flymake-mode in elisp is already quite dangerous it itself.

But it does make it slightly *more* dangerous, because you can visit a
file that would otherwise be safe, but be betrayed by a .dir-locals.el
that spoofs the load-path to run some attacker's code.  Not sure if this
is a reasonable scenario.

So should I push this to master with or without the safety spec?

João



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

* Re: [Emacs-diffs] scratch/allow-custom-load-paths-in-elisp-flymake 4ef9711: Allow custom load paths in elisp's byte-compilation Flymake
  2018-12-05  4:34   ` [Emacs-diffs] scratch/allow-custom-load-paths-in-elisp-flymake 4ef9711: Allow custom load paths in elisp's byte-compilation Flymake Stefan Monnier
  2018-12-05 15:14     ` João Távora
@ 2018-12-05 20:00     ` Glenn Morris
  2018-12-05 20:40       ` João Távora
  1 sibling, 1 reply; 18+ messages in thread
From: Glenn Morris @ 2018-12-05 20:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: João Távora, emacs-devel


>> +(put 'elisp-flymake-byte-compile-load-path 'safe-local-variable
>> +     (lambda (x) (and (listp x) (catch 'tag
>> +                                  (dolist (path x t) (unless (stringp path)
>> +                                                       (throw 'tag nil)))))))

AFAICS the above tests whether the value is valid, not whether it is safe.
This should probably be a risky-local-variable, like load-path is.
The default "." seems actively dangerous, in much the same way as having
"." in a shell's PATH is.



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

* Re: [Emacs-diffs] scratch/allow-custom-load-paths-in-elisp-flymake 4ef9711: Allow custom load paths in elisp's byte-compilation Flymake
  2018-12-05 20:00     ` Glenn Morris
@ 2018-12-05 20:40       ` João Távora
  2018-12-08 13:23         ` Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths) João Távora
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2018-12-05 20:40 UTC (permalink / raw)
  To: Glenn Morris; +Cc: Stefan Monnier, emacs-devel

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

On Wed, Dec 5, 2018, 20:00 Glenn Morris <rgm@gnu.org wrote:

>
> >> +(put 'elisp-flymake-byte-compile-load-path 'safe-local-variable
> >> +     (lambda (x) (and (listp x) (catch 'tag
> >> +                                  (dolist (path x t) (unless (stringp
> path)
> >> +                                                       (throw 'tag
> nil)))))))
>
> AFAICS the above tests whether the value is valid, not whether it is safe.
> This should probably be a risky-local-variable, like load-path is.
> The default "." seems actively dangerous, in much the same way as having
> "." in a shell's PATH is.
>

Glenn,

I'm aware of the difference between valid and safe. One of us is missing
something:

1. The new variable I introduce only has any effect when flymake-mode is
enabled in elisp buffers.
2. Currently, flymake-mode is pretty unsafe since it byte-compiles the file
which can run arbitrary compile-time code in that file.
3. It doesn't make sense for an attacker to use the new variable as a
file-local var since it's much easier to add some eval-when-compile instead
to the file itself.
4. The only attack I could envision is to set the var in
dir-locals.el, assuming the attacker had access to that file and not the
other .el files that live in that directory.

As i tried to explain, I added the validity spec to the variable, precisely
because I thought 4 was pretty far-fetched, and couldn't find any other
plausible scenario. Can you?

João

>

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

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

* Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-05 20:40       ` João Távora
@ 2018-12-08 13:23         ` João Távora
  2018-12-08 15:36           ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2018-12-08 13:23 UTC (permalink / raw)
  To: emacs-devel; +Cc: Glenn Morris, Stefan Monnier

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

> On Wed, Dec 5, 2018, 20:00 Glenn Morris <rgm@gnu.org wrote:
>
>  >> +(put 'elisp-flymake-byte-compile-load-path 'safe-local-variable
>  >> +     (lambda (x) (and (listp x) (catch 'tag
>  >> +                                  (dolist (path x t) (unless (stringp path)
>  >> +                                                       (throw 'tag nil)))))))
>
>  AFAICS the above tests whether the value is valid, not whether it is safe.
>  This should probably be a risky-local-variable, like load-path is.
>  The default "." seems actively dangerous, in much the same way as having
>  "." in a shell's PATH is.
>
> Glenn,
>
> As i tried to explain, I added the validity spec to the variable,
> precisely because I thought 4 was pretty far-fetched, and couldn't
> find any other plausible scenario. Can you?

Hello again,

In the absence of further comments I was going to push this change today
to master (before I discovered that I already did so some days ago,
inadvertently, when I was pushing another flymake-related change, so
sorry about that).

But I'd like to continue the discussion of elisp-flymake-byte-compile's
safety.  I think something should be done to address it, even if
flymake-mode never makes it into emacs-lisp-mode-hook.  

To illustrate the dimension of the problem, some time ago I was editing
an .el file that had some macros in it and macroexpansions of said
macros in it.  In the middle of writing the macro body, I wrote the list

   (delete-directory default-directory)

I never compiled this file or executed this form explicitly in any way.
I merely typed it out, in the wrong place at the wrong time :-) A few
minutes later I discovered that my project directory was completely
wiped out: elisp-flymake-byte-compile has deleted when to byte-compiling
my buffer for warnings.

Thanks to Emacs's buffers, git, and auto-save strategies, it was easy to
recover the lost directory, but obviously it could have been much more
serious that this...

So here's what could improve the situation:

1. Create a elisp-flymake-maybe-enable function that checks the buffer
   for top-level forms that _could_ make it unsafe for byte-compiling on
   the fly.  This would include, but not limited to, eval-and-compile,
   eval-when-compile, defmacro, cl-defmacro, any "unknown" top-level
   form.  This will generate a lot of false positives (positive meaning
   "unsafe") but perhaps it could be made to generate 0 false negatives
   and still successfully vet a good number of elisp files.

2. In elisp-flymake-byte-compile, disable a significant chunk of Emacs's
   system interface in the slave emacs, including file-system write
   access and network access.  Either pass a switch to the subprocess
   invocation or do the byte-compilation in a dynamic environment where
   most of these primitives are disabled, i.e. via cl-letf.  It would be
   nice to exempt `load` from this.

3. After 1. and/or 2. re-evaluate the relative safety of
   elisp-flymake-byte-compile-load-path

I think 2. would be easier to do.  Some elisp files checked this way
would possibly report false diagnostics, but at least it would be
slightly safer to enable flymake in elisp-mode.

João   



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-08 13:23         ` Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths) João Távora
@ 2018-12-08 15:36           ` Stefan Monnier
  2018-12-10  0:20             ` João Távora
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2018-12-08 15:36 UTC (permalink / raw)
  To: João Távora; +Cc: Glenn Morris, emacs-devel

> 1. Create a elisp-flymake-maybe-enable function that checks the buffer
>    for top-level forms that _could_ make it unsafe for byte-compiling on
>    the fly.  This would include, but not limited to, eval-and-compile,
>    eval-when-compile, defmacro, cl-defmacro, any "unknown" top-level
>    form.  This will generate a lot of false positives (positive meaning
>    "unsafe") but perhaps it could be made to generate 0 false negatives
>    and still successfully vet a good number of elisp files.

I was thinking that we can probably do it without a separate check:
In bytecomp.el, when working in "flymake" mode (a mode in which the
output bytecode is not actually needed) we'd treat eval-when/and-compile
just like `progn`, and we'd mark some other macros as "unsafe"
(in which case we'd treat the corresponding calls as if they expanded
to nil) and when we see a defmacro, we use `unsafep` to decide whether
that macro should be treated as unsafe (so we could still macro expand
locally defined macros as long as they're simple enough).

Also, in that mode, we'd likely skip byte-opt altogether as well as
compiler macros.

The most obvious remaining holes there would be macros defined by
installed packages and whose expansion includes execution of some of its
arguments (cl-eval-when being the most obvious one, but there are many
more subtle ones).


        Stefan



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-08 15:36           ` Stefan Monnier
@ 2018-12-10  0:20             ` João Távora
  2018-12-10  2:22               ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2018-12-10  0:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Glenn Morris, emacs-devel

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> 1. Create a elisp-flymake-maybe-enable function that checks the buffer
>>    for top-level forms that _could_ make it unsafe for byte-compiling on
>>    the fly.  This would include, but not limited to, eval-and-compile,
>>    eval-when-compile, defmacro, cl-defmacro, any "unknown" top-level
>>    form.  This will generate a lot of false positives (positive meaning
>>    "unsafe") but perhaps it could be made to generate 0 false negatives
>>    and still successfully vet a good number of elisp files.
>
> I was thinking that we can probably do it without a separate check:

You mean do it directly in elisp-flymake--batch-compile-for-flymake?
Yeah, that's definitely a good idea.

> In bytecomp.el, when working in "flymake" mode (a mode in which the
> output bytecode is not actually needed) we'd treat eval-when/and-compile
> just like `progn`, and we'd mark some other macros as "unsafe"
> (in which case we'd treat the corresponding calls as if they expanded
> to nil) and when we see a defmacro, we use `unsafep` to decide whether
> that macro should be treated as unsafe

Trying to parse this... You mean when we see a call to a macro, not when
we see a `defmacro' top-level form, right?

> (so we could still macro expand
> locally defined macros as long as they're simple enough).

> Also, in that mode, we'd likely skip byte-opt altogether as well as
> compiler macros.

OK. What's byte-opt BTW? Optimization?  

> The most obvious remaining holes there would be macros defined by
> installed packages and whose expansion includes execution of some of its
> arguments (cl-eval-when being the most obvious one, but there are many
> more subtle ones).

So basically, in your proposal, package authors would use sth. like
(declare (flymake-safe t)) in their defmacros?  But then we would have
to prompt the user to accept or reject these marks right?

I don't think I completely understood your idea: what about, for
example, eglot.el's macrology that checks LSP interface destructuring at
compile-time?  There are some eval-when-compile's there right now:

  (eval-and-compile
    (defvar eglot--lsp-interface-alist
      `(
        (CodeAction (:title) (:kind :diagnostics :edit :command))
        (Command (:title :command) (:arguments))
        ...)
        "Alist of (INTERFACE REQUIRED-KEYS OPTIONAL-KEYS)"))

And then

   (eglot--dbind ((Command) title not-really-a-key) some-lsp-object
     (do-something-with title not-really-a-key))

gives me a nice warning that not-really-a-key isn't in the "Command"
interface.  How would that work in your new elisp-flymake-byte-compile?

Also, what do you think of my option 2 to disable most of the system
interface when flymaking?

João



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-10  0:20             ` João Távora
@ 2018-12-10  2:22               ` Stefan Monnier
  2018-12-10 23:17                 ` João Távora
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2018-12-10  2:22 UTC (permalink / raw)
  To: João Távora; +Cc: Glenn Morris, emacs-devel

>>> 1. Create a elisp-flymake-maybe-enable function that checks the buffer
>>>    for top-level forms that _could_ make it unsafe for byte-compiling on
>>>    the fly.  This would include, but not limited to, eval-and-compile,
>>>    eval-when-compile, defmacro, cl-defmacro, any "unknown" top-level
>>>    form.  This will generate a lot of false positives (positive meaning
>>>    "unsafe") but perhaps it could be made to generate 0 false negatives
>>>    and still successfully vet a good number of elisp files.
>>
>> I was thinking that we can probably do it without a separate check:
> You mean do it directly in elisp-flymake--batch-compile-for-flymake?
> Yeah, that's definitely a good idea.

I'm saying "without another traversal of the code".
I.e. elisp-flymake--batch-compile-for-flymake would just call the
byte-compiler in a different way (either via a new entry-point, or by
let-binding some new variable) to cause it to be more careful (and not
worry about generating correct&efficient code).

>> In bytecomp.el, when working in "flymake" mode (a mode in which the
>> output bytecode is not actually needed) we'd treat eval-when/and-compile
>> just like `progn`, and we'd mark some other macros as "unsafe"
>> (in which case we'd treat the corresponding calls as if they expanded
>> to nil) and when we see a defmacro, we use `unsafep` to decide whether
>> that macro should be treated as unsafe
>
> Trying to parse this... You mean when we see a call to a macro, not when
> we see a `defmacro' top-level form, right?

Right.

>> (so we could still macro expand
>> locally defined macros as long as they're simple enough).
>
>> Also, in that mode, we'd likely skip byte-opt altogether as well as
>> compiler macros.
>
> OK. What's byte-opt BTW? Optimization?  

See lisp/emacs-lisp/byte-opt.el
It's two byte-optimizers: one applied on the Elisp code after
macro-expansion, and another applied on the byte-code.

>> The most obvious remaining holes there would be macros defined by
>> installed packages and whose expansion includes execution of some of its
>> arguments (cl-eval-when being the most obvious one, but there are many
>> more subtle ones).
>
> So basically, in your proposal, package authors would use sth. like
> (declare (flymake-safe t)) in their defmacros?

The macros defined in the flymake'd file wouldn't need any declaration:
they'd all be treated as suspicious and checked via `unsafep`.

For the macros defined in already-loaded packages, I'm not sure
what would be the better option between a whitelist (such as the `flymake-safe`
declaration above) or a blacklist or something in-between.

> But then we would have to prompt the user to accept or reject these
> marks right?

Since they're in the files we already trust I don't think that's needed.

> I don't think I completely understood your idea: what about, for
> example, eglot.el's macrology that checks LSP interface destructuring at
> compile-time?  There are some eval-when-compile's there right now:
>
>   (eval-and-compile
>     (defvar eglot--lsp-interface-alist
>       `(
>         (CodeAction (:title) (:kind :diagnostics :edit :command))
>         (Command (:title :command) (:arguments))
>         ...)
>         "Alist of (INTERFACE REQUIRED-KEYS OPTIONAL-KEYS)"))
>
> And then
>
>    (eglot--dbind ((Command) title not-really-a-key) some-lsp-object
>      (do-something-with title not-really-a-key))
>
> gives me a nice warning that not-really-a-key isn't in the "Command"
> interface.  How would that work in your new elisp-flymake-byte-compile?

It would only work once `eglot--dbind` is defined by a pakage in
`load-path` (and after that package defined eglot--lsp-interface-alist).

But not if you just open eglot.el.

> Also, what do you think of my option 2 to disable most of the system
> interface when flymaking?

Providing ways to run Elisp in a confined environment would be useful in
various circumstances, but it's non-trivial.


        Stefan



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-10  2:22               ` Stefan Monnier
@ 2018-12-10 23:17                 ` João Távora
  2018-12-11 14:03                   ` Stefan Monnier
  2018-12-11 19:30                   ` Sandboxing (was: Safety of elisp-flymake-byte-compile) Stefan Monnier
  0 siblings, 2 replies; 18+ messages in thread
From: João Távora @ 2018-12-10 23:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Glenn Morris, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I'm saying "without another traversal of the code".
> I.e. elisp-flymake--batch-compile-for-flymake would just call the
> byte-compiler in a different way (either via a new entry-point, or by
> let-binding some new variable) to cause it to be more careful (and not
> worry about generating correct&efficient code).

OK. Got it.

> The macros defined in the flymake'd file wouldn't need any declaration:
> they'd all be treated as suspicious and checked via `unsafep`.
>
> For the macros defined in already-loaded packages, I'm not sure
> what would be the better option between a whitelist (such as the `flymake-safe`
> declaration above) or a blacklist or something in-between.

Hmmm.  The way I'm understanding this now, I'd favor a blacklist,
because it seems that you are proposing that anything that is loaded in
the main Emacs (call it "host" or "master"), is already safe,
except for some known misbehaving macros.

>> But then we would have to prompt the user to accept or reject these
>> marks right?
>
> Since they're in the files we already trust I don't think that's
> needed.

Trust a file = loaded in the host Emacs - some known exceptions, right?

>> I don't think I completely understood your idea: what about, for
>> example, eglot.el's macrology that checks LSP interface destructuring at
>> compile-time?  There are some eval-when-compile's there right now:
>>
>>   (eval-and-compile
>>     (defvar eglot--lsp-interface-alist
>>       `(
>>         (CodeAction (:title) (:kind :diagnostics :edit :command))
>>         (Command (:title :command) (:arguments))
>>         ...)
>>         "Alist of (INTERFACE REQUIRED-KEYS OPTIONAL-KEYS)"))
>>
>> And then
>>
>>    (eglot--dbind ((Command) title not-really-a-key) some-lsp-object
>>      (do-something-with title not-really-a-key))
>>
>> gives me a nice warning that not-really-a-key isn't in the "Command"
>> interface.  How would that work in your new elisp-flymake-byte-compile?
>
> It would only work once `eglot--dbind` is defined by a pakage in
> `load-path` (and after that package defined
> eglot--lsp-interface-alist).
>
> But not if you just open eglot.el.


So as soon as I load eglot.el, or eglot.elc in the host Emacs, it would
start working?  If so, I could live with that.  Until it starts working
it could issue some diagnostics saying "this macro is not known to be
safe, so not checking".

Now, how would you transmit this information about safe and unsafe
macros to from the host Emacs to the slave byte-compiling Emacs which is
a separate process?  Via command-line parameters, an .el generated on
the fly (we already do this for the flymake'd file, btw), or something
else?
>
>> Also, what do you think of my option 2 to disable most of the system
>> interface when flymaking?
>
> Providing ways to run Elisp in a confined environment would be useful in
> various circumstances, but it's non-trivial.

I can understand that, but I'm not proposing a fully hermetic sandbox
just something that ameliorates the problem.

At least, the way I understand your solution for the "safe/unsafe" macro
problem it still doesn't seem to fix the fact that as soon as I type
"(launch-nuke)" into some already loaded macro in eglot.el, nukes are
potentially going to be launched by some unsuspecting macro-expansion
down below.  So I'd like at least to have a slave emacs with main
nuke-launching facilities disabled, even if there are almost certainly
ways to circumvent that.

João



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-10 23:17                 ` João Távora
@ 2018-12-11 14:03                   ` Stefan Monnier
  2018-12-14 12:00                     ` João Távora
  2018-12-11 19:30                   ` Sandboxing (was: Safety of elisp-flymake-byte-compile) Stefan Monnier
  1 sibling, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2018-12-11 14:03 UTC (permalink / raw)
  To: emacs-devel

> Trust a file = loaded in the host Emacs - some known exceptions, right?

I think rather than "loaded in the host Emacs" it'll have to be "in
load-path", because Emacs is generally very liberal about automatically
loading files from load-path, so anything in load-path is
pretty much already trusted.

[ I think someone™ should sit down and think hard about this in general
  (not only in the context of flymake), because "in load-path" is not as
  clearly defined as we might think, since we also sometimes load files
  from subdirectories within load-path.
  And hopefully, this someone should be well intentioned ;-)  ]

> So as soon as I load eglot.el, or eglot.elc in the host Emacs, it would
> start working?

Right, or even as soon as eglot is in your load-path.

> If so, I could live with that.  Until it starts working it could issue
> some diagnostics saying "this macro is not known to be safe, so not
> checking".

Sounds OK, yes.

> Now, how would you transmit this information about safe and unsafe
> macros to from the host Emacs to the slave byte-compiling Emacs which is
> a separate process?  Via command-line parameters, an .el generated on
> the fly (we already do this for the flymake'd file, btw), or something
> else?

If we use "in load-path" as the main criterion, then I think this
question is a non-issue, right?

> At least, the way I understand your solution for the "safe/unsafe" macro
> problem it still doesn't seem to fix the fact that as soon as I type
> "(launch-nuke)" into some already loaded macro in eglot.el, nukes are
> potentially going to be launched by some unsuspecting macro-expansion
> down below.

Yup.  That's the problem with the use of trust as a proxy for safety.
I think if we switch to Haskell or Coq instead of Elisp we
could make all those problems disappear ;-)


        Stefan




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

* Sandboxing (was: Safety of elisp-flymake-byte-compile)
  2018-12-10 23:17                 ` João Távora
  2018-12-11 14:03                   ` Stefan Monnier
@ 2018-12-11 19:30                   ` Stefan Monnier
  2018-12-14  1:35                     ` Sandboxing João Távora
  1 sibling, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2018-12-11 19:30 UTC (permalink / raw)
  To: emacs-devel

>> Providing ways to run Elisp in a confined environment would be useful in
>> various circumstances, but it's non-trivial.
> I can understand that, but I'm not proposing a fully hermetic sandbox
> just something that ameliorates the problem.

I think a first step might be to add a new boolean var
`disallow-unsafe-effects` and then go through the C code to check this
var whenever we do something "dangerous" (e.g. change a global var,
launch a process, ...).

I suspect that a boolean will be too coarse in the long run (we'll
probably want to split this into different domains, maybe with some kind
of capabilities, or maybe monitor the effect, or god knows what), but we
need to start somewhere and this will at least let us annotate the code
that is sensitive so it's easier afterwards to refine it.  It will also
let us see whether this affects performance-significant code.


        Stefan




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

* Re: Sandboxing
  2018-12-11 19:30                   ` Sandboxing (was: Safety of elisp-flymake-byte-compile) Stefan Monnier
@ 2018-12-14  1:35                     ` João Távora
  0 siblings, 0 replies; 18+ messages in thread
From: João Távora @ 2018-12-14  1:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Providing ways to run Elisp in a confined environment would be useful in
>>> various circumstances, but it's non-trivial.
>> I can understand that, but I'm not proposing a fully hermetic sandbox
>> just something that ameliorates the problem.
>
> I think a first step might be to add a new boolean var
> `disallow-unsafe-effects` and then go through the C code to check this
> var whenever we do something "dangerous" (e.g. change a global var,
> launch a process, ...).
>
> I suspect that a boolean will be too coarse in the long run (we'll
> probably want to split this into different domains, maybe with some kind
> of capabilities, or maybe monitor the effect, or god knows what), but we
> need to start somewhere

Indeed we do.  Just a couple of checks in src/fileio.c, delete_file and
delete_directory_internal would be a great start.  Then a couple more
and so on.

João



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-11 14:03                   ` Stefan Monnier
@ 2018-12-14 12:00                     ` João Távora
  2018-12-14 12:15                       ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2018-12-14 12:00 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Trust a file = loaded in the host Emacs - some known exceptions, right?
> I think rather than "loaded in the host Emacs" it'll have to be "in
> load-path", because Emacs is generally very liberal about automatically
> loading files from load-path, so anything in load-path is
> pretty much already trusted.
> [ I think someone™ should sit down and think hard about this in general
>   (not only in the context of flymake), because "in load-path" is not as
>   clearly defined as we might think, since we also sometimes load files
>   from subdirectories within load-path.
>   And hopefully, this someone should be well intentioned ;-)  ]

This is orthonogal to the question right?  I think for the time being we
can write some 'moderately-trusted-p (file)' function to hide that can
of worms.

>> So as soon as I load eglot.el, or eglot.elc in the host Emacs, it would
>> start working?
> Right, or even as soon as eglot is in your load-path.

Hmmm, there appears to be a contradiction here, or maybe I'm missing
something.  Can you explain exactly what will happen if I C-u M-x
byte-compile-file the eglot.el file?  That's the way I normally load .el
files: I usually don't put their directories in my load path unless they
have complicated dependencies, or I plan on keeping them in my config.

So, shouldn't we instead/also use 'features'?

>> If so, I could live with that.  Until it starts working it could issue
>> some diagnostics saying "this macro is not known to be safe, so not
>> checking".
> Sounds OK, yes.

Good to have this one nailed down.

>> Now, how would you transmit this information about safe and unsafe
>> macros to from the host Emacs to the slave byte-compiling Emacs which is
>> a separate process?  Via command-line parameters, an .el generated on
>> the fly (we already do this for the flymake'd file, btw), or something
>> else?
> If we use "in load-path" as the main criterion, then I think this
> question is a non-issue, right?

We still have to hand the host's load-path/features value to the slave
Emacs, right?

>> At least, the way I understand your solution for the "safe/unsafe" macro
>> problem it still doesn't seem to fix the fact that as soon as I type
>> "(launch-nuke)" into some already loaded macro in eglot.el, nukes are
>> potentially going to be launched by some unsuspecting macro-expansion
>> down below.
> Yup.  That's the problem with the use of trust as a proxy for safety.

Yup x 2.  With Flymake and macro expansions it's like you trust someone
to be reasonable, but then he turns into a nuke-lanching maniac just by
joking about it.  Better not have nukes handy by then.

João



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-14 12:00                     ` João Távora
@ 2018-12-14 12:15                       ` Stefan Monnier
  2018-12-14 13:09                         ` João Távora
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2018-12-14 12:15 UTC (permalink / raw)
  To: emacs-devel

> This is orthonogal to the question right?  I think for the time being we
> can write some 'moderately-trusted-p (file)' function to hide that can
> of worms.

Indeed.
Cute worms, by the way.

>>> So as soon as I load eglot.el, or eglot.elc in the host Emacs, it would
>>> start working?
>> Right, or even as soon as eglot is in your load-path.
> Hmmm, there appears to be a contradiction here, or maybe I'm missing
> something.  Can you explain exactly what will happen if I C-u M-x
> byte-compile-file the eglot.el file?  That's the way I normally load .el
> files: I usually don't put their directories in my load path unless they
> have complicated dependencies, or I plan on keeping them in my config.

If it is in your load-history, then it also qualifies to be "trusted",
even if it's not in your load-path.

>>> At least, the way I understand your solution for the "safe/unsafe" macro
>>> problem it still doesn't seem to fix the fact that as soon as I type
>>> "(launch-nuke)" into some already loaded macro in eglot.el, nukes are
>>> potentially going to be launched by some unsuspecting macro-expansion
>>> down below.
>> Yup.  That's the problem with the use of trust as a proxy for safety.
> Yup x 2.  With Flymake and macro expansions it's like you trust someone

It's particularly serious when that someone is yourself, because
when you mess up you can't even point fingers.


        Stefan




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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-14 12:15                       ` Stefan Monnier
@ 2018-12-14 13:09                         ` João Távora
  2018-12-14 13:27                           ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2018-12-14 13:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> If it is in your load-history, then it also qualifies to be "trusted",
> even if it's not in your load-path.

Oh, I didn't know about load-history. So is this usable?

    (defun moderately-trusted-p (file)
      (catch 'done
        (let ((file (expand-file-name file))
              (lhist load-history)
              (lpath load-path)
              probe)
          (and
           (string-match "\\.elc?$" file)
           (while (setq probe (pop lhist))
             (when (and (car probe)
                        (equal (file-name-sans-extension file)
                               (file-name-sans-extension (car probe))))
               (throw 'done t)))
           (while (setq probe (pop lpath))
             (when (equal (file-name-as-directory
                           (file-name-directory file))
                          (file-name-as-directory probe))
               (throw 'done t)))))))

Or do you want to write this in C for speed?  Where should this be
plugged into?  load in lread.c directly?

We should still hand load-path and load-history, or a part of
load-history down to the flymake'in slave emacs, right?

>>>> At least, the way I understand your solution for the "safe/unsafe" macro
>>>> problem it still doesn't seem to fix the fact that as soon as I type
>>>> "(launch-nuke)" into some already loaded macro in eglot.el, nukes are
>>>> potentially going to be launched by some unsuspecting macro-expansion
>>>> down below.
>>> Yup.  That's the problem with the use of trust as a proxy for safety.
>> Yup x 2.  With Flymake and macro expansions it's like you trust someone
> It's particularly serious when that someone is yourself, because
> when you mess up you can't even point fingers.

Because you probably just blew<M-DEL>chopp<M-DEL>... never mind...

João




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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-14 13:09                         ` João Távora
@ 2018-12-14 13:27                           ` Stefan Monnier
  2018-12-14 13:38                             ` João Távora
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2018-12-14 13:27 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel

> Oh, I didn't know about load-history. So is this usable?
>
>     (defun moderately-trusted-p (file)
>       (catch 'done
>         (let ((file (expand-file-name file))
>               (lhist load-history)
>               (lpath load-path)
>               probe)
>           (and
>            (string-match "\\.elc?$" file)
                                   ^
                                  \\'

>            (while (setq probe (pop lhist))
>              (when (and (car probe)
>                         (equal (file-name-sans-extension file)
>                                (file-name-sans-extension (car probe))))
>                (throw 'done t)))
>            (while (setq probe (pop lpath))
>              (when (equal (file-name-as-directory
>                            (file-name-directory file))
>                           (file-name-as-directory probe))
>                (throw 'done t)))))))

LGTM

> Or do you want to write this in C for speed?  Where should this be
> plugged into?  load in lread.c directly?

I don't understand the question.  AFAIK this would only be used within
bytecomp.el when processing a `defmacro` in order to decide whether it
should be added to byte-compile-macro-environment or not.

> We should still hand load-path and load-history, or a part of
> load-history down to the flymake'in slave emacs, right?

Ah, right, we'd then need to pass the `car` of load-history entries to
the slave.


        Stefan



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-14 13:27                           ` Stefan Monnier
@ 2018-12-14 13:38                             ` João Távora
  2018-12-14 14:13                               ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2018-12-14 13:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Oh, I didn't know about load-history. So is this usable?
>>
>>     (defun moderately-trusted-p (file)
>>       (catch 'done
>>         (let ((file (expand-file-name file))
>>               (lhist load-history)
>>               (lpath load-path)
>>               probe)
>>           (and
>>            (string-match "\\.elc?$" file)
>                                    ^
>                                   \\'
>

>>            (while (setq probe (pop lhist))
>>              (when (and (car probe)
>>                         (equal (file-name-sans-extension file)
>>                                (file-name-sans-extension (car probe))))
>>                (throw 'done t)))
>>            (while (setq probe (pop lpath))
>>              (when (equal (file-name-as-directory
>>                            (file-name-directory file))
>>                           (file-name-as-directory probe))
>>                (throw 'done t)))))))
>
> LGTM
>
>> Or do you want to write this in C for speed?  Where should this be
>> plugged into?  load in lread.c directly?
>
> I don't understand the question.  AFAIK this would only be used within
> bytecomp.el when processing a `defmacro` in order to decide whether it
> should be added to byte-compile-macro-environment or not.

Oh, right... of course, never mind.  Can you do that bit?  I suppose we
have to check some new var byte-compile-just-checking-p and
(moderately-trusted-p byte-compile-current-file), right?

Is it in `byte-compile-file-form-defalias'

>> We should still hand load-path and load-history, or a part of
>> load-history down to the flymake'in slave emacs, right?
>
> Ah, right, we'd then need to pass the `car` of load-history entries to
> the slave.

And not the load-path?

João



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

* Re: Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths)
  2018-12-14 13:38                             ` João Távora
@ 2018-12-14 14:13                               ` Stefan Monnier
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Monnier @ 2018-12-14 14:13 UTC (permalink / raw)
  To: emacs-devel

> Oh, right... of course, never mind.  Can you do that bit?  I suppose we
> have to check some new var byte-compile-just-checking-p and
                                                       ^^
No "-p" on variables (they're not predicates).

> (moderately-trusted-p byte-compile-current-file), right?
> Is it in `byte-compile-file-form-defalias'

I haven't looked into it yet.

>> Ah, right, we'd then need to pass the `car` of load-history entries to
>> the slave.
> And not the load-path?

I presumed that was already passed along.


        Stefan




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

end of thread, other threads:[~2018-12-14 14:13 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20181204233600.7907.75252@vcs0.savannah.gnu.org>
     [not found] ` <20181204233601.273DD209DC@vcs0.savannah.gnu.org>
2018-12-05  4:34   ` [Emacs-diffs] scratch/allow-custom-load-paths-in-elisp-flymake 4ef9711: Allow custom load paths in elisp's byte-compilation Flymake Stefan Monnier
2018-12-05 15:14     ` João Távora
2018-12-05 20:00     ` Glenn Morris
2018-12-05 20:40       ` João Távora
2018-12-08 13:23         ` Safety of elisp-flymake-byte-compile (Was Re: [Emacs-diffs] scratch/allow-custom-load-paths) João Távora
2018-12-08 15:36           ` Stefan Monnier
2018-12-10  0:20             ` João Távora
2018-12-10  2:22               ` Stefan Monnier
2018-12-10 23:17                 ` João Távora
2018-12-11 14:03                   ` Stefan Monnier
2018-12-14 12:00                     ` João Távora
2018-12-14 12:15                       ` Stefan Monnier
2018-12-14 13:09                         ` João Távora
2018-12-14 13:27                           ` Stefan Monnier
2018-12-14 13:38                             ` João Távora
2018-12-14 14:13                               ` Stefan Monnier
2018-12-11 19:30                   ` Sandboxing (was: Safety of elisp-flymake-byte-compile) Stefan Monnier
2018-12-14  1:35                     ` Sandboxing João Távora

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).