Ability to perform nested group searches using MemberOf field

[Borgquite]

Noted under the documentation that:

'TameMyCerts uses the MemberOf (https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-ada2/cc24555b-61c7-49a2-9748-167b8ce5a512) Active Directory attribute (against the mapped accounts domain).'

and that one cause is that 'nested' group memberships are not supported.

I wondered if it would be possible to lift this restriction using the LDAP_MATCHING_RULE_IN_CHAIN LDAP search string - so you can perform an LDAP search which returns whether a user is a (nested) member of a given group simply by using:

(memberof:1.2.840.113556.1.4.1941:=cn=Group1,OU=groupsOU,DC=x)

It's part of Active Directory and described here - would make role-based access control a possibility?

Just a thought

[virot]

After reading through the entire source.
TameMyCerts currently reads the MemberOf for the user and does not do a search per say.
A change like this could be implemented, but it would require rewrites and could/would affect performance on the connected Active Directory.

[Sleepw4lker]

Hello @Borgquite and @virot. I have this on my backlog and was thinking about mitigation the performance concerns by making the behavior configurable. But I also had talks with AD experts who told me it should be absolutely tolerable performance-wise.
So @virot if you plan to make another awesome contribution, you're highly welcome and i happily handle testing and documentation. Otherwise please give me some time to implement it myself ;).
Cheers

[virot]

My plan would be to do a boolean option.
One question, do we have version limits for Windows?? I know there are some "never" stuff that only exists in Windows 2016 and later..

PS C:> (Get-ADObject -SearchScope Base -SearchBase (Get-ADUser "$env:username") -Properties memberof -Filter *).'memberof'.count
10
PS C:> (Get-ADObject -SearchScope Base -SearchBase (Get-ADUser "$env:username") -Properties msds-memberOfTransitive -Filter *).'msds-memberOfTransitive'.count
57

But if we want to have support for Windows 2012 ADs, I would recommend to just code with old fashioned:
(get-adgroup -LDAPFilter "(member:1.2.840.113556.1.4.1941:=$(Get-ADUser "$env:username-s"))").count

Just doing Powershell, to give example pre implementation.

[Sleepw4lker]

When I started the project, I targeted it for Windows Server 2019 (thus .NET 4.7.2 as this is the pre-installed LTS version), but with support for all Windows OSes in mind that were supported by Microsoft at that time, including Windows Server 2012 R2.
Now that Windows Server 2012 R2 is out of support, and we are a security project, I would argue we could drop support for it.

I have built a Pester testing framework for everything that cannot be done with Unit tests / requires a fully deployed CA - so basically automatically create an AD Domain with CA and templates and then run against all supported OSes. I'll upload that soon, that should ease the testing a lot.

I think it is also time for a contributing document explaining some thoughts that I have architecture-wise.

Again, thank you so much for your work!

[Sleepw4lker]

@virot has implemented it and is now merged into main. Note that I've also ported the Project to .NET 8 now if you plan to custom-compile.

[Borgquite]

@Sleepw4lker @virot Is this still an issue?

https://blog.joeware.net/2021/04/19/6068/

[virot]

But of course Microsoft, has made it "special" :(
Reading that blog the big thing seems to be 4501.

Since we are reading the group membership, Tecnically, since we are using msds-memberOfTransitive instead of msds-tokenGroupNames this could affect us.

msds-memberOfTransitive should give both Security and Distribution groups..
msds-tokenGroupNames should only give Security

So do we think any user will be a member of more than 1017 groups?

Do we want to require only security groups?
If you have more than 1017 security groups, then you will have more than 1024 SIDs in your kerberos ticket and the world will go up in flames.. (There are a few that is always included.)

[virot]

I know it technially is another issue, but is so close.. "Domain Users" is not part of the MemberOf part, but rather PrimaryGroupID.

Should we just resolve that one too, then ALL groups would be accessable.

[Borgquite]

@virot My only thought is that you'd be surprised how many groups some ADs have, and the old style 'LDAP_MATCHING_RULE_IN_CHAIN LDAP' (1.2.840.113556.1.4.1941) does not appear to have the 'large number of groups' limitation and works in all versions of Windows Server...

But up to you!

https://jorgequestforknowledge.wordpress.com/2014/12/15/finding-all-groups-with-a-specific-direct-and-indirect-member-user/
https://jorgequestforknowledge.wordpress.com/2014/12/11/finding-all-direct-and-indirect-members-users-of-a-specific-group/

[gnugnug]

@Borgquite Performing a lookup with a LDAP_MATCHING_RULE_IN_CHAIN filter is extremely slow on my side. Do you have an example on how to run this sufficiently fast?

Regarding the MR of @virot some things could be changed in my opinion.

1. Use msds-TokenGroupNames instead of msds-memberOfTransitive:
   • The attributes in TameMyCerts configuration are called AllowedSecurityGroups and DisallowedSecurityGroups. It would be confusing if a nested membership in a distribution list could return a match.
   • By excluding distribution lists the number of returned groups is smaller, potentially causing less problems.
   • TokenGroupNames include the default group like "Default Users", so if you want to use this in your filters you could.
2. The other AD lookup in TameMyCerts is using DirectorySearcher.FindAll(), whereas the merged code uses DirectorySearcher.FindOne(), which only returns the first entry. I cannot imagine why a lookup with a SearchBase of a specific distinguished name would return more than one entry, but to keep it consistent you could use DirectorySearcher.FindAll() in the merged code as well and then err out if anything but exactly one entry is returned.
3. TameMyCerts reserved a flag to toggle the nested group searches. The code ignores this flag and bases the decision solely on the Windows version. I don't know under which usecase someone would want to ignore indirect group memberships, so either the flag should be removed or the code updated.

Instead of creating a new MR, which changes other contributors code, I would like to discuss the changes here first.

[Sleepw4lker]

Happy new year everyone.

I changed the logic to use msds-TokenGroupNames now. As this attribute is only available on Windows Server 2016 DCs (regardless of Domain Level), I've decided to handle it as follows:

• By default, only memberOf is being used, so the "old" behavior is default. You can enable the new "advanced" logic using the TmcFlag of value 0x4.
• When your DCs do not support the msds-TokenGroupNames attribute, the ActiveDirectoryObject will throw an exception that is caught by the DirectoryServicesValidator. The request is then properly denied and the log indicates that the attribute could not be retrieved.
• I will update the documentation as well pointing out that you must have your DCs on 2016 or newer, if you want to enumerate nested groups. Which should hopefully not be a big deal since 2012 R2 is already out of support by Microsoft.
• Using FindOne() should not matter in this case as we query an exactly identified object.
• I am however not sure if msds-TokenGroupNames has similar side-effects as msds-memberOfTransitive - needs more research.

Edit: I did some additional research and testing on the matter:

• the response seems not to use paging at all, and it gets capped at about 2050 groups.
• anyways, as there is a limit of group memberships of about 1010-1015, we should never hit that limit in real life.