package security auditing and isolation

package security auditing and isolation

[Ted Zlatanov]

After watching some discussions among the MELPA maintainers, I wanted to
ask what Emacs itself can do to audit and isolate packages from
modifying the user's system. This is a concern for almost all Emacs
users because any ELPA repository, its sources, and its signing process
can be compromised for at least a short time. I think it's good to
assume this will happen sooner or later, and to proactively defend Emacs
users from it.

I propose two specific pieces of functionality, which I think are
essential to this task:

1) tools for analyzing the source code and finding the function calls
that can be dangerous. This includes eval, calling the shell, etc.

I don't know how feasible this is, but IMHO it would be valuable. At the
very least it could make code reviews easier by focusing on the
potentially problematic sections. I think Emacs' core is the right place
to add such code, not modules or add-on packages.

2) establishing isolation levels in the C core. Simply put, not all
packages need to be able to run shell commands, delete or modify files,
and so on. When the user installs or updates a package, if the needed
access levels change, that should be noted and acknowledged.

Thanks
Ted

Re: package security auditing and isolation

[Stefan Monnier]

> I propose two specific pieces of functionality, which I think are
> essential to this task:
>
> 1) tools for analyzing the source code and finding the function calls
> that can be dangerous. This includes eval, calling the shell, etc.
>
> I don't know how feasible this is, but IMHO it would be valuable. At the
> very least it could make code reviews easier by focusing on the
> potentially problematic sections. I think Emacs' core is the right place
> to add such code, not modules or add-on packages.

Are you thinking of this to protect against accidental problems, or to
protect against a malicious attacker?

It seems like it would be completely ineffective against a malicious
attacker (especially if such an tool auditing is "standardized"), but
I can't imagine it being very effective for the accidental case either.

> 2) establishing isolation levels in the C core.  Simply put, not all
> packages need to be able to run shell commands, delete or modify files,
> and so on.  When the user installs or updates a package, if the needed
> access levels change, that should be noted and acknowledged.

That could be done, but it's a major restructuring, which will probably
require major changes to be able to isolate packages from each other.


        Stefan

Re: package security auditing and isolation

[Yuri Khan]

On Thu, Apr 6, 2017 at 9:15 PM, Ted Zlatanov <tzz@lifelogs.com> wrote:

> 2) establishing isolation levels in the C core. Simply put, not all
> packages need to be able to run shell commands, delete or modify files,
> and so on. When the user installs or updates a package, if the needed
> access levels change, that should be noted and acknowledged.

“This package is requesting the following permissions:

* Monitor all your keystrokes
* Modify buffer contents
* Access the kill ring
* Read and write files”

Re: package security auditing and isolation

[Ted Zlatanov]

On Thu, 06 Apr 2017 10:48:20 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> I propose two specific pieces of functionality, which I think are
>> essential to this task:
>> 
>> 1) tools for analyzing the source code and finding the function calls
>> that can be dangerous. This includes eval, calling the shell, etc.
>> 
>> I don't know how feasible this is, but IMHO it would be valuable. At the
>> very least it could make code reviews easier by focusing on the
>> potentially problematic sections. I think Emacs' core is the right place
>> to add such code, not modules or add-on packages.

SM> Are you thinking of this to protect against accidental problems, or to
SM> protect against a malicious attacker?

To help code reviews find malicious changes.

SM> It seems like it would be completely ineffective against a malicious
SM> attacker (especially if such an tool auditing is "standardized"), but
SM> I can't imagine it being very effective for the accidental case either.

Can you elaborate on what could make it effective? Or, alternatively,
why the idea is fundamentally flawed and if there are better ones?

The goal I stated is to "audit and isolate packages from modifying the
user's system... because any ELPA repository, its sources, and its
signing process can be compromised for at least a short time." If my
suggestion is ineffective, can you make suggestions to improve the
situation?

Another idea: add some UI in package.el to see what code changes are
getting installed with a package install update. That would be a quick
way to give users some visibility. It's not a solution, and it's poor
UI, but it's a start.

>> 2) establishing isolation levels in the C core.  Simply put, not all
>> packages need to be able to run shell commands, delete or modify files,
>> and so on.  When the user installs or updates a package, if the needed
>> access levels change, that should be noted and acknowledged.

SM> That could be done, but it's a major restructuring, which will probably
SM> require major changes to be able to isolate packages from each other.

If you, with your deep knowledge of the C core, could start a
*top-level* list of the necessary changes, that would be really helpful.

We don't have to do it all today, or this year. But we have to start
somewhere.

Ted

Re: package security auditing and isolation

[Stefan Monnier]

SM> Are you thinking of this to protect against accidental problems, or to
SM> protect against a malicious attacker?
> To help code reviews find malicious changes.

Then it's problematic: if it's a more or less standard procedure, you
can assume than any attacker will know about it and will hence use
workarounds to evade detection.

Such "heuristic detection" can only work via obscurity, either by
keeping the reviewing criteria secret or making them somehow
unpredictable (not sure what that could look like in this context).

