Can plugins add libraries to unix vm?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
47 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: New VMMaker/svn release

timrowledge

On 3-Jan-06, at 2:21 PM, Andreas Raab wrote:

> tim Rowledge wrote:
>> On 29-Dec-05, at 12:00 PM, David T. Lewis wrote:
>>> Attachments (in zip file):
>>>
>>> platforms-win32-plugins-FilePlugin-sqWin32FilePrims.c.diff -  
>>> Change  Win32
>>>     FilePlugin to use session ID from the interpreter rather  
>>> than  generate its
>>>     own value (for consistency, and also so as not to break OSPP  
>>> for Win32).
>> OK, that is up to andreas - I couldn't commit it if I wanted to.
>
> Now that's interesting. I can fix that easy enough but I'm curious  
> as to the reason for having a "global" session ID instead of a  
> local one. I always found it advantageous that (in all likelyhood)  
> you couldn't use the session ID for a file interchangeably with the  
> session ID of a socket. And I must say that I find the idea of  
> exposing what essentially amounts to a "VM secret" to other plugins  
> quite peculiar.
>
> Can somebody remind what the intent of that change was? I'm sure  
> there must've been some reason for it but I think I've missed the  
> discussion of that feature.
We've discussed it several times on this list and others since  
(believe it or not, I was amazed) November. November 2001 that is...

Part of my liking of the idea is reducing code duplication and the  
concomitant likelihood of mis-duplication - think of it as a service  
the VM core offers plugins. Why is hiding one plugin's session ID  
useful? I can't immediately think of anything devious one could  
usefully do with this secret info.

Obviously one doesn't have to use this facility but I can't see any  
reason not to.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
A low level language is one whose programs require attention to the  
irrelevant.



Reply | Threaded
Open this post in threaded view
|

Re: New VMMaker/svn release

Andreas.Raab
tim Rowledge wrote:
>> Can somebody remind what the intent of that change was? I'm sure  
>> there must've been some reason for it but I think I've missed the  
>> discussion of that feature.
>
> We've discussed it several times on this list and others since  (believe
> it or not, I was amazed) November. November 2001 that is...

Oh, I believe it. Coincidentally, this would also be the exact time
frame when I moved from the US to Germany and probably wasn't paying
close attention ;-)

> Part of my liking of the idea is reducing code duplication and the  
> concomitant likelihood of mis-duplication - think of it as a service  
> the VM core offers plugins. Why is hiding one plugin's session ID  
> useful? I can't immediately think of anything devious one could  
> usefully do with this secret info.

Actually it really isn't all that secret (which is part of the problem).
If you were to manifacture a file handle you could inspect an existing
one and copy the session ID out of it. The one thing it does do is
preventing mixing up file handles with other I/O handles if they have
the same size and different magic numbers.

> Obviously one doesn't have to use this facility but I can't see any  
> reason not to.

True. Although I think it is really valuable if a plugin can verify that
a handle is genuine (e.g., actually produced by that plugin). See the
following message.

Cheers,
   - Andreas

Reply | Threaded
Open this post in threaded view
|

Fake handles (was: Re: New VMMaker/svn release)

Andreas.Raab
In reply to this post by David T. Lewis
Hi Dave -

> The fileID is an opaque value that just happens to correspond to some
> data structure that we are not supposed to know about, but it is easier
> to handle the issue in the Win32OSProcessPlugin that to try to design a
> platform independent solution that can be incorporated into FilePlugin for
> all supported platforms.

*Gasp* If I've ever seen "evil" code, this got to be it ;-) Not because
it's badly written but because it's designed to break the (supposed)
encapsulation of the file structure. If the file plugin code ever
changes (such as when removing the fileSize) that code will make the VM
explode.

The main lesson for me here is that if it's *that* easy to fake a file
handle we've got a real problem with security. Basically, all of our I/O
operations are prone to this kind of attack and I don't even want to
think about all of the different ways in which this could be exploited.

In order to fix this, I just added a tiny little hash table to the
windows file plugin which checks whether a file handle is genuine, e.g.,
has been produced inside the the file plugin or not. Since that hash
table is not publicly accessible you can't store into it, thus you can
no longer fabricate a random file handle.

BTW, this is not designed "just so your evil code breaks" - as far as I
am concerned it's actually a *major* step forward towards a secure VM
design because one of the effects this achieves is that the handle
(token) that's used in the plugin is actually a capability to invoke a
given set of primitives and no others, and it is solely at the plugins
discretion which set of primitives to assign to which handle. Nice,
simple, and secure (just as it should be).

