Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make default ntlmrelayx dump SAM and LSA #1253

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

n00py
Copy link

@n00py n00py commented Feb 8, 2022

I've always wished that the default SMB relay dumped SAM and LSA, so I modified it a bit.

I'm sure there is something more that needs to be added, but from my testing so far this change is functional. Not sure if there was a reason to only do SAM, but if not it would be cool to have it do both.

@ShutdownRepo
Copy link
Contributor

This is great idea and would be an awesome addition to ntlmrelayx 👍
In my opinion there are a few things missing and this PR is a great opportunity to work a bit on ntlmrelayx's smbattack features!

There are some of my additions that are currently working on my local version (lootdir taken into account, filename now replicate what secretsdump does when saving files, LSA hashes are saved just like when using secretsdump) but there's another that I'm struggling to implement.

It is the ability to dump Kerberos keys for the machine account, which would be great for silver tickets and s4u2self abuses. This is, at the moment, not feasible for the following reason.

  1. The dumpCachedHashes() function relies on self.__printMachineKerberos() (function definition in secretsdump.py) to calculate the kerberos keys.
  2. Which fails because the function relies on self.__remoteOps.getMachineKerberosSalt() (function definition in secretsdump.py)
  3. Which fails because the function relies on self.__smbConnection.getServerName() (function definition in smb3.py)
  4. Which fails because ntlmrelayx's smbrelayclient.py doesn't retrieve and set the ServerName and ServerDNSDomainName during the sendNegotiate() operation.

Retrieving the values during the sendNegotiate() operation can be easily done but I'm stuck at passing the values from smbrelayclient.py's self.session to smbattack.py's self.__SMBConnection. I'll continue working on this in the days to come and will make a PR on @n00py's branch when I find a way to implement it.

@ShutdownRepo
Copy link
Contributor

Found a solution thanks to @p0dalirius and @wlayzz
Pushed a PR on @n00py's branch: n00py#1
If my PR gets added, and if @n00py's current PR gets merged as well, the new features will be as follows

  • ntlmrelayx's smbattack.py will dump SAM and LSA upon successful and admin sessions
  • ntlmrelayx's smbattack.py will take the --lootdir argument into account to export hashes into files
  • ntlmrelayx's smbattack.py will export the files with the same names used by secretsdump
  • ntlmrelayx's smbrelayclient.py will extract the server host, domain and dsndomainname during the sendNegotiate() operation. This allows to calculate the Kerberos salt used to calculate the Kerberos keys during the LSA dump.

Dumping the target's machine account's kerberos keys is essential as it allows silver ticket and s4u2self abuses and allows attackers to takeover the target without pass-the-hash, which can be limited sometimes.

ntlmrelayx output during the dump (running with --lootdir ntlmlootdir) 👇
Screenshot from 2022-02-09 11-50-54

ntlmrelayx lootdir after the dump 👇
Screenshot from 2022-02-09 11-56-40

Improved exporting and added Kerberos keys calculation
@gabrielg5 gabrielg5 added the in review This issue or pull request is being analyzed label Jan 26, 2023
@LukeLauterbach
Copy link

This is a solid addition, but doesn't seem to have gotten much traction in the past year. Is there something that I can help do to move this along?

@LukeLauterbach
Copy link

This doesn't really seem to be getting any traction. In case anyone sees this pull request and thinks "hey, I'd really like this," this pull request (along with many others) have been accepted into ThePorgs' fork located here.

@ShutdownRepo
Copy link
Contributor

ShutdownRepo commented Oct 6, 2023

This doesn't really seem to be getting any traction. In case anyone sees this pull request and thinks "hey, I'd really like this," this pull request (along with many others) have been accepted into ThePorgs' fork located here.

Upstream is merging a few PRs here and there. My fork will be less and less relevant hopefully. @anadrianmanrique could you probably take a look? This PR wouldn't impact much things and only adds up things. It should be an quick and easy review & merge

@anadrianmanrique
Copy link
Contributor

anadrianmanrique commented Oct 12, 2023

Hi! My concerns with this changes is that the default behavior wouldn't be as "atomic" as before. I'm trying to think a scenario dumping a huge amount of accounts and receiving in the middle of that a new connection to relay and also having to dump a big load of data. It looks to me like a behavior that the operator would like to configure. Opinions ?

@anadrianmanrique anadrianmanrique self-assigned this Oct 12, 2023
@ShutdownRepo
Copy link
Contributor

Hi! My concerns with this changes is that the default behavior wouldn't be as "atomic" as before. I'm trying to think a scenario dumping a huge amount of accounts and receiving in the middle of that a new connection to relay and also having to dump a big load of data. It looks to me like a behavior that the operator would like to configure. Opinions ?

Agreed, but imo let's default to SAM + LSA dump, which will be fitted to most users' needs. And then give the ability to either dump only SAM or LSA 🤷

@anadrianmanrique
Copy link
Contributor

ok, we discussed with the team, we concluded it would be nice to have this functionality available through a new parameter ( like --dump-lsa ) and not as default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review This issue or pull request is being analyzed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants