add ldap3 Kerberos authentication function#843
add ldap3 Kerberos authentication function#843azoxlpf wants to merge 5 commits intoPennyw0rth:mainfrom
Conversation
azoxlpf
requested review from
Marshall-Hallenbeck,
NeffIsBack,
mpgn and
zblurx
as code owners
|
Hi, first of all thanks for your work! We have discussed switching to ldap3 instead of impackets ldap implementation already internally a while ago and decided to not switch for several reasons:
Imo if we should need the TGT/TGS of the connection we should implement getter/setter methods in impacket to retrieve them from the connection. The missing CRUD methods are indeed very annoying, but i think before we implement and establish a different kerberos login method which enables you to do other things than with the standard kerberos login we should try to implement these methods in impacket. Otherwise we will run into situations where we have a different ldap connection than assumed and stuff will fall apart due to missing functions. In the meantime i would go for the same approach as other modules do that need the CRUD methods: Use the existing credentials in NetExec to establish a new ldap3 connection inside that module. That are at least my thoughts :) |
Thanks for the detailed feedback! Just to clarify: this PR does not aim to replace Impacket’s LDAP implementation or change how NetExec handles LDAP globally. It simply introduces a utility function (ldap3_kerberos_login) that creates a ldap3 connection when needed — for example, in upcoming features like --targetedkerberoast that require LDAP operations such as modify(), add(), etc. I completely understand the concerns about moving away from Impacket — but this PR is not proposing such a migration. If it helps, I’m happy to rename or further isolate the function to make it clear that it’s a helper, meant to be used only when needed, without affecting the core behavior. |
Okay got it! I think until we have CRUD methods in impackets LDAP version it is fine to provide a method to establish a ldap3 LDAP connection which can be used without having to invest time into implementing the ldap3 bind&auth part. I think we should indeed rename it to something like Furthermore, does this handshake require the gssapi package of linux? Does this work with native python code as you have implemented the TGT/TGS flow manually? Otherwise we might run into problems when running on other platforms such as Windows or MacOS. |
Got it ! I’ll rename it to get_ldap3_connection() and make it automatically reuse the credentials/args from the Impacket LDAP connection, respecting --kerberos exactly like the classic flow. It works with both NTLM and Kerberos (TGT/TGS built in pure Python), and does not depend on the system’s gssapi package, I tested successfully on Linux without it, and on Windows. I don’t have a Mac to test, but it should work as it’s platform-independent. Windows AES : 
Windows kcache : 
|
|
I would name it |
|
I’m fine with either naming. It’s up to you. |
|
I’ve updated the PR according to the feedback:
Let me know if anything else should be adjusted. |
|
Interesting pr :) |
There was a problem hiding this comment.
Without going too much indepth, here is a first review.
So, in order to ensure that modules if e.g. chained can call this function as often as needed we should:
- Move the main logic into
nxc/protocols/ldap/ldap3_conn.py(or something similarly called, but separate file) - Create a class that is a wrapper for the object instantiation
- Implement a singleton pattern for the instantiation:
if self.ldap3_conn: return self.ldap3_conn, else: self.ldap3_conn = LDAP3_CONN(args).create_conn(), return self.ldap3_conn - Rename function in ldap.py back to
get_ldap3_connectionto emphasize calling this function as often as needed
After that we should replace all manual creations of ldap3 connections in modules. Vice versa, when impacket.ldap finally has an implementation for CRUD functions we can rip out this workaround and replace it with impacket.ldap.
| """ | ||
| Return an ldap3.Connection. | ||
|
|
||
| - NTLM bind requires a plaintext password. |
There was a problem hiding this comment.
Is there no pass-the-hash implementation in ldap3?
There was a problem hiding this comment.
Yes, there is pass-the-hash support in ldap3 with NTLM. I think I initially misread the ldap3 documentation when I worked on that PR
| hash_only = (not self.password) and (bool(self.nthash) or bool(self.lmhash)) | ||
| aes_only = (not self.password) and bool(self.aesKey) | ||
| use_k = self.kerberos or hash_only or aes_only | ||
| use_cc = getattr(self, "use_kcache", False) |
There was a problem hiding this comment.
There should only be either password, hash, aesKey or kcache. A combination is not possible, so there should not be a use case for "only" variables. Take a look at the connection.py for the logic of netexec authentication.
| if hash_only and not self.kerberos: | ||
| self.logger.display("No -k supplied: switching to Kerberos because ldap3 NTLM doesn’t support hash-only auth.") | ||
|
|
||
| if not use_k: |
There was a problem hiding this comment.
Defining functions depending on states is dangerous. The check should be when the function is used and not on definition.
| self.logger.fail(f"NTLM bind failed: {last_err}") | ||
| return None | ||
|
|
||
| self.ldap_connection = conn |
There was a problem hiding this comment.
This attribute should never be overwritten. Instead, create another variable (e.g. self.ldap3_connection) which can then be used by modules/functions which explicitely need to use ldap3 instead of impacket.
| self.logger.info( | ||
| f"ldap3 NTLM bind over {'LDAPS:636' if conn.server.port == 636 else f'LDAP:{conn.server.port}'} established" | ||
| ) |
There was a problem hiding this comment.
Please don't scatter function calls like that. That just wastes space and decreases readability.
I’ve implemented the suggested changes:
I also took the opportunity to refactor the code to improve overall readability |
|
I’ll close it since @NeffIsBack implemented CRUD support in Impacket Add CRUD methods to ldap, it’ll be much easier to use that way |
4 participants
Description
This PR introduces a pure-Python LDAP bind routine that uses the
ldap3library with SPNEGO / Kerberos authentication (ldap3_kerberos_login). Until now NetExec bound over LDAP exclusively with Impacket’s low-levelLDAPConnection, which:modify(),add(),delete(), …).ldap3, on the other hand, offers a complete Pythonic API for directory operations but lacked a Kerberos path inside NetExec. The new function bridges that gap:a SASL bind through
ldap3,(
{"KDC_REP": blob, "cipher": …, "sessionKey": …}) so downstream code canconsume them without branching.
Why
ldap3matters--targetedkerberoastPR (temporary add/remove ofservicePrincipalName) Add targeted Kerberoasting by injecting/removing temporary SPNscan reuse without re-implementing ASN.1.
system libraries beyond the already-present
ldap3.Dependencies
ldap3,impacket,pyasn1andCryptodomeare already shipped with NetExec.Summary
Type of change
Insert an "x" inside the brackets for relevant items (do not delete options)
Setup guide for the review
Screenshots (if appropriate):
The capture shows the
--testhelper (a thin wrapper that simply calls ldap3_kerberos_login) succeeding with a clear-text password, an NT hash, AES-256 and AES-128 keys, and finally with a ticket pulled from the local ccache :Checklist:
Insert an "x" inside the brackets for completed and relevant items (do not delete options)
poetry run python -m ruff check . --preview, use--fixto automatically fix what it can)tests/e2e_commands.txtfile if necessary (new modules or features are required to be added to the e2e tests)