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

Quality of life improvements #1439

Merged
merged 10 commits into from
May 3, 2024
Merged

Conversation

RazzburyPi
Copy link
Contributor

Multiple small quality of life improvements including:

ntlmrelayx

  • writes certificates to file rather than outputting b64 to console
  • improved ability to continue relaying to ADCS web enrollment endpoint in order to request multiple certificates for different users

smbclient

  • Added ability to provide an output file that the smbclient mini shell will write commands and output to

secretsdump

  • Added ability to skip dumping of SAM or SECURITY hives when performing remote operations
  • Added ability to specify users to skip when dumping NTDS

@@ -76,7 +76,12 @@ def _run(self):
certificate = response.read().decode()

certificate_store = self.generate_pfx(key, certificate)
LOG.info("Base64 certificate of user %s: \n%s" % (self.username, base64.b64encode(certificate_store).decode()))
LOG.info("Writing certificate to %s.pfx" % self.username)

Choose a reason for hiding this comment

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

Why not to add a try except here to catch possible errors?

Choose a reason for hiding this comment

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

What if file exists already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@666Danger-sudo added try / except block where adcs relay attack will default to original behavior of printing b64 encoded certificate to console if writing to file fails. Also added ability for certificates to be written within lootdir specified in command line

Comment on lines 80 to 90
LOG.info("Writing certificate to %s/%s.pfx" % (self.config.lootdir, self.username))
try:
if not os.path.isdir(self.config.lootdir):
os.mkdir(self.config.lootdir)
f = open("%s/%s.pfx" % (self.config.lootdir, self.username), 'wb')
f.write(certificate_store)
f.close()
LOG.info("Certificate successfully written to file")
except Exception as e:
LOG.info("Unable to write certificate to file, printing B64 of certificate to console instead")
LOG.info("Base64 certificate of user %s: \n%s" % (self.username, base64.b64encode(certificate_store).decode()))
pass

Choose a reason for hiding this comment

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

Please see my pull request #1440. Does the same thing with less code. More easier to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@666Danger-sudo implemented your solution for opening the file to write the certificate.

@anadrianmanrique anadrianmanrique self-assigned this Apr 11, 2024
@anadrianmanrique anadrianmanrique added the in review This issue or pull request is being analyzed label Apr 11, 2024
@anadrianmanrique
Copy link
Contributor

Hi, I'm reviewing this PR. @RazzburyPi could you rebase it, in order to avoid conflicts and then continue with the reviewing process?
Thanks

@RazzburyPi
Copy link
Contributor Author

@anadrianmanrique - Rebased and pushed! Let me know if you need anything else, I appreciate your help!

@anadrianmanrique anadrianmanrique added medium Medium priority item and removed in review This issue or pull request is being analyzed labels Apr 25, 2024
if line[0] != '#':
print("# %s" % line, end=' ')
shell.onecmd(line)
else:
print(line, end=' ')
else:
f = open(options.outputfile, 'a')
Copy link
Contributor

Choose a reason for hiding this comment

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

input and output files aren't mutually exclusive. move these changes outside the else block and check options.outputfile to be active in order to avoid crash when not passing parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anadrianmanrique - Submitted commit moving the output file logic out of the if/else for the input file

@anadrianmanrique anadrianmanrique added the waiting for response Further information is needed from people who opened the issue or pull request label Apr 26, 2024
@anadrianmanrique
Copy link
Contributor

@RazzburyPi changes seemed to work as expected. Please apply the sugested changes in order to merge and close this PR.
Thanks!

@anadrianmanrique anadrianmanrique removed the waiting for response Further information is needed from people who opened the issue or pull request label May 3, 2024
@anadrianmanrique anadrianmanrique merged commit cb8467c into fortra:master May 3, 2024
9 checks passed
@anadrianmanrique
Copy link
Contributor

Thanks for the PR! merging changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Medium priority item
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants