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

Linux: Fix nested group resolution for RBAC claims #35279

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
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
54 changes: 45 additions & 9 deletions src/Security/Authentication/Negotiate/src/Internal/LdapAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -68,7 +69,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<string>());
}
else
{
Expand Down Expand Up @@ -96,9 +97,11 @@ public static async Task RetrieveClaimsAsync(LdapSettings settings, ClaimsIdenti
}
}

private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger, IList<string> retrievedClaims)
private static void GetNestedGroups(LdapConnection connection, ClaimsIdentity principal, string distinguishedName, string groupCN, ILogger logger, IList<string> retrievedClaims, HashSet<string> processedGroups)
{
var filter = $"(&(objectClass=group)(sAMAccountName={groupCN}))"; // This is using ldap search query language, it is looking on the server for someUser
retrievedClaims.Add(groupCN);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved it here from under if clause, but I'm actually not sure.
Basically what I did was compare the results of this code in Linux vs Windows.
I noticed one difference in their behaviour:

  1. When using filter below on Windows, the results include all groups, including distribution groups (email distribution lists).
  2. On Linux, distribution groups are not resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it warrants a separate issue.


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);

Expand All @@ -109,21 +112,54 @@ 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
string name = group.DistinguishedName;
retrievedClaims.Add(name);
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)
{
foreach (var member in memberof)
{
var groupDN = $"{Encoding.UTF8.GetString((byte[])member)}";
var nestedGroupCN = groupDN.Split(',')[0].Substring("CN=".Length);
GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger, retrievedClaims);
var nestedGroupDN = $"{Encoding.UTF8.GetString((byte[])member)}";
var nestedGroupCN = nestedGroupDN.Split(',')[0].Substring("CN=".Length);

if (processedGroups.Contains(nestedGroupDN))
{
// We need to keep track of already processed groups because circular references are possible with AD groups
continue;
}

GetNestedGroups(connection, principal, distinguishedName, nestedGroupCN, logger, retrievedClaims, processedGroups);
}
}
}
}

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<string, string>
{
{@"\", @"\5c" },
{"*", @"\2a" },
{"(", @"\28" },
{")", @"\29" },
{"\0", @"\00" },
};

var result = filterArgument;

foreach (var (source, target) in charactersToEscape)
{
result = Regex.Replace(result, @$"(?<!(?<!\\)\\(?:\\\\)*)({Regex.Escape(source)})", target);
};

return result;
}
}
}