Kees Cook kees@outflux.net writes:
ChangeLog: Black-box implementation of CryptProtectData/CryptUnprotectData
This is a resend, since it looks like current patches are making their way into CVS now. :) It was reviewed last week by several people, and includes docs, tests, etc.
I don't understand while you come up with such an elaborate scheme of storing things in the registry when it's clearly not the way this thing is supposed to work. If you can't figure out what Windows does, then just xoring the data with 0xdeadbeef or something like this would be at least as secure as your solution, and would actually be much closer to the proper behavior.
On Wednesday 13 April 2005 12:16, Alexandre Julliard wrote:
I don't understand while you come up with such an elaborate scheme of storing things in the registry when it's clearly not the way this thing is supposed to work. If you can't figure out what Windows does, then just xoring the data with 0xdeadbeef or something like this would be at least as secure as your solution, and would actually be much closer to the proper behavior.
What Windows does is described in detail at: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnsecure/ht...
Besides the complicated (and in my opinion braindead) procedure for key backup and restauration, it all basically melts down to the following (simplified):
When a user logs in to the system a hash of his password is computed. This hash is kept in memory in a system service called "Local Security Authority". CryptProtectData/CryptUnprotectData asks the LSA via RPC to en/de-crypt data using this hash as a symmetric key on behalf of the user.
Since the unix/linux login process doesn't provide such a functionality, this is not easy to replicate. I like Mike McCormacks' idea to apply "ssh-agent", but I didn't look into it in detail.
Bye, -- Michael Jung mjung@iss.tu-darmstadt.de
On Wed, Apr 13, 2005 at 12:16:44PM +0200, Alexandre Julliard wrote:
I don't understand while you come up with such an elaborate scheme of storing things in the registry when it's clearly not the way this thing is supposed to work. If you can't figure out what Windows does, then just xoring the data with 0xdeadbeef or something like this would be at least as secure as your solution, and would actually be much closer to the proper behavior.
Mostly I did this because there is some optional data (description, entropy). I didn't want to have to invent a data format to store all of that in, so I used the registry to do it instead.
Another reason I did it this way was so that it was easily to examine and change the information getting passed back from the Crypt*Data functions. But I suppose, I can just use FIXME's for this.
I don't like the ssh-agent idea because not everyone uses ssh-agent. If inventing a data format and XORing stuff is prefered, I can write it that way.
What direction should I take this?
Kees Cook kees@outflux.net writes:
I don't like the ssh-agent idea because not everyone uses ssh-agent. If inventing a data format and XORing stuff is prefered, I can write it that way.
What direction should I take this?
You should do this as close to Windows as possible, so that it's easier to adapt it to work correctly later on. If you do everything right except you replace the encryption step by a dummy XOR, then it's obvious how to fix it. With the registry approach, if someone wants to fix it they first have to rip out all the code and restart from scratch; that makes it much less likely that it ever will get fixed.
On Thu, Apr 14, 2005 at 03:44:34PM +0200, Alexandre Julliard wrote:
I don't like the ssh-agent idea because not everyone uses ssh-agent. If inventing a data format and XORing stuff is prefered, I can write it that way.
What direction should I take this?
You should do this as close to Windows as possible, so that it's easier to adapt it to work correctly later on. If you do everything right except you replace the encryption step by a dummy XOR, then it's obvious how to fix it. With the registry approach, if someone wants to fix it they first have to rip out all the code and restart from scratch; that makes it much less likely that it ever will get fixed.
I'd really like to get my Crypt*Protect data patches in, so I want to make sure that I do this rewrite in a way that'll bet accepted. If I understand correctly, you want me to:
- parse the Windows data format as best I can - produce output that looks like the Windows data format - do some kind of encryption on the data so that nothing needs to be stored to the computer between calls of CryptProtectData and CryptUnprotectData. (The existing patches intentionally avoid any encryption.)
From looking at Wine's configure.ac, it seems safe to depend on openssl
being available. Is that correct?
Hi Kees,
It seems to me that there is some misunderstanding involved here. I'll pick some comments from your previous posts and comment on them.
On Wednesday 13 April 2005 17:51, Kees Cook wrote:
Mostly I did this because there is some optional data (description, entropy). I didn't want to have to invent a data format to store all of that in, so I used the registry to do it instead.
You don't have to store this data. Actually it would be quite a bad idea to do so, for security reasons. What windows does conceptionally, is to compute a new key based on the following parameters: A hash of the user's login password, the description and the entropy. The client provided DATA_BLOB is then encrypted given this key and passed back to the user. It is not stored anywhere. In fact AFAIK, the Crypt(Un)ProtectData functions do not store anything whatsoever. The caller is responsible to store the encrypted DATA_BLOB somewhere. He also has to be able to restore in some way the entropy and the description, if he wants to decrypt the DATA_BLOB at some later time.
In my opinion, the Crypt(Un)ProtectData APIs should basically be implemented as no-ops at the moment (IMHO XOR-ing with some magic value is senseless in an open source project. I think it's not a good idea, since it gives the impression of security, where there is none). As far as I understand, passing back the input DATA_BLOB just as it was given by the caller should work just fine. A FIXME on the command line telling the user that his data is not actually protected would be appropriate.
- parse the Windows data format as best I can
DATA_BLOB's are opaque by nature. Applications should not expect anything of the format. So there is no benefit in trying to mimic the Windows data format. (Sometimes MSDN states that a format should be considered opaque, while certain components of Windows don't treat it this way. In these cases it's necessary to mimic the native binary format. As far as I know, that's not the case here.)
- produce output that looks like the Windows data format
Same as above.
- do some kind of encryption on the data so that nothing needs to be
stored to the computer between calls of CryptProtectData and CryptUnprotectData. (The existing patches intentionally avoid any encryption.
Like already said, we don't have access to a hash of the user's login password, so we can't provide real security here. Therefore I think we should not try to pretend it. IMO you should'nt do any encryption. Just pass back the DATA_BLOB.
That said, if the caller actually provides the pszDataDescription and pOptionalEntropy parameters, you could derive a key from those. The CryptoAPI is accessible by the Crypt* family of API's in advapi32.dll. All that is necessary for this to implement should be available in wine already.
Bye,
Michael Jung mjung@iss.tu-darmstadt.de writes:
In my opinion, the Crypt(Un)ProtectData APIs should basically be implemented as no-ops at the moment (IMHO XOR-ing with some magic value is senseless in an open source project. I think it's not a good idea, since it gives the impression of security, where there is none). As far as I understand, passing back the input DATA_BLOB just as it was given by the caller should work just fine. A FIXME on the command line telling the user that his data is not actually protected would be appropriate.
The idea of using a XOR is not to provide any security, it's just to make sure the code goes through the proper motions so that plugging in real encryption later on is easier. If you simply pass the data through you are not really exercising the functionality. Of course it would be even better to do true encryption with a hardcoded key; it still doesn't provide any security, but it's much closer to the desired end result, which makes it more likely that someone will be able to plug in the missing step.
Hi Kees,
On Wednesday 04 May 2005 12:43, Alexandre Julliard wrote:
Of course it would be even better to do true encryption with a hardcoded key; it still doesn't provide any security, but it's much closer to the desired end result, which makes it more likely that someone will be able to plug in the missing step.
Ok, I see.
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/seccrypto/s...
gives a pretty good introduction on how to derive a key from a password using CryptoAPI. You should hash the following: 1.) A placeholder for the user's login password, 2.) the pszDescription parameter (if present) and 3.) the pEntropy (if present).
If you apply the user's login name as the placeholder for 1.) you are even closer to Windows in the sense that one user can't decrypt another users DATA_BLOB's. (Well, he can of course. But only with some hacking involved.)
Bye,
On Wed, May 04, 2005 at 10:38:40AM +0200, Michael Jung wrote:
DATA_BLOB somewhere. He also has to be able to restore in some way the entropy and the description, if he wants to decrypt the DATA_BLOB at some later time.
Actually, the description is returned by CryptUnprotectData (and is stored in the clear in the "opaque" blob).
DATA_BLOB's are opaque by nature. Applications should not expect anything of the format. So there is no benefit in trying to mimic the Windows data format. (Sometimes MSDN states that a format should be considered opaque,
Well, no, applications don't care, but Wine's implementation (and future implementation) of Crypt*protectData should attempt a guess at it. This is what I've got so far:
/* * The data format returned by CryptProtectData seems to be something like:
DWORD count0; - how many "info0_*[16]" blocks follow (was always 1) BYTE info0_0[16]; - unknown information (was the same between calls) ... DWORD count1; - how many "info1_*[16]" blocks follow (was always 1) BYTE info1_0[16]; - unknown information (different between calls) ... DWORD null0; - NULL "end of records"? DWORD str_len; - length of WCHAR string including term WCHAR str[str_len]; - The "dataDescription" value DWORD unknown0; - unknown value (seems large, but only WORD large, same) DWORD unknown1; - unknown value (seems small, less than a BYTE, same) DWORD data_len; - length of data (was 16 bytes) md5? BYTE data[data_len]; - unknown data (fingerprint? changes) DWORD null1; - NULL ? (same) DWORD unknown2; - unknown value (seems large, but only WORD large, same) DWORD unknown3; - unknown value (seems small, less than a BYTE, same) DWORD salt_len; - salt length(?) - was 16 bytes==128b md5? BYTE salt[salt_len]; - salt for symmetric encryption (changes) DWORD cipher_len; - length of cipher(?) data - was close to plain len, symmetric block ciphers like 3DES are in 40 byte chunks, I think BYTE cipher[cipher_len]; - cipher text? (changes) DWORD fingerprint_len; - length of fingerprint(?) data BYTE fingerprint[fingerprint_len]; - SHA1 fingerprint(?) (20 bytes, changes) */
Like already said, we don't have access to a hash of the user's login password, so we can't provide real security here. Therefore I think we should not try to pretend it. IMO you should'nt do any encryption. Just pass back the DATA_BLOB.
I think what I'm going to do is use (for now) the uid appended to a string that includes the time. Maybe md5 it for fun. I really don't know yet. (Before encryption, though, I have to add on the entropy.) Something like: sprintf(salt,"Wine:%u:%u:",uid,time());
is accessible by the Crypt* family of API's in advapi32.dll. All that is necessary for this to implement should be available in wine already.
Cool, that's good news. I haven't had the chance to review that stuff yet.
-
Alexandre Julliard -
Kees Cook -
Michael Jung