One of the effects of that change is that it immediately makes the
session ID superfluous from a security POV. The worst that could ever
happen is that an "old" handle refers to a "new" file (if handles aren't
unique across sessions) but an invalid handle can never be used in any
of these operations. (note that from a security POV an old handle
referring to a new file is indistinguishable from cloning an existing
handle which is something that we don't deal with either right now -
fixing this would be straightforward if we were able to deal with oops
instead of third-party handles but that would require dealing with oop
relocation during GC; ouch).

And now that we got this done ;-) we can start talking about how to
expose the functionality that you need in a nice and cross-platform way.
Like, accessing standard file handles: What's wrong with a
primitiveGetStdFileHandle that takes an integer argument for the kind of
standard handle to retrieve (0 - stdin, 1 - stdout, 2 - stderr)? Sounds
simple enough to me. Any others you'd need?

Cheers,
   - Andreas

Reply | Threaded
Open this post in threaded view
|

Re: Fake handles (was: Re: New VMMaker/svn release)

David T. Lewis
On Tue, Jan 03, 2006 at 10:41:29PM -0800, Andreas Raab wrote:

> Hi Dave -
>
> > The fileID is an opaque value that just happens to correspond to some
> > data structure that we are not supposed to know about, but it is easier
> > to handle the issue in the Win32OSProcessPlugin that to try to design a
> > platform independent solution that can be incorporated into FilePlugin for
> > all supported platforms.
>
> *Gasp* If I've ever seen "evil" code, this got to be it ;-) Not because
> it's badly written but because it's designed to break the (supposed)
> encapsulation of the file structure. If the file plugin code ever
> changes (such as when removing the fileSize) that code will make the VM
> explode.

Sure. In fact this has happened quite recently. The SQFile struct was
changed to handle some alignment issues for the 64 bit VM. I had to
put out an update to OSPP to accommodate the change. If nobody had
been maintaining OSPP, this would have caused a problem. Thankfully
the VM did not explode, although I did have some failing prims in the
unit tests.

Manipulating fileID is a Bad Thing. In the case of OSPP, the tradeoff
was between doing that particular bad thing versus attempting to
maintain a package of platform-specific extensions as part of the
FilePlugin, SocketPlugin, and so forth. I chose to implement the
extensions separately, and this has worked well for me.

> The main lesson for me here is that if it's *that* easy to fake a file
> handle we've got a real problem with security. Basically, all of our I/O
> operations are prone to this kind of attack and I don't even want to
> think about all of the different ways in which this could be exploited.

Yes. I suspect this would be among the least of our security problems, but
you are absolutely right.

> In order to fix this, I just added a tiny little hash table to the
> windows file plugin which checks whether a file handle is genuine, e.g.,
> has been produced inside the the file plugin or not. Since that hash
> table is not publicly accessible you can't store into it, thus you can
> no longer fabricate a random file handle.
>
> BTW, this is not designed "just so your evil code breaks" - as far as I
> am concerned it's actually a *major* step forward towards a secure VM
> design because one of the effects this achieves is that the handle
> (token) that's used in the plugin is actually a capability to invoke a
> given set of primitives and no others, and it is solely at the plugins
> discretion which set of primitives to assign to which handle. Nice,
> simple, and secure (just as it should be).
>
> One of the effects of that change is that it immediately makes the
> session ID superfluous from a security POV. The worst that could ever
> happen is that an "old" handle refers to a "new" file (if handles aren't
> unique across sessions) but an invalid handle can never be used in any
> of these operations. (note that from a security POV an old handle
> referring to a new file is indistinguishable from cloning an existing
> handle which is something that we don't deal with either right now -
> fixing this would be straightforward if we were able to deal with oops
> instead of third-party handles but that would require dealing with oop
> relocation during GC; ouch).

This sounds like a pretty reasonable approach. It will have a rather
large impact on OSPP file operations (e.g. file locking on Unix), but
probably manageable. I would like to look it over and remind myself
of all the things that are going to break. If you can hold off until
say next week before diving into this I'd appreciate it.

> And now that we got this done ;-) we can start talking about how to
> expose the functionality that you need in a nice and cross-platform way.
> Like, accessing standard file handles: What's wrong with a
> primitiveGetStdFileHandle that takes an integer argument for the kind of
> standard handle to retrieve (0 - stdin, 1 - stdout, 2 - stderr)? Sounds
> simple enough to me. Any others you'd need?

