Imort system certificates in crypt32 at Wine
Donat Enikeev
donat at enikeev.net
Fri Oct 28 10:57:53 CDT 2016
Hi Jacek,
I've applied your recommendations and submitted new version to
@wine-patches
> Also, it seems that we don't really have tests using root store. It
> would be nice to have some.
Generic test for this and other common stores are submitted as well, it
took slightly more time, as appeared that certs for HKCU\MY are not
saved in the registry, but in %AppData%, even though it is explicitly
declared to use registry physical store.
However, this test doesn't cover expressing of system certs into
registry, as I hasn't figured out yet, how to do that in a portable
way, so that tests are reliably working in
a) the read-only context,
b) across distributions and
c) in the future, considering certificates revocation and expiration
(ftr I have 173 certificates in /etc/ssl/certs/ca-certificates.crt
shipped with distribution, not counting another hundreds of Mozilla)
Re-implementing certs import from scratch for the reference, so tests
are well-independent, would have taken notable amount of time, while
putting hacks into the code for the test-purposes only - looks
redundantly (and such patches will not be accepted, I suppose :). I am
still thinking on this, but probably you would have (or already have)
ideas how to do that in more elegant way
Looking forward
Best,
Donnie
On Чт, окт 27, 2016 at 2:15 , Jacek Caban <jacek at codeweavers.com>
wrote:
> Hi Donnie,
>
> On 26.10.2016 21:29, Donat Enikeev wrote:
>> Hi Jacek,
>>
>> Following your proposal over the previous patch, that it should be
>> a step forward to the implementation closer to the original
>> behaviour,
>>
>> I've prepared different version of patch:
>>
>> 1. sligthly changed existing functions saving certs to the registry
>> (to be able to pass the REG_OPTION_VOLATILE flag)
>> 2. replaced the RootStore entity and everything related with new
>> CRYPT_ImportSystemRootCertsToReg() that does actual import (once)
>> into REG_OPTION_VOLATILE keys
>> 3. start calling this import whenever an application is reaching
>> necessity to work with HKLM\Root certs store as previously, however
>> the latter call could be moved to the DllMain
>>
>> Does it look better?
>
> Yes, it looks better. I have a few comments from a quick review.
>
>> static const char * const CRYPT_knownLocations[] = {
>> "/etc/ssl/certs/ca-certificates.crt",
>> "/etc/ssl/certs",
>> @@ -492,6 +445,7 @@ static const char * const
>> CRYPT_knownLocations[] = {
>> "/etc/security/cacerts", /* Android */
>> };
>>
>> +/* This cert had been expired on 31.12.1999, shoult we delete it ?
>> */
>> static const BYTE authenticode[] = {
>>
>> 0x30,0x82,0x03,0xd6,0x30,0x82,0x02,0xbe,0xa0,0x03,0x02,0x01,0x02,0x02,0x01,0x01,
>>
>> 0x30,0x0d,0x06,0x09,0x2a,0x86,0x48,0x86,0xf7,0x0d,0x01,0x01,0x04,0x05,0x00,0x30,
>> @@ -555,6 +509,10 @@ static const BYTE authenticode[] = {
>>
>> 0x60,0xdc,0x82,0x1f,0xfb,0xce,0x74,0x06,0x7e,0xd6,0xf1,0xac,0x19,0x6a,0x4f,0x74,
>>
>> 0x5c,0xc5,0x15,0x66,0x31,0x6c,0xc1,0x62,0x71,0x91,0x0f,0x59,0x5b,0x7d,0x2a,0x82,
>> 0x1a,0xdf,0xb1,0xb4,0xd8,0x1d,0x37,0xde,0x0d,0x0f };
>> +
>> +/* Microsoft Root Authority
>> + * Valid to 31.12.2020, please consider update after this date
>> + */
>> static const BYTE rootauthority[] = {
>>
>> 0x30,0x82,0x04,0x12,0x30,0x82,0x02,0xfa,0xa0,0x03,0x02,0x01,0x02,0x02,0x0f,0x00,
>>
>> 0xc1,0x00,0x8b,0x3c,0x3c,0x88,0x11,0xd1,0x3e,0xf6,0x63,0xec,0xdf,0x40,0x30,0x0d,
>> @@ -622,6 +580,10 @@ static const BYTE rootauthority[] = {
>>
>> 0x04,0xe4,0xa8,0x45,0x04,0xc8,0x5a,0x33,0x38,0x6e,0x4d,0x1c,0x0d,0x62,0xb7,0x0a,
>>
>> 0xa2,0x8c,0xd3,0xd5,0x54,0x3f,0x46,0xcd,0x1c,0x55,0xa6,0x70,0xdb,0x12,0x3a,0x87,
>> 0x93,0x75,0x9f,0xa7,0xd2,0xa0 };
>> +
>> +/* Microsoft Root Authority Certificate
>> + * Valid to 09.05.2021, please consider update after this date
>> + */
>
> Please avoid unrelated changes in part of code that you don't touch.
> The
> patch changes enough places already.
>
>> static const BYTE rootcertauthority[] = {
>>
>> 0x30,0x82,0x05,0x99,0x30,0x82,0x03,0x81,0xa0,0x03,0x02,0x01,0x02,0x02,0x10,0x79,
>>
>> 0xad,0x16,0xa1,0x4a,0xa0,0xa5,0xad,0x4c,0x73,0x58,0xf4,0x07,0x13,0x2e,0x65,0x30,
>> @@ -790,55 +752,48 @@ static void
>> read_trusted_roots_from_known_locations(HCERTSTORE store)
>>
>> static HCERTSTORE create_root_store(void)
>> {
>> - HCERTSTORE root = NULL;
>> HCERTSTORE memStore = CertOpenStore(CERT_STORE_PROV_MEMORY,
>> X509_ASN_ENCODING, 0, CERT_STORE_CREATE_NEW_FLAG, NULL);
>>
>> if (memStore)
>> {
>> - CERT_STORE_PROV_INFO provInfo = {
>> - sizeof(CERT_STORE_PROV_INFO),
>> - sizeof(rootProvFuncs) / sizeof(rootProvFuncs[0]),
>> - rootProvFuncs,
>> - NULL,
>> - 0,
>> - NULL
>> - };
>> -
>> read_trusted_roots_from_known_locations(memStore);
>> add_ms_root_certs(memStore);
>> - root = CRYPT_ProvCreateStore(0, memStore, &provInfo);
>> }
>> - TRACE("returning %p\n", root);
>> - return root;
>> + TRACE("returning %p\n", memStore);
>> + return memStore;
>> }
>>
>> -static WINECRYPT_CERTSTORE *CRYPT_rootStore;
>> +static LONG root_certs_imported = 0;
>>
>> -WINECRYPT_CERTSTORE *CRYPT_RootOpenStore(HCRYPTPROV hCryptProv,
>> DWORD dwFlags)
>> +static const WCHAR certs_root_path[] =
>> +
>> {'S','o','f','t','w','a','r','e','\\','M','i','c','r','o','s','o','f','t','\\',
>> +
>> 'S','y','s','t','e','m','C','e','r','t','i','f','i','c','a','t','e','s','\\',
>> + 'R','o','o','t','\\',
>> 'C','e','r','t','i','f','i','c','a','t','e','s', 0};
>> +
>> +BOOL CRYPT_ImportSystemRootCertsToReg(void)
>> {
>> - TRACE("(%ld, %08x)\n", hCryptProv, dwFlags);
>> + HCERTSTORE store = NULL;
>> + HKEY key;
>> + BOOL ret = FALSE;
>> + LONG rc;
>>
>> - if (dwFlags & CERT_STORE_DELETE_FLAG)
>> - {
>> - WARN("root store can't be deleted\n");
>> - SetLastError(ERROR_ACCESS_DENIED);
>> - return NULL;
>> - }
>> - if (!CRYPT_rootStore)
>> - {
>> - HCERTSTORE root = create_root_store();
>> + TRACE("\n");
>> +
>> + if (root_certs_imported) return FALSE;
>
> To guarantee that it's called only once per process, INIT_ONCE would
> be
> nicer. However, in this case, we'd like to have it done only once per
> wineserver session. For that CreateMutex seems like a good choice.
>
> Also, there is no need to return BOOL if since return value is not
> used
> anyway.
>
> Also, it seems that we don't really have tests using root store. It
> would be nice to have some.
>
> Thanks,
> Jacek
More information about the wine-devel
mailing list