Bug 771652

Summary: Prevent Alert CLI scripts from being able to access RHQ server's internals
Product: [Other] RHQ Project Reporter: Lukas Krejci <lkrejci>
Component: Core ServerAssignee: Lukas Krejci <lkrejci>
Status: CLOSED CURRENTRELEASE QA Contact: Mike Foley <mfoley>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 4.3CC: hrupp, spinder
Target Milestone: ---   
Target Release: RHQ 4.3.0, JON 3.0.1   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: 4.3.0 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 787214 787218 787274 790023 (view as bug list) Environment:
Last Closed: 2013-08-31 10:11:36 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 760116, 786172, 787214, 787218, 787274, 790023    
Attachments:
Description Flags
test scripts
none
Failure of positive testcase #2
none
image of fail to verify for comment #23 none

Description Lukas Krejci 2012-01-04 13:34:14 UTC
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.

Comment 1 Lukas Krejci 2012-01-04 14:16:28 UTC
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.

Comment 2 Lukas Krejci 2012-01-09 14:05:55 UTC
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.

Comment 3 Lukas Krejci 2012-01-16 08:36:17 UTC
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.

Comment 4 Lukas Krejci 2012-01-16 08:39:35 UTC
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.

Comment 5 Lukas Krejci 2012-01-16 08:56:25 UTC
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'

Comment 6 Lukas Krejci 2012-01-16 11:48:34 UTC
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.

Comment 7 Lukas Krejci 2012-01-16 14:42:05 UTC
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.

Comment 8 Mike Foley 2012-02-02 19:19:43 UTC
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/

Comment 9 Mike Foley 2012-02-02 19:28:06 UTC
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$

Comment 10 Lukas Krejci 2012-02-03 13:23:05 UTC
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.

Comment 11 Lukas Krejci 2012-02-03 18:21:57 UTC
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.

Comment 12 Lukas Krejci 2012-02-06 22:22:19 UTC
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.

Comment 13 Mike Foley 2012-02-08 20:10:25 UTC
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.

Comment 14 Mike Foley 2012-02-08 23:10:00 UTC
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.

Comment 15 Simeon Pinder 2012-02-09 19:41:24 UTC
Moving this to ON_QA as new RC3 binary available here:
https://brewweb.devel.redhat.com//buildinfo?buildID=198086

Comment 16 Lukas Krejci 2012-02-13 14:11:16 UTC
Making this only target JON 3.0.1. A separate bug 790023 was created to track the progress in RHQ 4.3.0.

Comment 17 Lukas Krejci 2012-02-13 14:15:23 UTC
To recap, in JON 3.0.1, the following commit is related to this BZ:

http://git.fedorahosted.org/git/?p=rhq/rhq.git;a=commitdiff;h=1c7b0aefd0060c2daf15cb95512704934c7ee1af

Comment 18 Mike Foley 2012-02-21 21:14:40 UTC
Positive Testcase #1 = PASS

Positive Testcase #2 = Fail ... or I have questions.  See attached screenshot of unexpected failure.   I do have a running AS5 server.  Please review screenshot of error.   Please provide clarification on "AS 5 server running with JNDI directory remotely accessible using JNP (without authz)".   

 

//This test requires a remote JBoss AS 5 server running with JNDI directory remotely accessible using JNP (without authz)
//This script assumes that there is a bound object called "jmx" in the directory (which it should be)
var jbossHost = 'localhost';
var jbossJnpPort = 1299;

var env = new java.util.Hashtable();
env.put('java.naming.factory.initial', 'org.jboss.naming.NamingContextFactory');
env.put('java.naming.provider.url', "jnp://" + jbossHost + ":" + jbossJnpPort);
var ctx = new javax.naming.InitialContext(env);
var jmx = ctx.lookup('jmx');
assertNotNull(jmx);
pretty.print(jmx);

Comment 19 Mike Foley 2012-02-21 21:15:21 UTC
Created attachment 564785 [details]
Failure of positive testcase #2

Comment 20 Mike Foley 2012-02-21 21:27:19 UTC
Negative testcase #1 = PASS
Negative testcase #2 = PASS
Negative testcase #3 = PASS

Comment 21 Mike Foley 2012-02-21 21:33:26 UTC
Negative testcase #4 = PASS
Negative testcase #5 = PASS
Negative testcase #6 = PASS

Comment 22 Mike Foley 2012-02-22 15:42:15 UTC
email sent to lukas 2/22/2012 requesting clarification on test case #2
IRC requesting clarification on 2/23/2012

Comment 23 Mike Foley 2012-02-22 16:28:23 UTC
additional info from lukas regarding positive test case #2 

<mfoley> that BZ is almost ready to be closed
<lkrejci> mfoley: the EAP with no authentication can be downloaded for example here: http://download.devel.redhat.com/released/JBEAP-5/5.1.2/zip/jboss-eap-noauth-5.1.2.zip 
<mfoley> the 2nd positive testcase ... is either failing ... or i am not familiar with the setup steps required for AS5
<spinder> have to take my daughter to the doctor. Be back in abit. 
* spinder is now known as spinder-bbiab
* btison has quit (Ping timeout: 622 seconds)
<mfoley> so there is no configuration changes for AS5?  it is a totally separate download?
<lkrejci> mfoley: once you start it up (using jbossas/bin/run.sh), look for jboss-as/server/default/data/jnp-service.url
<mfoley> where do i look for that?  in a property file?
<lkrejci> mfoley: it can be configured, but i thought it'd be easier to just redownload this... also I'm not sure atm how to "unconfigure" the authz in AS
<lkrejci> mfoley: it is a file
<mfoley> ok ... and what needs to be done to that file?
<lkrejci> and in that file, you can figure out the jnp host and port
<lkrejci> and you need to put those values into the test JS file
<mfoley> ok

Comment 24 Mike Foley 2012-02-22 16:51:31 UTC
I am marking this BZ as fail to verify.  Specifically, positive test case #2 is failing.  I have followed the developer steps from comment #23.

1) installed JBoss EAP 5.1 NoAuth, as advised in comment #23
2) reviewed the contents of jnp-service.url

jnp://127.0.0.1:1099

3) made sure this matches the js in positive testcase #2 (documented in comment #18)

4) the test fails.  i am attaching an image to document the failure.

assigning back to development.

Comment 25 Mike Foley 2012-02-22 16:52:19 UTC
Created attachment 565039 [details]
image of fail to verify for comment #23

Comment 26 Lukas Krejci 2012-02-22 17:05:54 UTC
as can be seen in the attached image from comment #25, the script that ran is still using the wrong host and port combination.

It is not enough to just save the file with the updated values on the file system. You need to delete the alert notification with the old script version and create a new alert notification, where you need to upload the updated script file.

I know that I didn't specifically mention those steps but they are kind of implicit - if a script is to be run as an alert notification the server needs to be able to get at the script - as documented in the feature documentation (http://docs.redhat.com/docs/en-US/JBoss_Operations_Network/3.0/html/Setting_up_Monitoring_Alerts_and_Operations/alerts.html#init-cli-script-alerts), the scripts are stored in RHQ repositories.

Comment 27 Mike Foley 2012-02-22 19:03:20 UTC
positive testcase #2 passed JON 3.01 RC5

Comment 28 Heiko W. Rupp 2013-08-31 10:11:36 UTC
Bulk close of old bugs in VERIFIED state.