Bug 787218 - Methods not in CLI API documentation
Methods not in CLI API documentation
Status: CLOSED NOTABUG
Product: RHQ Project
Classification: Other
Component: Documentation (Show other bugs)
4.3
Unspecified Unspecified
urgent Severity unspecified (vote)
: ---
: JON 3.0.1
Assigned To: Deon Ballard
Mike Foley
:
Depends On: 771652 787274
Blocks: jon30-sprint10/rhq43-sprint10 786172 787214
  Show dependency treegraph
 
Reported: 2012-02-03 10:24 EST by Mike Foley
Modified: 2012-02-13 15:20 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 771652
Environment:
Last Closed: 2012-02-13 15:20:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Mike Foley 2012-02-03 10:24:35 EST
+++ 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@redhat.com 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@redhat.com>	
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@redhat.com>
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@redhat.com on 2012-01-09 09:05:55 EST ---

commit 772e9d247e524071a86d6a25b8e1615ed77e9fcf
Author: Lukas Krejci <lkrejci@redhat.com>
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@redhat.com 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@redhat.com>
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@redhat.com 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@redhat.com>
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@redhat.com 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@redhat.com>
Date:   Fri Jan 13 11:38:26 2012 +0100

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

--- Additional comment from lkrejci@redhat.com 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@redhat.com 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@redhat.com 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@redhat.com 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@redhat.com 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 15:00:07 EST
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 15:00:57 EST
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 04:03:06 EST
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 11:50:45 EST
per triage 2/6/2012 (asantos, ccrouch, loleary, mfoley)
Comment 5 Lukas Krejci 2012-02-13 09:37:46 EST
setting target release to JON 3.0.1, status ASSIGNED.

Note You need to log in before you can comment on or make changes to this bug.