Added CREDHIST support #1564
Added CREDHIST support #1564
Conversation
There was a problem hiding this comment.
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.
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.
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.
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 | ||
|
|
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.
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)