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.

Sign up for GitHub

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

Fix LDAP backoff retry logic for serverdown to actually use new connections #100

Merged
merged 11 commits into from
Feb 6, 2024
Merged

Fix LDAP backoff retry logic for serverdown to actually use new connections #100

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
de0ffda
fix: fix ldap backoff retry logic to actually create new connections,…
rvazarkar Feb 2, 2024
bba535d
chore: add some comments
rvazarkar Feb 2, 2024
514a27c
chore: DRY
rvazarkar Feb 2, 2024
e12785f
chore: more comment
rvazarkar Feb 2, 2024
81498cf
chore: simplify backoff delay
rvazarkar Feb 6, 2024
1e06142
fix: replace semaphore with manual reset event
rvazarkar Feb 6, 2024
765bf91
chore: bad import
rvazarkar Feb 6, 2024
dee2f88
chore: rename var for readability
rvazarkar Feb 6, 2024
7d7e669
chore: delete old comment
rvazarkar Feb 6, 2024
efa6a28
fix: fix retry logic again
rvazarkar Feb 6, 2024
42f0805
chore: run formatter, add a small sleep to prevent race
rvazarkar Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/CommonLib/Enums/LdapErrorCodes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ public enum LdapErrorCodes : int
{
Success = 0,
Busy = 51,
ServerDown = 81
ServerDown = 81,
LocalError = 82
}
}
194 changes: 76 additions & 118 deletions src/CommonLib/LDAPUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.DirectoryServices;
using System.DirectoryServices.ActiveDirectory;
using System.DirectoryServices.Protocols;
using System.IO.Compression;
using System.Linq;
using System.Net;
using System.Net.Sockets;
Expand All @@ -18,9 +19,7 @@
using SharpHoundCommonLib.LDAPQueries;
using SharpHoundCommonLib.OutputTypes;
using SharpHoundCommonLib.Processors;
using SharpHoundRPC;
using SharpHoundRPC.NetAPINative;
using SharpHoundRPC.Wrappers;
using Domain = System.DirectoryServices.ActiveDirectory.Domain;
using SearchScope = System.DirectoryServices.Protocols.SearchScope;
using SecurityMasks = System.DirectoryServices.Protocols.SecurityMasks;
Expand Down Expand Up @@ -53,8 +52,8 @@ private static readonly ConcurrentDictionary<string, ResolvedWellKnownPrincipal>
private readonly ConcurrentDictionary<string, Domain> _domainCache = new();
private readonly ConcurrentDictionary<string, string> _domainControllerCache = new();
private static readonly TimeSpan MinBackoffDelay = TimeSpan.FromSeconds(2);
private static readonly TimeSpan MaxBackoffDelay = TimeSpan.FromSeconds(10);
private static readonly TimeSpan BackoffDelayMultiplier = TimeSpan.FromSeconds(2);
private static readonly TimeSpan MaxBackoffDelay = TimeSpan.FromSeconds(20);
private static readonly int BackoffDelayMultiplier = 2;
private const int MaxRetries = 3;

