ou=users, ou=system - user cannot see their own entry

• Key: DIRSERVER-606
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Critical
• Assignee: Emmanuel Lecharny
• Reporter: Marc Batchelor
• Affects Version/s: 1.0-RC1

Environment

JDK 1.4.1
Tried both JXplorer, and from ACEGI security

Description

User binds to ApacheDS as a user under ou=users, ou=system. The user cannot see their own entry to get their own attributes.

Documentation states: Users cannot see other user entries under the 'ou=users,ou=system' entry.

Agreed and understood. But, the user, after binding with the directory, cannot even find their own entry to get their own attributes.

Comments

[Stefan Zoerner]

Able to reproduce. It is just like Marc describes. Starting from a default RC1, I used the admin to add an entry like this:

dn: cn=Fiona Apple,ou=users,ou=system
objectclass: top
objectclass: person
cn: Fiona Apple
sn: Apple
userpassword: machine

Performing a
$ ldapsearch -h localhost -p 10389 -D "cn=Fiona Apple,ou=users,ou=system" -w machine -s one -b "ou=users,ou=system" "(objectClass=*)" dn
gives no results

I assume an error in the OldAuthorizationService component. If I comment this interceptor out in the server.xml (name=oldAuthorizationService), the search op above gives Fionas entry (and all others).

$ ldapsearch -h localhost ...
version: 1
dn: cn=Fiona Apple,ou=users,ou=system

dn: cn=Kate Bush,ou=users,ou=system
$

This small patch (1 line) allows users to search their own entry. It changes a condition within the isSearchable method of OldAuthorizationService.

There is still a problem left, if the DN of the user contains uppercase letters, or if s/he uses uppercase letters in the bind DN. In these cases, the issue still exists. The reason for this is the equals method of org.apache.directory.shared.ldap.name.LdapName, which is used frequently in OldAuthorizationService, and which is case sensitive.

Two options here:
a) modify equals() in LdapName to ignore case
b) perform special checks in OldAuthorizationService which ignore case

[Emmanuel Lecharny]

I have a different behavior. I also have created the same entry (with LdapBrowser, and it's not easy, becuase if you don't create a file to store the password, then there is no way you can modify the entry after having added it.)

Now, if I try to do :
ldapsearch -h localhost -p 10389 -D "cn=fiona apple,ou=users,ou=system" -w machine -s sub -b "ou=users,ou=system" "(objectClass=*)" dn

I got :
ldap_sasl_interactive_bind_s: Insufficient access (50)
        additional info: failed on search operation

I changed the command to :
ldapsearch -x -h localhost -p 10389 -D "cn=fiona apple,ou=users,ou=system" -w machine -s sub -b "ou=users,ou=system" "(objectClass=*)" dn

(the -x is for Simple authentication)

Now, I get this error :
ldap_bind: Invalid credentials (49)
        additional info: Bind failed


Of course, if I use the admin DN to authenticate, it works :
ldapsearch -x -h localhost -p 10389 -D "uid=admin,ou=system" -w secret -s sub -b "ou=users,ou=system" "(objectClass=*)" dn

gives the following result :

# extended LDIF
#
# LDAPv3
# base <ou=users,ou=system> with scope sub
# filter: (objectClass=*)
# requesting: dn
#

# users, system
dn: ou=users,ou=system

# Fiona Apple, users, system
dn: cn=Fiona Apple,ou=users,ou=system

# search result
search: 2
result: 0 Success

# numResponses: 3
# numEntries: 2

[Emmanuel Lecharny]

Ooops...

I have debugged the server, and I realized that the problem I had was related to the way the password was created. As you just can import it from a file, if you create this file with a standard editor, it adds a '\a' at the end of it.

Now, I have the correct result

ldapsearch -x -h localhost -p 10389 -D "cn=fiona apple,ou=users,ou=system" -w machine -s sub -b "ou=users,ou=system" "(objectClass=*)"

gives :

# extended LDIF
#
# LDAPv3
# base <ou=users,ou=system> with scope sub
# filter: (objectClass=*)
# requesting: ALL
#

# users, system
dn: ou=users,ou=system
objectClass: top
objectClass: organizationalUnit
ou: users

# search result
search: 2
result: 0 Success

# numResponses: 2
# numEntries: 1

[Emmanuel Lecharny]

Ok, I think that we can fix the code this way :

...
                if ( dn.startsWith( USER_BASE_DN ) || dn.startsWith( GROUP_BASE_DN ) )
                {
                        // check that it's not the user himself.
                 if ( dn.equals( dnParser.parse( principalDn.toString() ) ) )
                 {
                 return true;
                 }
                
                    return false;
                }
...

The problem is that, if you do it, you will get the userPassword :

ldapsearch -x -h localhost -p 10389 -D "cn=Fiona apple,ou=users,ou=system" -w machine -s one -b "ou=users,ou=system" "(objectClass=*)"
# extended LDIF
...
# Fiona Apple, users, system
dn: cn=Fiona Apple,ou=users,ou=system
...
userpassword:: bWFjaGluZQ==

and it's easy to check that bWFjaGluZQ== is the base 64 encoded value for "machine"...

Is it what we want? I don't think so.

wdyt ?

[Stefan Zoerner]

Emmanuel, I recommend to fix it like this (attached file patch_DIRSERVER-606_2.txt) instead. Your proposal has two disadvantages (as far as I understand the situation):

