directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Karasulu <akaras...@apache.org>
Subject Re: svn commit: r1144962 - /directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/LdapCoreSessionConnection.java
Date Tue, 12 Jul 2011 07:19:26 GMT
On Tue, Jul 12, 2011 at 9:19 AM, Kiran Ayyagari <kayyagari@apache.org> wrote:
> On Tue, Jul 12, 2011 at 11:23 AM, Emmanuel Lécharny
> <elecharny@apache.org> wrote:
>> On 7/12/11 2:21 AM, Alex Karasulu wrote:
>>>
>>> On Mon, Jul 11, 2011 at 9:55 AM, Emmanuel Lecharny<elecharny@gmail.com>
>>>  wrote:
>>>>
>>>> I'm not sure it"s a good idea to setup a default session, at least to
>>>> admin.
>>>> If we consider the normal (ie, not embedded) server, we don't set any
>>>> session, the default session is Anonymous (of course if allowed). IMO,
>>>> this
>>>> might be a security breach too.
>>>>
>>>> What was the rational for this modificatioon, Alex ?
>>>
>>> First there was a big null pointer exception due to this not being
>>> set. Second taking a big step back I thought about it and if I have a
>>> handle on DirectoryService I can pretty much do anything anyway. If
>>> I'm using CoreSessions and DirectoryServices I can use any kind of
>>> session there's no security barrier there. So IMO there's no security
>>> issue here to defaulting to an admin session.
>>
>> Make sense. I'm just wondering if we shouldn't mimic the way the LDAP server
>> works by forcing the session to use an anonymous principal by default,
>> instead of an admin one. I shouldn't have used the term 'security issue',
>> it's not really a problem in this case, what I had in mind is that if
>> someone want to use a Admin session, it's probably better to require that he
>> explicitly create such a session. Call it 'protection against stupid
>> move'...
>>
>> PS : NPE ? ouch...
>>
> this NPE is a valid one in this case, note that this is a connection
> implementation and a user is supposed to call bind()
> but the constructor LdapCoreSessionConnection( CoreSession session )
> is entirely different, in this case the user knows what he is
> doing, so my preference would be to move the assignment to this
> constructor and assign the passed session (i.e.,
>
>  this.session = session;

There were actually a few places where this could happen. If session
was passed in as a method argument sure we can just set it as you did
above. But I don't think we had that option in the method where the
change was induced.

>  rather than
>
> this.session = directoryService.getAdminSession(); in setDirectoryService())
>
> what we already know is that DS is available and user/app can do
> anything if has got access to, but more important is
> the usage from an app developer's POV, if I have a web app that allows
> users to connect to the server using LdapCoreSessionConnection
> then assigning admin session by default during initialization will be
> a serious security issue.

LDAP applications rarely align their authorization schema with LDAP
security. Most applications just connect as admin and handle lookups
on behalf of their users.

But I think you and Emmanuel both make a good case here. We need to
solve this better since some applications like the self service
applications we've spoken about writing might use direct LDAP
security. However I think we don't just go with an anonymous session
or a admin session. We need a means to make this decision better.

We should require a bind to set the exact session.

-- 
Best Regards,
-- Alex

Mime
View raw message