private readonly ConcurrentDictionary<string, LdapConnection> _globalCatalogConnections = new();
Expand All @@ -66,6 +65,7 @@ private static readonly ConcurrentDictionary<string, ResolvedWellKnownPrincipal>
private readonly ConcurrentDictionary<string, string> _netbiosCache = new();
private readonly PortScanner _portScanner;
private LDAPConfig _ldapConfig = new();
private readonly SemaphoreSlim _semaphoreSlim = new(1, 1);
rvazarkar marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Creates a new instance of LDAP Utils with defaults
Expand Down Expand Up @@ -517,8 +517,7 @@ public IEnumerable<string> DoRangedRetrieval(string distinguishedName, string at
//Allow three retries with a backoff on each one if we get a "Server is Busy" error
retryCount++;
Thread.Sleep(backoffDelay);
backoffDelay = TimeSpan.FromSeconds(Math.Min(
backoffDelay.TotalSeconds * BackoffDelayMultiplier.TotalSeconds, MaxBackoffDelay.TotalSeconds));
backoffDelay = GetNextBackoff(retryCount);
continue;
}
catch (Exception e)
Expand Down Expand Up @@ -874,38 +873,78 @@ public IEnumerable<ISearchResultEntry> QueryLDAP(string ldapFilter, SearchScope
}catch (LdapException le) when (le.ErrorCode == (int)LdapErrorCodes.ServerDown &&
retryCount < MaxRetries)
{
/*A ServerDown exception indicates that our connection is no longer valid for one of many reasons.
However, this function is generally called by multiple threads, so we need to be careful in recreating
the connection. Using a semaphore, we can ensure that only one thread is actually recreating the connection
while the other threads that hit the ServerDown exception simply wait. The initial caller will hold the semaphore
and do a backoff delay before trying to make a new connection which will replace the existing connection in the
_ldapConnections cache. Other threads will retrieve the new connection from the cache instead of making a new one
This minimizes overhead of new connections while still fixing our core problem*/
var isSemaphoreHeld = _semaphoreSlim.CurrentCount == 0;
rvazarkar marked this conversation as resolved.
Show resolved Hide resolved
//Always increment retry count
retryCount++;
Thread.Sleep(backoffDelay);
backoffDelay = TimeSpan.FromSeconds(Math.Min(
backoffDelay.TotalSeconds * BackoffDelayMultiplier.TotalSeconds, MaxBackoffDelay.TotalSeconds));
conn = CreateNewConnection(domainName, globalCatalog, skipCache);
if (conn == null)

_semaphoreSlim.Wait(cancellationToken);
try
{
_log.LogError("Unable to create replacement ldap connection for ServerDown exception. Breaking loop");
yield break;
if (!isSemaphoreHeld)
{
//If no one is holding this semaphore, we're the first entrant into this logic, so its our responsibility
//to make the new LDAP connection
Thread.Sleep(backoffDelay);
backoffDelay = GetNextBackoff(retryCount);
//Explicitly skip the cache so we don't get the same connection back
conn = CreateNewConnection(domainName, globalCatalog, true);
if (conn == null)
{
_log.LogError(
"Unable to create replacement ldap connection for ServerDown exception. Breaking loop");
yield break;
}

_log.LogInformation("Created new LDAP connection after receiving ServerDown from server");
}
else
{
//If the semaphore is already held, we're just waiting until we get the semaphore, at which point a new connection should be available
backoffDelay = GetNextBackoff(retryCount);
conn = CreateNewConnection(domainName, globalCatalog);
}
}
finally
{
//Always release
_semaphoreSlim.Release();
}

_log.LogInformation("Created new LDAP connection after receiving ServerDown from server");
continue;
}catch (LdapException le) when (le.ErrorCode == (int)LdapErrorCodes.Busy && retryCount < MaxRetries) {
retryCount++;
Thread.Sleep(backoffDelay);
backoffDelay = TimeSpan.FromSeconds(Math.Min(
backoffDelay.TotalSeconds * BackoffDelayMultiplier.TotalSeconds, MaxBackoffDelay.TotalSeconds));
backoffDelay = GetNextBackoff(retryCount);
continue;
}
catch (LdapException le)
{
if (le.ErrorCode != 82)
if (le.ErrorCode != (int)LdapErrorCodes.LocalError)
{
if (throwException)
{
throw new LDAPQueryException(
$"LDAP Exception in Loop: {le.ErrorCode}. {le.ServerErrorMessage}. {le.Message}. Filter: {ldapFilter}. Domain: {domainName}.",
$"LDAP Exception in Loop: {le.ErrorCode}. {le.ServerErrorMessage}. {le.Message}. Filter: {ldapFilter}. Domain: {domainName}",
le);
else
_log.LogWarning(le,
"LDAP Exception in Loop: {ErrorCode}. {ServerErrorMessage}. {Message}. Filter: {Filter}. Domain: {Domain}",
le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName);
}

_log.LogWarning(le,
"LDAP Exception in Loop: {ErrorCode}. {ServerErrorMessage}. {Message}. Filter: {Filter}. Domain: {Domain}",
le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName);
}

if (le.ErrorCode == (int)LdapErrorCodes.ServerDown)
{
throw new LDAPQueryException(
$"LDAP Exception in Loop: {le.ErrorCode}. {le.ServerErrorMessage}. {le.Message}. Filter: {ldapFilter}. Domain: {domainName}",
le);
}

yield break;
}
catch (Exception e)
Expand Down Expand Up @@ -979,99 +1018,15 @@ public virtual IEnumerable<ISearchResultEntry> QueryLDAP(string ldapFilter, Sear
string[] props, string domainName = null, bool includeAcl = false, bool showDeleted = false,
string adsPath = null, bool globalCatalog = false, bool skipCache = false, bool throwException = false)
{
var queryParams = SetupLDAPQueryFilter(
ldapFilter, scope, props, includeAcl, domainName, includeAcl, adsPath, globalCatalog, skipCache);

if (queryParams.Exception != null)
{
if (throwException) throw queryParams.Exception;

_log.LogWarning(queryParams.Exception, "Failed to setup LDAP Query Filter");
yield break;
}
var conn = queryParams.Connection;
var request = queryParams.SearchRequest;
var pageControl = queryParams.PageControl;

PageResultResponseControl pageResponse = null;

var backoffDelay = MinBackoffDelay;
var retryCount = 0;

while (true)
{
SearchResponse response;

try
{
_log.LogTrace("Sending LDAP request for {Filter}", ldapFilter);
response = (SearchResponse)conn.SendRequest(request);
if (response != null)
pageResponse = (PageResultResponseControl)response.Controls
.Where(x => x is PageResultResponseControl).DefaultIfEmpty(null).FirstOrDefault();
}
catch (LdapException le) when (le.ErrorCode == (int)LdapErrorCodes.Busy && retryCount < MaxRetries)
{
retryCount++;
Thread.Sleep(backoffDelay);
backoffDelay = TimeSpan.FromSeconds(Math.Min(
backoffDelay.TotalSeconds * BackoffDelayMultiplier.TotalSeconds, MaxBackoffDelay.TotalSeconds));
continue;
}
catch (LdapException le) when (le.ErrorCode == (int)LdapErrorCodes.ServerDown &&
retryCount < MaxRetries)
{
retryCount++;
Thread.Sleep(backoffDelay);
backoffDelay = TimeSpan.FromSeconds(Math.Min(
backoffDelay.TotalSeconds * BackoffDelayMultiplier.TotalSeconds, MaxBackoffDelay.TotalSeconds));
conn = CreateNewConnection(domainName, globalCatalog, skipCache);
if (conn == null)
{
_log.LogError("Unable to create replacement ldap connection for ServerDown exception. Breaking loop");
yield break;
}

_log.LogInformation("Created new LDAP connection after receiving ServerDown from server");
continue;
}
catch (LdapException le)
{
if (le.ErrorCode != 82)
if (throwException)
throw new LDAPQueryException(
$"LDAP Exception in Loop: {le.ErrorCode}. {le.ServerErrorMessage}. {le.Message}. Filter: {ldapFilter}. Domain: {domainName}",
le);
else
_log.LogWarning(le,
"LDAP Exception in Loop: {ErrorCode}. {ServerErrorMessage}. {Message}. Filter: {Filter}. Domain: {Domain}",
le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName);
yield break;
}
catch (Exception e)
{
if (throwException)
throw new LDAPQueryException(
$"Exception in LDAP loop for {ldapFilter} and {domainName ?? "Default Domain"}", e);

_log.LogWarning(e, "Exception in LDAP loop for {Filter} and {Domain}", ldapFilter,
domainName ?? "Default Domain");
yield break;
}

if (response == null || pageResponse == null) continue;

if (response.Entries == null)
yield break;

foreach (SearchResultEntry entry in response.Entries)
yield return new SearchResultEntryWrapper(entry, this);

if (pageResponse.Cookie.Length == 0 || response.Entries.Count == 0)
yield break;
return QueryLDAP(ldapFilter, scope, props, new CancellationToken(), domainName, includeAcl, showDeleted,
adsPath, globalCatalog, skipCache, throwException);
}

pageControl.Cookie = pageResponse.Cookie;
}
private static TimeSpan GetNextBackoff(int retryCount)
{
return TimeSpan.FromSeconds(Math.Min(
BackoffDelayMultiplier * (retryCount + 1) * retryCount,
rvazarkar marked this conversation as resolved.
Show resolved Hide resolved
MaxBackoffDelay.TotalSeconds));
}

/// <summary>
Expand Down Expand Up @@ -1560,8 +1515,11 @@ private async Task<LdapConnection> CreateLDAPConnection(string domainName = null

connection.AuthType = authType;

if (!skipCache)
_ldapConnections.TryAdd(targetServer, connection);
_ldapConnections.AddOrUpdate(targetServer, connection, (s, ldapConnection) =>
{
ldapConnection.Dispose();
return connection;
});

return connection;
}
Expand Down
Loading