You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by jo...@gmail.com on 2011/09/06 23:13:01 UTC

Re: Review Request: Securing Features and RPC

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1666/#review1772
-----------------------------------------------------------


Hey Ryan,

The original intent of the <exports type="rpc">fooKey</exports> syntax is that it is simply a declaration that the feature has in it somwhere:
gadgets.rpc.register("fooKey", /* impl */);

As a corollary, <uses type="rpc">barKey</uses> means that there's a code path that does gadgets.rpc.call("barKey", ...); Between these two, intelligent validation and wiring could be done.

This comes with the caveat that the syntax hasn't actually been put to practice yet, and real world utility trumps abstract thinking every time. This said, I do assert that this syntax ought to be purely declarative, as part of a manifest describing the feature.

You could use the <uses> pieces to describe which rpc endpoints are _candidates_ for use by the gadget -- but I wouldn't use this for permissions, since those may differ container by container.

--j

- johnfargo


On 2011-08-26 16:05:12, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1666/
> -----------------------------------------------------------
> 
> (Updated 2011-08-26 16:05:12)
> 
> 
> Review request for shindig and johnfargo.
> 
> 
> Summary
> -------
> 
> We want to allow administrators of containers to secure the features gadgets are allowed to use. We can do this by doing two things to the container. 
> 
> 1.) When the gadget preloads a gadget and requests the iFrame URL for the gadget (indicating the container wants to render the gadget) we check the list of allowed features for that gadget in that container. In theory the administrator of the container would setup this list. If the gadget wants to use a feature that is not in the list approved by the administrator the preload fails. 
> 
> 2.) Secure what RPC service id a gadget can call. A gadget should only be able to call the RPC service ids from the features the administrator has approved for that gadget in that container. If the gadget tried to call and RPC service id that is not in a feature the administrator has approved than the RPC call will be "blocked".
> 
> THIS PATCH IS INCOMPLETE.  I just want to start to get feedback on this before I get too deep and have to go back and change something major.  Currently this patch implements number 1 above and part of number 2.  There no code yet to arbitrate the RPC calls, right now this patch only returns the allowed RPC service IDs in the metadata request, but it doesn't do anything with them.
> 
> One of the major concerns I have is my use of <export type="rpc"..../> in the feature XML files.  I am not sure if I am using them in the intended way.  Hoping John H can take a look at that.
> 
> 
> This addresses bug SHINDIG-1601.
>     https://issues.apache.org/jira/browse/SHINDIG-1601
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeProcessor.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/FeatureSecurityInfoTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/ContainerSecurityInfoTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/BasicFeatureSecurityStoreTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/FeatureSecurityModule.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/FeatureSecurityStore.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/BasicFeatureSecurityStore.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/pom.xml 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetException.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/RenderingContext.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/config/feature-security.json PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1157104 
> 
> Diff: https://reviews.apache.org/r/1666/diff
> 
> 
> Testing
> -------
> 
> Created/updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>


Re: Review Request: Securing Features and RPC

Posted by John Hjelmstad <fa...@google.com>.
On Tue, Sep 6, 2011 at 5:25 PM, Ryan Baxter <rb...@gmail.com> wrote:

>
>
> > On 2011-09-06 21:13:01, johnfargo wrote:
> > > Hey Ryan,
> > >
> > > The original intent of the <exports type="rpc">fooKey</exports> syntax
> is that it is simply a declaration that the feature has in it somwhere:
> > > gadgets.rpc.register("fooKey", /* impl */);
> > >
> > > As a corollary, <uses type="rpc">barKey</uses> means that there's a
> code path that does gadgets.rpc.call("barKey", ...); Between these two,
> intelligent validation and wiring could be done.
> > >
> > > This comes with the caveat that the syntax hasn't actually been put to
> practice yet, and real world utility trumps abstract thinking every time.
> This said, I do assert that this syntax ought to be purely declarative, as
> part of a manifest describing the feature.
> > >
> > > You could use the <uses> pieces to describe which rpc endpoints are
> _candidates_ for use by the gadget -- but I wouldn't use this for
> permissions, since those may differ container by container.
> > >
> > > --j
>
> Awesome John this will work nicely.  I will be sure to use this in v2 of
> the patch.
>
> I am not going to use "uses" for permissions directly.  Basically the admin
> would specify which features a gadget is allowed to you and based on that
> information we can infer the RPC services the gadget calls by looking up the
> features in the features registry.  That list can then be returned to
> container to try to arbitrate the RPC calls made by the gadget.  Based on
> our other email thread it looks like we will only be able to arbitrate calls
> make from the gadget to the container...
>

