[wix-devs] Issue 4875

Thu Apr 30 16:30:10 PDT 2020

Update on next planned build here: https://www.firegiant.com/blog/2020/4/29/wix-online-meeting-187-highlights/

-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Rob Mensching via wix-devs
Sent: Tuesday, April 28, 2020 10:30 AM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Rob Mensching <rob at firegiant.com>
Subject: Re: [wix-devs] Issue 4875

Thanks. I anticipate we will be having a build as early as this week for continued ARM64 fixes and this PR will get included in it.

Regards,

  Rob Mensching
  CEO
  FireGiant
_______________________________________________________________
 FireGiant  |  Dedicated support for the WiX toolset  |  http://www.firegiant.com/

-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Friday, April 17, 2020 3:39 PM
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

Hey Rob, I took this pull request into v3.14. You said to ping you if you hadn't done a build for that by the next meeting (which happened two days ago).

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

> 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/Certific
> ate-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/ 
> ____________________________________________________________________
> WiX Toolset Developer Mailing List provided by FireGiant 
> http://www.firegiant.com/
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/ ____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/