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

Added CREDHIST support #1564

Merged
merged 2 commits into from
Oct 11, 2023
Merged

Added CREDHIST support #1564

merged 2 commits into from
Oct 11, 2023

Conversation

w0rmh013
Copy link
Contributor

@w0rmh013 w0rmh013 commented Jun 3, 2023

Added support in dpapi.py for CREDHIST functions - closes issue #978.

(Also moved user key derivation functions to dpapi.py instead of the examples/dpapi.py)

@anadrianmanrique anadrianmanrique added the medium Medium priority item label Aug 17, 2023
Copy link
Collaborator

@0xdeaddood 0xdeaddood left a comment

Choose a reason for hiding this comment

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

Hi @w0rmh013!
Thanks for the PR! It works great. I have some suggestions before merging it. Please let me know what you think

Comment on lines 483 to 502
keys = []

# Handle key options
if self.options.key and self.options.sid:
key = unhexlify(self.options.key[2:])
keys = deriveKeysFromUserkey(self.options.sid, key)

elif self.options.sid and self.options.key is None:
# Do we have a password?
if self.options.password is None:
# Nope let's ask it
from getpass import getpass
password = getpass("Password:")
else:
password = options.password
keys = deriveKeysFromUser(self.options.sid, password)

else:
chf.dump()
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be good to add a check in case the CREDHIST file is empty. Additionally, I believe we don't need to request the user's SID (we can obtain it from the CREDHIST file, as many tools with this functionality do). What do you think?

Suggested change
keys = []
# Handle key options
if self.options.key and self.options.sid:
key = unhexlify(self.options.key[2:])
keys = deriveKeysFromUserkey(self.options.sid, key)
elif self.options.sid and self.options.key is None:
# Do we have a password?
if self.options.password is None:
# Nope let's ask it
from getpass import getpass
password = getpass("Password:")
else:
password = options.password
keys = deriveKeysFromUser(self.options.sid, password)
else:
chf.dump()
return
if not chf.credhist_entries:
print('The CREDHIST file is empty')
return
# Handle key options
if self.options.key:
key = unhexlify(self.options.key[2:])
keys = deriveKeysFromUserkey(chf.credhist_entries_list[0].sid, key)
# Do we have a password?
elif self.options.password:
keys = deriveKeysFromUser(chf.credhist_entries_list[0].sid, options.password)
else:
chf.dump()
print('Cannot decrypt (specify -key or -password)')
return

# Wrong key
if real_key is None:
chf.dump()

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we get a wrong key, we should display a message:

Suggested change
print('Cannot decrypt (wrong key or password)')
return

Comment on lines 524 to 526

else:
real_key = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

The variable is not used

Suggested change
else:
real_key = None
else:

if chf.credhist_entries_list[self.options.entry].pwdhash is not None:
chf.dump()
return

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change

# A CREDHIST command
credhist = subparsers.add_parser('credhist', help='CREDHIST related functions')
credhist.add_argument('-file', action='store', required=True, help='CREDHIST file')
credhist.add_argument('-sid', action='store', help='SID of the user')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the SID parameter based on the previous changes

Suggested change
credhist.add_argument('-sid', action='store', help='SID of the user')

examples/dpapi.py Outdated Show resolved Hide resolved
@w0rmh013
Copy link
Contributor Author

@0xdeaddood Hi! I've made the requested changes in a new commit.

Slightly modified the password handling since you always want to receive either a password or a key (a password prompt is shown if neither were chosen).

Moreover, changed the output so when you want a specific entry, only that entry will be shown at the end.

@0xdeaddood
Copy link
Collaborator

Thanks @w0rmh013! I'm going to check it!

@gabrielg5 gabrielg5 merged commit 5674780 into fortra:master Oct 11, 2023
9 checks passed
@gabrielg5
Copy link
Collaborator

many thanks @w0rmh013 and @0xdeaddood! 🎉

abbra pushed a commit to abbra/impacket that referenced this pull request Nov 27, 2023
* Added CREDHIST support
* Added fixes from suggestions
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.

4 participants