dirkjanm / ldapdomaindump

Active Directory information dumper via LDAP
MIT License
1.15k stars 182 forks source link

revert the minimum required version of ldap3 #25

Closed rmaksimov closed 4 years ago

rmaksimov commented 4 years ago

I saw you changed the minimum version of ldap3 back from 2.6.1, but I have faced with an issue using the version 2.5.1

If there is an object's DN containing trailing spaces (as the following)

CN=Any Group Name\ ,OU=Any Sub OU Name,OU=Any OU Name,DC=CONTOSO,DC=COM

LDAPDomainDump crashes (ldap3 2.5.1)

$ ldapdomaindump -o dump -u 'contoso.com\username' -p password 10.0.0.10
[*] Connecting to host...
[*] Binding to host
[+] Bind OK
[*] Starting domain dump
Traceback (most recent call last):
  File "/usr/local/bin/ldapdomaindump", line 3, in <module>
    ldapdomaindump.main()
  File "/usr/local/lib/python3.7/dist-packages/ldapdomaindump/__init__.py", line 944, in main
    dd.domainDump()
  File "/usr/local/lib/python3.7/dist-packages/ldapdomaindump/__init__.py", line 428, in domainDump
    rw.generateUsersByGroupReport(self)
  File "/usr/local/lib/python3.7/dist-packages/ldapdomaindump/__init__.py", line 778, in generateUsersByGroupReport
    grouped = dd.sortUsersByGroup(dd.users)
  File "/usr/local/lib/python3.7/dist-packages/ldapdomaindump/__init__.py", line 380, in sortUsersByGroup
    ugroups = [self.getGroupCnFromDn(group) for group in user.memberOf.values]
  File "/usr/local/lib/python3.7/dist-packages/ldapdomaindump/__init__.py", line 380, in <listcomp>
    ugroups = [self.getGroupCnFromDn(group) for group in user.memberOf.values]
  File "/usr/local/lib/python3.7/dist-packages/ldapdomaindump/__init__.py", line 363, in getGroupCnFromDn
    cn = self.unescapecn(dn.parse_dn(dnin)[0][1])
  File "/usr/local/lib/python3.7/dist-packages/ldap3/utils/dn.py", line 292, in parse_dn
    if not _validate_attribute_value(attribute_value):
  File "/usr/local/lib/python3.7/dist-packages/ldap3/utils/dn.py", line 208, in _validate_attribute_value
    raise LDAPInvalidDnError('invalid final character')
ldap3.core.exceptions.LDAPInvalidDnError: invalid final character

And everything is fine while using the version 2.6.1

$ ldapdomaindump -o dump -u 'contoso.com\username' -p password 10.0.0.10
[*] Connecting to host...
[*] Binding to host
[+] Bind OK
[*] Starting domain dump
[+] Domain dump finished

The bug has been fixed in ldap3 version 2.6.1: https://github.com/cannatag/ldap3/commit/64cac9ac1d54404af6f75291f9fae0632471f6b6

dirkjanm commented 4 years ago

hey, by default ldapdomaindump should install with ldap3 2.6.1. However to prevent errors while other packages (such as impacket) are still on 2.5.1, this is an accepted version as well. I'm aware 2.5.1 crashes in some environments but it's better to leave this version in despite this bug in my opinion.

rmaksimov commented 4 years ago

Hey, dirkjanm Thanks for the reply

You are totally right it should install the latest available version or use an already installed one (impacket case). But the latter case is bad, because while using version 2.5.1 you don’t get the full dump of a domain.

It is not ldapdomaindump problem as well, but its dependencies. Of course the best way would be to backport those changes in ldap3 to the version 2.5.1, plus migrate impacket and Co to the latest version of ldap3. It looks that ldapdomaindump is stable with the version 2.6.1 and it works as expected. Anyway it is up to you ;)

dirkjanm commented 4 years ago

Yes dependencies are hard :) i agree with your points that the current situation is not ideal, but having a conflicting version with impacket, which directly depends on ldapdomaindump and does not get version updates frequently, is even worse in my opinion. So closing this one, but thanks for thinking along!