Skip to content

Commit

Permalink
Adjust logging levels for LDAP failures (#76477)
Browse files Browse the repository at this point in the history 
This change increases the logging level for LDAP failures in 3 cases:

1. When a search fails, the failure details are now logged at DEBUG
rather than TRACE. The errors are useful for diagnostic purposes and
it should not be necessary to turn on TRACE to see them

2. When a failure occurs when attempting to bind using the configured
"bind_dn" and password, this is now a WARN rather than DEBUG. Failures
for user supplied credentials (e.g. basic auth) are still logged as
DEBUG because these are typically not under the control of the cluster
administrator.

3. When a failure occurs while attempting to retrieve a connection
from an LDAP connection pool, this will be logged at a WARN level.
This is almost always an error that the cluster administrator should
be aware of and seek to resolve.

In some cases this may cause 2 sets of log messages (one from LDAP and
one from the authentication service) however it is not always the case
that both are logged, and even when they are they may have different
levels of detail (for example, the authentication service message does
not always include the DN of the bind user).

Backport of: #76058
  • Loading branch information
@tvernum
tvernum committed Aug 13, 2021
1 parent 0a82466 commit 6a6706f
Show file tree
Hide file tree
Showing 4 changed files with 339 additions and 219 deletions.
46 changes: 27 additions & 19 deletions .../main/java/org/elasticsearch/xpack/security/authc/ldap/ActiveDirectorySessionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.unboundid.ldap.sdk.ServerSet;
import com.unboundid.ldap.sdk.SimpleBindRequest;
import com.unboundid.ldap.sdk.controls.AuthorizationIdentityRequestControl;

import org.apache.logging.log4j.Logger;
import org.elasticsearch.ElasticsearchSecurityException;
import org.elasticsearch.action.ActionListener;
Expand All @@ -25,16 +26,16 @@
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
import org.elasticsearch.core.CharArrays;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.RealmSettings;
import org.elasticsearch.xpack.core.security.authc.ldap.ActiveDirectorySessionFactorySettings;
import org.elasticsearch.xpack.core.security.authc.ldap.PoolingSessionFactorySettings;
import org.elasticsearch.xpack.core.security.authc.ldap.support.LdapSearchScope;
import org.elasticsearch.core.CharArrays;
import org.elasticsearch.xpack.core.ssl.SSLService;
import org.elasticsearch.xpack.security.authc.ldap.support.LdapMetadataResolver;
import org.elasticsearch.xpack.security.authc.ldap.support.LdapSession;
Expand Down Expand Up @@ -146,7 +147,7 @@ void getUnauthenticatedSessionWithoutPool(String user, ActionListener<LdapSessio
}
try {
final LDAPConnection connection = LdapUtils.privilegedConnect(serverSet::getConnection);
LdapUtils.maybeForkThenBind(connection, bindCredentials, threadPool, new AbstractRunnable() {
LdapUtils.maybeForkThenBind(connection, bindCredentials, true, threadPool, new AbstractRunnable() {

@Override
public void onFailure(Exception e) {
Expand Down Expand Up @@ -245,7 +246,7 @@ final void authenticate(LDAPConnection connection, String username, SecureString
final byte[] passwordBytes = CharArrays.toUtf8Bytes(password.getChars());
final SimpleBindRequest userBind = new SimpleBindRequest(bindUsername(username), passwordBytes,
new AuthorizationIdentityRequestControl());
LdapUtils.maybeForkThenBind(connection, userBind, threadPool, new ActionRunnable<LdapSession>(listener) {
LdapUtils.maybeForkThenBind(connection, userBind, false, threadPool, new ActionRunnable<LdapSession>(listener) {
@Override
protected void doRun() throws Exception {
final ActionRunnable<LdapSession> searchRunnable = new ActionRunnable<LdapSession>(listener) {
Expand All @@ -269,7 +270,7 @@ protected void doRun() throws Exception {
searchRunnable.run();
} else {
final SimpleBindRequest bind = new SimpleBindRequest(bindDN, CharArrays.toUtf8Bytes(bindPassword.getChars()));
LdapUtils.maybeForkThenBind(connection, bind, threadPool, searchRunnable);
LdapUtils.maybeForkThenBind(connection, bind, true, threadPool, searchRunnable);
}
}
});
Expand Down Expand Up @@ -431,31 +432,38 @@ void netBiosDomainNameToDn(LDAPInterface ldapInterface, String netBiosDomainName
finalLdapConnection.getConnectedAddress(),
finalLdapConnection.getSSLSession() != null ? ldapsPort : ldapPort));
final byte[] passwordBytes = CharArrays.toUtf8Bytes(password.getChars());
final SimpleBindRequest bind = bindDN.isEmpty()
final boolean bindAsAuthenticatingUser = this.bindDN.isEmpty();
final SimpleBindRequest bind = bindAsAuthenticatingUser
? new SimpleBindRequest(username, passwordBytes)
: new SimpleBindRequest(bindDN, CharArrays.toUtf8Bytes(bindPassword.getChars()));
LdapUtils.maybeForkThenBind(searchConnection, bind, threadPool, new ActionRunnable<String>(listener) {
ActionRunnable<String> body = new ActionRunnable<String>(listener) {
@Override
protected void doRun() throws Exception {
search(searchConnection, "CN=Configuration," + domainDN, LdapSearchScope.SUB_TREE.scope(), filter,
timeLimitSeconds, ignoreReferralErrors,
ActionListener.wrap(
results -> {
IOUtils.close(searchConnection);
handleSearchResults(results, netBiosDomainName, domainNameCache, listener);
}, e -> {
IOUtils.closeWhileHandlingException(searchConnection);
listener.onFailure(e);
}),
"ncname");
search(
searchConnection,
"CN=Configuration," + domainDN,
LdapSearchScope.SUB_TREE.scope(),
filter,
timeLimitSeconds,
ignoreReferralErrors,
ActionListener.wrap(results -> {
IOUtils.close(searchConnection);
handleSearchResults(results, netBiosDomainName, domainNameCache, listener);
}, e -> {
IOUtils.closeWhileHandlingException(searchConnection);
listener.onFailure(e);
}),
"ncname"
);
}

@Override
public void onFailure(Exception e) {
IOUtils.closeWhileHandlingException(searchConnection);
listener.onFailure(e);
}
});
};
LdapUtils.maybeForkThenBind(searchConnection, bind, bindAsAuthenticatingUser == false, threadPool, body);
}
} catch (LDAPException e) {
listener.onFailure(e);
Expand Down
5 changes: 3 additions & 2 deletions ...ecurity/src/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapSessionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
import com.unboundid.ldap.sdk.LDAPConnection;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.SimpleBindRequest;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.core.CharArrays;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.util.concurrent.AbstractRunnable;
import org.elasticsearch.core.CharArrays;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
Expand Down Expand Up @@ -99,7 +100,7 @@ public void onFailure(Exception e) {
void loop() {
final String template = userDnTemplates[loopIndex++];
final SimpleBindRequest bind = new SimpleBindRequest(buildDnFromTemplate(username, template), passwordBytes);
LdapUtils.maybeForkThenBind(connection, bind, threadPool, this);
LdapUtils.maybeForkThenBind(connection, bind, false, threadPool, this);
}
}.loop();
} catch (LDAPException e) {
Expand Down
9 changes: 5 additions & 4 deletions ...c/main/java/org/elasticsearch/xpack/security/authc/ldap/LdapUserSearchSessionFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import com.unboundid.ldap.sdk.LDAPInterface;
import com.unboundid.ldap.sdk.SearchResultEntry;
import com.unboundid.ldap.sdk.SimpleBindRequest;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRunnable;
import org.elasticsearch.common.settings.SecureString;
Expand Down Expand Up @@ -106,7 +107,7 @@ void getSessionWithPool(LDAPConnectionPool connectionPool, String user, SecureSt
void getSessionWithoutPool(String user, SecureString password, ActionListener<LdapSession> listener) {
try {
final LDAPConnection connection = LdapUtils.privilegedConnect(serverSet::getConnection);
LdapUtils.maybeForkThenBind(connection, bindCredentials, threadPool, new AbstractRunnable() {
LdapUtils.maybeForkThenBind(connection, bindCredentials, true, threadPool, new AbstractRunnable() {
@Override
protected void doRun() throws Exception {
findUser(user, connection, ActionListener.wrap((entry) -> {
Expand All @@ -117,10 +118,10 @@ protected void doRun() throws Exception {
final String dn = entry.getDN();
final byte[] passwordBytes = CharArrays.toUtf8Bytes(password.getChars());
final SimpleBindRequest userBind = new SimpleBindRequest(dn, passwordBytes);
LdapUtils.maybeForkThenBind(connection, userBind, threadPool, new AbstractRunnable() {
LdapUtils.maybeForkThenBind(connection, userBind, false, threadPool, new AbstractRunnable() {
@Override
protected void doRun() throws Exception {
LdapUtils.maybeForkThenBind(connection, bindCredentials, threadPool, new AbstractRunnable() {
LdapUtils.maybeForkThenBind(connection, bindCredentials, true, threadPool, new AbstractRunnable() {

@Override
protected void doRun() throws Exception {
Expand Down Expand Up @@ -183,7 +184,7 @@ void getUnauthenticatedSessionWithPool(LDAPConnectionPool connectionPool, String
void getUnauthenticatedSessionWithoutPool(String user, ActionListener<LdapSession> listener) {
try {
final LDAPConnection connection = LdapUtils.privilegedConnect(serverSet::getConnection);
LdapUtils.maybeForkThenBind(connection, bindCredentials, threadPool, new AbstractRunnable() {
LdapUtils.maybeForkThenBind(connection, bindCredentials, true, threadPool, new AbstractRunnable() {
@Override
protected void doRun() throws Exception {
findUser(user, connection, ActionListener.wrap((entry) -> {
Expand Down

0 comments on commit 6a6706f

Please sign in to comment.