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.

Sign up for GitHub

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

Jump to bottom

add new security-questions module #295

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

add new security-questions module #295

wants to merge 9 commits into from

Conversation

Adamkadaban
Copy link
Contributor

@Adamkadaban Adamkadaban commented May 8, 2024 •
edited
Loading

Description

this module will dump security questions and answers for all users on the machine assuming run as administrator

requires this PR to work: fortra/impacket#1743

Reference: https://hackback.zip/2024/05/08/Remotely-Dumping-Windows-Security-Questions-With-Impacket.html

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Only tested against windows 10 and windows 11.
Ran e2e tests

Screenshots

image
example output shows <Username> - <Question>: <Answer> for each user

Checklist

  • 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 and existing e2e tests pass locally with my changes
  • My code follows the style guidelines of this project (should be covered by Ruff above)
  • If reliant on 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: add docs for security-questions module NetExec-Wiki#11)

@Adamkadaban Adamkadaban mentioned this pull request May 8, 2024
Merged
@NeffIsBack NeffIsBack added this to the v1.3.0 milestone May 8, 2024
@Marshall-Hallenbeck
Copy link
Collaborator

@Adamkadaban did you run Ruff against this? If you are using poetry: poetry run python -m ruff check . --preview

@Adamkadaban
Copy link
Contributor Author

did you run Ruff against this?

@Marshall-Hallenbeck Oh sorry, I didn't - but I have now. made the necessary changes and all checks are passing

@Dfte
Copy link
Contributor

Dfte commented May 12, 2024

Quick question but since these informations are dumped calling the SAMR RPC endpoint, can't we simply add this module in the code --sam option ?

@Adamkadaban
Copy link
Contributor Author

Adamkadaban commented May 12, 2024 •
edited
Loading

@Dfte I considered this too. I would say yes, but afaik --sam currently only tries to dump passwords in the sam database.
There are a ton of things we can dump with samr that currently aren't being dumped, so I assumed the best option would be a module

Adamkadaban added a commit to Adamkadaban/NetExec-Wiki that referenced this pull request May 12, 2024
@Adamkadaban
add docs for Pennyw0rth/NetExec#295
6424fc7
@Adamkadaban Adamkadaban mentioned this pull request May 12, 2024
Open
NeffIsBack
NeffIsBack requested changes Jun 1, 2024
View reviewed changes
Copy link
Contributor

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what is happening here, but when i swap out the print() for the exception logging this is the traceback:
image

nxc/modules/security-questions.py Outdated Show resolved Hide resolved
@Adamkadaban
Copy link
Contributor Author

@NeffIsBack

Not sure what is happening here, but when i swap out the print() for the exception logging this is the traceback:

This looks like the very verbose logging that impacket provides when an information class doesn't exist.

I think this means your impacket is outdated

@NeffIsBack
Copy link
Contributor

@NeffIsBack

Not sure what is happening here, but when i swap out the print() for the exception logging this is the traceback:

This looks like the very verbose logging that impacket provides when an information class doesn't exist.

I think this means your impacket is outdated

Oh yes, it's late, missed the fortra PR notice. Gonna update impacket and try again👍🏼

@NeffIsBack
Copy link
Contributor

Hmm okay so with impacket being updated i get an error on the dc and no output at all on the win11 machine. There should be a local account set up with sec questions.
image
image

The DC debug output:
image

The Win11 debug output (looks like its running fine, but doesn't find anything?):
image

Marshall-Hallenbeck
Marshall-Hallenbeck requested changes Jun 18, 2024
View reviewed changes
Copy link
Collaborator

@Marshall-Hallenbeck Marshall-Hallenbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, just requesting some variable renames so it's Python3-Pythonic.

nxc/modules/security-questions.py
self.getSAMRResetInfo(context)

def getSAMRResetInfo(self, context):
stringbinding = f"ncacn_np:{self.__targetIp}[\\pipe\\samr]"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be "string_binding"

nxc/modules/security-questions.py

def getSAMRResetInfo(self, context):
stringbinding = f"ncacn_np:{self.__targetIp}[\\pipe\\samr]"
rpctransport = transport.DCERPCTransportFactory(stringbinding)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to "rpc_transport"

nxc/modules/security-questions.py

# obtain server handle for samr connection
resp = samr.hSamrConnect(dce)
serverHandle = resp["ServerHandle"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to "server_handle"

nxc/modules/security-questions.py

# obtain domain handle for samr connection
resp = samr.hSamrOpenDomain(dce, serverHandle=serverHandle, domainId=resp["DomainId"])
domainHandle = resp["DomainHandle"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to "domain_handle"

nxc/modules/security-questions.py
domainHandle = resp["DomainHandle"]

status = STATUS_MORE_ENTRIES
enumerationContext = 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to "enumeration_context"

nxc/modules/security-questions.py
r = samr.hSamrOpenUser(dce, domainHandle, samr.MAXIMUM_ALLOWED, user["RelativeId"])
info = samr.hSamrQueryInformationUser2(dce, r["UserHandle"], samr.USER_INFORMATION_CLASS.UserResetInformation)

resetData = info["Buffer"]["Reset"]["ResetData"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to "reset_data"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mpgn mpgn mpgn left review comments

@Marshall-Hallenbeck Marshall-Hallenbeck Marshall-Hallenbeck requested changes

@NeffIsBack NeffIsBack NeffIsBack requested changes

@zblurx zblurx Awaiting requested review from zblurx zblurx is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
new module
Projects
None yet
Milestone
v1.3.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Adamkadaban @Marshall-Hallenbeck @Dfte @NeffIsBack @mpgn