Bug 790023

This is a separate BZ for tracking the progress of this fix in RHQ 4.3.0.

+++ This bug was initially created as a clone of Bug #771652 +++

Description of problem:
The alert scripts, by the virtue of running in the server currently have full access to all classes inside the RHQ's EAR. This includes local SLSBs, entity manager, raw JDBC connections etc.

This is a problem because a) we don't guarantee any API/DB schema stability across releases (unlike the stability of the remote API), b) users can easily circumvent the permissions given to the script user and c) it is too easy for the users to either shoot themselves in the foot or do outright malicious things in the script to the RHQ server itself.

Note that scripts already can't exit the JVM, which is checked for using a custom security context in which the scripts are running.

Version-Release number of selected component (if applicable):
4.2.0 and up

How reproducible:
always

Steps to Reproduce:
1. create a user that doesn't have privileges to uninventory some resource.
2. create a CLI script alert on that resource that will run as the above user:

context = new javax.naming.InitialContext();
subjectManager = context.lookup('SubjectManagerBean/local');
overlord = subjectManager.getOverlord();

resourceManager = context.lookup('ResourceManagerBean/local');
resourceManager.uninventoryResource(overlord, alert.alertDefinition.resource.id);

  
Actual results:
After the alert fires, the resource is automatically univentoried.

Expected results:
The script shouldn't be able to escalate its permissions like that.

--- Additional comment from lkrejci on 2012-01-04 09:16:28 EST ---

The trick is to disallow the scripts from doing JNDI lookups in the RHQ server.

As for the testing, you can take some inspiration from the unit tests that have been implemented for this functionality:
http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=blob;f=modules/enterprise/server/client-api/src/test/java/org/rhq/enterprise/client/security/test/EjbAccessTest.java;hb=HEAD

http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=blob;f=modules/integration-tests/jndi-access/jndi-access-test/src/test/java/org/rhq/jndi/test/JndiAccessTest.java;hb=HEAD

The fix might influence the LDAP integration in RHQ server. The unit tests pass but I think it would be worth doing some more complex manual testing to make sure nothing broke.

The following commits implement that functionality in master:

commit http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commitdiff;h=6945397a12e96fcaa7c92161e5f474abd98f7cad
Author:	Lukas Krejci <lkrejci>	
Date:   Tue, 3 Jan 2012 14:26:38 +0000 (15:26 +0100)

Merge branch 'lkrejci/dissalow-alert-scripts-from-accessing-local-slsbs'

commit http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commitdiff;h=51fbaef6a41e324118e069b7b5b8915526c2dc34
Author: Lukas Krejci <lkrejci>
Date:   Wed Jan 4 14:15:57 2012 +0100

    Adding support for Ldap, Dir, Event and EventDir contexts in addition to
    the normal JNDI contexts in our JNDI security wrappers. This should unbreak
    the LDAP support in the RHQ server.
    
    Fixed enterprise/server/itests tests.

--- Additional comment from lkrejci on 2012-01-09 09:05:55 EST ---

commit 772e9d247e524071a86d6a25b8e1615ed77e9fcf
Author: Lukas Krejci <lkrejci>
Date:   Mon Jan 9 14:05:09 2012 +0100

    Fixing the application of various decorators to JNDI contexts to support
    contexts that implement more than 1 context interface (like LdapCtx which
    implements both LdapContext and EventDirContext).
    
    This should fix the RHQ's LDAP integration for good.

--- Additional comment from lkrejci on 2012-01-16 03:36:17 EST ---

commit http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commitdiff;h=0e36c3ee8f1954486b1c12ee22ddd4b90cf7e685
Author: Lukas Krejci <lkrejci>
Date:   Mon Jan 9 16:57:11 2012 +0100

    reverting the secured JNDI access... This functionality is still failing
    some tests and so it won't re-emerge in master until I fix them.

--- Additional comment from lkrejci on 2012-01-16 03:39:35 EST ---

commit http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commitdiff;h=3bf2fbfd118d30e2c0c9b616ff12ba7bc9e9492c
Author: Lukas Krejci <lkrejci>
Date:   Fri Jan 13 11:38:18 2012 +0100

Putting back the original version of secured JNDI access so that the latest
    batch of fixes can be merged into it.
    
    This reverts commit 0e36c3ee8f1954486b1c12ee22ddd4b90cf7e685.

--- Additional comment from lkrejci on 2012-01-16 03:56:25 EST ---

commit http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commitdiff;h=94b7a562f4efb9e4d9922e090ceca49deef5c1fd
Author: Lukas Krejci <lkrejci>
Date:   Fri Jan 13 11:38:26 2012 +0100

Merge branch 'lkrejci/dissalow-alert-scripts-from-accessing-local-slsbs'

--- Additional comment from lkrejci on 2012-01-16 06:48:34 EST ---

Things to test:

1) Ldap integration works

2) No AccessControlException in the server logs (unless caused by CLI alerts
that should have caused it as specified below)

3) Create a series of CLI alerts that try to get at
EJBs/EntityManager/SessionManager - look at the tests in
http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=blob;f=modules/enterprise/server/itests/src/test/java/org/rhq/enterprise/client/security/test/JndiAccessTest.java;hb=HEAD
- all of this should not be possible and throw AccessControlExceptions.

4) create CLI alerts that do JNDI lookup against a remote location - this
should work ok. Look at
http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=blob;f=modules/integration-tests/jndi-access/jndi-access-test/src/test/java/org/rhq/jndi/test/JndiAccessTest.java;hb=HEAD

When I say "should/should not be possible" in the above, I mean that the CLI
alert notification shouldn't/should fail with an AccessControlException when
the alert fires.

--- Additional comment from lkrejci on 2012-01-16 09:42:05 EST ---

Java 6u27 changed the way Rhino script engine is secured. Our implementation, though, should support both the pre-u27 and post-u27 JREs and the access should be controlled.

--- Additional comment from mfoley on 2012-02-02 14:19:43 EST ---

regression testing on LDAP


  - LDAP Role mapping without SSL:    
https://tcms.engineering.redhat.com/run/33188/

  - LDAP Role mapping with SSL:      
https://tcms.engineering.redhat.com/run/33182/

--- Additional comment from mfoley on 2012-02-02 14:28:06 EST ---

I am getting an error on step #2 of the repro steps .... as show below.
 

This might be an issue ... or the repro steps needs to be clarified.  Lukas ... thoughts?


mfoley@localhost:7080$ context = new javax.naming.InitialContext();
InitialContext:
	    environment: 
	nameInNamespace: 

mfoley@localhost:7080$ subjectManager = context.lookup('SubjectManagerBean/local');
TypeError: Cannot find function lookup in object javax.script.SimpleScriptContext@51af7c57. (<Unknown source>#1)
subjectManager = context.lookup('SubjectManagerBean/local'); 
^

mfoley@localhost:7080$ s          

saveBytesToFile   scriptUtil        sleep             subject
mfoley@localhost:7080$

--- Additional comment from lkrejci on 2012-02-03 08:23:05 EST ---

note that this is currently NOT in JON 3.0.1. As the target release field says, this is only in RHQ 4.3.0.

as of commit cb78bd893aa614d0fcc5760f2a25a73175fe6c2e I updated the testcases to not use the variable called "context" which seems to be predefined (and least in the CLI).

That being said, to test this bug, the scripts need to run on the RHQ server (not in the standalone CLI), because the secured JNDI is only applied to scripts running on server - there is no point in doing that on the client.

So in order to test this bug, you need to be running those scripts as CLI alerts because that is currently the only possible way of running CLI scripts on the serverside.

--- Additional comment from lkrejci on 2012-02-03 13:21:57 EST ---

Created attachment 559341 [details]
test scripts

I'm attaching a number of scripts that can be, one by one, specified as notifications on an alert definition.

The negative-testcase-*.js files should run with error when the alert fires (the error should state that there has been an AccessControlException denying access due to the lack of AllowRhqServerInternalsAccessPermission.

The positive-testcase-*.js files should on the other hand run successfully without an error.

The positive-testcase-2.js requires an external remotely accessible JBoss AS5 server running with no authentication on JNP port. Prior to uploading the script as the alert notification it has to be edited to point to the external JBoss AS5 server (one has to specify the host and JNP port of the server).

Apart from verifying that the above scripts can run as alert notifications as specified, the LDAP integration needs to be retested because the fix to this bug influences the way LDAP connection is realized.

--- Additional comment from lkrejci on 2012-02-06 17:22:19 EST ---

This has been merged into release/jon3.0.x: http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commitdiff;h=1c7b0aefd0060c2daf15cb95512704934c7ee1af

Things to test:

1) Create a single Alert definition with a number of CLI script notifications - one per each file in the attached test scripts (you of course need to first define a repository where to upload those scripts in Administration -> Repositories)

2) Wait for an alert based on the above definition to fire. Verify that the notifications created from the positive-testcase-*.js files ran successfully (remember to configure the positive-testcase-2.js) and the all the notifications created from the negative-testcase-*.js files failed with an AccessControlException.

3) Perform all the LDAP-related tests.

As part of the dev testing, the steps 1 and 2 were done successfully on a local build of JON 3.0.1.

--- Additional comment from mfoley on 2012-02-08 15:10:25 EST ---

adding data from #jboss-on relevant to this BZ .... 

<spinder> this showed up because with the recent JNDI changes caused the extra copy of server.jar in the ear .. 
<spinder> which put an old copy of ProductInfo.properties in the classpath ahead of the ones we intended to load.
<spinder> so fixes that went in to RC3 changed the landscape a little and it bit us. 
<spinder> the dependency hierarchy has been there for a while.  But the extra .props file has not. 


this is causing spinder to report JON productization issues in his pre-qual of RC#3.  

<spinder> well it caused the branding issues that I saw in RC3. But other than that I'm not sure. 
<mfoley> ok ... how did the extra .jar (presumably identical) ...cause a branding issue? i want to learn more
<mfoley> the more i know ... the more i will be able to find other issues
<spinder> basically it has an old copy of the ProductInfo.properties file within the jar itself. At build time we generate the correct one and it was being loaded correctly.

--- Additional comment from mfoley on 2012-02-08 18:10:00 EST ---

adding a comment to expand on comment #12.  in a post-scrum phone conversation on 2/6/2012 (crouch, lukas, mfoley) lukas further elaborated that he additionally performed a 1st pass of integration testing of this BZ.

--- Additional comment from spinder on 2012-02-09 14:41:24 EST ---

Moving this to ON_QA as new RC3 binary available here:
https://brewweb.devel.redhat.com//buildinfo?buildID=198086