[Mondrian] Fwd: Re: xmla security header processing

Julian Hyde jhyde at pentaho.com
Tue Jun 21 14:41:57 EDT 2011


Looks good; but where do you configure that actual XML that
DiscoverDatasources will return? Could that XML go in web.xml also?
 
(For the other devs on the list, some context. The problem is with an XMLA
servlet that gives access to two or more olap4j sources. It is not
appropriate for either of those sources to answeer the DiscoverDatasources
request individually. Only the servlet knows the whole story. It needs to
answer the request before opening an olap4j connection. Therefore the
metadata needs to come from the servlet, not from an olap4j driver.)


  _____  

From: mondrian-bounces at pentaho.org [mailto:mondrian-bounces at pentaho.org] On
Behalf Of Michele Rossi
Sent: Tuesday, June 21, 2011 10:29 AM
To: Mondrian developer mailing list
Subject: [Mondrian] Fwd: Re: xmla security header processing




Sent from my iPhone

Begin forwarded message:



From: "Julian Hyde" <jhyde at pentaho.com>
Date: 21 June 2011 19:23:53 CEST
To: "'Michele Rossi'" <michele.rossi at gmail.com>
Subject: RE: [Mondrian] Re: xmla security header processing
Reply-To: <jhyde at pentaho.com>





again, to devs list please.


  _____  

From: Michele Rossi [mailto:michele.rossi at gmail.com] 
Sent: Tuesday, June 21, 2011 8:52 AM
To:  <mailto:jhyde at pentaho.com> jhyde at pentaho.com
Subject: Re: [Mondrian] Re: xmla security header processing


hi Julian, 
I've implemented the first easy bit so that we can test whether the
packChange process works.
You can now optionally specify the return values for "discover datasources"
in the web.xml configuration. If you do the DISCOVER_DATASOURCES rowset will
not require a new OlapConnection.

Could you please let me know if this change is ok and if you can read my
packed change?

Tomorrow I will start the more complex bit - the use of commons-dbcp to pool
the OlapConnections.


thanks,
Michele

web_xml_xmla.png

On 20 June 2011 21:29, Julian Hyde < <mailto:jhyde at pentaho.com>
jhyde at pentaho.com> wrote:



The two should be equivalent. Let's go with the 'unpackChange' approach
since it seems to be working.
 
The only problem is with NonEmptyTest.java. It refused to overwrite a
writable file -- which is sensible if you think about it.
 
I think you should go with the version of NonEmptyTest.java in the .tar.gz
file. If you have made changes to NonEmptyTest.java that you want to keep,
you will have to manually apply them. 
 
Julian


  _____  


From: Michele Rossi [mailto: <mailto:michele.rossi at gmail.com>
michele.rossi at gmail.com] 

Sent: Monday, June 20, 2011 8:33 AM
To:  <mailto:jhyde at pentaho.com> jhyde at pentaho.com 

Subject: Re: [Mondrian] Re: xmla security header processing


hi Julian, 
I've been able to apply the packed change but the patching continues to be
elusive.
The two approaches are equivalent right?

I've included the output of the "unpackChange" and "patch" commands.

Thanks!
Michele

mrossi at PI-mrossi-L-1
/cygdrive/c/Work/thirdparty/mondrian_perforce/open/mondrian
$ unpackChange -c 14233
../../../mondrian_scripts/patches_and_packed_changes/changelist14233.tar.gz
File(s) not opened on this client.
//open/mondrian/src/main/mondrian/tui/XmlaSupport.java#27 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/RowsetDefinition.java#81 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/XmlaConstants.java#13 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/XmlaHandler.java#76 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/XmlaRequest.java#11 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/XmlaServlet.java#40 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/XmlaUtil.java#31 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/impl/DefaultSaxWriter.java#12 -
file(s) up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/impl/DefaultXmlaRequest.java#19 -
file(s) up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/src/main/mondrian/xmla/impl/DefaultXmlaServlet.java#30 -
file(s) up-to-date.
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/testsrc/main/mondrian/rolap/NonEmptyTest.java#145 - updating
C:\Work\thirdparty\mondrian_perforce\open\mondrian\test
src\main\mondrian\rolap\NonEmptyTest.java
Can't clobber writable file
C:\Work\thirdparty\mondrian_perforce\open\mondrian\testsrc\main\mondrian\rol
ap\NonEmptyTest.java
Change 14233 belongs to client jhyde.marmite2.
//open/mondrian/testsrc/main/mondrian/xmla/test/XmlaTest.java#23 - file(s)
up-to-date.
Change 14233 belongs to client jhyde.marmite2.
New change number is 14233




mrossi at PI-mrossi-L-1
/cygdrive/c/Work/thirdparty/mondrian_perforce/open/mondrian
$ patch -p l <
../../../mondrian_scripts/patches_and_packed_changes/jhyde2.patch
patch: **** strip count l is not a number

