[wix-devs] Issue 4875

[Jason Stephenson]

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