black-box implementation of CryptProtectData/CryptUnprotectData
Mike McCormack
mike at codeweavers.com
Mon Apr 4 01:01:53 CDT 2005
Kees Cook wrote:
> This patch implements a functional replacement for crypt32.dll's
> CryptProtectData and CryptUnprotectData. It does _not_ perform any
> encrypt/decryption, but rather tracks the cipher/entropy/plain triplets
> so that programs depending on the calls will operate correctly.
Perhaps you could make it work "right" by using a key stored in ssh-agent?
> diff -u -p -u -p -r1.92 ChangeLog
Just writing a ChangeLog entry like this is OK:
ChangeLog:
* Added black-box implementation of CryptProtectData/CryptUnprotectData.
You don't need to try patch ChangeLog, because it's going to change alot
as patches are applied. Just write the message you want to put in there.
> + hr = HRESULT_FROM_WIN32(RegOpenKeyExW(HKEY_CURRENT_USER, wszProtectDataMap, 0, KEY_READ, &hkeyMap));
> + if (!SUCCEEDED(hr))
Why do you convert the error code to a HRESULT here? Since you don't do
it elsewhere in your code, why not compare the returned value to
ERROR_SUCCESS, like you do below?
> + if (RegEnumKeyExW(hkeyMap, dwIndex, wszIndexKey, &dwIndexKeyLength, NULL, NULL, NULL, NULL) != ERROR_SUCCESS)
> + break;
Personally, I prefer the following, as it makes the lines shorter, makes
it easier to add a printf("%ld\n",r); and makes the comparison more obvious.
r = RegEnumKeyExW(hkeyMap, ...
if( r != ERROR_SUCCES )
break;
> + if (WINE_TRACE_ON(crypt))
> + wine_dbg_printf("\tChecking Map Index %s\n", debugstr_w(wszIndexKey));
You don't need the WINE_TRACE_ON() check, because TRACE already does
that for the default debug channel, so the following is the same:
TRACE("\tChecking Map Index %s\n", debugstr_w(wszIndexKey);
> + if (!bFound) {
> + TRACE("no matches\n");
> + }
Sometimes you used K&R style brackets and indenting, sometimes you used
ANSI C style. It's better to choose one or the other and stick to it.
> + return SUCCEEDED(RegSetValueExW(hkeyOpen,wszName,0,dwType,
> + pData.pbData,pData.cbData));
The SUCCEEDED() macro is only for HRESULT values, so the above is going
to succeed in alot of cases where it shouldn't.
> + }
> +
> + DWORD dwDisposition;
In an effort to maintain portability, we don't use C99 style variable
declarations.
Mike
More information about the wine-devel
mailing list