-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Support for cracking DPAPI masterkey files from XP to Win10 #2521
Conversation
Some feedback,
Overall, the new code is looking great 👍 |
|
Thanks for the review! |
|
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.
Dry review
src/dpapimkv1_fmt_plug.c
Outdated
#define OMP_SCALE 64 | ||
#endif | ||
#endif | ||
#include "memdbg.h" |
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.
nitpick: Please include all system headers, then a blank line, the all local ones.
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.
👍
src/dpapimkv1_fmt_plug.c
Outdated
#ifdef SIMD_COEF_32 | ||
#define ALGORITHM_NAME "SHA1/NTLM PBKDF2-SHA1-DPAPI-variant 3DES " SHA1_ALGORITHM_NAME | ||
#else | ||
#define ALGORITHM_NAME "SHA1/NTLM PBKDF2-SHA1-DPAPI-variant 3DES 32/" ARCH_BITS_STR |
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.
We're supposed to list primitives here, so I believe NTLM should be replaced with MD4.
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.
Also, what exactly does "-DPAPI-variant" mean here? It seems to me a perfectly normal PBKDF2-HMAC-SHA1 is one of the primitives, perhaps a correct string would be just "SHA1/MD4 PBKDF2-SHA1 3DES"
(plus the arch stuff)?
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.
You're right concerning the NTLM part, I'll change that
Concerning the "-DPAPI-variant", actually there is a tweak in MS implementation of PBKDF2 (instead of re-using the same salt during the XORing scheme in the loop as in the RFC, they use the result of the XORing of each iteration... Dunno if it is on purpose or just a misunderstanding of them)
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.
Oh, I think I see now: That's what happens when DPAPI_CRAP_LOGIC
is defined, right? In that case, "PBKDF2-SHA1-DPAPI-variant" is probably the better description.
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.
Yes exactly!
src/dpapimkv1_fmt_plug.c
Outdated
{ | ||
/* Convert key to UTF-16LE (--encoding aware) */ | ||
enc_to_utf16(saved_key[index], PLAINTEXT_LENGTH, (UTF8*)key, strlen(key)); | ||
} |
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'm delighted to see you implement proper encoding support 👍
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.
Haha, actually all the credit goes to @kholia as he did this 👍
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.
Oh, so the credit bounces back to me because I brute forced that into his head over several years 😆
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.
Oh, I see what you did there: smart move to get credits back 😆 😜
|
src/pbkdf2_hmac_sha512.h
Outdated
@@ -108,8 +108,12 @@ static void _pbkdf2_sha512(const unsigned char *S, int SL, int R, uint64_t *out, | |||
SHA512_Update(&ctx, tmp_hash, SHA512_DIGEST_LENGTH); | |||
SHA512_Final(tmp_hash, &ctx); | |||
|
|||
for (j = 0; j < SHA512_DIGEST_LENGTH/sizeof(uint64_t); j++) | |||
for (j = 0; j < SHA512_DIGEST_LENGTH/sizeof(uint64_t); j++){ |
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.
Please add one space before the curly bracket (twice in this file)
Ok, now the main remarks should be addressed. Please check, I may have forgotten some things. Remaining before merge:
Am I missing something? |
src/dpapimk_fmt_plug.c
Outdated
|
||
#include <string.h> | ||
#include <assert.h> | ||
#include <errno.h> |
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.
Both <assert.h> and <errno.h> includes can be removed.
src/dpapimk_fmt_plug.c
Outdated
#define KEY_LEN1 24 | ||
#define IV_LEN1 8 | ||
#define DIGEST_LEN1 20 | ||
|
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.
Nitpick: The values of these define statements can be aligned to match the existing alignment. It might look better :-)
src/dpapimk_fmt_plug.c
Outdated
}; | ||
|
||
|
||
|
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.
Tighten line spacing (i.e. remove extra newlines).
src/dpapimk_fmt_plug.c
Outdated
static char *ptrSID; | ||
|
||
memset(&cs, 0, sizeof(cs)); | ||
ctcopy += FORMAT_TAG_LEN; /* skip over "$DPAPImkv2$*" */ |
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.
Comment needs to be updated to reflect the new format tag.
src/dpapimk_fmt_plug.c
Outdated
return (void *)&cs; | ||
} | ||
|
||
|
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.
Tighten line spacing (i.e. remove extra newlines).
src/dpapimk_fmt_plug.c
Outdated
SHA_CTX ctx; | ||
MD4_CTX ctx2; | ||
|
||
int i; |
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.
Tighten line spacing. Various variable declarations can be clubbed together without newlines.
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.
All done and merged in previous commit 👍
Ok, now everything should be ready for merge, can you take a look please? Remaining:
Let me know when it is okay for you, I'll merge all commits in the first one in order to keep history clean. |
@Fist0urs This looks ready to merge now. Go ahead with the squashing process. Once done, I will happily merge this PR 👍 |
The wiki and documentation stuff can be done afterwards for sure. |
Squash done, thank you very much for your time guys! 👍 |
Thanks 👍 |
Unfortunately, this broke backwards compatibility to 1.8.0-jumbo-1 release. Looks like --format=efs is gone, and hashes like these are not considered valid anymore:
Is it possible to consider the old hashes as valid, and just convert the hashes in split()? Do we need a new document describing format name changes, and how to convert the old hashes? Are there other formats affected as well? I didn't really check. |
Hi @frank-dittrich, I'll add the backward compatibility in order to support older versions of this hashformat. Cheers! |
Summary
tl;dr: DPAPI is an API offered to Windows developpers by Microsoft to protect some data (password, certificates, data, etc.). DPAPI data protection relies on the currently logged on user's password. In order to encrypt/decrypt data, Windows derivates the user's password to unprotect "masterkey files", then extract a symetric master key from them and uses it to proceed further encryption/decryption.
DPAPI is massively used (Chrome, Skype, Dropbox, EFS, Credman, IE, etc.).
Cracking DPAPI masterkey file can be useful in different scenarii:
phishing attacks, where all you have is an authenticated access but not the actual credentials
if the workstation is harden and no mscash are stored, trying to break DPAPI masterkeys is an alternative (when a user interactively logs on, his roaming profil is created and his masterkeys are imported on the target workstation)
The support of this algorithm is splitted in 2 separate formats, dpapimkv1 and dpapimkv2 as pbkdf2 routines do not rely on the same hashing algorithm, thus a bit annoying for sse optimization.
Also provided is DPAPImk2john that extract the hash from the masterkey file.
Concerning the commit:
Finally, the format of input hash was defined for possible other algorithm.
This addresses #898
I'll implement these algorithms in hashcat as soon as I have time, using the same input hashing form.
Other Information
Tests have been done using ASAN, compiler was gcc version 6.3.0 and version 5.4.0
Concerning benchmarks:
which is better than I thought regarding the high number of iterations (it can actually change from one operating system version to another and from a workstation to a server version).
Cheers!