Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No password check when DefaultTlsDirContextAuthenticationStrategy is activated #430

Closed
derTobsch opened this issue Nov 18, 2016 · 7 comments · Fixed by #503
Closed

No password check when DefaultTlsDirContextAuthenticationStrategy is activated #430

derTobsch opened this issue Nov 18, 2016 · 7 comments · Fixed by #503
Assignees
Milestone

Comments

@derTobsch
Copy link
Contributor

We stumbled over this problem one week ago and thought it would be nice to tell you about this.

When the DefaultTlsDirContextAuthenticationStrategy is set as authentication strategy on the LdapContextSource every password will be accepted, because there happens no bind with user credentials against ldap.

After deep debugging into your code and a lot of searching on the web, we found this post in the spring forum from Jul 18th, 2013.
http://forum.spring.io/forum/spring-projects/data/ldap/129629-ldap-with-tls-authentication-issues

mwebb describes very detailed the problem and gives an code example approach to fix this.

It appears that SimpleDirContextAuthenticationStrategy and DefaultTlsDirContextAuthenticationStrategy are not symmetrical in their behaviour. I resolved this be creating a custom TlsDirContextAuthenticationStrategy and adding a ctx.reconnect() to the applyAuthentication(), after the environment settings for a simple bind have been set, as follows:

private void applyAuthentication(LdapContext ctx, String userDn, String password) throws NamingException {
ctx.addToEnvironment(Context.SECURITY_AUTHENTICATI ON, SIMPLE_AUTHENTICATION);
ctx.addToEnvironment(Context.SECURITY_PRINCIPAL, userDn);
ctx.addToEnvironment(Context.SECURITY_CREDENTIALS, password);
// Force reconnect with user credentials
ctx.reconnect(null)
}

The problem he describes was exactly the same we had. The line ctx.reconnect(null) fixed it for us.

@alex-sherwin
Copy link
Contributor

FWIW, I ran up against this problem this week as well.

Lots of hair pulling as to how the LdapTemplate.authenticate(..) suite of functions was not actually testing the specified user/password via a bind operation.

@rwinch
Copy link
Member

rwinch commented Sep 28, 2017

Thank you for the report. Unless someone comes up with another example, I'm closing this as invalid.

Please see the explanation here #432 (comment)

@rwinch rwinch closed this as completed Sep 28, 2017
@espenhw
Copy link

espenhw commented Sep 29, 2017

Five minutes of playing with @derTobsch's example project shows that this is not true; the exact same thing happens with anonymous search disabled. See my updated version here: https://github.com/espenhw/tls-bug-demo

Edit: I can also reproduce this on an OpenLDAP installation with 'disallow bind_anon' set; this is not reflected in the example project

@alex-sherwin
Copy link
Contributor

Not sure why this is closed as invalid? This is pretty easy to replicate and clear to check in the code what is happening. The implementation claims to authenticate a user when it does no such thing

rwinch added a commit that referenced this issue Oct 6, 2017
@rwinch
Copy link
Member

rwinch commented Oct 6, 2017

@alex-sherwin @espenhw Thank you for the additional feedback. This is now merged into master.

Please see my follow up comments on the PR.

@rwinch rwinch added this to the 2.3.2 milestone Oct 13, 2017
@rwinch rwinch self-assigned this Oct 13, 2017
@mschneid
Copy link

mschneid commented Feb 7, 2019

Since Java 9 this workaround is no longer necessary. Even worse, it leads to a memory leak and an open socket. Since Java 9, every call to LdapCtx#reconnect(Control[]) initiates a new LdapClient without calling LdapClient#close(Control[], boolean) before. This leads to the situation that the former LdapClient remains alive and is never garbage collected because of the worker thread of the LDAP connection.

@rwinch
Copy link
Member

rwinch commented Feb 7, 2019

@mschneid This issue is closed. Can you please create a new issue with details and a minimal but complete sample to reproduce?

rwinch pushed a commit that referenced this issue Mar 4, 2020
With the fix #430 the `DefaultTlsDirContextAuthenticationStrategy` was
extended by the call of `ctx.reconnect(null);`. Due to the Java bug
JDK-8217606 this call creates a second connection while the first
connection remains open forever.

fixes #502
see JDK-8217606 https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8217606
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants