Bug 787218

Summary: Methods not in CLI API documentation
Product: [Other] RHQ Project Reporter: Mike Foley <mfoley>
Component: DocumentationAssignee: Deon Ballard <dlackey>
Status: CLOSED NOTABUG QA Contact: Mike Foley <mfoley>
Severity: unspecified Docs Contact:
Priority: urgent    
Version: 4.3CC: hrupp, lkrejci
Target Milestone: ---   
Target Release: JON 3.0.1   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 771652 Environment:
Last Closed: 2012-02-13 20:20:58 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: 771652, 787274    
Bug Blocks: 760116, 786172, 787214    

Description Mike Foley 2012-02-03 15:24:35 UTC
+++ This bug was initially created as a clone of Bug #771652 +++  Methods in repro steps not in CLI API documentation.  Specifically, subjectManager.getOverlord.  Also ... the context object. 


----------------

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.

Comment 1 Deon Ballard 2012-02-03 20:00:07 UTC
Hey, Lukas (and Mike).

I'm unclear on this. The subjectManager.getOverlord method -- does this not exist in the public API? Or was it somehow skipped when the remote API doc was generated?

Comment 2 Deon Ballard 2012-02-03 20:00:57 UTC
Also, note to self, 'context' should be one of those things that is listed as being treated differently in the JON CLI v a server script.

Comment 3 Lukas Krejci 2012-02-06 09:03:06 UTC
SubjectManager.getOverlord() is not part of RHQ's public remote API.

However (without the fix for bug 771652), the CLI alert scripts are able to get at our local APIs. It is slightly more complicated but definitely possible. Note also that the scripts running in the CLI itself (i.e. connecting to RHQ server remotely) have no such possibility.

So I think we should not document such possibility although we know it exists (again, the fix for bug 771652 denies the alert scripts the ability to lookup our local APIs and therefore do any "nasty" unsupported things, but the fix is not part of JON 3.0.1).

Comment 4 Mike Foley 2012-02-06 16:50:45 UTC
per triage 2/6/2012 (asantos, ccrouch, loleary, mfoley)

Comment 5 Lukas Krejci 2012-02-13 14:37:46 UTC
setting target release to JON 3.0.1, status ASSIGNED.