Unless workarounds are *really* difficult or impossible, of course.
But in the current Emacs design, I'd expect any need for a workaround
would be trivial to satisfy (e.g. call (intern (concat "shel" "l-command"))
to avoid detection) or end up making it more difficult for the
non-malicious packages to do their job.

> Can you elaborate on what could make it effective? Or, alternatively,
> why the idea is fundamentally flawed and if there are better ones?

Rather than try and detect dangerous patterns, we'd have to make
"unsafe" behavior impossible, via something like isolation.

SM> That could be done, but it's a major restructuring, which will probably
SM> require major changes to be able to isolate packages from each other.
> If you, with your deep knowledge of the C core, could start a
> *top-level* list of the necessary changes, that would be really helpful.

Just trying to design the system will be a significant effort.
I'm not really interested, sorry.


        Stefan

Re: package security auditing and isolation

[Ted Zlatanov]

On Thu, 06 Apr 2017 14:19:23 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

SM> Are you thinking of this to protect against accidental problems, or to
SM> protect against a malicious attacker?
>> To help code reviews find malicious changes.

SM> Then it's problematic: if it's a more or less standard procedure, you
SM> can assume than any attacker will know about it and will hence use
SM> workarounds to evade detection.

I don't buy that argument from either end. Right now, attackers need no
workarounds so the cost-benefit analysis is heavily in their favor.

SM> Such "heuristic detection" can only work via obscurity, either by
SM> keeping the reviewing criteria secret or making them somehow
SM> unpredictable (not sure what that could look like in this context).

We have to operate openly, because it's the only practical choice other
than forming a cabal or ignoring the problem. Heuristics are not what I
had in mind.

Here are some questions for the list:

a) Can the parse tree of a package be analyzed safely (without running
code in the package)? Is it deterministic?

b) If the parse tree of a package is analyzed, and only has whitelisted
functions such as `string-equal' in it, does that make the package safe?

c) Can the parse tree of a package be compared deterministically at two
separate VCS checkpoints to find what's changed?

d) Can the changes to the parse tree between two VCS checkpoints be
signed by a reviewer?

>> Can you elaborate on what could make it effective? Or, alternatively,
>> why the idea is fundamentally flawed and if there are better ones?

SM> Rather than try and detect dangerous patterns, we'd have to make
SM> "unsafe" behavior impossible, via something like isolation.
...
SM> Just trying to design the system will be a significant effort.
SM> I'm not really interested, sorry.

Right. Thanks for your comments.

Ted

Re: package security auditing and isolation

[Stefan Monnier]

> We have to operate openly, because it's the only practical choice other
> than forming a cabal or ignoring the problem.

Agreed.

> Heuristics are not what I had in mind.

You mentioned "finding the function calls that can be dangerous", which
to me sounds like the usual flawed approach of "anti-virus" looking for the
known exploits.

> a) Can the parse tree of a package be analyzed safely (without running
> code in the package)? Is it deterministic?

Yes, currently the reader is pretty much unaffected by Elisp code.

> b) If the parse tree of a package is analyzed, and only has whitelisted
> functions such as `string-equal' in it, does that make the package safe?

We have unsafep.el which tries to use such a white-list approach.
I doubt it's really bulletproof, but in any case I don't think you could
write a useful Elisp package with that subset.
[ IIRC it was introduced as part of ses.el to check safety of formulas
  in SES spreadsheets, so full-generality was not a goal.  ]

> c) Can the parse tree of a package be compared deterministically at two
> separate VCS checkpoints to find what's changed?

Yes, of course.

> d) Can the changes to the parse tree between two VCS checkpoints be
> signed by a reviewer?

Technically, yes, of course.


        Stefan

Re: package security auditing and isolation

[Clément Pit-Claudel]

On 2017-04-06 15:26, Ted Zlatanov wrote:
> a) Can the parse tree of a package be analyzed safely (without running
> code in the package)? Is it deterministic?

Yes if you mean the parse tree, but no if you mean the expanded syntax tree:  you need to run macros to see the full AST, and macros can run arbitrary code.  You could apply a first analysis pass to the macros, decide that they are safe, expand, and run the analysis again; but see (b)

> b) If the parse tree of a package is analyzed, and only has whitelisted
> functions such as `string-equal' in it, does that make the package safe?

Just looking at the parse tree isn't enough, because macros.  The AST is better, but still no: it's not hard to crash Emacs from ELisp, e.g. by causing stack overflows.  That would allow you to escape most protections, I expect.  Without going to such extremes, it's hard to think of what a good whitelist would look like.

Re: package security auditing and isolation

[Ted Zlatanov]

On Thu, 06 Apr 2017 16:12:22 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> a) Can the parse tree of a package be analyzed safely (without running
>> code in the package)? Is it deterministic?

SM> Yes, currently the reader is pretty much unaffected by Elisp code.

On Thu, 6 Apr 2017 16:17:17 -0400 Clément Pit-Claudel <cpitclaudel@gmail.com> wrote: 

CP> Yes if you mean the parse tree, but no if you mean the expanded syntax tree: you
CP> need to run macros to see the full AST, and macros can run arbitrary code. You
CP> could apply a first analysis pass to the macros, decide that they are safe,
CP> expand, and run the analysis again; but see (b)

I think it's OK to leave macros unexpanded and only show to the auditor
a diff that highlights the unsafe things. In other words, call `unsafep'
or something like it on the new parts of the parse tree, and mark the
unsafe pieces for review. The auditor can decide if the macros can be
trusted, together with any other potentially unsafe code.