mrossi at PI-mrossi-L-1
/cygdrive/c/Work/thirdparty/mondrian_perforce/open/mondrian
$ patch -p 1 <
../../../mondrian_scripts/patches_and_packed_changes/jhyde2.patch
patching file src/main/mondrian/tui/XmlaSupport.java
Reversed (or previously applied) patch detected!  Assume -R? [n] Y
Apply anyway? [n] Y
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file
src/main/mondrian/tui/XmlaSupport.java.rej
patching file src/main/mondrian/xmla/RowsetDefinition.java
Reversed (or previously applied) patch detected!  Assume -R? [n]
Apply anyway? [n] y
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 41.
Hunk #3 succeeded at 6436 with fuzz 2 (offset 12 lines).
2 out of 3 hunks FAILED -- saving rejects to file
src/main/mondrian/xmla/RowsetDefinition.java.rej
patching file src/main/mondrian/xmla/XmlaConstants.java
Reversed (or previously applied) patch detected!  Assume -R? [n] Y
Apply anyway? [n] y
Hunk #1 succeeded at 9 with fuzz 2 (offset -38 lines).
Hunk #2 FAILED at 26.
1 out of 2 hunks FAILED -- saving rejects to file
src/main/mondrian/xmla/XmlaConstants.java.rej
patching file src/main/mondrian/xmla/XmlaHandler.java
Reversed (or previously applied) patch detected!  Assume -R? [n]



On 16 June 2011 23:11, Julian Hyde < <mailto:jhyde at pentaho.com>
jhyde at pentaho.com> wrote:



Two options for the patch file:
 
1. The patch file was the wrong format. I fixed it. See attached. Apply it
using 'patch -p 1 < jhyde2.patch'.
 
For this you will need to install cygwin, including the patch command.
 
OR...
 
2. I have also attached a file generated by packChange. See attached. Apply
using 'unpackChange changelist14233.tar.gz'.
 
For this you will need to install cygwin, plus the packChange and
unpackChange scripts from
<http://p4webhost.eigenbase.org:8080/open/util/bin/>
http://p4webhost.eigenbase.org:8080/open/util/bin/. Put them somewhere on
cygwin's path, e.g. /usr/local/bin, and give them execute access 'chmod 755
...'.
 
To check out files for edit, use the user 'considerate_guest'. The password
is 'consider8'.
 
Julian
 
 


  _____  


From: Michele Rossi [mailto: <mailto:michele.rossi at gmail.com>
michele.rossi at gmail.com] 

Sent: Wednesday, June 15, 2011 6:47 AM 

To:  <mailto:jhyde at pentaho.com> jhyde at pentaho.com; Mondrian developer
mailing list

Subject: Re: [Mondrian] Re: xmla security header processing


hi Julian, 
sorry for the long delay, I've only just started working on this task again.

I have been trying to apply your patch but unfortunately I get an error
message, "Invalid patch file".
I haven't found a way to apply your patch from the "p4" command line tool (I
have googled for a considerable amount of time, no luck. I use the p4
eclipse plugin 2010.1/275861, I couldn't do with the March 2011 p4v client
either).

Could we consider switching to "perforce shelving" instead of patches?
Or some other way to allow non-authorised committers to deal with the
mondrian perforce in a more efficient way? For example we could create a
special branch where everybody can commit too.
Then you could decide what things you want to merge back to the main branch.
It's just an idea.. I am obviously not too good with patch files.

Back to the XMLA servlet:

1. it looks like we need very different connection creation policies: I need
to create authenticated connections and I need to associate connections to
sessions.
If it's something that you don't want could we think about allowing the
injection of different OlapConnectionFactory implementations?
That way I could make an OlapConnectionFactory that caches connections and
associates them with user sessions while the default behaviour would be what
it is now.

2. I am happy to use the container-level session to store authentication
credentials - that is something that you suggested and that I will implement
asap

3. We have a problem with the first request that SimbaO2X sends without
authentication.
The request is "Discover Datasources".
Are you happy if I allow users to provide their own hard-coded response to
such request?
It could go in the web.xml.
When such configuration is not hard coded the system would behave as now and
would try to obtain it from a OlapConnection object.


many thanks,
Michele



On 26 May 2011 15:49, Michele Rossi < <mailto:michele.rossi at gmail.com>
michele.rossi at gmail.com> wrote:


hi Julian, 
I've replied to some of your questions inline.

thanks,
Michele


On 25 May 2011 00:43, Julian Hyde < <mailto:jhyde at pentaho.com>
jhyde at pentaho.com> wrote:


Michele,
 
As promised, I've reviewed your changes. 
 
All in all, the change looks useful. I can see that we need to introduce the
notion of 'sessions' due to the way Simba O2X sends its requests. But I am
concerned at how many places username & password are now present in the API,
and the result is so confusing it's difficult to say that there are not
security holes.
 
If you can address my concerns, I will commit your changes. I've changed
quite a lot of the code; see the patch attached. Can you use that patch as
starting point when fixing the issues I raise below. And when you have made
changes, please generate a similar patch using 'p4 diff -du' from the
command line.
 
Detailed comments...
 
0. Would it be simpler if the servlet looked for the <Security> tag and
session information and copied these into the HTTP headers? Then we could
use HTTP athentication and session lifecycle management. Just an idea.


My main problem is to create an authenticated OlapConnection.
Hence I need the credentials to be processed in the code that creates the
connection and not by the container.
 

 
1. MondrianServerImpl.getConnection is not used. Removed it.
 
