Kerberos and authentication related fixes #895

Marshall-Hallenbeck · 2025-08-22T16:09:23Z

Description

Fixes Nonexistent directory in output file path causes FileNotFoundError stacktrace #878 by improving input parsing when kcache is defined (ensuring file exists, user can read it, etc). Additionally, this improves setting the target domain & user and informs the user if there are discrepancies.
Fixes nopac module with null session results in "KerberosException" stacktrace #889 by checking if the username exists, because you can't request a Kerberos Principal for NULL
Fixes --local-groups over Kerberos session with invalid FQDN results in "KerberosError" stacktrace #892 by improving error handling and error output, suggesting to the user they use FQDN when working with Kerberos and --local-groups
Fixes --local-groups STATUS_LOGON_FAILURE with correct credentials #871 by passing hashes through to the SAMR/LSA DCE transports, allowing users to run --local-groups with -H

Type of change

Insert an "x" inside the brackets for relevant items (do not delete options)

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Deprecation of feature or functionality
This change requires a documentation update
This requires a third party update (such as Impacket, Dploot, lsassy, etc)

Setup guide for the review

No unusual setup needed

Screenshots (if appropriate):

Can provide

Checklist:

Insert an "x" inside the brackets for completed and relevant items (do not delete options)

I have ran Ruff against my changes (via poetry: poetry run python -m ruff check . --preview, use --fix to automatically fix what it can)
I have added or updated the tests/e2e_commands.txt file if necessary (new modules or features are required to be added to the e2e tests)
New and existing e2e tests pass locally with my changes
- Hung on --nla-screenshot, probably need to look into this (unrelated to this PR though)
If reliant on changes of third party dependencies, such as Impacket, dploot, lsassy, etc, I have linked the relevant PRs in those projects
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation (PR here: https://github.com/Pennyw0rth/NetExec-Wiki)

NeffIsBack · 2025-08-23T13:33:31Z

Hey i know it initially makes sense to just fix multiple issues in one PR, but this makes it really hard to understand which lines were added in order to resolve which issues. I think next time it would be better to split up the bug fixes into separate PRs

Marshall-Hallenbeck · 2025-08-23T14:36:55Z

Hey i know it initially makes sense to just fix multiple issues in one PR, but this makes it really hard to understand which lines were added in order to resolve which issues. I think next time it would be better to split up the bug fixes into separate PRs

Yeah I did for other issues, but this PR covers Kerberos/auth all in one.

If you want to see each issue fixed, I did a separate commit for each issue, so you can easily check each of those if you want to see them separate.

Alternatively, I can close this and separate each commit into a PR, but the commits are pretty clean IMO.

NeffIsBack · 2025-08-23T14:45:05Z

All good for now and yes I could go through each commit but it just makes it harder to put it together at the end. Just for the future I think we should keep them separate

NeffIsBack

We should never alter the supplied args. This is the single source of truth where we can get information of the actual supplied information and not something nxc decided to set at some point.
Although the check if the ccache file exists was deeply needed, i don't think the main function is the right place for parsing the ticket information. That should probably be done in the connection itself, probably the login() function.

Rest looks good

NeffIsBack · 2025-08-23T13:38:30Z

nxc/netexec.py

+        krb_domain, krb_username, krb_tgt, krb_tgs = CCache.parseFile()
+        nxc_logger.debug(f"Kerberos cache file parsed: {krb_username}, {krb_domain}, {krb_tgt}, {krb_tgs}")
+
+        if len(args.username) > 1:
+            nxc_logger.error(f"Only one username can be used when using Kerberos cache! The domain\\user for this cache file is '{krb_domain}\\{krb_username}'")
+            exit(1)
+
+        if args.username and krb_username != args.username[0]:
+            nxc_logger.error(f"Kerberos cache file username '{krb_username}' does not match provided username '{args.username[0]}', setting username to '{krb_username}'")
+        else:
+            nxc_logger.debug(f"No username provided, using the one from the cache file: {krb_username}")
+
+        if args.domain and krb_domain != args.domain:
+            nxc_logger.error(f"Kerberos cache file domain '{krb_domain}' does not match provided domain '{args.domain}', setting domain to '{krb_domain}'")
+        else:
+            nxc_logger.debug(f"No domain provided, using the one from the cache file: {krb_domain}")
+
+        args.username = [krb_username]
+        args.domain = krb_domain
+        # at this point the user should be a single user, but in a list, because we still parse credentials the same later, and it expects a list
+        nxc_logger.debug(f"Domain set to {args.domain} and username set to {args.username} for kerberos cache login")
+
+        if args.password:
+            nxc_logger.display("Setting password is irrelevant when using Kerberos cache, ignoring")


Imo we should nearly never accept args.username, args.password or args.domain when using --use-kcache. This is just prone to fail and the information is in anyway in the ticket. This is the way to go when using kcaches imo:

So should we just completely ignore the parameters if they are passed? Or just stop all execution and tell them not to pass any user/pass/domain? In my first iteration I didnt do any of this setting, but currently if they pass in a user or domain with kkache it just fails without telling them why.

I think as it already behaves like we would ignore the users input we should just continue doing that and warn the user.

So add a warning that it's not actually used? Because if we keep it the same as we have it now (accept them but just completely ignore them) it's a bit confusing.

Sure, sounds good if that makes it clearer👍

NeffIsBack · 2025-08-23T13:39:12Z

nxc/netexec.py

+        args.username = [krb_username]
+        args.domain = krb_domain


i don't think we should override the arguments at any point in time. This can lead to unintended behavior on some edge cases

Like what? Unfortunately at this place and time the user/domain arent set, so we can do that later I guess, I think in connection?

I think we should just not consider the arg and just pass in the username&domain in the connection.py where we evaluate if we do ticket authentication. But we have to be careful that the user does not overwrite the domain with the arg parameter

So that's what my other code was doing, making sure it matched and then setting it properly, which I think is useful.

Agreed, but imo instead of overwriting the arg and then read the arg at a later point in time we should just read the info from the ticket at the place where we need/parse the info, without clobbering the supplied args.
I think we should always keep args as is, because this is the only "source of truth" where we can check what was actually supplied.

nxc/modules/nopac.py

NeffIsBack · 2025-08-23T13:54:28Z

nxc/protocols/ldap.py

+        if self.args.use_kcache:  # already parsed kcache domain and username in main()
+            self.domain = self.args.domain
+            self.username = self.args.username[0]


If we don't specify a user via an arg (and don't mess with supplied args somewhere else) this would break when not specifying a username: nxc smb <ip> --use-kcache