In that case we need to make a canonical representation (parse tree) of
a package's code before macros are expanded. That can be the mechanical
part of the code audit (together with the signing process, which is an
easier problem). Can Emacs make (a) easy with a package or with C code?

The package metadata is probably the right place to define what
constitutes a package's parse tree, and then that subportion of the
metadata will have to signed together with the parse tree. I don't know
if "parse tree" is the right term, either.

I think one benefit of signing the parse tree is that packages can then
be copied between various VCS systems and ELPA repositories as long as
the code remains the same. IOW you don't have to trust the ELPA
repository or the VCS system, you only trust the auditor(s) that signed
the parse tree of the actual package you're about to install.

Ted

Re: package security auditing and isolation

[Tim Cross]

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

This is a very interesting and important thread. Some great points from
both Ted and Stefan. This is also a very difficult problem, but one we do
need to consider and see what we can do to improve the current situation.

I don't think we can automate the process and even our abilities to have
functions/libraries which can assist with verifying/checking packages will
be limited as I suspect you will need a reasonable high level of
understanding of elisp, emacs and security threats in order to use such
tools effectively. However, this isn't to say we shouldn't try to improve
the situation.

In an ideal world, we would probably have a team of specialists who would
review new packages, provide an assessment of their relative security risks
and sign the code. This is very unlikely.

A common mistake made in security is to focus too much on the technology
and technical solutions to the problem rather than focus on the process and
people. In addition to looking at tools/libraries which would help with
analysis of code, I think we also need to look at the processes associated
with uploading of packages to repositories, retrieving packages from
repositories and their installation to identify weaknesses and see what we
can do to tighten up those weaknesses. A good example of this is Stefan's
other post regarding the org package. In addition to identifying security
weaknesses in the process, we need to identify usability weaknesses which
result in people doing the wrong thing - either because doing the right
thing is too hard or just simply a lack of awareness.

I also think it would be a good idea to see what we can do with respect to
improving awareness and understanding of security risks and how to manage
them for end users. While there is likely quite a few who will not care,
there are some who do and we should try to empower them as much as possible
by providing information/documentation on what they can do, what they
should look out for, how to assess code security risks etc. This
information could also be very useful in identifying what sort of tools
might be useful and/or what features they would require.

Something else which might be worth considering would be some way to share
analysis results. Knowing that someone has at least tried to look at a
package is likely to provide at least some additional confidence over a
package - we would need to have some protection against 'false' reviews.

A good starting point might be to either add a section to the manual or on
an emacs wiki which covers techniques and approaches that could be used for
assessing security implications for a package and how to harden your
package.el setup etc. This could even become the start of a requirements
gathering for what tools or support libraries are needed to assist in this
type of activity.



On 7 April 2017 at 07:57, Ted Zlatanov <tzz@lifelogs.com> wrote:

> On Thu, 06 Apr 2017 16:12:22 -0400 Stefan Monnier <
> monnier@iro.umontreal.ca> wrote:
>
> >> a) Can the parse tree of a package be analyzed safely (without running
> >> code in the package)? Is it deterministic?
>
> SM> Yes, currently the reader is pretty much unaffected by Elisp code.
>
> On Thu, 6 Apr 2017 16:17:17 -0400 Clément Pit-Claudel <
> cpitclaudel@gmail.com> wrote:
>
> CP> Yes if you mean the parse tree, but no if you mean the expanded syntax
> tree: you
> CP> need to run macros to see the full AST, and macros can run arbitrary
> code. You
> CP> could apply a first analysis pass to the macros, decide that they are
> safe,
> CP> expand, and run the analysis again; but see (b)
>
> I think it's OK to leave macros unexpanded and only show to the auditor
> a diff that highlights the unsafe things. In other words, call `unsafep'
> or something like it on the new parts of the parse tree, and mark the
> unsafe pieces for review. The auditor can decide if the macros can be
> trusted, together with any other potentially unsafe code.
>
> In that case we need to make a canonical representation (parse tree) of
> a package's code before macros are expanded. That can be the mechanical
> part of the code audit (together with the signing process, which is an
> easier problem). Can Emacs make (a) easy with a package or with C code?
>
> The package metadata is probably the right place to define what
> constitutes a package's parse tree, and then that subportion of the
> metadata will have to signed together with the parse tree. I don't know
> if "parse tree" is the right term, either.
>
> I think one benefit of signing the parse tree is that packages can then
> be copied between various VCS systems and ELPA repositories as long as
> the code remains the same. IOW you don't have to trust the ELPA
> repository or the VCS system, you only trust the auditor(s) that signed
> the parse tree of the actual package you're about to install.
>
> Ted
>
>
>


-- 
regards,

Tim

--
Tim Cross

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