The open opcode

[Jonathan Leto]

Howdy,

I have been playing with some code that allows me to intercept access
to the File and FileHandle PMC from PIR [0], which allows me to add
some security to those when accessed from PL/Parrot. But I currently
do not have a way to intercept calls to the "open" opcode. Defining a
subroutine called "open" in the global namespace does not seem to
override it.

Chromatic and I talked on IRC about a "security" runcore, which would
allow replacing opcodes. The point was brought up that if you have
something that allows replacing opcodes, then that itself causes
another security hole. But if, at the end of replacing the necessary
ops, you then replace the "op replacer", that hole is closed.

A "security" runcore is definitely a great idea, and will be necessary
for many advanced security features for Parrot, but if someone can
think of a simpler way to replace the "open" opcode at run-time, I am
all ears. Would it be possible to replace the "open" opcode with a
dynop? I think that would be the smallest possible feature that Parrot
could add to give PL/Parrot the filesystem security that it needs.

Would making the open opcode a dynop create a noticeable decrease in
performance? That is the only reason I can see that we would not want
to go that route.

Duke

[0] - http://gist.github.com/373558

PS: Thanks to Austin++ for the idea about intercepting PMC's.

--
Jonathan "Duke" Leto
jona...@leto.net
http://leto.net
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev

--
You received this message because you are subscribed to the Google Groups "parrot.dev" group.
To post to this group, send email to parro...@googlegroups.com.
To unsubscribe from this group, send email to parrot-dev+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/parrot-dev?hl=en.

[Moritz Lenz]

Jonathan Leto wrote:
> Would making the open opcode a dynop create a noticeable decrease in
> performance? That is the only reason I can see that we would not want
> to go that route.

As I mentioned on #parrot, it's not clear to me why 'open' needs to be
an opcode (or a dynop, for that matter) at all. It sounds like the thing
you'd find as a method call in a (core) library, really.

So deprecating 'open' as an opcode (and replacing it by an appropriate
library function, if that doesn't exist already) seems to solve that
problem :-)

Cheers,
Moritz

[Jonathan Leto]

Howdy,

moritz++ has a point. Can we deprecate the "open" opcode and tell
people to use the "open" method on either File or FileHandle PMC? The
downside is that we have to wait quite a while, due to the deprecation
cycle, for this to come into effect.

Would it be possible to make the open opcode use the File or
FileHandle PMC behind the scenes? That would fix my problem and not
require changing anything to end users as well as avoiding the
deprecation cycle.

Duke

On Thu, Apr 22, 2010 at 1:16 PM, Moritz Lenz <mor...@faui2k3.org> wrote:
> Jonathan Leto wrote:
>> Would making the open opcode a dynop create a noticeable decrease in
>> performance? That is the only reason I can see that we would not want
>> to go that route.
>
> As I mentioned on #parrot, it's not clear to me why 'open' needs to be
> an opcode (or a dynop, for that matter) at all. It sounds like the thing
> you'd find as a method call in a (core) library, really.
>
> So deprecating 'open' as an opcode (and replacing it by an appropriate
> library function, if that doesn't exist already) seems to solve that
> problem :-)
>
> Cheers,
> Moritz
> _______________________________________________
> http://lists.parrot.org/mailman/listinfo/parrot-dev
>



--

Jonathan "Duke" Leto
jona...@leto.net
http://leto.net

[Geoffrey Broadwell]

On Thu, 2010-04-22 at 13:43 -0700, Jonathan Leto wrote:
> moritz++ has a point. Can we deprecate the "open" opcode and tell
> people to use the "open" method on either File or FileHandle PMC? The
> downside is that we have to wait quite a while, due to the deprecation
> cycle, for this to come into effect.
>
> Would it be possible to make the open opcode use the File or
> FileHandle PMC behind the scenes? That would fix my problem and not
> require changing anything to end users as well as avoiding the
> deprecation cycle.

+1 to deprecating the opcode
+1 to immediately changing open to use PMCs in the background *if* all
functionality can be duplicated perfectly (so users of the opcode see no
change)


-'f

[chromatic]

On Thursday 22 April 2010 at 13:43, Jonathan Leto wrote:

> Would it be possible to make the open opcode use the File or
> FileHandle PMC behind the scenes? That would fix my problem and not
> require changing anything to end users as well as avoiding the
> deprecation cycle.

How does that fix the problem? I'm afraid I don't follow this step.

-- c

[Andrew Whitworth]