2. It is a bad idea to pool connections. If you want to pool connections,
use a connection pool. It is an even worse idea to pool connections based on
session ID -- that implies that you are trying to make sessions stateful,
and it is much better if the XMLA servlet is stateless.
 
So, I removed ConnectionHolder and the "connections" member.


The problem is that Simba will create something like 15 connections before
you can even start configuring a Pivot in Excel. 

In general I believe that creating a new connection for each request is  an
anti-pattern as it leads to a substantial waste of resources.

In my case in particular this approach wouldn't work at all as I cache the
metadata objects at the connection level. 
So before displaying a Pivot in Excel I'd have to populate the whole
metadata cache N times, once for each connection I create.

Connection pooling is the technique normally used by more traditional web
apps that require access to a database for this problem and it can be
implemented either at the application level (e.g. commons-dbcp) and or at
the container level (tomcat can handle pool of connections to databases) .

I am aware that my implementation didn't clean up un-used connections.
I didn't want to submit too much code at once and risk wasting time if you
didn't like all the rest of the work.
 

 
I see why we need to save username & password so that they can be retrieved
by subsequent requests in the same session. But your implementation never
removed entries from that table. I added code to remove the entry when we
see an 'end of session'. It would be nice if we could hook into HTTP session
management (see #0 above).


We can definitely save the credentials in the Http Session. The new version
of Simba supports Cookies too which means that this approach could work.
I will start thinking about this approach.
 

 
3. In removing the "connections" member, I may have broken the mechanism by
which subsequent requests in the same XMLA session inherit the same
password. (Difficult to tell without unit tests.) If so, I apologize.
 
But it is not obvious how session context is conveyed along with the
XmlaRequest. Would it make sense to replace 'String
XmlaRequest.getSessionId()' (and getUsername() and getPassword()) with a
'SessionInfo XmlaRequest.getSessionInfo()' method that contains all session
context?


Yes definitely - although the end result is very similar this approach is
probably more flexible.
 

 
4. Please convince me that you are not opening up a security hole, as
follows. The context contains a role name. It also may contain username and
password, if the client happens to be using basic authentication. Suppose
that a malicious client specified a valid username and password in its HTTP
headers, but also specified a role that had greater privileges than the
username and password allowed. Would that client be able to gain greater
privileges than it deserved?


I don't think the Xmla Servlet supports basic authentication - you could
enable basic authentication at the container level but I really don't know
much about this.
The credentials I am interested in are provided in the SOAP payload by
SimbaO2X, they don't come from HTTP headers.

Also how are clients currently specifying their "role" ?
What stops them from choosing anything they like as their role?
(I will also start doing my own research on these points)


 
It needs to be clear and foolproof in the code whether the client is
trusted. If trusted, it can adopt whichever role it choses. If not, it must
take the role assigned by olap4j after authentication.

 
I don't fully understand the significance of the role - it's not something I
use in my driver but I know that mondrian uses it. Is it a standard JDBC
concept or something mondrian specific?
I will find out more about it.


 
5. What does your comment "we'd need to inject the HttpServletRequest into
this method" mean?


What I meant was that if we wanted to make the host name that made the
request (IP of the client machine) available in the context, we would need
to have access to the HttpServletRequest.
I need that information for licensing reasons.
 

 
6. There is a magic id for un-authenticated session. Is this a security
hole?


Unfortunately Simba fires off the very first XMLA "DISCOVER" request
(Discover Datasources) without providing any authentication credentials.
It's as if Simba expected the server side to know about its data sources
without opening a database connection.
We could approach this problem in other ways, for example by providing the
list of data sources in a different way.
I will think about it.

In any case an un-authenticated connection is not a security hole per-se.
The behaviour depends on the particular implementation of Olap4j.
Some drivers might refuse to execute queries on OlapConnections created
without credentials.
Other might not need authenticated connections at all.
 

 
7. Would it make sense for the servlet to refuse basic authentication
requests if they were not over HTTPS? (And not just the first request, which
carries username/password. Subsequent requests carry an authenticated
session ID, so they must be protected also.)

 
Again I need to go back and study  the relation between the xmla servlet and
basic authentication as I don't understand it yet.


 
8. I fixed XmlaBasicTest. Maybe other tests are broken also. Can you please
ensure that the tests run clean before you re-submit.


Will do. 

 
9. Add unit test for basic authentication. (It's hard for me to review this
code if it is not used in the test suite.)


Ok maybe by Basic Authentication you mean the SOAP authentication provided
by Excel / Simba.
I will look into it as discussed above.
 

 
10. Is the security model assuming that session ids are hard to guess?



Yes that is always expected by session IDs - you want to prevent an attacker
from guessing a valid ID.
So I can make the session ID generation algo a bit better.
 




-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.pentaho.org/pipermail/mondrian/attachments/20110621/7c209fa8/attachment.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/png
Size: 33039 bytes
Desc: not available
Url : http://lists.pentaho.org/pipermail/mondrian/attachments/20110621/7c209fa8/attachment.png 


More information about the Mondrian mailing list