From d3c95bef492c2dcfeeaabfa5cca8eb8b13332e2b Mon Sep 17 00:00:00 2001
From: Tom Spencer <tomspencer989@gmail.com>
Date: Fri, 7 Jul 2023 18:00:11 -0700
Subject: [PATCH 1/2] Remove unnecessary calls to DRSCrackNames in LDAP and
 full DRSUAPI DC Syncs

---
 impacket/examples/secretsdump.py | 86 ++++++++++++++------------------
 1 file changed, 37 insertions(+), 49 deletions(-)

diff --git a/impacket/examples/secretsdump.py b/impacket/examples/secretsdump.py
index 3905407e71..b35d9c0e60 100644
--- a/impacket/examples/secretsdump.py
+++ b/impacket/examples/secretsdump.py
@@ -69,7 +69,7 @@
 from impacket.ldap.ldap import SimplePagedResultsControl, LDAPSearchError
 from impacket.ldap.ldapasn1 import SearchResultEntry
 from impacket.dcerpc.v5 import transport, rrp, scmr, wkst, samr, epm, drsuapi
-from impacket.dcerpc.v5.dtypes import NULL
+from impacket.dcerpc.v5.dtypes import NULL, SID
 from impacket.dcerpc.v5.rpcrt import RPC_C_AUTHN_LEVEL_PKT_PRIVACY, DCERPCException, RPC_C_AUTHN_GSS_NEGOTIATE
 from impacket.dcerpc.v5.dcom import wmi
 from impacket.dcerpc.v5.dcom.oaut import IID_IDispatch, IDispatch, DISPPARAMS, DISPATCH_PROPERTYGET, \
@@ -548,10 +548,18 @@ def DRSGetNCChanges(self, userEntry):
         request['pmsgIn']['V8']['uuidDsaObjDest'] = self.__NtdsDsaObjectGuid
         request['pmsgIn']['V8']['uuidInvocIdSrc'] = self.__NtdsDsaObjectGuid
 
+        # Convert string SID to 2.4.2.2 packet SID
+        tsid = SID()
+        tsid.fromCanonical(userEntry)
+        packetSid  = pack("<B", tsid.fields['Revision'])
+        packetSid += pack("<B", tsid.fields['SubAuthorityCount'])
+        packetSid += tsid.fields['IdentifierAuthority'].fields['Value']
+        packetSid += tsid.fields['SubAuthority']
+
         dsName = drsuapi.DSNAME()
-        dsName['SidLen'] = 0
-        dsName['Guid'] = string_to_bin(userEntry[1:-1])
-        dsName['Sid'] = ''
+        dsName['SidLen'] = len(packetSid)
+        dsName['Guid'] = 0
+        dsName['Sid'] = packetSid
         dsName['NameLen'] = 0
         dsName['StringName'] = ('\x00')
 
@@ -638,7 +646,7 @@ def getDomainUsersLDAP(self, searchFilter):
         try:
             LOG.debug('Search Filter=%s' % searchFilter)
             sc = SimplePagedResultsControl(size=100)
-            resp = self.__ldapConnection.search(searchFilter=searchFilter, attributes=['msDS-PrincipalName'], sizeLimit=0, searchControls=[sc])
+            resp = self.__ldapConnection.search(searchFilter=searchFilter, attributes=['msDS-PrincipalName','objectSid'], sizeLimit=0, searchControls=[sc])
         except LDAPSearchError:
             raise
 
@@ -647,17 +655,21 @@ def getDomainUsersLDAP(self, searchFilter):
         for item in resp:
             if isinstance(item, SearchResultEntry):
                 msDSPrincipalName = ''
+                userSid = ''
                 try:
                     for attribute in item['attributes']:
                         if str(attribute['type']) == 'msDS-PrincipalName':
                             msDSPrincipalName = attribute['vals'][0].asOctets().decode('utf-8')
+                        if str(attribute['type']) == 'objectSid':
+                            tsid = SID(attribute['vals'][0].asOctets())
+                            userSid = tsid.formatCanonical()
                 except Exception as e:
                     LOG.debug("Exception", exc_info=True)
                     LOG.error('Skipping item, cannot process due to error %s' % str(e))
                     pass
                 else:
-                    LOG.debug('DA msDS-PrincipalName: %s' % msDSPrincipalName)
-                    domainUsers.append(msDSPrincipalName)
+                    LOG.debug('DA msDS-PrincipalName: %s, SID: %s' % (msDSPrincipalName, userSid))
+                    domainUsers.append([msDSPrincipalName, userSid])
 
         return domainUsers
 