I don't really like the 0, 1, 2 convention, because it's an ancient
Unix convention, and really they are supposed to be treated as
opaque handles (seriously). It's probably OK to implement primitiveGetStdOut,
primitiveGetStdErr, and primitiveGetStdErr in the FilePlugin as long
as they are optional primitives and fail in reasonable ways.
that these are really still unix conventions that have been adopted
elsewhere, and may be rather platform-dependent. They behave differently
on Win32 and unix, and I don't know what RiscOS does about them. Tim?

Pipe handles are definitely needed, and given Squeak's current approach
to files and sockets, these should be treated as file handles. Anonymous
pipes don't have path names, so the process of opening them is of course
different.  Pipes are very platform-dependent, and I'm not sure this
belongs in the core plugins. I'll see if I can think of a way to do
this that is not too "evil" ;)

Dave


Reply | Threaded
Open this post in threaded view
|

Re: Fake handles (was: Re: New VMMaker/svn release)

David T. Lewis
On Wed, Jan 04, 2006 at 07:22:43AM -0500, David T. Lewis wrote:

> On Tue, Jan 03, 2006 at 10:41:29PM -0800, Andreas Raab wrote:
>
> > And now that we got this done ;-) we can start talking about how to
> > expose the functionality that you need in a nice and cross-platform way.
> > Like, accessing standard file handles: What's wrong with a
> > primitiveGetStdFileHandle that takes an integer argument for the kind of
> > standard handle to retrieve (0 - stdin, 1 - stdout, 2 - stderr)? Sounds
> > simple enough to me. Any others you'd need?
>
> I don't really like the 0, 1, 2 convention, because it's an ancient
> Unix convention, and really they are supposed to be treated as
> opaque handles (seriously). It's probably OK to implement primitiveGetStdOut,
> primitiveGetStdErr, and primitiveGetStdErr in the FilePlugin as long
> as they are optional primitives and fail in reasonable ways.
> that these are really still unix conventions that have been adopted
> elsewhere, and may be rather platform-dependent. They behave differently
> on Win32 and unix, and I don't know what RiscOS does about them. Tim?
>
> Pipe handles are definitely needed, and given Squeak's current approach
> to files and sockets, these should be treated as file handles. Anonymous
> pipes don't have path names, so the process of opening them is of course
> different.  Pipes are very platform-dependent, and I'm not sure this
> belongs in the core plugins. I'll see if I can think of a way to do
> this that is not too "evil" ;)

After giving this a little more thought, I can see two feasible ways
to expose the required functionality while still allowing OSPP to exist
as as optional external plugin.

1) Move primitiveCreatePipe, primitiveGetStdIn, primitiveGetStdOut, and
primitiveGetStdErr out of the OSPP plugins and into FilePlugin, and sort
out the platform differences in the support code.

2) Have FilePlugin provide a primitiveOpenHandle that could be invoked
in a manner similar to the current StandardFileStream>>primOpen:writable:,
except that the supplied parameter would be something representing a (HANDLE)
or (FILE *), e.g. "primOpenHandle: aByteArray ofLength: 4 writable: true".

Option #1 is undesirable because both the concepts and the implementations
are quite platform-specific. But it is workable as long as the VM maintainers
don't mind, and as long as at least the Unix and Win32 implementations
actually get done.

Option #2 would work well, but does not address the security concern. If
spoofing a fileID is a security concern, then spoofing a HANDLE would
presumably be equally bad. But it does at least get rid of the outside
manipulation of private data structures.

In either case it would be helpful to add a primitiveFileHandle that
answers the platform-specific (HANDLE) or (FILE *) corresponding to the
certified, non-fake fileID. This would be used by platform-specific
extensions (e.g. file locking, fcntl functions) and would eliminate
the need for knowledge of the SQFile struct in other plugins. Ditto
for the SocketPlugin, if only for the sake of uniformity.

>From my own point of view, either of these would be OK. I also have no
objection to leaving things as they are, although I would prefer that
the various FilePlugin implementations use the same sessionIdentifier.
The workaround I've had to use for the last few years is truly awful.

Dave


Reply | Threaded
Open this post in threaded view
|

Re: New VMMaker/svn release

David T. Lewis
In reply to this post by timrowledge
Tim,

Some belated followup on this. The patches for sqMemoryAccess.h do
not seem to have been committed to SVN as of SVN 1352. I'm attaching
a copy of this patch along with the others that I currently apply
to the platforms tree in order to do a Unix VM build.