That's correct -- unless we were to take on substantially more work in the
form of:
A) Introducing a mandatory intermediary auth-wrapper IFRAME, which in turn
IFRAMEs the gadget itself. This has actually been done in various fashions,
but never really formalized. The general idea is that the intermediary frame
is wired to proxy all requests between container and gadget, but be
controlled by the container.

This is tricky on many levels: layout, security model, rpc (prevent calls
from anyone except intermediary), and so on.

B) Cajoling gadget contents and injecting a security layer accordingly. This
mandates all sorts of wiring changes.

Long story short, not advisable. Fortunately, at least as far as I'm aware
there's no real push from gadget dev's perspective to limit container access
to its RPCs, just the other way around.

-j


>
>
> - Ryan
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1666/#review1772
> -----------------------------------------------------------
>
>
> On 2011-08-26 16:05:12, Ryan Baxter wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/1666/
> > -----------------------------------------------------------
> >
> > (Updated 2011-08-26 16:05:12)
> >
> >
> > Review request for shindig and johnfargo.
> >
> >
> > Summary
> > -------
> >
> > We want to allow administrators of containers to secure the features
> gadgets are allowed to use. We can do this by doing two things to the
> container.
> >
> > 1.) When the gadget preloads a gadget and requests the iFrame URL for the
> gadget (indicating the container wants to render the gadget) we check the
> list of allowed features for that gadget in that container. In theory the
> administrator of the container would setup this list. If the gadget wants to
> use a feature that is not in the list approved by the administrator the
> preload fails.
> >
> > 2.) Secure what RPC service id a gadget can call. A gadget should only be
> able to call the RPC service ids from the features the administrator has
> approved for that gadget in that container. If the gadget tried to call and
> RPC service id that is not in a feature the administrator has approved than
> the RPC call will be "blocked".
> >
> > THIS PATCH IS INCOMPLETE.  I just want to start to get feedback on this
> before I get too deep and have to go back and change something major.
>  Currently this patch implements number 1 above and part of number 2.  There
> no code yet to arbitrate the RPC calls, right now this patch only returns
> the allowed RPC service IDs in the metadata request, but it doesn't do
> anything with them.
> >
> > One of the major concerns I have is my use of <export type="rpc"..../> in
> the feature XML files.  I am not sure if I am using them in the intended
> way.  Hoping John H can take a look at that.
> >
> >
> > This addresses bug SHINDIG-1601.
> >     https://issues.apache.org/jira/browse/SHINDIG-1601
> >
> >
> > Diffs
> > -----
> >
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeProcessor.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/FeatureSecurityInfoTest.javaPRE-CREATION
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/ContainerSecurityInfoTest.javaPRE-CREATION
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/BasicFeatureSecurityStoreTest.javaPRE-CREATION
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/FeatureSecurityModule.javaPRE-CREATION
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/FeatureSecurityStore.javaPRE-CREATION
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/BasicFeatureSecurityStore.javaPRE-CREATION
> >   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/pom.xml1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetException.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/RenderingContext.java1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js1157104
> >   http://svn.apache.org/repos/asf/shindig/trunk/config/container.js1157104
> >
> http://svn.apache.org/repos/asf/shindig/trunk/config/feature-security.jsonPRE-CREATION
> >
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java1157104
> >
> > Diff: https://reviews.apache.org/r/1666/diff
> >
> >
> > Testing
> > -------
> >
> > Created/updated unit tests.
> >
> >
> > Thanks,
> >
> > Ryan
> >
> >
>
>

