http://bugs.winehq.org/show_bug.cgi?id=20340
Summary: CryptImportKey fails chromium's base_unittests.exe in HMACTest.* Product: Wine Version: 1.1.31 Platform: PC OS/Version: Linux Status: NEW Keywords: download, source, testcase Severity: normal Priority: P2 Component: crypt32 AssignedTo: wine-bugs@winehq.org ReportedBy: dank@kegel.com
HMACTest.HmacSafeBrowsingResponseTest now passes with the patch Dmitry sent, but the other two test cases in The other two test cases in http://src.chromium.org/viewvc/chrome/trunk/src/base/hmac_unittest.cc still fail. They also appear to be CryptImportKey tests. The test cases are probably about as easy to extract as HmacSafeBrowsingResponseTest was, just read http://src.chromium.org/viewvc/chrome/trunk/src/base/hmac_win.cc and http://src.chromium.org/viewvc/chrome/trunk/src/base/hmac.h and write the same thing in C in wine test idiom. (Licensing on that code is Apache, so if you copy more than a trivial number of lines you'd need to insert a reference to the license header http://src.chromium.org/viewvc/chrome/trunk/src/LICENSE.)
http://bugs.winehq.org/show_bug.cgi?id=20340
Juan Lang juan_lang@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |juan_lang@yahoo.com
--- Comment #1 from Juan Lang juan_lang@yahoo.com 2009-10-20 11:27:27 --- Is this still an issue with today's Wine? I'm curious whether the default provider had anything to do with it.
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #2 from Dan Kegel dank@kegel.com 2009-10-20 11:31:15 --- I'll try it soon (might take me a while, I have to take the bus to work today). In the meantime, you can run it yourself with something like wget http://kegel.com/wine/chromium/chromium-tests.tar.bz2 tar -xjvf chromium-tests.tar.bz2 cd src/chrome/Debug wine base_unittests.exe --gtest_filter=HMACTest.*
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #3 from Juan Lang juan_lang@yahoo.com 2009-10-20 17:39:03 --- It still fails for me with today's Wine. From a +crypt log: trace:crypt:CryptImportKey (0x1528d0, 0xca7bb8, 32, 0x0, 00000100, 0xca7b88) trace:crypt:RSAENH_CPImportKey (hProv=00000001, pbData=0xca7bb8, dwDataLen=32, hPubKey=00000000, dwFlags=00000100, phKey=0x152ac4) trace:crypt:import_key blob type: 8 trace:crypt:new_key alg = "RC2", dwKeyLen = 160 trace:crypt:new_key key len 160 out of bounds (40, 128)
This is from these lines in hmac_win.cc: if (!CryptImportKey(plat_->provider_, &key_blob_storage[0], key_blob_storage.size(), 0, CRYPT_IPSEC_HMAC_KEY, &plat_->hkey_)) { NOTREACHED();
Wine's enhanced provider is restricted to 128 bits, and it ignores the CRYPT_IPSEC_HMAC_KEY flag. MSDN states that passing CRYPT_IPSEC_HMAC_KEY: "Allows for the import of an RC2 key that is larger than 16 bytes. If this flag is not set, calls to the CryptImportKey function with RC2 keys that are greater than 16 bytes fail, and a call to GetLastError will return NTE_BAD_DATA."
More tests needed, of course.
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #4 from Juan Lang juan_lang@yahoo.com 2009-10-21 16:20:38 --- With current Wine, the failures are: [ RUN ] HMACTest.RFC2202TestCases [8:9:1021/141210:491:FATAL:hmac_win.cc(70)] Check failed: false.
and: [ RUN ] HMACTest.HMACObjectReuse [8:9:1021/141211:1254:FATAL:hmac_win.cc(70)] Check failed: false.
Those failures are on the following line: if (!CryptImportKey(plat_->provider_, &key_blob_storage[0], key_blob_storage.size(), 0, CRYPT_IPSEC_HMAC_KEY, &plat_->hkey_)) { NOTREACHED();
That is, the key isn't imported. From a +crypt trace: trace:crypt:CryptImportKey (0x1528c8, 0xca7bb8, 32, 0x0, 00000100, 0xca7b88) trace:crypt:RSAENH_CPImportKey (hProv=00000001, pbData=0xca7bb8, dwDataLen=32, hPubKey=00000000, dwFlags=00000100, phKey=0x152abc) trace:crypt:import_key blob type: 8 trace:crypt:new_key alg = "RC2", dwKeyLen = 160 trace:crypt:new_key key len 160 out of bounds (5, 128)
That is, even the current key length limit of 128 is insufficient to make the app happy. Applying the following hacky patch to rsaenh: diff --git a/dlls/rsaenh/rsaenh.c b/dlls/rsaenh/rsaenh.c index 1b23391..de6da03 100644 --- a/dlls/rsaenh/rsaenh.c +++ b/dlls/rsaenh/rsaenh.c @@ -159,7 +159,7 @@ typedef struct tagKEYCONTAINER static const PROV_ENUMALGS_EX aProvEnumAlgsEx[5][RSAENH_MAX_ENUMALGS+1] = { { - {CALG_RC2, 40, 5, 128,0, 4,"RC2", 24,"RSA Data + {CALG_RC2, 40, 5, 640,0, 4,"RC2", 24,"RSA Data {CALG_RC4, 40, 40, 56,0, 4,"RC4", 24,"RSA Data {CALG_DES, 56, 56, 56,0, 4,"DES", 31,"Data Enc {CALG_SHA, 160,160, 160,CRYPT_FLAG_SIGNING, 6,"SHA-1", 30,"Secure H
results in the following sorts of errors instead: [ RUN ] HMACTest.RFC2202TestCases .\hmac_unittest.cc(129): error: Value of: 0 Expected: memcmp(cases[i].digest, digest, kDigestSize) Which is: 1
That is, just adjusting the key length limit results in the hash producing bad values. I believe the problem is the lack of HMAC support in rsaenh. libtomcrypt, on which rsaenh was built, has an HMAC implementation, but it wasn't included as part of rsaenh.
I believe reverting Dmitry's patch is more correct than leaving it in, and I sent some patches to do so. This one's going to need more work.
http://bugs.winehq.org/show_bug.cgi?id=20340
Juan Lang juan_lang@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends on| |20337
--- Comment #5 from Juan Lang juan_lang@yahoo.com 2009-10-21 16:23:42 --- Adding dependency on (currently "fixed") bug.
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #6 from Juan Lang juan_lang@yahoo.com 2009-10-21 17:01:13 --- Oops, I misspoke when I said rsaenh doesn't support HMAC. It does (see free_hmac_info, copy_hmac_info, etc. in rsaenh.c), but I'd guess CryptImportKey needs to use the HMAC code when passed the CRYPT_IPSEC_HMAC_KEY flag.
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #7 from Juan Lang juan_lang@yahoo.com 2009-10-21 17:16:01 --- Here's a little more from a +crypt log: trace:crypt:CryptImportKey (0x1528c8, 0xca8270, 28, 0x0, 00000100, 0xca67d8) trace:crypt:RSAENH_CPImportKey (hProv=00000001, pbData=0xca8270, dwDataLen=28, hPubKey=00000000, dwFlags=00000100, phKey=0x152abc) trace:crypt:import_key blob type: 8 trace:crypt:new_key alg = "RC2", dwKeyLen = 128 trace:crypt:CryptCreateHash (0x1528c8, 0x8009, 0x152ab8, 00000000, 0xca67d4)
x8009 is CALG_HMAC in CryptCreateHash.
The blob type is 8, which is PLAINTEXTKEYBLOB. This code in rsaenh.c's import_plaintext_key is incorrect when CRYPT_IPSEC_HMAC_KEY is passed: memcpy(pCryptKey->abKeyValue, pbKeyStream, *pKeyLen);
The reason is abKeyValue is declared as: BYTE abKeyValue[RSAENH_MAX_KEY_SIZE];
and RSAENH_MAX_KEY_SIZE is: #define RSAENH_MAX_KEY_SIZE 48
This is fine for the current key length of 128 bits, but not for the largest the app passes (640 = 80 bytes). Testing on Windows showed that there is no key length limit when CRYPT_IPSEC_HMAC_KEY is passed, so I think CRYPT_IPSEC_HMAC_KEY implies the data are passed to an HMAC stream, rather than imported directly.
http://bugs.winehq.org/show_bug.cgi?id=20340
Juan Lang juan_lang@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends on|20337 |
--- Comment #8 from Juan Lang juan_lang@yahoo.com 2009-10-22 10:35:47 --- I changed my mind on the dependency. I believe 20337 and 20340 were manifestations of the same bug: Wine doesn't have support for the CRYPT_IPSEC_HMAC_KEY flag. I've described more about the problem here, so I'm going to mark that one as a dup of this.
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #9 from Juan Lang juan_lang@yahoo.com 2009-10-22 10:36:48 --- *** Bug 20337 has been marked as a duplicate of this bug. ***
http://bugs.winehq.org/show_bug.cgi?id=20340
Juan Lang juan_lang@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |4781
http://bugs.winehq.org/show_bug.cgi?id=20340
Juan Lang juan_lang@yahoo.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|crypt32 |rsaenh
--- Comment #10 from Juan Lang juan_lang@yahoo.com 2009-11-03 12:01:35 --- Now that there's an rsaenh component, changing component.
http://bugs.winehq.org/show_bug.cgi?id=20340
Dan Kegel dank@kegel.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Summary|CryptImportKey fails |CryptImportKey |chromium's |CRYPT_IPSEC_HMAC_KEY |base_unittests.exe in |support missing, causes |HMACTest.* |failure in chromium's | |base_unittests.exe in | |HMACTest.*
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #11 from Juan Lang juan_lang@yahoo.com 2011-01-11 18:28:57 CST --- I converted the test cases, which are from RFC 2202 section 3, to unit tests for Wine: http://www.winehq.org/pipermail/wine-patches/2011-January/097765.html
http://bugs.winehq.org/show_bug.cgi?id=20340
--- Comment #12 from Juan Lang juan_lang@yahoo.com 2011-01-13 17:15:48 CST --- Patches sent: http://www.winehq.org/pipermail/wine-patches/2011-January/097817.html http://www.winehq.org/pipermail/wine-patches/2011-January/097818.html
http://bugs.winehq.org/show_bug.cgi?id=20340
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |FIXED
--- Comment #13 from Austin English austinenglish@gmail.com 2011-01-14 12:56:02 CST --- (In reply to comment #12)
Patches sent: http://www.winehq.org/pipermail/wine-patches/2011-January/097817.html http://www.winehq.org/pipermail/wine-patches/2011-January/097818.html
Fixed by: http://source.winehq.org/git/wine.git/?a=commitdiff;h=5ccf2bd9982bc7b7cb8e8c... http://source.winehq.org/git/wine.git/?a=commitdiff;h=c91afb9733b2227935c0ba...
http://bugs.winehq.org/show_bug.cgi?id=20340
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #14 from Alexandre Julliard julliard@winehq.org 2011-01-21 13:44:02 CST --- Closing bugs fixed in 1.3.12.