Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 369689 - [DS] BundleSignerCondition and Declarative Services
Summary: [DS] BundleSignerCondition and Declarative Services
Status: RESOLVED INVALID
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: Compendium (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: equinox.compendium-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-25 10:56 EST by Missing name Mising name CLA
Modified: 2012-01-27 10:50 EST (History)
2 users (show)

See Also:


Attachments
bundles to reproduce the bug (54.47 KB, application/zip)
2012-01-26 06:34 EST, Missing name Mising name CLA
no flags Details
Startup log with stacktrace (5.21 KB, text/plain)
2012-01-26 06:39 EST, Missing name Mising name CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Missing name Mising name CLA 2012-01-25 10:56:19 EST
Build Identifier: Version: Indigo Service Release 1 Build id: 20110916-0149

We tried to create a ConditionalPermissionInfo which allows only our bundles to register some specific services. So we signed all our bundles and installed the following ConditionalPermissionInfo:

"DENY{ [ org.osgi.service.condpermadmin.BundleSignerCondition \"*, O=OurOrganisation, L=Vienna, C=Austria\" \"!\" ] (org.osgi.framework.ServicePermission \"com.smart.test.TestAPI\" \"REGISTER\") }"

(--> means: all bundles not signed by us are not allowed to register com.smart.test.TestAPI services)

This works totally fine as long as we create our services in the activator and register them via the BundleContext. But as we start to register your service with Declarative Services we receive the following AccessControlException:

java.security.AccessControlException: access denied (org.osgi.framework.ServicePermission com.smart.test.TestAPI register)

This exception occurs because DS only registered a ServiceReg object instead of our service to archive lazy loading. Creation and Registration happens in org.eclipse.equinox.internal.ds.InstanceProcess#registerService(ServiceComponentProp scp, boolean factory, ComponentInstanceImpl ci) line 501.

Maybe it would be better to create and register the "proxy" object (ServiceReg) inside of a PrivilegedAction or something familiar to grant the "proxy" object (ServiceReg) the AccessControlContext of the bundle which provides the component definition for DS.

Reproducible: Always

Steps to Reproduce:
1. register a ConditionalPermissionInfo which has a condition and permission like: all bundles not signed by us are not allowed to register a specific service
e.g.: "DENY{ [ org.osgi.service.condpermadmin.BundleSignerCondition \"*, O=OurOrganisation, L=Vienna, C=Austria\" \"!\" ] (org.osgi.framework.ServicePermission \"com.smart.test.TestAPI\" \"REGISTER\") }"
2. create a bundle which is signed and registers a "com.smart.test.TestAPI" implementation via Declarative Services
3. activate security and start the environment
Comment 1 Thomas Watson CLA 2012-01-25 11:32:28 EST
I think you need to grant the DS implementation bundle permission to register all services.  Otherwise there is no amount of doPriv that the DS impl can do to avoid this permission check.
Comment 2 Stoyan Boshev CLA 2012-01-25 14:07:02 EST
Please check the security considerations in DS specification in the OSGI compandium specification document. This is chapter "112.9 Security".

Anyway, I think if you are seeing the AccessControlException for the ServicePermission then the problem might not be not in DS implementation. SCR has successfully accuired the BC and has started using it. Perhaps the FW impl is not making the correct priviledge call in the implementation of BC.registerService()?

Could you post the whole exception stack of the AccessControlException you are seeing? 
It would also be very helpful if you prepare a test bundle/s to reproduce this.
Comment 3 BJ Hargrave CLA 2012-01-25 14:48:51 EST
The permission check in the framework only cares about the names under which the service is to be registered. It does not care about the service object for the permission check.

DS must include the ACC of the bundle in the register call to make sure the bundle itself has permission to register the service. DS must not be able to register a service on behalf of a bundle that the bundle itself cannot do.

So DS must be granted permission to register any service and DS must include the ACC of the bundle when registering/getting services on behalf of the bundle.
Comment 4 Missing name Mising name CLA 2012-01-26 06:34:07 EST
Created attachment 210108 [details]
bundles to reproduce the bug

The attached EclipseBug369689.zip contains 6 eclipse projects.

For reproducing the error you will only need to:
- import the "Config" project into your eclipse workspace
- set the "Bug369689.target" target contained inside of the Config project
- launch the "Bug369689.launch" also contained inside of the Config project

structure of the "Config" project:
 +- keystores
 |  +- signing.ks (used to sign the bundles)
 |  +- trusted.ks (used to run the environment)
 |  +- README (commands used to create the keystores and passwords)
 |
 +- signedBundles (contains the exported and signed test bundles)
 |
 +- Bug369689.launch (the launch config I used)
 +- Bug369689.target (the target containing the signed bundles from Config/signedBundles)

The other 5 plugins contain the source of the test bundles.
 * CommandLineExtension - an extension of the OSGi console to run the registered services
 * SecurityManager - installes the ConditionalPermissionInfo's
 * TestAPI - contains the interface for the services
 * TestImplRegViaActivator - registers a service implementation manually via the Activator
 * TestImplRegViaDS - registers a service implementation via DS


Some hints:
- Before launching make sure that the signed exported bundles from the target are used to the test environment and not the one from the workspace.
- Check if the "Clear the configuration area before launching" flag is set before starting
Comment 5 Missing name Mising name CLA 2012-01-26 06:39:14 EST
Created attachment 210109 [details]
Startup log with stacktrace

This Log was created with the test bundles attached previously.

It contains:
 - the stacktrace 
 - an output of the ss-command
 - an the output of the service call where you can see that the manual registration works and the DS registration doesn't
Comment 6 Thomas Watson CLA 2012-01-26 09:22:10 EST
(In reply to comment #3)
> The permission check in the framework only cares about the names under which
> the service is to be registered. It does not care about the service object for
> the permission check.
> 
> DS must include the ACC of the bundle in the register call to make sure the
> bundle itself has permission to register the service. DS must not be able to
> register a service on behalf of a bundle that the bundle itself cannot do.
> 

I thought this could be done using Bundle.hasPermission against the bundle that defines the component.

> So DS must be granted permission to register any service and DS must include
> the ACC of the bundle when registering/getting services on behalf of the
> bundle.

I agree, the DS impl must be granted all service permissions.  There is no changes we can make to the DS impl or framework impl to avoid that since the DS impl is performing a privileged action on behalf of another bundle.  The DS code must be granted the necessary permission to do that.  I think this bug is invalid.
Comment 7 BJ Hargrave CLA 2012-01-26 10:05:21 EST
(In reply to comment #6)
> I thought this could be done using Bundle.hasPermission against the bundle that
> defines the component.

This assumes that DS knows exactly how to do the permission check. It is much cleaner for DS to have servicepermission to register/get all services and just include the bundle's ACC in the call to the framework. There is less risk then of the DS impl performing the checks incorrectly. 

> I agree, the DS impl must be granted all service permissions.  There is no
> changes we can make to the DS impl or framework impl to avoid that since the DS
> impl is performing a privileged action on behalf of another bundle.  The DS
> code must be granted the necessary permission to do that.  I think this bug is
> invalid.

We need to confirm that the test case grants the necessary permissions to the DS impl and also that the DS impl performs the security tests (either Bundle.hasPermission or doPriv over bundle's ACC).

I am quite sure the framework's permission checks are correct. Don't know about the DS impl or the whether the test case has properly configured the permissions for the DS impl. 

In looking at the stack trace, I don't see any indication the DS impl has a doPriv in effect. Even though it is DS's own thread, the creation of that thread will have collected an ACC from the parent thread. Perhaps that thread creation was done under a doPriv?
Comment 8 Thomas Watson CLA 2012-01-26 10:45:04 EST
(In reply to comment #7)
> (In reply to comment #6)
> > I thought this could be done using Bundle.hasPermission against the bundle that
> > defines the component.
> 
> This assumes that DS knows exactly how to do the permission check. It is much
> cleaner for DS to have servicepermission to register/get all services and just
> include the bundle's ACC in the call to the framework. There is less risk then
> of the DS impl performing the checks incorrectly. 
> 
Keep in mind there was no standard way to get the ACC until the R5 core specification (Equinox Juno), bug358894.
Comment 9 BJ Hargrave CLA 2012-01-26 10:58:30 EST
(In reply to comment #8)
> Keep in mind there was no standard way to get the ACC until the R5 core
> specification (Equinox Juno), bug358894.

True, but the DS impl can do:

new AccessControlContext(new ProtectionDomain[] {componentImplementationClass.getProtectionDomain()});

This does require the DS impl to be granted RuntimePermission("getProtectionDomain").

This also assumes the component implementation class comes from the bundle which may not be the case.
Comment 10 Stoyan Boshev CLA 2012-01-26 15:10:11 EST
The DS implementation does use Bundle.hasPermission to check whether the component's bundle has the necessary permissions.

The current problem seems to be that the DS bundle is not granted the ServicePermission to register the service com.ourorganisation.test.SimpleAPI. 

Rene, I would suggest you to grant the DS bundle ServicePermission to register/get any services. 

As a side note, I think the DS specification shall explicitly state the requirement for the ServicePermission granted to the DS bundle.
Comment 11 Missing name Mising name CLA 2012-01-27 06:06:28 EST
After adding the following permissions to the DS bundle it worked as expected.

"ALLOW{ [org.osgi.service.condpermadmin.BundleLocationCondition \"initial@reference:file:plugins/org.eclipse.equinox.ds_1.3.1.R37x_v20110701.jar/\"] (org.osgi.framework.ServicePermission \"*\" \"REGISTER,GET\") (java.lang.RuntimePermission \"accessDeclaredMembers\") }"

so required is:
Comment 12 Missing name Mising name CLA 2012-01-27 06:12:14 EST
- org.osgi.framework.ServicePermission "*" "REGISTER,GET" << to allow registration
- java.lang.RuntimePermission "accessDeclaredMembers" << to call the activator methods

Would be fine if someone could add this to the documentation.

Sorry for that bug, it was my fault. Now it works as expected normal signed bundles can declare the services via DS and unsigned bundles trying to declare a bundle produce the console output:

!ENTRY org.eclipse.equinox.ds 2 0 2012-01-27 12:02:27.377
!MESSAGE [SCR] Cannot satisfy component 'TestImplRegViaDSUnSigned' because its bundle does not have permissions to register service with interface com.ourorganisation.test.SimpleAPI

and are listed as unsatisfied DS entries.

osgi> ls
All Components:
ID	State			Component Name			Located in bundle
1	Active		TestImplRegViaDS			TestImplRegViaDS(bid=2)
2	Unsatisfied		TestImplRegViaDSUnSigned			TestImplRegViaDSUnSigned(bid=3)


Thanks for your help and again I'm sorry.
Comment 13 Thomas Watson CLA 2012-01-27 10:50:09 EST
(In reply to comment #12)
> Thanks for your help and again I'm sorry.

No problem.  It is good to get the issue documented and the resolution realized!  Even if that means the original "bug" turned out to be invalid.

I opened bug369952 to consider using the component bundle's ACC instead of bundle.hasPermission.