From 5c2f44ae3a026d59079af609ebbd9fff52a54bde Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Fri, 24 Jan 2020 00:57:54 -0800 Subject: [PATCH] GUACAMOLE-936: Clean up referral logic and logging. Ensure associated LDAP connections are always closed. --- .../auth/ldap/LDAPConnectionService.java | 66 ++++++++-------- .../auth/ldap/ObjectQueryService.java | 78 ++++++++++++++----- 2 files changed, 94 insertions(+), 50 deletions(-) diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java index 7e88f8abe..67ed4dec0 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/LDAPConnectionService.java @@ -40,8 +40,6 @@ import org.apache.guacamole.GuacamoleServerException; import org.apache.guacamole.GuacamoleUnsupportedException; import org.apache.guacamole.auth.ldap.conf.ConfigurationService; import org.apache.guacamole.auth.ldap.conf.EncryptionMethod; -import org.apache.guacamole.net.auth.credentials.CredentialsInfo; -import org.apache.guacamole.net.auth.credentials.GuacamoleInvalidCredentialsException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -123,7 +121,8 @@ public class LDAPConnectionService { * bound. * * @throws GuacamoleException - * If an error occurs while binding to the LDAP server. + * If the configuration details relevant to binding to the LDAP server + * cannot be read. */ public LdapNetworkConnection bindAs(Dn userDN, String password) throws GuacamoleException { @@ -165,46 +164,49 @@ public class LDAPConnectionService { } /** - * Generate a new LdapNetworkConnection object for following a referral - * with the given LdapUrl, and copy the username and password - * from the original connection. - * - * @param referralUrl - * The LDAP URL to follow. - * - * @param ldapConfig - * The connection configuration to use to retrieve username and - * password. - * - * @param hop - * The current hop number of this referral - once the configured - * limit is reached, this method will throw an exception. + * Establishes a new network connection to the LDAP server indicated by the + * given LDAP referral URL. The credentials used to bind with the referred + * LDAP server will be the same as those used to bind with the original + * connection. * + * @param ldapConnection + * The LDAP connection that bind credentials should be copied from. + * + * @param url + * The URL of the referred LDAP server to which a new network + * connection should be established. + * * @return - * A LdapNetworkConnection object that points at the location - * specified in the referralUrl. - * - * @throws GuacamoleException - * If an error occurs parsing out the LdapUrl object or the - * maximum number of referral hops is reached. + * A LdapNetworkConnection representing a network connection to the + * LDAP server specified in the URL, or null if the specified URL is + * invalid. */ - public LdapNetworkConnection getReferralConnection(LdapUrl referralUrl, - LdapConnectionConfig ldapConfig, int hop) - throws GuacamoleException { + public LdapNetworkConnection getReferralConnection( + LdapNetworkConnection ldapConnection, String url) { - if (hop >= confService.getMaxReferralHops()) - throw new GuacamoleServerException("Maximum number of referrals reached."); - + LdapConnectionConfig ldapConfig = ldapConnection.getConfig(); LdapConnectionConfig referralConfig = new LdapConnectionConfig(); // Copy bind name and password from original config referralConfig.setName(ldapConfig.getName()); referralConfig.setCredentials(ldapConfig.getCredentials()); - + + LdapUrl referralUrl; + try { + referralUrl = new LdapUrl(url); + } + catch (LdapException e) { + logger.debug("Referral URL \"{}\" is invalid.", url, e); + return null; + } + // Look for host - if not there, bail out. String host = referralUrl.getHost(); - if (host == null || host.isEmpty()) - throw new GuacamoleServerException("Referral URL contains no host."); + if (host == null || host.isEmpty()) { + logger.debug("Referral URL \"{}\" is invalid as it contains " + + "no hostname.", url ); + return null; + } referralConfig.setLdapHost(host); diff --git a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java index e1fa2bb45..fcae4d67b 100644 --- a/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java +++ b/extensions/guacamole-auth-ldap/src/main/java/org/apache/guacamole/auth/ldap/ObjectQueryService.java @@ -23,6 +23,7 @@ import com.google.inject.Inject; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -38,7 +39,6 @@ import org.apache.directory.api.ldap.model.filter.EqualityNode; import org.apache.directory.api.ldap.model.filter.ExprNode; import org.apache.directory.api.ldap.model.filter.OrNode; import org.apache.directory.api.ldap.model.filter.PresenceNode; -import org.apache.directory.api.ldap.model.message.Referral; import org.apache.directory.api.ldap.model.message.SearchRequest; import org.apache.directory.api.ldap.model.name.Dn; import org.apache.directory.api.ldap.model.url.LdapUrl; @@ -46,6 +46,8 @@ import org.apache.directory.ldap.client.api.LdapConnectionConfig; import org.apache.directory.ldap.client.api.LdapNetworkConnection; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; +import org.apache.guacamole.auth.ldap.conf.ConfigurationService; +import org.apache.guacamole.auth.ldap.conf.LDAPGuacamoleProperties; import org.apache.guacamole.net.auth.Identifiable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -70,6 +72,12 @@ public class ObjectQueryService { @Inject private LDAPConnectionService ldapService; + /** + * Service for retrieving LDAP server configuration information. + */ + @Inject + private ConfigurationService confService; + /** * Returns the identifier of the object represented by the given LDAP * entry. Multiple attributes may be declared as containing the identifier @@ -204,13 +212,21 @@ public class ObjectQueryService { public List search(LdapNetworkConnection ldapConnection, Dn baseDN, ExprNode query, int searchHop) throws GuacamoleException { + // Refuse to follow referrals if limit has been reached + int maxHops = confService.getMaxReferralHops(); + if (searchHop >= maxHops) { + logger.debug("Refusing to follow further referrals as the maximum " + + "number of referral hops ({}) has been reached. LDAP " + + "search results may be incomplete. If further referrals " + + "should be followed, consider setting the \"{}\" " + + "property to a larger value.", maxHops, LDAPGuacamoleProperties.LDAP_MAX_REFERRAL_HOPS.getName()); + return Collections.emptyList(); + } + logger.debug("Searching \"{}\" for objects matching \"{}\".", baseDN, query); - LdapConnectionConfig ldapConnectionConfig = ldapConnection.getConfig(); - // Search within subtree of given base DN - SearchRequest request = ldapService.getSearchRequest(baseDN, - query); + SearchRequest request = ldapService.getSearchRequest(baseDN, query); // Produce list of all entries in the search result, automatically // following referrals if configured to do so @@ -219,22 +235,48 @@ public class ObjectQueryService { try (SearchCursor results = ldapConnection.search(request)) { while (results.next()) { - if (results.isEntry()) { + // Add entry directly if no referral is involved + if (results.isEntry()) entries.add(results.getEntry()); - } - else if (results.isReferral() && request.isFollowReferrals()) { - Referral referral = results.getReferral(); - for (String url : referral.getLdapUrls()) { - LdapNetworkConnection referralConnection = - ldapService.getReferralConnection( - new LdapUrl(url), - ldapConnectionConfig, searchHop++ - ); - entries.addAll(search(referralConnection, baseDN, query, - searchHop)); + // If a referral must be followed to obtain further results, + // retrieval of those results depends on whether such referral + // following is enabled + else if (results.isReferral()) { + + // Follow received referrals only if configured to do so + if (request.isFollowReferrals()) { + for (String url : results.getReferral().getLdapUrls()) { + + // Connect to referred LDAP server to retrieve further results, ensuring the network + // connection is always closed when it will no longer be used + try (LdapNetworkConnection referralConnection = ldapService.getReferralConnection(ldapConnection, url)) { + if (referralConnection != null) { + logger.debug("Following referral to \"{}\"...", url); + entries.addAll(search(referralConnection, baseDN, query, searchHop + 1)); + } + else + logger.debug("Could not follow referral to " + + "\"{}\" as the URL is invalid.", url); + } + catch (GuacamoleException e) { + logger.warn("Referral to \"{}\" could not be followed: {}", url, e.getMessage()); + logger.debug("Failed to follow LDAP referral.", e); + } + + } } + // Log if referrals may be applicable but they aren't being + // followed + else + logger.debug("Referrals to one or more other LDAP " + + "servers were received but are being " + + "ignored because following of referrals is " + + "not enabled. If referrals must be " + + "followed, consider setting the \"{}\" " + + "property to \"true\".", LDAPGuacamoleProperties.LDAP_FOLLOW_REFERRALS.getName()); + } } @@ -244,7 +286,7 @@ public class ObjectQueryService { } catch (CursorException | IOException | LdapException e) { throw new GuacamoleServerException("Unable to query list of " - + "objects from LDAP directory.", e); + + "objects from LDAP directory: " + e.getMessage(), e); } }