Skip to content

[Daclread] New module option - Filter interesting-only permissions #973

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

Open
njutn95 wants to merge 1 commit into
base: main
Choose a base branch
from
+24 −9
Open

[Daclread] New module option - Filter interesting-only permissions #973

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 24 additions & 9 deletions nxc/modules/daclread.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import sys
import traceback
from os.path import isfile
import re

OBJECT_TYPES_GUID = {}
OBJECT_TYPES_GUID.update(SCHEMA_OBJECTS)
Expand Down Expand Up @@ -219,19 +220,21 @@ def __init__(self, context=None, module_options=None):
self.ace_type = "allowed"
self.rights = None
self.rights_guid = None
self.interesting_only = False

def options(self, context, module_options):
"""
Be careful, this module cannot read the DACLS recursively.
For example, if an object has particular rights because it belongs to a group, the module will not be able to see it directly, you have to check the group rights manually.

TARGET The objects that we want to read or backup the DACLs, specified by its SamAccountName
TARGET_DN The object that we want to read or backup the DACL, specified by its DN (useful to target the domain itself)
PRINCIPAL The trustee that we want to filter on
ACTION The action to realise on the DACL (read, backup)
ACE_TYPE The type of ACE to read (Allowed or Denied)
RIGHTS An interesting right to filter on ('FullControl', 'ResetPassword', 'WriteMembers', 'DCSync')
RIGHTS_GUID A right GUID that specify a particular rights to filter on
TARGET The objects that we want to read or backup the DACLs, specified by its SamAccountName
TARGET_DN The object that we want to read or backup the DACL, specified by its DN (useful to target the domain itself)
PRINCIPAL The trustee that we want to filter on
ACTION The action to realise on the DACL (read, backup)
ACE_TYPE The type of ACE to read (Allowed or Denied)
RIGHTS An interesting right to filter on ('FullControl', 'ResetPassword', 'WriteMembers', 'DCSync')
RIGHTS_GUID A right GUID that specify a particular rights to filter on
INTERESTING_ONLY Whether to exclude common or default permissions and only focus on more specific, significant ones
Copy link
Copy Markdown
Contributor

@azoxlpf azoxlpf May 29, 2026

Choose a reason for hiding this comment

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

Please reword this, the filter is based on trustee SID (RID ≥ 1000) and access-mask patterns, not on “common permissions"


Based on the work of @_nwodtuhs and @BlWasp_.
"""
Expand Down Expand Up @@ -278,6 +281,9 @@ def options(self, context, module_options):
if "RIGHTS_GUID" in module_options:
self.rights_guid = module_options["RIGHTS_GUID"]

if "INTERESTING_ONLY" in module_options:
self.interesting_only = module_options["INTERESTING_ONLY"].lower() in ["true", "1", "yes"]
Copy link
Copy Markdown
Contributor

@azoxlpf azoxlpf May 29, 2026

Choose a reason for hiding this comment

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

Please use .get() to avoid relying on key presence and keep it consistent


def on_login(self, context, connection):
"""On a successful LDAP login we perform a search for the targets' SID, their Security Descriptors and the principal's SID if there is one specified"""
context.log.highlight("Be careful, this module cannot read the DACLS recursively.")
Expand Down Expand Up @@ -362,13 +368,22 @@ def resolveSID(self, sid):
return ""

# Parses a full DACL
# - dacl : the DACL to parse, submitted in a Security Desciptor format
# - dacl : the DACL to parse, submitted in a Security Descriptor format
def parse_dacl(self, dacl):
parsed_dacl = []
self.context.log.debug("Parsing DACL")
for ace in dacl["Data"]:
parsed_ace = self.parse_ace(ace)
parsed_dacl.append(parsed_ace)
if self.interesting_only:
Copy link
Copy Markdown
Contributor

@azoxlpf azoxlpf May 29, 2026

Choose a reason for hiding this comment

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

Use .get() and continue when keys are missing, or skip unsupported ACEs early

# only process SIDs > 1000
if (re.search(r"\(S-1-5-.*-[1-9]\d{3,}\)$", parsed_ace["Trustee (SID)"]) and
Copy link
Copy Markdown
Contributor

@azoxlpf azoxlpf May 29, 2026

Choose a reason for hiding this comment

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

Please switch to parsing the canonical SID (e.g. extract RID from ace['Ace']['Sid'] before resolveSID) instead of regex on the formatted "Name (S-1-5-...)" string, more reliable and avoids LDAP lookups for ACEs you’ll drop anyway.

(re.search(r"GenericAll|Write|Create|Delete|FullControl|Modify|ControlAccess", parsed_ace["Access mask"]) or
("ExtendedRight" in parsed_ace["Access mask"] and "ACCESS_ALLOWED" in parsed_ace["ACE Type"]))
Copy link
Copy Markdown
Contributor

@azoxlpf azoxlpf May 29, 2026

Choose a reason for hiding this comment

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

This will KeyError on standard ACCESS_ALLOWED_ACE / ACCESS_DENIED_ACE because parse_ace() overwrites the dict and drops "ACE Type" (see L416–419). Please either keep "ACE Type" in the standard-ACE branch

Copy link
Copy Markdown
Contributor

@azoxlpf azoxlpf May 29, 2026

Choose a reason for hiding this comment

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

Please switch "ExtendedRight in ..." to an explicit match. It accidentally matches AllExtendedRights and never matches the real flag names (ControlAccess, etc.). Use the actual permission names produced by the enums instead of a substring check.

):
parsed_dacl.append(parsed_ace)
else:
parsed_dacl.append(parsed_ace)

return parsed_dacl

# Parses an access mask to extract the different values from a simple permission
Expand Down