From c6c45166bfffabceb6d49fb6ff4df1bbc01152b0 Mon Sep 17 00:00:00 2001 From: RoadTrain Date: Mon, 9 Aug 2021 23:41:42 +0300 Subject: [PATCH 1/3] LdapAdapter: Fix nested claims retrieval -- use CN instead of DN --- .../Authentication/Negotiate/src/Internal/LdapAdapter.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs index bac5beb97309..20e6cc1062e9 100644 --- a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs +++ b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs @@ -110,16 +110,15 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr } var group = searchResponse.Entries[0]; //Get the object that was found on ldap - string name = group.DistinguishedName; - retrievedClaims.Add(name); + retrievedClaims.Add(groupCN); var memberof = group.Attributes["memberof"]; // You can access ldap Attributes with Attributes property if (memberof != null) { foreach (var member in memberof) { - var groupDN = $"{Encoding.UTF8.GetString((byte[])member)}"; - var nestedGroupCN = groupDN.Split(',')[0].Substring("CN=".Length); + var nestedGroupDN = $"{Encoding.UTF8.GetString((byte[])member)}"; + var nestedGroupCN = nestedGroupDN.Split(',')[0].Substring("CN=".Length); GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger, retrievedClaims); } } From bba75b7f89869a20770c30f796edaba16f07a990 Mon Sep 17 00:00:00 2001 From: RoadTrain Date: Tue, 10 Aug 2021 05:09:01 +0300 Subject: [PATCH 2/3] LdapAdapter: Keep track of already processed group to account for possible circular references --- .../Negotiate/src/Internal/LdapAdapter.cs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs index 20e6cc1062e9..7448de8a32d8 100644 --- a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs +++ b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs @@ -68,7 +68,7 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti if (!settings.IgnoreNestedGroups) { - GetNestedGroups(settings.LdapConnection, identity, distinguishedName, groupCN, logger, retrievedClaims); + GetNestedGroups(settings.LdapConnection, identity, distinguishedName, groupCN, logger, retrievedClaims, new HashSet()); } else { @@ -96,8 +96,10 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti } } - private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger, IList retrievedClaims) + private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger, IList retrievedClaims, HashSet processedGroups) { + retrievedClaims.Add(groupCN); + var filter = $"(&(objectClass=group)(sAMAccountName={groupCN}))"; // This is using ldap search query language, it is looking on the server for someUser var searchRequest = new SearchRequest(distinguishedName, filter, SearchScope.Subtree, null); var searchResponse = (SearchResponse)connection.SendRequest(searchRequest); @@ -109,8 +111,10 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr logger.LogWarning($"More than one response received for query: {filter} with distinguished name: {distinguishedName}"); } - var group = searchResponse.Entries[0]; //Get the object that was found on ldap - retrievedClaims.Add(groupCN); + var group = searchResponse.Entries[0]; // Get the object that was found on ldap + var groupDN = group.DistinguishedName; + + processedGroups.Add(groupDN); var memberof = group.Attributes["memberof"]; // You can access ldap Attributes with Attributes property if (memberof != null) @@ -119,7 +123,14 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr { var nestedGroupDN = $"{Encoding.UTF8.GetString((byte[])member)}"; var nestedGroupCN = nestedGroupDN.Split(',')[0].Substring("CN=".Length); - GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger, retrievedClaims); + + if (processedGroups.Contains(nestedGroupDN)) + { + // We need to keep track of already processed groups because circular references are possible with AD groups + return; + } + + GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger, retrievedClaims, processedGroups); } } } From 7f72499bb428bc1828d13e14367dc3c69c05b4d7 Mon Sep 17 00:00:00 2001 From: RoadTrain Date: Wed, 11 Aug 2021 00:13:22 +0300 Subject: [PATCH 3/3] LdapAdapter: Escape LDAP filter arguments --- .../Negotiate/src/Internal/LdapAdapter.cs | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs index 7448de8a32d8..fa17a145a938 100644 --- a/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs +++ b/src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Security.Claims; using System.Text; +using System.Text.RegularExpressions; using System.Threading.Tasks; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; @@ -100,7 +101,7 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr { retrievedClaims.Add(groupCN); - var filter = $"(&(objectClass=group)(sAMAccountName={groupCN}))"; // This is using ldap search query language, it is looking on the server for someUser + var filter = $"(&(objectClass=group)(sAMAccountName={EscapeLdapFilterArgument(groupCN)}))"; // This is using ldap search query language, it is looking on the server for someUser var searchRequest = new SearchRequest(distinguishedName, filter, SearchScope.Subtree, null); var searchResponse = (SearchResponse)connection.SendRequest(searchRequest); @@ -127,7 +128,7 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr if (processedGroups.Contains(nestedGroupDN)) { // We need to keep track of already processed groups because circular references are possible with AD groups - return; + continue; } GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger, retrievedClaims, processedGroups); @@ -135,5 +136,30 @@ private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity pr } } } + + private static string EscapeLdapFilterArgument(string filterArgument) + { + // LDAP filter arguments must be escaped according to RFC 4515: + // https://tools.ietf.org/search/rfc4515#section-3 + // This is important because AD groups can have characters like '(' or ')' in their names, + // and openldap will fail to parse the filter expression if they are not escaped. + var charactersToEscape = new Dictionary + { + {@"\", @"\5c" }, + {"*", @"\2a" }, + {"(", @"\28" }, + {")", @"\29" }, + {"\0", @"\00" }, + }; + + var result = filterArgument; + + foreach (var (source, target) in charactersToEscape) + { + result = Regex.Replace(result, @$"(?