Unrelated to the patch I sent for sqMemoryAccess.h, but affecting
the same source file, you had asked if the oopForPointer macro was
defined wrong and I had replied that I thought that it did look
wrong. However, I just got around to trying a build with the macro
defined as:
  #define oopForPointer(ptr) ((sqInt)((ptr) - sqMemoryBase))

and this does *not* work (blows up with a compiler error). So I
don't know what is right here, but you should not make that change
until somebody figures it out.

For reference, here is a summary of the patches (attached) that I
am currently using. Two patches are required, and the other two
are nice to have.  

  platforms-unix-vm-sqUnixCharConv.c.diff -  Provide missing
  sqGetFilenameFromString() function. This function must be present
  in order to build a unix VM.

  platforms-Cross-vm-sqMemoryAccess.h.diff - Missing memory access
  macros.  Required for Linux build.

  platforms-unix-vm-sqUnixExternalPrims.c.diff - Provide meaningful
  error message if e.g. vm-display-X11 fails to load for some reason.
  Nice to have but not required for successful VM build.

  platforms-unix-config-config.h.in.diff - Define #SQAIO_H. Required
  for OSPP only, nice to have.

I'll note also that I cannot build an MPEG plugin for Unix with
the current sources, but I assume this a new issue related to
John's recent upgrades and I have not looked into it yet.

Dave

On Tue, Jan 03, 2006 at 01:34:35PM -0800, tim Rowledge wrote:

>
> On 29-Dec-05, at 12:00 PM, David T. Lewis wrote:
> >
> > Attachments (in zip file):
> >
> > platforms-win32-plugins-FilePlugin-sqWin32FilePrims.c.diff - Change  
> > Win32
> >     FilePlugin to use session ID from the interpreter rather than  
> > generate its
> >     own value (for consistency, and also so as not to break OSPP  
> > for Win32).
> OK, that is up to andreas - I couldn't commit it if I wanted to.
> >
> > platforms-Cross-vm-sqMemoryAccess.h.diff - Add missing memory  
> > access macros
> >     (the inline functions are complete, but some of the  
> > corresponding macros
> >     are missing).
> OK, made the changes. Am I hallucinating or should we really make the
> #define oopForPointer(ptr) ((sqInt)(ptr))
> read
> #define oopForPointer(ptr) ((sqInt)(ptr-sqMemoryBase))
> instead?
>
>
> >
> > platforms-unix-vm-sqUnixExternalPrims.c.diff - Provide meaningful  
> > console
> >     error message if an external module (e.g. vm-display-X11) fails  
> > to load.
> That's one for Ian.
> >
> > platforms-unix-config-config.h.in.diff - Add a definition to unix/
> > config/config.in
> >     to #define SQAIO_H "sqaio.h" due to renaming aio.h to sqaio.h.  
> > Permits OSPP
> >     and AIO plugins to be backward compatible with older source  
> > trees. (Note
> >     to Tim: RiscOS uses a copy of aio.h in its platform tree,  
> > should sqaio.h
> >     be moved to Cross?)
> Well, maybe it should.  I suppose if it is used by two or more  
> platforms it makes a reasonable candidate for Cross. It is another  
> posixy rather than ansi-ish file though.
>
>
> tim
> --
> tim Rowledge; [hidden email]; http://www.rowledge.org/tim
> Never write software that anthropomorphizes the machine. They hate that.
>

vmPatchesForSVN1352.zip (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: New VMMaker/svn release

timrowledge

On 12-Mar-06, at 9:35 AM, David T. Lewis wrote:

> Tim,
>
> Some belated followup on this. The patches for sqMemoryAccess.h do
> not seem to have been committed to SVN as of SVN 1352. I'm attaching
> a copy of this patch along with the others that I currently apply
> to the platforms tree in order to do a Unix VM build.
>
> Unrelated to the patch I sent for sqMemoryAccess.h, but affecting
> the same source file, you had asked if the oopForPointer macro was
> defined wrong and I had replied that I thought that it did look
> wrong. However, I just got around to trying a build with the macro
> defined as:
>   #define oopForPointer(ptr) ((sqInt)((ptr) - sqMemoryBase))
>
> and this does *not* work (blows up with a compiler error). So I
> don't know what is right here, but you should not make that change
> until somebody figures it out.

Weird; I can't spot any differences with the memory access header.  
The version I have (updated today from squeakvm.org) seems correct to  
me. Can you double check and if there are diffs send me the *whole*  
file. I don't read unix diffs very well.


tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Useful random insult:- Several nuts over fruitcake minimum.



123