-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Added CREDHIST support #1564
Conversation
There was a problem hiding this 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
examples/dpapi.py
Outdated
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 |
There was a problem hiding this comment.
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?
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() | ||
|
There was a problem hiding this comment.
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:
print('Cannot decrypt (wrong key or password)') | |
return |
examples/dpapi.py
Outdated
|
||
else: | ||
real_key = None |
There was a problem hiding this comment.
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
else: | |
real_key = None | |
else: |
if chf.credhist_entries_list[self.options.entry].pwdhash is not None: | ||
chf.dump() | ||
return | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
examples/dpapi.py
Outdated
# 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') |
There was a problem hiding this comment.
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
credhist.add_argument('-sid', action='store', help='SID of the user') |
@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. |
Thanks @w0rmh013! I'm going to check it! |
many thanks @w0rmh013 and @0xdeaddood! 🎉 |
* Added CREDHIST support * Added fixes from suggestions
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)