Re: Review Request: Securing Features and RPC

Posted by Ryan Baxter <rb...@gmail.com>.

> On 2011-09-06 21:13:01, johnfargo wrote:
> > Hey Ryan,
> > 
> > The original intent of the <exports type="rpc">fooKey</exports> syntax is that it is simply a declaration that the feature has in it somwhere:
> > gadgets.rpc.register("fooKey", /* impl */);
> > 
> > As a corollary, <uses type="rpc">barKey</uses> means that there's a code path that does gadgets.rpc.call("barKey", ...); Between these two, intelligent validation and wiring could be done.
> > 
> > This comes with the caveat that the syntax hasn't actually been put to practice yet, and real world utility trumps abstract thinking every time. This said, I do assert that this syntax ought to be purely declarative, as part of a manifest describing the feature.
> > 
> > You could use the <uses> pieces to describe which rpc endpoints are _candidates_ for use by the gadget -- but I wouldn't use this for permissions, since those may differ container by container.
> > 
> > --j

Awesome John this will work nicely.  I will be sure to use this in v2 of the patch.  

I am not going to use "uses" for permissions directly.  Basically the admin would specify which features a gadget is allowed to you and based on that information we can infer the RPC services the gadget calls by looking up the features in the features registry.  That list can then be returned to container to try to arbitrate the RPC calls made by the gadget.  Based on our other email thread it looks like we will only be able to arbitrate calls make from the gadget to the container...


- Ryan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1666/#review1772
-----------------------------------------------------------


On 2011-08-26 16:05:12, Ryan Baxter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1666/
> -----------------------------------------------------------
> 
> (Updated 2011-08-26 16:05:12)
> 
> 
> Review request for shindig and johnfargo.
> 
> 
> Summary
> -------
> 
> We want to allow administrators of containers to secure the features gadgets are allowed to use. We can do this by doing two things to the container. 
> 
> 1.) When the gadget preloads a gadget and requests the iFrame URL for the gadget (indicating the container wants to render the gadget) we check the list of allowed features for that gadget in that container. In theory the administrator of the container would setup this list. If the gadget wants to use a feature that is not in the list approved by the administrator the preload fails. 
> 
> 2.) Secure what RPC service id a gadget can call. A gadget should only be able to call the RPC service ids from the features the administrator has approved for that gadget in that container. If the gadget tried to call and RPC service id that is not in a feature the administrator has approved than the RPC call will be "blocked".
> 
> THIS PATCH IS INCOMPLETE.  I just want to start to get feedback on this before I get too deep and have to go back and change something major.  Currently this patch implements number 1 above and part of number 2.  There no code yet to arbitrate the RPC calls, right now this patch only returns the allowed RPC service IDs in the metadata request, but it doesn't do anything with them.
> 
> One of the major concerns I have is my use of <export type="rpc"..../> in the feature XML files.  I am not sure if I am using them in the intended way.  Hoping John H can take a look at that.
> 
> 
> This addresses bug SHINDIG-1601.
>     https://issues.apache.org/jira/browse/SHINDIG-1601
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerServiceTest.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/FakeProcessor.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/FeatureSecurityInfoTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/ContainerSecurityInfoTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/security/BasicFeatureSecurityStoreTest.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerService.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/FeatureSecurityModule.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/FeatureSecurityStore.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/features/FeatureRegistry.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/security/BasicFeatureSecurityStore.java PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/pom.xml 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGuiceModule.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetException.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/RenderingContext.java 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 1157104 
>   http://svn.apache.org/repos/asf/shindig/trunk/config/feature-security.json PRE-CREATION 
>   http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerTest.java 1157104 
> 
> Diff: https://reviews.apache.org/r/1666/diff
> 
> 
> Testing
> -------
> 
> Created/updated unit tests.
> 
> 
> Thanks,
> 
> Ryan
> 
>