I agree this is maybe not the best place to parse it, but i don't think the main function of netexec.py is the place to go

Where would be a good place to do it? I considered the alternatives and figured in the main function would be best, so we can stop all execution if something is messed up.

This should probably go to place in connectino.py where we check if we use kcache as auth method. Probably best to format/supply all attributes that should go into the actual kerberos login function there:

NetExec/nxc/connection.py

Lines 545 to 552 in d4c38ea

if self.args.use_kcache:

self.logger.debug("Trying to authenticate using Kerberos cache")

with sem:

username = self.args.username[0] if len(self.args.username) else ""

password = self.args.password[0] if len(self.args.password) else ""

self.kerberos_login(self.domain, username, password, "", "", self.kdcHost, True)

self.logger.info("Successfully authenticated using Kerberos cache")

return True

That way this is also done for all kerberos protocols that implement kerberos auth and doesn't have to be done in smb.py and ldap.py as well

nxc/protocols/ldap.py

NeffIsBack · 2025-08-24T11:24:13Z

nxc/connection.py

-                username = self.args.username[0] if len(self.args.username) else ""
-                password = self.args.password[0] if len(self.args.password) else ""
-                self.kerberos_login(self.domain, username, password, "", "", self.kdcHost, True)
+                username = self.args.username[0]  # we check this in the main() function when using kcache and set it appropriately already


This will crash if we don't mess with the args somewhere else

Yeah, this is reliant on the username being set in the main function.

NeffIsBack · 2025-08-24T11:30:47Z

nxc/protocols/smb.py