the open opcode calls src/io/api.c:Parrot_io_open, which does redirect
to the appropriate PMC type. We've done some encapsulation-breaking
for performance issues (saves a PCC call if we can just muck with
FileHandle's internals directly). In any case, the PMC is in control.
So it's a moot point to wonder whether making the open opcode use the
PMC would be an improvement for security or not: we do it anyway.

--Andrew Whitworth

[Jonathan Leto]

Howdy,

On Thu, Apr 22, 2010 at 4:34 PM, Andrew Whitworth <wknig...@gmail.com> wrote:
> the open opcode calls src/io/api.c:Parrot_io_open, which does redirect
> to the appropriate PMC type. We've done some encapsulation-breaking
> for performance issues (saves a PCC call if we can just muck with
> FileHandle's internals directly). In any case, the PMC is in control.
> So it's a moot point to wonder whether making the open opcode use the
> PMC would be an improvement for security or not: we do it anyway.
>
> --Andrew Whitworth
>

The issue I am getting at is that the the encapsulation-breaking makes
it so that when I override the File/FileHandle PMC's, the open opcode
still knows how to call the originals. Can we make the open opcode
actually look up the File/FileHandle PMC in the current interpreter?

Duke

--
Jonathan "Duke" Leto
jona...@leto.net
http://leto.net

[Allison Randal]

On 4/23/10 1:35 AM, Jonathan Leto wrote:
>
> The issue I am getting at is that the the encapsulation-breaking makes
> it so that when I override the File/FileHandle PMC's, the open opcode
> still knows how to call the originals. Can we make the open opcode
> actually look up the File/FileHandle PMC in the current interpreter?

The deeper problem here is that monkeypatching File/FileHandle isn't
really the right solution to secure I/O.

Allison

[Jonathan Leto]

Howdy,

> The deeper problem here is that monkeypatching File/FileHandle isn't really
> the right solution to secure I/O.

I totally agree with you, but I prefer to call it "duck punching." ;)

> Allison

I have attached a small patch that allows me to secure PL/Parrot
against the open opcode. It comes at the cost of a single
Parrot_PMC_typenum(interp,"FileHandle") lookup, per call of the open
opcode, which is an extremely tiny performance hit and unbreaks
encapsulation.

If you compile Parrot with the attached patch, then run
intercept_io.pir, you will see that all IO calls are properly
intercepted. This script can easily be turned into a test to verify
that the "feature" I am adding works as expected and continues to
work, since PL/Parrot will depend on it.

I fully intend to continue working on improving the security of
Parrot, but this tiny step allows PL/Parrot to be considered for
deployment into production systems and gives Parrot an entirely new
niche to thrive in.

I would greatly appreciate the comments of my fellow Parrot hackers as
to whether this patch (with tests) can be applied.

[Austin Hastings]

It's not a case of modifying FileHandle at all. The trick is to simply
replace the FileHandle pmc with a different pmc - one which implements
chroot, or fails, or throws an exception, or whatever. The interpreter
maintains a pmc dictionary, and replacing FileHandle is a snap (as it
should be).

There are a bunch of fairly obvious extensions to this suggestion, in
that there are a lot of ops that really should either be converted to
methods, or should be converted to method-shortcuts. E.g., PPSU, incr,
decr should probably be shortcuts. But having open as an opcode is
pretty silly - I don't think anyone is measuring parrot performance in
Millions of FileOpens per Second, or anything.


=Austin

[Geoffrey Broadwell]

On Thu, 2010-04-22 at 19:34 -0400, Andrew Whitworth wrote:
> the open opcode calls src/io/api.c:Parrot_io_open, which does redirect
> to the appropriate PMC type. We've done some encapsulation-breaking
> for performance issues (saves a PCC call if we can just muck with
> FileHandle's internals directly).

Honest question: How did this ever turn up as a performance issue? Was
this a response to performance problems before the PCC
refactoring/optimization?

I'm trying to imagine a case (even a completely contrived one) in which
the addition of a PCC call during the file open operation becomes a
bottleneck, and I'm afraid I'm failing, unless I've grossly
underestimated the cost of a PCC call in my calculations ....


-'f

[Allison Randal]

On 4/23/10 2:03 AM, Jonathan Leto wrote:
>
> I have attached a small patch that allows me to secure PL/Parrot
> against the open opcode. It comes at the cost of a single
> Parrot_PMC_typenum(interp,"FileHandle") lookup, per call of the open
> opcode, which is an extremely tiny performance hit and unbreaks
> encapsulation.

I'm okay with this patch, as long as it's temporary (a non-specific
time-frame something like "we replace it before 2.9").

On what to replace it with:

- That direct usage of the FileHandle typeid already should have been
wrapped in a call to Parrot_get_ctx_HLL_type.

- We shouldn't be ASSERTing based on typenum there anyway, to allow for
HLL mapping of FileHandle.

- If masking entire core PMCs becomes a standard security strategy we
might want to consider a feature similar to HLL mapping in the security
layer, that cleanly substitutes the custom PMC for the core PMC.

- I'd like to see security features built into FileHandle (jonathan and
I have talked about this on IRC), altering behavior based on an
interpreter or system-wide security level setting.

- Selective enabling/disabling of opcodes is still on the menu. I'm not
as certain about selective overriding of opcodes. It's not possible now,
but might be with Lorito.

[Allison Randal]

On 4/23/10 3:17 AM, Geoffrey Broadwell wrote:
>
> Honest question: How did this ever turn up as a performance issue? Was
> this a response to performance problems before the PCC
> refactoring/optimization?
>
> I'm trying to imagine a case (even a completely contrived one) in which
> the addition of a PCC call during the file open operation becomes a
> bottleneck, and I'm afraid I'm failing, unless I've grossly
> underestimated the cost of a PCC call in my calculations ....

I was surprised too. Before the I/O refactor year before last, chromatic
and I had a running gag about I/O optimizations: "The slowest thing
about I/O is... I/O."

Turns out, that's not true. If you load I/O down with a pile of PCC
method invocations, you can slow it down. Substantially.

Allison

[Andrew Whitworth]

In the case of the open operation, avoiding the PCC call is probably
not a huge savings. Open is not called often enough to make a huge
difference either way. Operations like read/write are both called far
more often so the saves there really do add up over time. I have no
problem making the uncommon case more expensive if it gives us better
security utility. I'm sure we could find performance savings elsewhere
in the IO subsystem to make up for it.

--Andrew Whitworth

[Allison Randal]

Andrew Whitworth wrote:
> In the case of the open operation, avoiding the PCC call is probably
> not a huge savings. Open is not called often enough to make a huge
> difference either way. Operations like read/write are both called far
> more often so the saves there really do add up over time. I have no
> problem making the uncommon case more expensive if it gives us better
> security utility. I'm sure we could find performance savings elsewhere
> in the IO subsystem to make up for it.

A method call isn't what's needed here anyway. All it needs is to look
up the FileHandle class from the namespace instead of instantiating it
from the type id.

Allison