Fix ACE SID comparion in RBCD module

[jheysel-r7]

Description

This fixes a broken SID comparison. action_remove was failing to remove the target ACE because of a type mismatch in how it compared SIDs. Before the module was doing:

aces.delete_if { |ace| ace.body.sid == delegate_from[:objectSid].first }

The two sides of that comparison aren't the same datatype. ace.body.sid is a Rex::Proto::MsDtyp::MsDtypSid, and when you compare it with ==, it uses its human-readable string form - something like "S-1-5-21-2324486357-...". But delegate_from[:objectSid].first is the raw binary SID straight from LDAP ie:Net::BER::BerIdentifiedString

The module was comparing a formatted SID string against raw bytes, which never matches. The delete_if block always returned false, and the ACE was never removed.

The fix is to parse the raw binary into a MsDtypSid first, so both sides are comparing on the same type. This mirrors what #_action_write_create already does before handing the SID off to build_ace, so it brings #action_remove in line with the existing pattern.

This also fixes the same bug in #_action_write_update - its "already configured" check had the identical type mismatch, meaning it would never detect an existing duplicate entry.

Related Issue:

This is was discovered when making these changes: #21647 however I figured it made sense to separate these PRs as they're different issues.

Breaking Changes

None

Reviewer Notes

Verification Steps

• Run the READ action of the rbcd module and verify there are no entries present for a specific machine account
• Run the WRITE action and verify an entry gets written to the attribute
• Run the READ action to be extra super duper certain the entry was written successfully
• Run the REMOVE action, it should say the entry was removed
• Run the READ action once more to verify the entry to was removed

Test Evidence

Before

msf auxiliary(admin/ldap/rbcd) > set DELEGATE_FROM MINION1$
DELEGATE_FROM => MINION1$
msf auxiliary(admin/ldap/rbcd) > remove
[*] Running module against 172.16.199.200
[*] Discovering base DN automatically
[*] No DACL ACEs matched. No changes are necessary.
[*] Auxiliary module execution completed
msf auxiliary(admin/ldap/rbcd) > set DELEGATE_TO MINION1$
DELEGATE_TO => MINION1$
msf auxiliary(admin/ldap/rbcd) > remove
[*] Running module against 172.16.199.200
[*] Discovering base DN automatically
[*] No DACL ACEs matched. No changes are necessary.
[*] Auxiliary module execution completed
msf auxiliary(admin/ldap/rbcd) > read
[*] Running module against 172.16.199.200
[*] Discovering base DN automatically
[*] Allowed accounts:
[*]   S-1-5-21-2324486357-3075865580-3606784161-2604 (MINION1$)
[*] Auxiliary module execution completed
msf auxiliary(admin/ldap/rbcd) > set DELEGATE_TO MINION1
DELEGATE_TO => MINION1
msf auxiliary(admin/ldap/rbcd) > set DELEGATE_FROM MINION1
DELEGATE_FROM => MINION1
msf auxiliary(admin/ldap/rbcd) > remove
[*] Running module against 172.16.199.200
[*] Discovering base DN automatically
[*] No DACL ACEs matched. No changes are necessary.
[*] Auxiliary module execution completed
msf auxiliary(admin/ldap/rbcd) >

After

msf auxiliary(admin/ldap/rbcd) > remove
[*] Running module against 172.16.199.200
[*] Discovering base DN automatically
[*] Removed 1 matching ACE.
[+] Successfully updated the msDS-AllowedToActOnBehalfOfOtherIdentity attribute.
[*] Auxiliary module execution completed
msf auxiliary(admin/ldap/rbcd) > read
[*] Running module against 172.16.199.200
[*] Discovering base DN automatically
[*] Allowed accounts:
[*] Auxiliary module execution completed

Environment

Field	Details
Operating System	Windows 2019 domain controller

AI Usage Disclosure

kiro-cli

Pre-Submission Checklist

• No sensitive information (IP addresses, credentials, API keys, hashes) in code or documentation
• Tested on the target environment specified in the Environment section above

[github-actions]

Thanks for your pull request! As part of our landing process, we manually verify that all modules work as expected.

We've added the additional-testing-required label to indicate that additional testing is required before this pull request can be merged.
For maintainers, this means visiting here.

[smcintyre-r7]

Yeah this looks right to me, I've got no changes to request. I was able to reproduce the original issue and then validate that these changes fix it.

New and fixed on the left, old and busted on the right. I ran the old test second, so you'll see that's why the first read output is slightly different because an empty object exists vs being entirely absent.

[smcintyre-r7]

Release Notes

This fixes a bug in the Role Based Constrained Delegation (RBCD) module that prevented Access Control Entries (ACEs) from being removed due to a type mismatch while comparing Security Identifiers (SIDs).