-        elif self.args.use_kcache:  # Fixing domain trust, just pull the auth domain out of the ticket
-            self.domain = CCache.parseFile()[0]


Why removing that? How do we ensure that we have the correct domain?

Because this is done in the main() function now. But we might change that per your other comments.

…errors occur; fixes #892

…generate-tgt test

…ling error

Signed-off-by: Marshall Hallenbeck <Marshall.Hallenbeck@gmail.com>

NeffIsBack · 2025-09-08T10:02:33Z

@Marshall-Hallenbeck any update on this?

Marshall-Hallenbeck · 2025-09-09T23:34:06Z

@Marshall-Hallenbeck any update on this?

We still have on-going discussions from your comments.

NeffIsBack · 2025-10-05T22:35:21Z

@Marshall-Hallenbeck any update on this?

We still have on-going discussions from your comments.

Then let's discuss them 😁

Marshall-Hallenbeck self-assigned this Aug 22, 2025

Marshall-Hallenbeck requested review from NeffIsBack, mpgn and zblurx as code owners August 22, 2025 16:09

Marshall-Hallenbeck added enhancement New feature or request bug-fix This Pull Request fixes a bug labels Aug 22, 2025

Marshall-Hallenbeck changed the title ~~Kerberos-related fixes~~ Kerberos and authentication related fixes Aug 22, 2025

NeffIsBack requested changes Aug 24, 2025

View reviewed changes

NeffIsBack linked an issue Aug 24, 2025 that may be closed by this pull request

FileNotFoundError stacktrace if ccache file doesn't exist when using --use-kcache #869

Open

Marshall-Hallenbeck and others added 7 commits August 25, 2025 10:26

fix(local_shares): add checks and improve error messages if Kerberos …

21fa894

…errors occur; fixes #892

fix(nopac): exit module if there is no username; fixes #889

5df52a4

feat(tgt): check if output file has .ccache in the name already; add …

85bb82b

…generate-tgt test

fix(tgt): check if a username is passed in and catch errors; fix spel…

6ab122f

…ling error

fix(kcache): improve kcache parsing and domain/user setting; fixes #869

fe25369

fix(samr/lsa): support hash authentication; fixes #871

68b44ed

remove unnecessary comment

8d574e2

Signed-off-by: Marshall Hallenbeck <Marshall.Hallenbeck@gmail.com>

Marshall-Hallenbeck force-pushed the marshall-kerberos-cache-fixes branch from 0091e38 to 8d574e2 Compare August 25, 2025 14:26

Marshall-Hallenbeck and others added 3 commits September 9, 2025 18:34

Merge branch 'main' into marshall-kerberos-cache-fixes

b2339f5

Merge branch 'main' into marshall-kerberos-cache-fixes

5b5616a

fix(nopac): remove confusing text

8ab2073

	if self.args.use_kcache:
	self.logger.debug("Trying to authenticate using Kerberos cache")
	with sem:
	username = self.args.username[0] if len(self.args.username) else ""
	password = self.args.password[0] if len(self.args.password) else ""
	self.kerberos_login(self.domain, username, password, "", "", self.kdcHost, True)
	self.logger.info("Successfully authenticated using Kerberos cache")
	return True

		elif self.args.use_kcache: # Fixing domain trust, just pull the auth domain out of the ticket
		self.domain = CCache.parseFile()[0]

Kerberos and authentication related fixes #895

Are you sure you want to change the base?

Kerberos and authentication related fixes #895

Conversation

Marshall-Hallenbeck commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Setup guide for the review

Screenshots (if appropriate):

Checklist:

Uh oh!

NeffIsBack commented Aug 23, 2025

Uh oh!

Marshall-Hallenbeck commented Aug 23, 2025

Uh oh!

NeffIsBack commented Aug 23, 2025

Uh oh!

NeffIsBack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NeffIsBack commented Sep 8, 2025

Uh oh!

Marshall-Hallenbeck commented Sep 9, 2025

Uh oh!

NeffIsBack commented Oct 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Marshall-Hallenbeck commented Aug 22, 2025 •

edited

Loading