Add support for decrypting System DPAPI secrets #305

cobyge · 2023-07-07T21:24:06Z

Closes #274 .

This PR adds support for decrypting system DPAPI secrets.
I have also added support for decrypting Wireguard configuration from DPAPI (As an example).

Questions I have:

Are there any concrete usages for DPAPI decryption you'd like to see implemented? I'd be happy to take a look at integrating them
The current implementation only allows decrypting DPAPI blobs encrypted with the System user. Any other user would require either a password, or a Domain Backup Key. I'm not sure what the right way to accept those would be (Especially since I assume most of the time we don't have the plaintext password, and accepting a different target to extract the Backup Key from (the DC) would be odd). I would like to hear any thoughts about this.
Is the current API decrypt_dpapi_system_blob(blob: bytes) good enough? Should there be a different API? See the Wireguard code for an example usage
Unless I create a way to encrypt DPAPI from python, testing DPAPI blobs in plugins will be pretty annoying. Each plugin author will need to encrypt their data on a Windows machine, then extract their DPAPI secrets, and insert into the test (Look at my test for an example of what info is needed). I'm not sure what the correct way to handle this is.
While this theoretically supports DPAPI from versions earlier than Vista, this does not support the format of Windows Server 2000 and older, and has not been tested on Windows XP 🤷‍♂️. I'm debating just removing the Vista support altogether, but I'm not quite sure if it's needed anywhere.

I'd love to get some feedback on the code, as most of the work I did was the research into DPAPI itself, and I'm not 100% sure that the way I implemented the module inside of Dissect was the correct way.

Added DPAPI support to Wireguard

Schamper · 2023-07-10T09:41:10Z

Wow, awesome work @cobyge! I've assigned myself and @pyrco to review this, but please allow some time for the review as it's obviously quite large!

Schamper

Apologies for taking so long. I have some other thoughts for improvements as well, but if you could address some of these comments first, I'd like to make some commits directly to your branch afterwards if you don't mind!

dissect/target/plugins/os/windows/dpapi/blob.py

dissect/target/plugins/os/windows/dpapi/crypto.py

dissect/target/plugins/os/windows/dpapi/masterkey.py

dissect/target/plugins/os/windows/dpapi/blob.py

tests/data/dpapi/master_keys/d8ef5e00-328a-4919-9ab6-c058f493a6e3

dissect/target/plugins/os/windows/dpapi/types/dpapi.py

cobyge · 2023-08-05T22:26:52Z

Going over the review now, I'm having serious thought about removing pre-vista support, mostly because I can't test it (AFAIK there is no harness for that so I would have to replicate the registry from an old version of windows).
Thoughts about this?

…riable naming convention, and other review changes

cobyge · 2023-08-05T23:13:56Z

Okay, I'm done with the requested changes.
Let me know if I missed something or if you want additional changes.
In regard to your review comment, feel free to push directly to the branch :)

Schamper · 2023-08-07T01:09:00Z

I've pushed some suggested changes. I haven't fully read into the details of DPAPI, so let me know what you think.

Going over the review now, I'm having serious thought about removing pre-vista support, mostly because I can't test it (AFAIK there is no harness for that so I would have to replicate the registry from an old version of windows).
Thoughts about this?

Ideally we also support <Vista down the line, but I'm fine with adding that at a later stage. In some brief Googling I also saw that Win2000 requires some different structures for the master key file, so there's also that.

Regarding the big changes to how you used dissect.cstruct, that mostly comes down to a bit of optimisation + magic reduction. I liked the approach though, and I've long been considering adding a native UUID type to cstruct, as well as a offsetof helper that would help solve the issues you had completely within the syntax. I've already added a special EOF array length indicator in an experimental rewrite branch of dissect.cstruct to easily read until the end of the buffer!

codecov · 2023-08-07T01:09:42Z

Codecov Report

Merging #305 (e5f9124) into main (9adde7e) will increase coverage by 0.18%.
The diff coverage is 84.14%.

@@            Coverage Diff             @@
##             main     #305      +/-   ##
==========================================
+ Coverage   73.28%   73.47%   +0.18%     
==========================================
  Files         245      249       +4     
  Lines       19544    19925     +381     
==========================================
+ Hits        14322    14639     +317     
- Misses       5222     5286      +64

Flag	Coverage Δ
unittests	`73.47% <84.14%> (+0.18%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
dissect/target/plugins/apps/vpns/wireguard.py	`86.53% <62.50%> (-11.29%)`	⬇️
dissect/target/plugins/os/windows/sam.py	`92.55% <66.66%> (-0.72%)`	⬇️
...sect/target/plugins/os/windows/dpapi/master_key.py	`75.00% <75.00%> (ø)`
dissect/target/plugins/os/windows/dpapi/blob.py	`82.69% <82.69%> (ø)`
dissect/target/helpers/hashutil.py	`85.18% <85.71%> (-1.09%)`	⬇️
dissect/target/plugins/os/windows/dpapi/crypto.py	`87.57% <87.57%> (ø)`
dissect/target/plugins/os/windows/dpapi/dpapi.py	`90.32% <90.32%> (ø)`
dissect/target/plugin.py	`90.40% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

cobyge

I've pushed a couple of lines of fixes, and added some questions as comments.

I'm going to push another commit which removes "support" for pre-vista logic

cobyge · 2023-08-08T20:09:45Z

dissect/target/plugins/apps/vpns/wireguard.py

-        config_globs = self.os_config_globs.get(target.os, [])
-        for path in config_globs:
+        self.configs: list[Path] = []
+        for path in self.CONFIG_GLOBS:


Is there a specific reason to attempt to read all files, instead of just the relevant ones?

