[wix-devs] Issue 4875

[Jason Stephenson]

I'm not sure I fully understand... exactly what do you mean by "Ignore certs in other physical stores"?
Is there any thoughts on this making it into 3x, and if so what kind of time lines am I looking at?
I don't want to pressure an integration (although I believe the fix is correct), i'd just like to manage my PM's expectations.

Regards,
Jason

________________________________
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> on behalf of Sean Hall via wix-devs <wix-devs at lists.wixtoolset.org>
Sent: 25 March 2020 23:13
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Sean Hall <r.sean.hall at gmail.com>
Subject: Re: [wix-devs] Issue 4875

Thanks for looking into this. I was going to request a more granular flag
than CERT_STORE_MAXIMUM_ALLOWED_FLAG, but I see that API doesn't have any
other options. This looks like the correct fix.

In v4, I'm curious if we can change the behavior to target the Default
physical store with the CERT_STORE_PROV_SYSTEM_REGISTRY flag to ignore
certs in the other physical stores.

On Thu, Mar 26, 2020 at 1:51 AM Jason Stephenson via wix-devs <
wix-devs at lists.wixtoolset.org> wrote:

> Hi,
>
> After a week or so of digging I believe I have arrived at a low risk fix
> to https://github.com/wixtoolset/issues/issues/4875.
> I have provided a minimal assessment of the issue on the aforementioned
> link, and previous discussions have occurred here:
> http://windows-installer-xml-wix-toolset.687559.n2.nabble.com/Certificate-install-to-local-machine-fails-with-code-26352-td1121050.html
>
> Summarizing these chains into one conversation, the issue is:
>
>   *   Have a WiX installer that installs a certificate. For example:
>
>        <Component Id="InstallCertificate" Guid="$(var.InstallCertGuid)"
> KeyPath="yes">
>                 <iis:Certificate Id="RootCa" BinaryKey="RootCaBinary"
> Name="My Root CA" StoreLocation="localMachine" StoreName="root"
> Request="no" Overwrite="yes" />
>         </Component>
>
>   *   Have the certificate already present on the machine, in either the
> Enterprise or Group Policy physical stores.  In my case this is because we
> also roll the certificate out via Group Policy, but a manual import can be
> used to reproduce the issue.
>   *   WiX installer will fail. At its lowest level: dutil.lib
> ::CertAddCertificateContextToStore(CERT_STORE_ADD_REPLACE_EXISTING) will
> fail with ACCESS_DENIED.
>
> Previously all conversations centered around changing
> CERT_STORE_ADD_REPLACE_EXISTING to CERT_STORE_ADD_USE_EXISTING. Indeed, I
> too thought this was the correct change, and suggested it the github issue.
> This change does appear to "make things work" in a simple.exe application,
> but the same code fails when executed as part of a CustomAction.
> CERT_STORE_ADD_REPLACE_EXISTING or CERT_STORE_ADD_USE_EXISTING would always
> fail with ACCESS_DENIED.
>
> It is my belief that deferred custom actions are executed with subtly
> different sets of default privileges than the equivalent binary executed by
> SYSTEM (using psExec), which is the crux of this issue.
> More specifically:
>
>   1.  When ::CertOpenStore(CERT_STORE_PROV_SYSTEM, 0, NULL,
> CERT_SYSTEM_STORE_LOCAL_MACHINE, "root") is called from simple.exe:
>      *   CERT_SYSTEM_STORE_LOCAL_MACHINE is opened as RW
>      *   CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY is opened as R
>      *   CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPISE is opened as R
>   2.  When ::CertOpenStore(CERT_STORE_PROV_SYSTEM, 0, NULL,
> CERT_SYSTEM_STORE_LOCAL_MACHINE, "root") is called as part of a differed
> custom action:
>      *   CERT_SYSTEM_STORE_LOCAL_MACHINE is opened as RW
>      *   CERT_SYSTEM_STORE_LOCAL_MACHINE_GROUP_POLICY is not opened or is
> opened with 0 permissions.
>      *   CERT_SYSTEM_STORE_LOCAL_MACHINE_ENTERPISE is not opened or is
> opened with 0 permissions.
>
> This explains why changing from CERT_STORE_ADD_REPLACE_EXISTING to
> CERT_STORE_ADD_USE_EXISTING works for simple.exe, but not for the WiX
> deferred custom action.
> As it turns out, the fix is relatively simple. The ::CertOpenStore call in
> scacertexec.cpp should include the CERT_STORE_MAXIMUM_ALLOWED_FLAG which
> explicitly asks for RW permissions.
>
> I have done some testing with my change and the following behaviors are
> observed:
>
>
>   1.  With no existing certificate, installed certificate is written to
> the Registry physical store. (Existing behavior)
>   2.  With an existing certificate in the Registry physical store,
> installed certificate replaces it in the Registry physical store. (Existing
> behavior)
>   3.  With an existing certificate in the Group Policy physical store,
> installed certificate replaces it in the Group Policy physical store,
> instead of erroring. (New behavior)
>   4.  With an existing certificate in the Enterprise physical store,
> installed certificate replaces it in the Enterprise physical store, instead
> of erroring. (New behavior)
>   5.  With an existing certificate, regardless of which physical store it
> was in, uninstalls will correctly remove it. (New behavior as previously
> installs would fail)
>
> My v4 MR is here : https://github.com/wixtoolset/Iis.wixext/pull/4
> My v3 MR is here : https://github.com/wixtoolset/wix3/pull/511
>
> Should you guys agree on my analysis and conclusions it is my hope that
> this bug fix makes it into both Major WiX versions.
>
> Regards,
> Jason Stephenson
>
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant
> http://www.firegiant.com/
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/