[Draft] POC ldap3 implementation for schannel authentication #412
Conversation
LightxR
requested review from
zblurx,
Marshall-Hallenbeck,
NeffIsBack and
mpgn
as code owners
LightxR
changed the title
|
The problem with ldap3 is that kerberos auth is not native, you need to manually install using apt or whatever |
|
I get your point but not sure to understand if it will be a problem for this PR : I only replace the impacket.ldap import and the code is still using impacket.krb5 so this should be OK.
|
Ok, i read the code a little bit to fast then, if this is working without any additional package this is an awesome PR ! |
|
As you already wrote it into the title i converted the PR into a draft :) |
|
And of course thanks for the PR! |
There was a problem hiding this comment.
First of all thanks for the PR! This would be really cool to have!
There are many changes in the PR that seems to revert previous changes. Could it be that they were accidentally commited?
With them in here it is nearly impossible to track what have been changed (and should have an impact on the new feature) and what not.
The same applies to the patch from the impacket ldap file. Currently i don't get a diff because the whole file is added. I think it would be best if we PR&merge it into the Pennyw0rth impacket fork so we can test it with this PR. Could you open up a PR to the fork?
When both changes are done i can review the PR and give proper feedback.
Fyi if you want to use a local version (or just some other repo) for impacket you can change that in the pyproject.toml file.
This is how you can use a local repo:
[tool.poetry.dependencies]
my-package = { path = "../my/path" }
| if is_admin: | ||
| self.context.log.debug("User is admin!") | ||
| self.admin_privs = True | ||
| return True | ||
| else: |
There was a problem hiding this comment.
Something is off here. Multiple lines are fixes introduced in the last months. Perhaps the commit containing this change reverted them.
| if is_admin: | ||
| self.admin_privs = True | ||
| return True | ||
| else: |
|
@LightxR if you need help with git, hit me up on discord :) |
|
I will update my PR and make modifications from the last netexec version. Regarding my code and more precisely the ldap section, the goal of the PR was to not use impacket.ldap anymore and to create a netexec ldap lib (based on ldap3) to handle ldap things. For the POC, I called it ldap3_patch (which is a bad name) just to search and replace ldap_impacket in the whole netexec code. Another approach could be taken, just let me know and I will update the PR in consequence. As for git, I will ask you help for sure not to mess things up :D |
|
Thanks for the explaination! However, i think maintaining a patched version of an ldap library inside of NetExec is neither the right place, nor feasible to maintain in the future. This should be strictly seperated, to avoid conflicts or the need to repatch the file every time something in impacket gets updated or patched. Is this only possible with ldap3 at the moment or is there a counterpart in impacket? From my point of view we have two options how to handle this:
My suggestion is, that we implement the features needed for cert auth and PR it. I can definitely help with that if i am needed |
not compatible with kerberos by default so not the best option ;) |
|
@LightxR in a perfect world where the pr in impacket is merged quickly, is it possible to directly update impacket ldap implementation ? |
|
Right now the POC is quite dirty as you still need to provide username and domain to be able to use the pfx certificate but the ldap module is already "usable" as is (and kerberos is working fine^^), there are still work regarding the ldap3 implementation : laps flag and some modules (get-userPassword.py, get-desc-users.py, groupmembership.py, get-unixUserPassword.py) have not been modified and don't work in consequence. I completely agree on the fact that working on impacket librairy is a more solid choice because the tool is currently stable with it and had a recent nice PR regarding channel binding :) |
|
Hey, Nice PR idea @LightxR, I'm currently struggling to read gMSA password using Netexec on a server where LDAPS is not accessible and this PR would make this possible by using Start TLS I guess. @NeffIsBack & @mpgn Would https://github.com/skelsec/msldap also be a candidate to replace Impacket LDAP maybe ? Regards. |
I have no problem to review and test the PR, but there is so many conflict with main branch that is starting to be archeology work :'( |
|
Following the last conversation, I put on hold the work on ldap3 implementation and started looking for how it could be possible to add schannel support on impacket.ldap.
|
POC for ldap3 implementation.
The main goal of this PR is to provide schannel authentication support to Netexec through ldap3 implementation.
Future improvements could be ldap signing and channel binding support (available on ldap3 dev branch at the moment). Great article about it here : https://offsec.almond.consulting/ldap-authentication-in-active-directory-environments.html
A workaround is possible right now to bypass channel binding with a fallback on port 389 and StartTLS, see another great article here : https://offsec.almond.consulting/bypassing-ldap-channel-binding-with-starttls.html. This code logic can be added into the actual code.
Another feature worth to consider is the pkinit function from Certipy.