Windows: Avoid launching as admin when NSIS installer ends. #12985

pull JeremyRand wants to merge 1 commits into bitcoin:master from JeremyRand:nsis-de-elevate changing 1 files +2 −1
  1. JeremyRand commented at 1:36 pm on April 14, 2018: contributor

    The Bitcoin Core NSIS script runs with elevated privileges. Unfortunately, this means that it launches Bitcoin Core itself with elevated privileges when the user chooses to launch Bitcoin Core at the end of the installation procedure. This PR works around the issue by having explorer.exe launch Bitcoin Core. Seems to be a similar approach to what http://nsis.sourceforge.net/ShellExecAsUser_plug-in does, but without a plugin.

    I’ve tested this with Sysinternals Process Explorer on Windows 10 32-bit. I wouldn’t expect any differences in behavior on other Windows releases, but if anyone would like to test on other Windows releases, feel free.

    h/t to “UK” at https://mdb-blog.blogspot.se/2013/01/nsis-lunch-program-as-user-from-uac.html?showComment=1410158039989#c2463780017054126736 for the sample code.

    Fixes #7990.

  2. Avoid launching as admin when NSIS installer ends. 
    The Bitcoin Core NSIS script runs with elevated privileges.  Unfortunately, this means that it launches Bitcoin Core itself with elevated privileges when the user chooses to launch Bitcoin Core at the end of the installation procedure.  This commit works around the issue by having explorer.exe launch Bitcoin Core.  Seems to be a similar approach to what http://nsis.sourceforge.net/ShellExecAsUser_plug-in does, but without a plugin.

h/t to "UK" at https://mdb-blog.blogspot.se/2013/01/nsis-lunch-program-as-user-from-uac.html?showComment=1410158039989#c2463780017054126736 for the sample code.

Fixes #7990.
    7d8a8cc25f
  3. fanquake added the label Windows on Apr 14, 2018
  4. ken2812221 approved
  5. ken2812221 commented at 9:04 am on April 15, 2018: contributor
    Tested ACK 7d8a8cc
  6. fanquake commented at 2:00 am on April 16, 2018: member
    @ken2812221 Could you provide the basic steps you used to test this, so anyone else can recreate on a Windows VM?
  7. ken2812221 commented at 6:09 am on April 16, 2018: contributor

    @fanquake I just add wallet=C:\Windows\xxxx to my config file.

    if bitcoin core has higher privilege, it can successfully create the directory, otherwise it crash

  8. laanwj commented at 6:12 am on April 16, 2018: member
    Concept ACK - would like to see some more tested ACKs.
  9. jonasschnelli commented at 10:53 am on April 17, 2018: contributor
    Gitian build is here: https://bitcoin.jonasschnelli.ch/build/573
  10. fanquake commented at 4:56 am on April 18, 2018: member

    I’ve tested this using the method @ken2812221 suggested. Basically create a bitcoin.conf with

    0wallet=C:\Windows\xxxx

    If you then install 0.16.0, and choose to run at the end of the installation, Core will error on startup, because we can’t specify a path as a wallet name. 

    02018-04-18 04:48:31 Using 4 threads for script verification
12018-04-18 04:48:31 scheduler thread start
22018-04-18 04:48:31 Using wallet directory C:\Users\User\AppData\Roaming\Bitcoin\wallets
32018-04-18 04:48:31 init message: Verifying wallet(s)...
42018-04-18 04:48:41 Shutdown: In progress...
52018-04-18 04:48:41 scheduler thread interrupt
62018-04-18 04:48:41 Shutdown: done

    However if you then install the gitian build provided by @jonasschnelli, and launch Core at the end of the installation, we get an exception. Looking at the debug.log, we no longer have access to C:\Windows\. 

    02018-04-18T04:40:39Z init message: Verifying wallet(s)...