Mostly readability and ease of expansion. Especially the Unix-y based paths would quickly get hairy and easy to forget when adding new operating system support to Dissect (e.g. BSD). It's a small amount of paths so I feel it's okay to "bruteforce" it.

cobyge · 2023-08-08T20:14:15Z

dissect/target/plugins/os/windows/dpapi/blob.py


-        self.blob = self._blob.blob
+        # All the blob data between the version, provider and sign fields
+        self.blob = data[20 : -(self._blob.signLength + 4)]


I'm slightly worried about this . It's basically hardcoded magic values which (theoretically but super unlikely) could change.
I don't think that it will ever change, so if you prefer this as opposed to another solution, that's fine

Made it more dynamic, does that satisfy? I intend to add a offsetof function to cstruct to make this easier/more concise.

dissect/target/plugins/os/windows/dpapi/crypto.py

dissect/target/plugins/os/windows/dpapi/dpapi.py

Schamper

@cobyge apologies again for the long delay. I've updated your branch and resolved the conflicts. It's good to go from my side, but I'd like a final review from @pyrco, which has been planned into the current sprint.

dissect/target/helpers/hashutil.py

dissect/target/plugins/apps/vpns/wireguard.py

dissect/target/plugins/os/windows/dpapi/crypto.py

Schamper · 2023-09-20T09:03:04Z

@cobyge once again thank you very much for this contribution, and my sincere apologies for the time it has taken. This is one of the best contributions we've gotten so far, and I'm excited to see what you'll bring next 😄

Co-authored-by: Schamper <1254028+Schamper@users.noreply.github.com>

Added support for decrypting System DPAPI secrets

25ecbf5

Added DPAPI support to Wireguard

Schamper requested review from Schamper and pyrco July 7, 2023 22:50

pyrco changed the title ~~Added support for decrypting System DPAPI secrets~~ Add support for decrypting System DPAPI secrets Jul 26, 2023

Schamper requested changes Aug 2, 2023

View reviewed changes

cobyge added 2 commits August 6, 2023 02:06

Refactored crypto.py, removed DES support, refactored to adhere to va…

b20f082

…riable naming convention, and other review changes

Pushed test data to LFS

8b070da

Suggested changes

09138e9

Small fixes

4c26ea8

cobyge commented Aug 8, 2023

View reviewed changes

cobyge requested a review from Schamper August 8, 2023 20:45

cobyge and others added 6 commits August 8, 2023 23:47

Removed pre-vista "support"

6a53acf

Fixed iv access if None

c0813ed

Small fixes

2a3b904

Make blob offset dynamic

2db8006

Remove unused import

8339046

Merge branch 'main' into feature/add_dpapi

66c285e

Schamper previously approved these changes Sep 15, 2023

View reviewed changes

Fix erronous merge

79bca33

Schamper dismissed their stale review via 79bca33 September 15, 2023 13:20

Fix circular import

f160324

pyrco requested changes Sep 19, 2023

View reviewed changes

dissect/target/helpers/hashutil.py Outdated Show resolved Hide resolved

dissect/target/plugins/apps/vpns/wireguard.py Show resolved Hide resolved

dissect/target/plugins/os/windows/dpapi/crypto.py Outdated Show resolved Hide resolved

Address comments

74dd79d

Schamper requested a review from pyrco September 19, 2023 20:09

pyrco approved these changes Sep 20, 2023

View reviewed changes

Schamper approved these changes Sep 20, 2023

View reviewed changes

Merge branch 'main' into feature/add_dpapi

e5f9124

Schamper merged commit 2db8496 into fox-it:main Sep 20, 2023
10 checks passed

cobyge deleted the feature/add_dpapi branch September 20, 2023 17:27

Poeloe pushed a commit that referenced this pull request Feb 29, 2024

Add support for decrypting System DPAPI secrets (#305)

5bce890

Co-authored-by: Schamper <1254028+Schamper@users.noreply.github.com>

Zawadidone pushed a commit to Zawadidone/dissect.target that referenced this pull request Apr 5, 2024

Add support for decrypting System DPAPI secrets (fox-it#305)

fcb0954

Co-authored-by: Schamper <1254028+Schamper@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for decrypting System DPAPI secrets #305

Add support for decrypting System DPAPI secrets #305

cobyge commented Jul 7, 2023

Schamper commented Jul 10, 2023

Schamper left a comment

cobyge commented Aug 5, 2023 •

edited

Loading

cobyge commented Aug 5, 2023

Schamper commented Aug 7, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading

cobyge left a comment

cobyge Aug 8, 2023

Schamper Aug 22, 2023

cobyge Aug 8, 2023

Schamper Aug 22, 2023

Schamper left a comment

Schamper commented Sep 20, 2023

Add support for decrypting System DPAPI secrets #305

Add support for decrypting System DPAPI secrets #305

Conversation

cobyge commented Jul 7, 2023

Schamper commented Jul 10, 2023

Schamper left a comment

Choose a reason for hiding this comment

cobyge commented Aug 5, 2023 • edited Loading

cobyge commented Aug 5, 2023

Schamper commented Aug 7, 2023 • edited Loading

codecov bot commented Aug 7, 2023 • edited Loading

Codecov Report

cobyge left a comment

Choose a reason for hiding this comment

cobyge Aug 8, 2023

Choose a reason for hiding this comment

Schamper Aug 22, 2023

Choose a reason for hiding this comment

cobyge Aug 8, 2023

Choose a reason for hiding this comment

Schamper Aug 22, 2023

Choose a reason for hiding this comment

Schamper left a comment

Choose a reason for hiding this comment

Schamper commented Sep 20, 2023

cobyge commented Aug 5, 2023 •

edited

Loading

Schamper commented Aug 7, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading