SEC-981: BindAuthenticator swaps read-write and read-only context operations

[spring-projects-issues]

["Jürgen Failenschmid":https://jira.spring.io/secure/ViewProfile.jspa?name=jfai](Migrated from ["SEC-981":https://jira.spring.io/browse/SEC-981?redirect=false]) said:

BindAuthenticator's BindWithSpecificDnContextSource class contains these methods:

    public DirContext getReadOnlyContext() throws DataAccessException {
        return ctxFactory.getReadWriteContext(userDn.toString(), password);
    }

    public DirContext getReadWriteContext() throws DataAccessException {
        return getReadOnlyContext();
    }

The implementation of getReadOnlyContext() should get a read-only context from the context factory, not a read-write context. Vice versa for method getReadWriteContext().

[spring-projects-issues]

["Luke Taylor":https://jira.spring.io/secure/ViewProfile.jspa?name=luke] said:

Both methods use the same implementation to obtain a context with the user's credentials. This is intentional, given that the intention is to bind as that user, and the class is privat to BindAuthenticator so has no impact elsewhere.

Please explain why you think this is an issue at all, let alone a critical bug.

[spring-projects-issues]

["Jürgen Failenschmid":https://jira.spring.io/secure/ViewProfile.jspa?name=jfai] said:

If a read-only context is requested, then any modify operation with that context should fail. If read-only cannot be supported, then the method should raise an exception.

[spring-projects-issues]

["Luke Taylor":https://jira.spring.io/secure/ViewProfile.jspa?name=luke] said:

There is nothing anywhere that specifies that this must be the case and in any case it is dependent on how the directory is configured. If you look at Spring LDAP's TransactionAwareContextSourceProxy, the implementation is:

public DirContext getReadOnlyContext() throws NamingException {
    return getReadWriteContext();
}

and in AbstractContextSource, both methods have the same behaviour by default. Again, please explain what difference this makes to you, given that it is an internal class which is only used by BindAuthenticator.

[spring-projects-issues]

["Jürgen Failenschmid":https://jira.spring.io/secure/ViewProfile.jspa?name=jfai] said:

I'm only pointing out improvements. I've configured a context source that checks for read-only access and read-write access (for auditing purposes) and I have to give the Spring Security stuff write access, although it really shouldn't need it.

Just because other classes are implemented sloppyly as well, doesn't make it right: don't excuse bad (method) behavior by pointing to other bad (method) behavior. I'll deal with the other Spring packages with similar issues separately.

Having set all of that, I really appreciate the hard work that went into Spring Security!

[spring-projects-issues]

["Luke Taylor":https://jira.spring.io/secure/ViewProfile.jspa?name=luke] said:

It would not be an improvement. The whole point is that authentication as the user takes place, regardless of whether the operation is read-only or otherwise. In fact it would open a severe security hole if it were changed to call ctxFactory.getReadOnlyContext() - the context would then be authenticated as the manager user (or anonymously).

The definition of what is a read-only or read-write context is subjective, just as it is for database connections. You don't have to make a special case for read-only operations but you have the option if the framework allows it. I would avoid references to "sloppy" implementations or "bad behaviour" if you are raising the issue again with the Spring LDAP guys.