12018-04-18T04:40:39Z Using BerkeleyDB version Berkeley DB 4.8.30: (April  9, 2010)
22018-04-18T04:40:39Z Using wallet wallet.dat
32018-04-18T04:40:39Z 
4
5************************
6EXCEPTION: N5boost10filesystem16filesystem_errorE       
7boost::filesystem::create_directory: Access is denied: "C:\Windows\xxxx"       
8C:\Program Files\Bitcoin\bitcoin-qt.exe in Runaway exception
  11. laanwj commented at 6:37 am on April 18, 2018: member
    @fanquake Thanks for testing. To be clear: All in all that seems a positive result? I suppose you used C:\windows as a path that bitcoin core is supposed to not have access to?
  12. fanquake commented at 6:54 am on April 18, 2018: member
    @laanwj Yes, I think it’s a positive result. Core should not have access to write files into C:\Windows\.
  13. laanwj added this to the milestone 0.16.1 on Apr 18, 2018
  14. laanwj added the label Needs backport on Apr 18, 2018
  15. laanwj commented at 9:51 am on April 18, 2018: member
    Right - it’s similar to /usr/. I’ve kind of forgot about windows things somewhat. Looks good to me, then. Also added “to backport” as sane installer behavior is very important.
  16. laanwj merged this on Apr 18, 2018
  17. laanwj closed this on Apr 18, 2018
  18. laanwj referenced this in commit 615f7c2884 on Apr 18, 2018
  20. fanquake referenced this in commit c366c12a8e on Apr 18, 2018
  21. JeremyRand commented at 9:41 pm on April 18, 2018: contributor
    @Michagogo I’m not sure whom at Microsoft Ericlaw talked to (he doesn’t give any citation), but Microsoft has recommended the approach of using explorer.exe for this.
  22. Michagogo commented at 0:25 am on April 19, 2018: contributor

    I’m not sure if launching explorer.exe with the program as an argument is exactly equivalent to what that page describes (calling various functions directly).

    On Thu, Apr 19, 2018 at 12:42 AM JeremyRand notifications@github.com wrote:

    @Michagogo https://github.com/Michagogo I’m not sure whom at Microsoft Ericlaw talked to (he doesn’t give any citation), but Microsoft has recommended https://blogs.msdn.microsoft.com/oldnewthing/20131118-00/?p=2643 the approach of using explorer.exe for this.

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/12985#issuecomment-382538461, or mute the thread https://github.com/notifications/unsubscribe-auth/ACcNnuY2XCFhgZWIGqt1gT0Qh3biAlPSks5tp7NDgaJpZM4TVEfn .

  23. JeremyRand commented at 5:36 am on April 19, 2018: contributor

    @Michagogo the Microsoft page I linked says:

    The solution here is to go back to Explorer and ask Explorer to launch the program for you. Since Explorer is running as the original unelevated user, the program (in this case, the Web browser) will run as Bob.

    So while I haven’t audited the Microsoft sample code to verify that it does that (Windows C API’s aren’t my specialty), I think it’s reasonable to infer that the Microsoft sample code matches the Microsoft description (which matches what this PR does).

  24. Michagogo commented at 7:49 am on April 19, 2018: contributor

    Windows C APIs aren’t my specialty either, so maybe someone with more experience could weigh in, but that code definitely doesn’t just run explorer.exe with an argument. Just by skimming it, it’s clear that it somehow looks at the environment to get some kind of handle or something on the running shell and call a function on that. That may, however, be equivalent - I don’t have enough deep Windows knowledge to say for sure what the difference is.

    On Thu, Apr 19, 2018 at 8:38 AM JeremyRand notifications@github.com wrote:

    @Michagogo https://github.com/Michagogo the Microsoft page I linked says:

    The solution here is to go back to Explorer and ask Explorer to launch the program for you. Since Explorer is running as the original unelevated user, the program (in this case, the Web browser) will run as Bob.

    So while I haven’t audited the Microsoft sample code to verify that it does that (Windows C API’s aren’t my specialty), I think it’s reasonable to infer that the Microsoft sample code matches the Microsoft description (which matches what this PR does).

    — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/12985#issuecomment-382615974, or mute the thread https://github.com/notifications/unsubscribe-auth/ACcNnvgiN0-Jy6P-unbmA2Ig5JDQ7qDVks5tqCK5gaJpZM4TVEfn .

  25. fanquake referenced this in commit 0684cf9b58 on Apr 26, 2018
  26. laanwj referenced this in commit feba12fe85 on May 16, 2018
  27. fanquake removed the label Needs backport on May 16, 2018
  28. HashUnlimited referenced this in commit 1fe255a53e on Jun 29, 2018
  29. PastaPastaPasta referenced this in commit fa652559f0 on Apr 3, 2020
  30. ckti referenced this in commit 4f5d896c9e on Mar 28, 2021
  31. DrahtBot locked this on Sep 8, 2021
  32. JeremyRand deleted the branch on Feb 13, 2022


JeremyRand ken2812221 fanquake laanwj jonasschnelli Michagogo

Labels
Windows

Milestone
0.16.1

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-24 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me