Add parsing for 'Export' hives and boot key computation from class names
Summary
This PR enables the parsing of registry hive files exported using regedit.exe or reg.exe export, along with the manual computation of the boot key. This functionality allows secretsdump.py to dump SAM and LSA secrets from these files.
registry-read.py / winregistry.py
The registry files exported via regedit are UTF-16 LE text files, which differ from the binary format produced by reg save. I implemented a parser in winregistry.py to handle these exported text files, and they can now be read using registry-read.py with the -format export option.
Boot key recovery
In secretsdump.py, boot key recovery typically relies on extracting class names from four specific registry keys within the LSA hive. However, the "export" hive format does not contain this metadata, making it impossible to retrieve the class names directly. You can recover the class names using two techniques outlined in the article “Dumping LSA secrets: a story about task decorrelation” written by @Dfte which inspired this PR.
Once you recover the class names, you can compute the bootKey using the following flags:
secretsdump.py
I introduced the -sam-export and -security-export flags to support dumping SAM and LSA secrets from these exported registry files:
MAN THIS IS SO HOOOOOOOOOT !!! Thanks you for that PR!!
Hello @MaxToffy,
Will start checking this PR! Can you solve those conflicts when you have a moment please?
Thanks!
hey hello!
been checking the code and testing a bit this feature.
I've found some differences when running registry-read with both formats. But wanted to analyze and clarify them better before giving you some feedback on that (will share it later or at most, tomorrow, though)
Code wise, there are some comments I do wanna share with you
registry-read
- Perhaps it's better implementing a common interface
Registryand separate parsers (SaveRegistryParserandExportRegistryParser) inheriting from this abstract class- It'd be good defining a factory function, as well, so it's all there how to instantiate one class or the other. In this sense, it can be automatically detected which parser to use (the magic hive "regf" or "Windows Registry Editor" can be used to detect which one). An exception should be triggered if none of those is matched
- In
ExportRegistryParser.get_classI'd trigger an exception clarifying why it could not be obtained
secretsdump
- If going for previous bullets, I'd remove new parameters (
-sam-exportand-system-export) as we can reuse already existing ones
Thanks! will share the other update soon
Hey,
Thanks a lot for this first part of the review! I’ll try to resolve the conflict tomorrow as I think it's quite simple.
From what I remember (since it’s been quite a while since I worked on this), I also noticed some differences between the two formats.
My conclusion back then was that they hold slightly different data, especially for specific data types, and since it didn’t impact secretsdump output, I moved on. Maybe you can find a better explanation.
Regarding the code review, I completely agree with all your points. That said, I probably won’t be able to implement those improvements myself as I’m busy with other projects at the moment.
Thanks again for the the feedback, I’m looking forward to the following review
First time fixing conflicts, so it was kind of a hassle, but it should be good now.
Hey hello!
At the end, those differences I mentioned seem to be missing keys when exporting (instead of saving registry hives); it's not an issue in the registry-read example but in the actual source it is parsing
Something I didn't find quite well is the number of parameters added to secretsdump, to specify the parts to build the bootkey. Was thinking a bit about it, perhaps doing another script that could return the bootkey based in the different available options (whether from the system hive or from these actual 4 values); and one can then use that returned value as input in secretsdump is a clearer option
Was discussing it with other maintainers and didn't reach an agreement
As we are approaching code freeze for a new release, in order to merge these enhancements, can we split this PR into 2 different ones:
- one for the
registry-readchanges (can be merged) - and the other one for the
secretsdumpchanges (to continue discussing these options around its parameters)
Side note for the secretsdump discussion, can be considered similar to regsecrets (at https://github.com/fortra/impacket/pull/1898) in a sense. As it is adding a new way of gathering secrets.
Should we eventually merge all these 3 in a single example? Should we have one example "per technique"?
I checked, and have permissions to edit the PR. So we can move forward however we want. But wanted to get your insights on this before doing anything.
Thanks!
Hey,
Thanks for the clarification on the differences.
Regarding the arguments in secretsdump, I agree with you, and it also bothered me when I first implemented it. I think the best solution would indeed be to separate the bootkey part in another script, as you suggested. For example :
getbootkey.py -jd 'd4a06ce5' -gbg '5ff6ce0a' -skew1 'eda5c80c' -data '9737f8c8'
The only thing I’d suggest is adding some indication for the end user. For example, adding a note to the -system parameter description stating that it doesn’t support the .reg (Registration Entries) format, since it doesn’t contain the metadata required for the bootkey computation.
Similarly, for the -bootkey parameter, we could add something like:
"bootkey for SYSTEM hive (compute manually using
getbootkey.py)."
As for splitting the PR into two, that works for me. Can you handle it on your own, or is there something you’d need me to do?
On your side note about secretsdump, I actually have two takes (not counting regsecrets, since I think it’s good that it stays a separate example).
-
If we keep in mind that it’s technically possible to mix formats in a single command (using one format for
-samand another for-security), then it would make sense to have just one example. -
On the other hand, it’s probably cleaner to make a separate example dedicated to .reg files (and that would also mean the description changes I suggested earlier wouldn’t be needed). The only caveat I can think of is that such an example might not benefit from the updates that are pushed to the
secretsdump.pyexample.
Let me know what you think.
Yes, think I can split the PR from here (I mean, remove changesets from this one and create another one including those)
Agree with adding the clarification for the hive type support in secretsdump
And for the new issue (Adding ,reg support to secresdump), will make sure of linking with this one and including in the description what we've commented here
Hello!
I've implemented commented changes on this PR
- Reverted changes in
secretsdump - Refactored registry parser in winregistry.py (
Registryis an abstract class inherited by bothsaveRegistryParserandexportRegistryParser) - Created a factory function
get_registry_parserthat will return right parser based in the file to parse
Once this is merged in master, will submit a new PR with changesets removed from this one to follow up with what was talked in previous comment
If you have a moment there and can take a look at these last changes @MaxToffy, thanks! I'll route this through another maintainer, though Thanks!
Hey,
Thanks for your implementations, it's much easier now that it uses the same arguments as usual. I tested it and checked the code, and everything looks good to me.
I just made a small edit to the description for the system flag to make it a bit clearer, hope that’s okay.
Thanks again for the work!