osgi / bugzilla-archive

Archive of OSGi Alliance Specification Bugzilla bugs. The Specification Bugzilla system was decommissioned with the move to GitHub. The issues in this repository are imported from the Specification Bugzilla system for archival purposes.
0 stars 1 forks source link

Framework SPI for Declarative Services #267

Closed bjhargrave closed 17 years ago

bjhargrave commented 18 years ago

Original bug ID: BZ#288 From: glyn.normington@springsource.com Reported version: R4

bjhargrave commented 18 years ago

Comment author: glyn.normington@springsource.com

This came up in JSR 291 - see the mailing list archives for April 2006.

The requirement is to define a SPI to the core framework that allows Declarative Services (DS) to be constructed strictly on top of the framework plus SPI (i.e. without framework implementation specific dependencies).

The main issue that the CPEG have been turning over appears to be the security of such a SPI when security is turned off. See the CPEG mailing list archives for details.

bjhargrave commented 18 years ago

Comment author: glyn.normington@springsource.com

A nice summary posted here for ease of reference.

"Kevin Riff" <kriff@ espial.com> wrote on 27/04/2006 18:12:01:

I think I'll just sum up my position and then drop the whole thing as I'm clearly not convincing anyone.

As I understand it, you want to introduce an API that would give developers access to another bundle's BundleContext so that features like Declarative Services can be implemented in a portable way that will work on anyone's framework implementation. Sounds like a good idea to me.

But BJ points out that this is a very dangerous thing and could easily be abused. It needs to be accessible only to "trusted" bundles. Unfortunately most frameworks are deployed without a SecurityManager, or if a security manager is installed then all bundles are granted AllPermission (which amounts to the same thing). Up till now, the framework has been "reasonably" secure even if Java's security features are disabled but this new API creates a huge vulnerability. The proposal then is to create some kind of ad hoc security mechanism that would function even on totally unsecure deployments.

I think Peter said it best: You can't have your cake and eat it too. You can't talk about the notion of a "trusted" bundle unless Java's security mechanisms are in place. Without the SecurityManager there is no trust, so a "trusted" bundle is an oxymoron. For this reason, I'm convinced that any ad hoc approach to security will ultimately fail.

If your code doesn't run in a properly locked-down environment then run it without a SecurityManager if you must. Or grant your bundle (and anything else it needs) AllPermission. Either way you have to accept the consequences that go with an unsecure system. I don't see why we should compromise our security model for the benefit of people who either can't or won't fix their own security issues.

I had suggested simply denying access to everybody if the SecurityManager is not present. People don't seem to like this idea because it requires that a SecurityManager always be installed if you need to use the API. That doesn't seem like a big deal to me since you can always grant everybody AllPermission if you want. However the more I think about it the more I realize it still doesn't work. A nefarious bundle could easily get around it (by installing its own SecurityManager for example) so it doesn't actually improve the situation.

So in short, my position is that the API should be protected by a standard permission check using the built-in Java 2 fine-grained permissions model. And secondly, we have to accept that if Java's security features aren't turned on, or they're thwarted by granting AllPermission to untrustworthy code, then protecting any part of the system is actually impossible.

bjhargrave commented 18 years ago

Comment author: glyn.normington@springsource.com

A helpful suggestion.

Olivier Gruber <ogruber@ us.ibm.com> wrote on 28/04/2006 11:29:22:

Why not use services?

It would have the same set of issues, but it does not impact cpeg APIs. It would also be more easily optional and replaceable/modifiable later on.

I always wondered why we do not surface some of containerisms as optional services. This super bundle context is a typical example, we could have a service providing the functionality of finding out BundleContext of other bundles.

With security turned on, it is secure. Without security turned on, it is not secure. Nothing more or less that the super bundle concept... But, the major point is that it does not impact core APIs. It is just a service, and it is optional.

Why services are not a good path for those optional behaviors? In particular, services are known concepts that can be optional so provisioning will be overall simpler if we start modelling optional behaviors as services. It just fits with the overall flexibility of available services and the corresponding deployability of bundles.

bjhargrave commented 18 years ago

Comment author: Richard Hall <richard.s.hall@sun.com>

This appears to be an issue for Spring integration too.

bjhargrave commented 18 years ago

Comment author: @bjhargrave

This appears to be an issue for Spring integration too.

Yes. This came up in the spring-osgi f2f discussions. I mentioned that we did not include Bundle.getBundleContext due to "security" concerns. But the reality is the without security turned on, people just get it via reflection. The spring-osgi code has a method which uses reflection to obtain the BundleContext for a bundle which works on Equinox, Felix and Knopflerfish. So who are we kidding by not providing a Bundle.getBundleContext method? Just ourselves I think :-). With no security manager, anyone can obtain the BundleContext for a bundle with reflection.

We should just add a getBundleContext method and protect it with a permission. It is a lot cleaner that forcing everyone to use reflection and is no less secure.

bjhargrave commented 18 years ago

Comment author: Jeff McAffer <jeff_mcaffer@ca.ibm.com>

we might consider something like the following on Bundle /**

/**

bjhargrave commented 18 years ago

Comment author: @bjhargrave

we might consider something like the following on Bundle /**

  • Returns the bundle context associated with the bundle that
  • loaded the calling method. Returns null if the bundle is not activated. **/ public static BundleContext getBundleContext();