1) Call to DnParser.parse is not synchronized (I assume it should, because other calls in the class are)

2) At least one case is not fixed:

$ ldapsearch -h localhost -p 10389 -D "uid=admin,ou=system" -w ***** -b "uid= admin,ou=system" "(objectClass=*)" dn

display the admin entry, but

$ ldapsearch -h localhost -p 10389 -D "uid=Admin,ou=system" -w ***** -b "uid= admin,ou=system" "(objectClass=*)" dn

does not (it displays nothing), because principalDn is not normalized in isSearchable for all cases. We should consider to normalize all principalDn occurrences in this class. There might be other problems due to the many LdapName.equals() calls.

[Emmanuel Lecharny]

Stefan,

DnParser is internally synchronized, so there is no need to synchronize it outside. DnParser.parse should be totally stateless, but actually it's not, this is why it has been synchronized. However, this is not a problem.

What I did may not be enough, as you stated, and your patch is better, but the question is : do we need to handle this bug? The problem is that if we fix it, user can see it's password, and super-user can see *all* passwords. Is that an expected feature? I don't think so. Alex pointed that this is OldAuthz, so it should be replaced by the newer one, with ACIs. I think that's very valid. Another solution could be to avoid sending userPassword in the response.

For Alex it's like it is legacy interceptor. May be we should suppress it from configuration.

Patching the server is quite easy, we can do it in five minutes, and your patch is ok (regardless to synchronization). But do we have to do it? That's the main point...

wdyt ?

btw, we must merge LdapName and LdapDN. We reached the conclusion with Alex that LdapName must be deleted, to avoid confusion with jdk 1.5 LdapName behavior. As we have another implementation (LdapDN) which is totally thread safe (without synchronization) and twice faster, this is the way to go. But it's not an simple patch, because LdapName is used all over the code. We may have a impact analysis before doing this move. A confluence page has been added for this task, but it's far from being complete.

[Stefan Zoerner]

Emmanuel,

currently (1.0 RC1), the OldAuthorizationService is enabled in the configuration by default.
I think we have therefore two options:

a) keep it enabled
In this case we should fix the bugs within the service (like this one), and try to have it work as described in the documentation (or update the documentation)
http://directory.apache.org/subprojects/apacheds/docs/users/authentication.html
In these docs the OldAuthorizationService is interpreted as "minimal built-in rules", not legacy.
How about renaming OldAuthorizationService to MinimalAuthorizationService (or something like that), if we decide to keep it?

b) disable or (even better) remove the service
In this case, we have to update the docs, and to ensure that the default ACI configuration is reasonable secure.

Password thing: I think it should be possible to allow users to read passwords. In some occasions someone may need it (if s/he wishes to write his/her own replication, for instance). I prefer to forbid it by default via ACIs (and allow users to change this behavior).

[Emmanuel Lecharny]

regarding your proposal :

a) Yeah, we can do that. This is for RC2, and its better to have something that works, even if you can read your password, than something that is badly broken.

a-2) I agree with "minimal built-in rules". I used legacy, because it was something which will be removed soon. I don't know when, may be in 1.1 or in 1.0

b) Yes, we have to update the doc. Feel free to ask Alex about the intricacy of ACI usage with the newer AuthorizationService. As he wrote the code, he is the one who know it the best. We sure need this doco if we want users not to be puzzled as we are...

Password things : I'm not very confortable with it. I don't like the fact for instance that I can read my passwords in Firefox. The problem is if you can do that, then you will have to be paranoïd : each time you go to have a copy, lock your computer... I don't see any occasion where clear password need to be shown to the user, even if the files that contains the entries is not crypted (eh eh, another improvment ...). Well, this is another problem, and we can fill a JIRA for that, too :)

[Alex Karasulu]

I like the renaming idea. I guess this OldAuthZService will always hang around as long as people don't want to grok the ACI based AuthZService. Don't know if we should bother deleting this. I guess its best to rename to Minimal/Default AuthZService as you guys suggest.

But this problem should be solved. Users should be allowed to see their own entry. Users should be able to see and update their own entries for self service perhaps? WDYT? Sometimes I think another application with superuser access can do this and regular users can be shielded from their own attributes but this does not play well when logging in as yourself to the directory.

[Emmanuel Lecharny]

So let's go for the renaming. I'll do it as soon I'm back home. DefaultAuthZService seems ok.

What we can do is to apply Stefan patch as is, and latter we can improve the password exposure (for instance, returns stars instead of real password). At least we can discuss about it later.

wdyt ? need a vote ?

[Alex Karasulu]

That sounds like a good plan E. No I don't think we need a vote.

[Emmanuel Lecharny]

Fixed. The patch submitted by Stefan has been applied, and the OldAuthorizationService class and reference into server.xml file have been changed to DefaultAuthorizationService and defaultAuthorizationService.

I have tested the different cases I found in all the comments, and it seems to be ok. A junit testcase would be appreciated.

[Stefan Zoerner]

Thanks Emmanuel! I have created a JUnit test in apachsds/server-unit (DefaultAuthorizationTest), which tests the situation described in this issue. Currently I have unfortunately no svn access -- I will commit it to the 1.0 branch tonight if I have again.

One question left: I have noticed that you have changed server.xml in apacheds/server-main to reflect the new service name. I assume, that it is also necessary to modify it in server-installers/src/main/installers?

[Emmanuel Lecharny]

closed