@@ -2587,7 +2599,7 @@ def dump(self):
                         formatOffered = drsuapi.DS_NT4_ACCOUNT_NAME_SANS_DOMAIN
 
                     crackedName = self.__remoteOps.DRSCrackNames(formatOffered,
-                                                                 drsuapi.DS_NAME_FORMAT.DS_UNIQUE_ID_NAME,
+                                                                 drsuapi.DS_STRING_SID_NAME,
                                                                  name=self.__justUser)
 
                     if crackedName['pmsgOut']['V1']['pResult']['cItems'] == 1:
@@ -2617,25 +2629,15 @@ def dump(self):
                         LOG.error(str(e))
                 elif self.__ldapFilter is not None:
                     resp = self.__remoteOps.getDomainUsersLDAP(self.__ldapFilter)
-                    formatOffered = drsuapi.DS_NAME_FORMAT.DS_NT4_ACCOUNT_NAME
-                    for user in resp:
-                        crackedName = self.__remoteOps.DRSCrackNames(formatOffered,
-                                                                     drsuapi.DS_NAME_FORMAT.DS_UNIQUE_ID_NAME,
-                                                                     name=user)
-
-                        if crackedName['pmsgOut']['V1']['pResult']['cItems'] == 1:
-                            if crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status'] != 0:
-                                raise Exception("%s: %s" % system_errors.ERROR_MESSAGES[
-                                    0x2114 + crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status']])
-
-                            userRecord = self.__remoteOps.DRSGetNCChanges(crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['pName'][:-1])
-                            #userRecord.dump()
-                            replyVersion = 'V%d' % userRecord['pdwOutVersion']
-                            if userRecord['pmsgOut'][replyVersion]['cNumObjects'] == 0:
-                                raise Exception('DRSGetNCChanges didn\'t return any object!')
-                        else:
-                            LOG.warning('DRSCrackNames returned %d items for user %s, skipping' % (
-                                        crackedName['pmsgOut']['V1']['pResult']['cItems'], user))
+                    for (user,userSid) in resp:
+
+                        userRecord = self.__remoteOps.DRSGetNCChanges(userSid)
+                        #userRecord.dump()
+
+                        replyVersion = 'V%d' % userRecord['pdwOutVersion']
+                        if userRecord['pmsgOut'][replyVersion]['cNumObjects'] == 0:
+                            raise Exception('DRSGetNCChanges didn\'t return any object!')
+
                         try:
                             self.__decryptHash(userRecord,
                                                userRecord['pmsgOut'][replyVersion]['PrefixTableSrc']['pPrefixEntry'],
@@ -2666,28 +2668,14 @@ def dump(self):
                                     LOG.debug('Skipping SID %s since it was processed already' % userSid)
                                 continue
 
-                            # Let's crack the user sid into DS_FQDN_1779_NAME
-                            # In theory I shouldn't need to crack the sid. Instead
-                            # I could use it when calling DRSGetNCChanges inside the DSNAME parameter.
-                            # For some reason tho, I get ERROR_DS_DRA_BAD_DN when doing so.
-                            crackedName = self.__remoteOps.DRSCrackNames(drsuapi.DS_NAME_FORMAT.DS_SID_OR_SID_HISTORY_NAME,
-                                                                         drsuapi.DS_NAME_FORMAT.DS_UNIQUE_ID_NAME,
-                                                                         name=userSid)
-
-                            if crackedName['pmsgOut']['V1']['pResult']['cItems'] == 1:
-                                if crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status'] != 0:
-                                    LOG.error("%s: %s" % system_errors.ERROR_MESSAGES[
-                                        0x2114 + crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status']])
-                                    break
-                                userRecord = self.__remoteOps.DRSGetNCChanges(
-                                    crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['pName'][:-1])
-                                # userRecord.dump()
-                                replyVersion = 'V%d' % userRecord['pdwOutVersion']
-                                if userRecord['pmsgOut'][replyVersion]['cNumObjects'] == 0:
-                                    raise Exception('DRSGetNCChanges didn\'t return any object!')
-                            else:
-                                LOG.warning('DRSCrackNames returned %d items for user %s, skipping' % (
-                                crackedName['pmsgOut']['V1']['pResult']['cItems'], userName))
+                            userRecord = self.__remoteOps.DRSGetNCChanges(userSid)
+                            # userRecord.dump()
+
+                            replyVersion = 'V%d' % userRecord['pdwOutVersion']
+
+                            if userRecord['pmsgOut'][replyVersion]['cNumObjects'] == 0:
+                                raise Exception('DRSGetNCChanges didn\'t return any object!')
+
                             try:
                                 self.__decryptHash(userRecord,
                                                    userRecord['pmsgOut'][replyVersion]['PrefixTableSrc']['pPrefixEntry'],

From 2a3a1ff3672c1b2547f37e45500f67f876608954 Mon Sep 17 00:00:00 2001
From: Tom Spencer <tomspencer989@gmail.com>
Date: Wed, 12 Jul 2023 23:13:15 -0700
Subject: [PATCH 2/2] Support graceful fallback for DCs that don't support SID
 lookups

---
 impacket/examples/secretsdump.py | 103 +++++++++++++++++++++++++------
 1 file changed, 83 insertions(+), 20 deletions(-)

diff --git a/impacket/examples/secretsdump.py b/impacket/examples/secretsdump.py
index b35d9c0e60..586a00adb5 100644
--- a/impacket/examples/secretsdump.py
+++ b/impacket/examples/secretsdump.py
@@ -535,22 +535,24 @@ def DRSCrackNames(self, formatOffered=drsuapi.DS_NAME_FORMAT.DS_DISPLAY_NAME,
         resp = drsuapi.hDRSCrackNames(self.__drsr, self.__hDrs, 0, formatOffered, formatDesired, (name,))
         return resp
 
-    def DRSGetNCChanges(self, userEntry):
-        if self.__drsr is None:
-            self.__connectDrds()
+    # Wrapper for calling _DRSGetNCChanges with a GUID
+    def DRSGetNCChangesGuid(self, userGuid):
+        dsName = drsuapi.DSNAME()
+        dsName['SidLen'] = 0
+        dsName['Guid'] = string_to_bin(userGuid[1:-1])
+        dsName['Sid'] = ''
+        dsName['NameLen'] = 0
+        dsName['StringName'] = ('\x00')
+        dsName['structLen'] = len(dsName.getData())
 
-        LOG.debug('Calling DRSGetNCChanges for %s ' % userEntry)
-        request = drsuapi.DRSGetNCChanges()
-        request['hDrs'] = self.__hDrs
-        request['dwInVersion'] = 8
+        return self._DRSGetNCChanges(userGuid, dsName)
 
-        request['pmsgIn']['tag'] = 8
-        request['pmsgIn']['V8']['uuidDsaObjDest'] = self.__NtdsDsaObjectGuid
-        request['pmsgIn']['V8']['uuidInvocIdSrc'] = self.__NtdsDsaObjectGuid
+    # Wrapper for calling _DRSGetNCChanges with a SID
+    def DRSGetNCChangesSid(self, userSid):
 
         # Convert string SID to 2.4.2.2 packet SID
         tsid = SID()
-        tsid.fromCanonical(userEntry)
+        tsid.fromCanonical(userSid)
         packetSid  = pack("<B", tsid.fields['Revision'])
         packetSid += pack("<B", tsid.fields['SubAuthorityCount'])
         packetSid += tsid.fields['IdentifierAuthority'].fields['Value']
@@ -562,9 +564,23 @@ def DRSGetNCChanges(self, userEntry):
         dsName['Sid'] = packetSid
         dsName['NameLen'] = 0
         dsName['StringName'] = ('\x00')
-
         dsName['structLen'] = len(dsName.getData())
 
+        return self._DRSGetNCChanges(userSid, dsName)
+
+    def _DRSGetNCChanges(self, userEntry, dsName):
+        if self.__drsr is None:
+            self.__connectDrds()
+
+        LOG.debug('Calling DRSGetNCChanges for %s ' % userEntry)
+        request = drsuapi.DRSGetNCChanges()
+        request['hDrs'] = self.__hDrs
+        request['dwInVersion'] = 8
+
+        request['pmsgIn']['tag'] = 8
+        request['pmsgIn']['V8']['uuidDsaObjDest'] = self.__NtdsDsaObjectGuid
+        request['pmsgIn']['V8']['uuidInvocIdSrc'] = self.__NtdsDsaObjectGuid
+
         request['pmsgIn']['V8']['pNC'] = dsName
 
         request['pmsgIn']['V8']['usnvecFrom']['usnHighObjUpdate'] = 0
@@ -2574,6 +2590,7 @@ def dump(self):
                 LOG.info('Using the DRSUAPI method to get NTDS.DIT secrets')
                 status = STATUS_MORE_ENTRIES
                 enumerationContext = 0
+                lookupBySid = True
 
                 # Do we have to resume from a previously saved session?
                 if self.__resumeSession.hasResumeData():
@@ -2599,7 +2616,7 @@ def dump(self):
                         formatOffered = drsuapi.DS_NT4_ACCOUNT_NAME_SANS_DOMAIN
 
                     crackedName = self.__remoteOps.DRSCrackNames(formatOffered,
-                                                                 drsuapi.DS_STRING_SID_NAME,
+                                                                 drsuapi.DS_NAME_FORMAT.DS_UNIQUE_ID_NAME,
                                                                  name=self.__justUser)
 
                     if crackedName['pmsgOut']['V1']['pResult']['cItems'] == 1:
@@ -2607,7 +2624,7 @@ def dump(self):
                             raise Exception("%s: %s" % system_errors.ERROR_MESSAGES[
                                 0x2114 + crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status']])
 
-                        userRecord = self.__remoteOps.DRSGetNCChanges(crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['pName'][:-1])
+                        userRecord = self.__remoteOps.DRSGetNCChangesGuid(crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['pName'][:-1])
                         #userRecord.dump()
                         replyVersion = 'V%d' % userRecord['pdwOutVersion']
                         if userRecord['pmsgOut'][replyVersion]['cNumObjects'] == 0:
@@ -2629,11 +2646,34 @@ def dump(self):
                         LOG.error(str(e))
                 elif self.__ldapFilter is not None:
                     resp = self.__remoteOps.getDomainUsersLDAP(self.__ldapFilter)
-                    for (user,userSid) in resp:
+                    formatOffered = drsuapi.DS_NAME_FORMAT.DS_NT4_ACCOUNT_NAME
+                    for (user, userSid) in resp:
 
-                        userRecord = self.__remoteOps.DRSGetNCChanges(userSid)
+                        # Try to lookup by SID, but fallback to DSCrackNames for GUID lookups otherwise
+                        if lookupBySid:
+                            try:
+                                userRecord = self.__remoteOps.DRSGetNCChangesSid(userSid)
+                            except drsuapi.DCERPCSessionError as e:
+                                LOG.debug("SID lookup unsuccessful, falling back to DRSCrackNames/GUID lookups")
+                                lookupBySid = False
+
+                        # We may need to run the above request again if it failed so this can't be an else
+                        if not lookupBySid:
+                            crackedName = self.__remoteOps.DRSCrackNames(formatOffered,
+                                                                         drsuapi.DS_NAME_FORMAT.DS_UNIQUE_ID_NAME,
+                                                                         name=user)
+
+                            if crackedName['pmsgOut']['V1']['pResult']['cItems'] == 1:
+                                if crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status'] != 0:
+                                    raise Exception("%s: %s" % system_errors.ERROR_MESSAGES[
+                                        0x2114 + crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status']])
+
+                                userRecord = self.__remoteOps.DRSGetNCChangesGuid(crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['pName'][:-1])
+                            else:
+                                LOG.warning('DRSCrackNames returned %d items for user %s, skipping' % (
+                                            crackedName['pmsgOut']['V1']['pResult']['cItems'], user)
+)
                         #userRecord.dump()
-
                         replyVersion = 'V%d' % userRecord['pdwOutVersion']
                         if userRecord['pmsgOut'][replyVersion]['cNumObjects'] == 0:
                             raise Exception('DRSGetNCChanges didn\'t return any object!')
@@ -2668,11 +2708,34 @@ def dump(self):
                                     LOG.debug('Skipping SID %s since it was processed already' % userSid)
                                 continue
 
-                            userRecord = self.__remoteOps.DRSGetNCChanges(userSid)
-                            # userRecord.dump()
+                            # Try to lookup by SID, but fallback to DSCrackNames for GUID lookups otherwise
+                            if lookupBySid:
+                                try:
+                                    userRecord = self.__remoteOps.DRSGetNCChangesSid(userSid)
+                                except drsuapi.DCERPCSessionError as e:
+                                    LOG.debug("SID lookup unsuccessful, falling back to DRSCrackNames/GUID lookups")
+                                    lookupBySid = False
+
+                            # We may need to run the above request again if it failed so this can't be an else
+                            if not lookupBySid:
+                                crackedName = self.__remoteOps.DRSCrackNames(drsuapi.DS_NAME_FORMAT.DS_SID_OR_SID_HISTORY_NAME,
+                                                                             drsuapi.DS_NAME_FORMAT.DS_UNIQUE_ID_NAME,
+                                                                             name=userSid)
+
+                                if crackedName['pmsgOut']['V1']['pResult']['cItems'] == 1:
+                                    if crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status'] != 0:
+                                        LOG.error("%s: %s" % system_errors.ERROR_MESSAGES[
+                                            0x2114 + crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['status']])
+                                        break
+                                    userRecord = self.__remoteOps.DRSGetNCChangesGuid(
+                                        crackedName['pmsgOut']['V1']['pResult']['rItems'][0]['pName'][:-1])
 
-                            replyVersion = 'V%d' % userRecord['pdwOutVersion']
+                                else:
+                                    LOG.warning('DRSCrackNames returned %d items for user %s, skipping' % (
+                                    crackedName['pmsgOut']['V1']['pResult']['cItems'], userName))
 
+                            # userRecord.dump()
+                            replyVersion = 'V%d' % userRecord['pdwOutVersion']
                             if userRecord['pmsgOut'][replyVersion]['cNumObjects'] == 0:
                                 raise Exception('DRSGetNCChanges didn\'t return any object!')