We cant do this on Bundle since Bundle is an interface. We could put such a method on FrameworkUtil but such an API is not needed for the use case suggested by this bug which is obtaining the context of an given bundle. I believe that the static method is for the use case where some code wants to know the context of the bundle from which it is loaded.

bjhargrave commented 18 years ago

Comment author: @bjhargrave

CPEG agreed to leave the static getBundleContext method as part of bug BZ#4. This bug will only consider the non-static getBundleContext method in this bug.

bjhargrave commented 18 years ago

Comment author: @bjhargrave

Created attachment 20 New Bundle.getBundleContext method

Here is the proposed method to obtain the BundleContext for a bundle:

/**
 * Returns this bundle's {@ link BundleContext}. The returned <code>BundleContext</code>
 * can be used by the caller to act on behalf of this bundle.
 * 
 * <p>
 * If this bundle is not in the {@ link #STARTING}, {@ link #ACTIVE}, 
 * or {@ link #STOPPING} states, then this bundle has no valid 
 * <code>BundleContext</code>. This method will return <code>null</code>
 * if this bundle has no valid <code>BundleContext</code>.
 * 
 * @ return A <code>BundleContext</code> for this bundle or <code>null</code> if this bundle
 *         has no valid <code>BundleContext</code>.
 * @ throws java.lang.IllegalStateException If this bundle has been
 *         uninstalled.
 * @ throws java.lang.SecurityException If the caller does not have the
 *         appropriate <code>AdminPermission[this,CONTEXT]</code>, and the
 *         Java Runtime Environment supports permissions.
 * @ since 1.4
 */
public BundleContext getBundleContext();

The attached patch also includes changes to AdminPermission to add a new context action and to update the version of the org.osgi.framework package to version 1.4.

Attached file: patch.txt (text/plain, 5435 bytes) Description: New Bundle.getBundleContext method

bjhargrave commented 18 years ago

Comment author: @bjhargrave

After discussion in CPEG, I commited the changes minus the throwing of the IllegalStateException.

The Framework RI needs to be updated to implement this method and DS RI should be changed to use this methods afterwords.

Given this new API, the Declarative Services specification needs to be updated. Section 112.8.1 can be removed or replaced with a reference to this new method. Section 112.9 needs to be updated to state that a DS implementation will need AdminPermission[*,CONTEXT] so it can call the new method.

bjhargrave commented 18 years ago

Comment author: @bjhargrave

Assigned to Peter to make specification text changes

bjhargrave commented 18 years ago

Comment author: @pkriens

  1. Added 4.3.14, explaining getBundleContext and its threats
  2. Added a row in the AdminPermission for context
  3. Changed 112 SCR to remove the section 112.8.1 that discussed access to the Bundle Context
  4. Added a section for the required Admin Permission
bjhargrave commented 17 years ago

Comment author: glyn.normington@springsource.com

I have communicated this back to the JSR 291 EG.

See: http://bundles.osgi.org/pipermail/jsr-291-eg/2006-October/000182.html

bjhargrave commented 17 years ago

Comment author: @bjhargrave

While looking ad adding a TCK test for this, I realized we have not specified the behavior of getBundle(0).getBundleContext(). That is, what should getBundleContext return when called on the System Bundle? Do we really want to return the system bundle's context? What are the risks of providing access to the system bundle's context?

I think we should not return the system bundle's context and instead throw some runtime exception (e.g. UnsupportedOperationException).

bjhargrave commented 17 years ago

Comment author: @bjhargrave

Also, we need to state the behavior when called on a fragment bundle. I think we should return null for fragment bundles.

bjhargrave commented 17 years ago

Comment author: @pkriens

Aren't we then making the same mistake again as before? There is no security without a security manager?

Are there valid reasons why the system bundle would NOT have a bundlecontext? I assume all implementations have a context to register their services.

bjhargrave commented 17 years ago

Comment author: @tjwatson

If the system bundle cannot have a BundleContext then this implies that it cannot define any declarative services and may have other implications. I prefer to allow the system bundle to have a BundleContext just like other bundles otherwise it introduces some confusing inconsistencies.

bjhargrave commented 17 years ago

Comment author: @bjhargrave

OK, So we allow getBundle(0).getBundleContext() to return the system bundle's context. The TCK test I added last night checks that getBundle(0).getBundleContext() returns a bundle context. :-)

What about fragment bundles? In the TCK test for this API I added last night, the test require that getBundleContext() return null for fragment bundles under the argument that fragment bundles have not valid bundle context. The tests passed on the RI. So I think we need to add some text to the spec which states that fragment bundles do not have a valid bundle context.

bjhargrave commented 17 years ago

Comment author: @tjwatson

According to the javadoc of Bundle.getBundleContext ...

"If this bundle is not in the STARTING, ACTIVE, or STOPPING states, then this bundle has no valid BundleContext. This method will return null if this bundle has no valid BundleContext."

Since a fragment cannot be started there is no way for it to get into the STARTING, ACTIVE or STOPPING state. This implies it will never have a valid BundleContext. Having said that, I still think the javadoc should add a sentence stating ... "A fragment bundle will never have a valid BundleContext."

bjhargrave commented 17 years ago

Comment author: @bjhargrave

OK, I updated the javadoc to list fragments as having no valid bundle context.

bjhargrave commented 17 years ago

Comment author: @bjhargrave

Resolved.