Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boxstarter broke my Win10 Administrator Privileges #358

Closed
VinciShark opened this issue Oct 21, 2018 · 25 comments
Closed

boxstarter broke my Win10 Administrator Privileges #358

VinciShark opened this issue Oct 21, 2018 · 25 comments
Labels
No Response / Stale Used on issues when additional information is requested, and no response has been given

Comments

@VinciShark
Copy link

Makes my Windows10 PC get Administrator Privileges everywhere.
Never notice me before restart my PC, or let me to choose when to restart to continue.
……
And now? I am reinstalling my Windows10 OS……

@VinciShark VinciShark changed the title The worst thing Nodejs recommanded to me is boxstarter boxstarter broke my Win10 Administrator Privileges Oct 21, 2018
@gep13
Copy link
Member

gep13 commented Oct 21, 2018

@VinciShark sorry to hear that you are having issues. Can you please provide some more information about exactly what you are/were seeing? The information that you have provided isn’t enough to figure out exactly what is/was going on.

@pauby pauby added the 0 - _Triaging Issue is accepted, but a milestone has yet to be added for the issue label Oct 21, 2018
@mpeson
Copy link

mpeson commented Oct 30, 2018

It seems that the installer disables Window's UAC. I'm no security expert but that doesn't seem to be something that any installer should be doing, even if it's meant to be temporary.

Also while messing up the security settings the installer reboots the PC without any warning whatsoever.

@pauby
Copy link
Member

pauby commented Oct 30, 2018

@mpeson

Also while messing up the security settings the installer reboots the PC without any warning whatsoever.

Boxstarter is aimed at bootstrapping machines. So when a required reboot is detected it's done. This is documented. If it has to wait on you rebooting the machine then it defeats the purpose.

It seems that the installer disables Window's UAC. I'm no security expert but that doesn't seem to be something that any installer should be doing, even if it's meant to be temporary.

Boxstarter does not disable UAC. There is however a command called Disable-UAC. If the script you ran has that command then it will disable UAC as you said. The responsibility for enabling it again lies with the script creator, not Boxstarter, using Enable-UAC.

UPDATE: Boxstarter does disable UAC. See Matt's explanation.

@VinciShark
Copy link
Author

It seems that the script disable my UAC, while it fails to re-enable it.
But don't know why.
It is an entire new Windows10 PC, whithout any strange 【computer housekeeper】

@pauby
Copy link
Member

pauby commented Oct 30, 2018

@VinciShark Where did the script come from (I want to ask specifically if it came from NodeJS as we're getting increasing number of issues created because of this)?

@mwrock
Copy link
Member

mwrock commented Oct 30, 2018

Just want to add a little context/history here. @gep13 pinged me about this general issue in a twitter DM and this seems like the best place to respond. Gary's DM included this twitter thread where these same security concerns were raised and it was pointed out that elevating the process would be a better path than disabling UAC.

These are all totally valid points!

Here is why I added the disable/re-enable UAC behavior rather than taking the normal path of elevating. For those unfamiliar with the mechanics of "elevating" that basically boils down to throwing up the "run as admin" prompt where a user consents to allow the process elevated admin privileges. Boxstarter's primary use case at that time (and I think largely still is) is to allow a user to "bootstrap" their machine with a single command, walk away, go to bed and come back with everything set up as they like. The key dilemma here in terms of UAC and elevation is what to do after a reboot. When Boxstarter reboots the machine, it interactively logs the user back in after the reboot into an interactive windows session. Prior to the reboot, boxstarter saves a script into the user's "startup" folder so that the script kicks off as soon as the new session logs in. However with UAC on, the script will not run as admin and lots of subsequent setup operations will very likely fail. Boxstarter could elevate but that would mean that someone has to "man the helm" which boxstarter never assumes.

So that was the motivation. If I were to implement this all over again I would have done it differently. Instead of putting a script in the "startup" folder (which totally does not work in server core SKU machines), I would have created a scheduled task to run this script upon startup. This way the task would not need an interactive session and could run with elevated privileges without prompting as long as it ran under an admin user's credentials and has a RunLevel set to Highest.

The biggest downside to the task approach is that after a reboot, boxstarter would continue completely unseen. That might be fine if you are away but often (likely in the nodejs case) you are watching the screen and want to have a sense that things are a happening. Also, there are some installers that MUST be run in an interactive context. I think back when I created Boxstarter (2012), I needed a couple apps that had the interactive requirement and today I'd likely say I don't want to have anything to do with an app that has such a requirement :).

I will happily leave it up to the very capable Chocolatey team to decide the best path forward here and wish I had more time to help but at least wanted to give some background into the present state of things.

@andrewtchilds
Copy link

andrewtchilds commented Oct 31, 2018

Also, there are some installers that MUST be run in an interactive context.

If the vast majority of installers don't require an interactive session, I say it's worth going the task scheduler route (at least by default). Surely there's other ways to give the user the status of their Boxstarter script, at least when they log back in.

As an aside, this is probably part of the reason why MDT uses the built-in Administrator account, where UAC is off, during desktop deployment.

@flcdrg
Copy link
Member

flcdrg commented Oct 31, 2018

Thanks @mwrock for the history and context.

Changing to scheduled tasks is something to consider, but as flagged it probably is a breaking change so if it ever happens is likely to happen in a future major release. For now, the new maintainers are still finding their feet with the existing code base and taking over the weight from Matt, so expect more incremental changes in the short term.

@andrewtchilds One thing to consider is that there's also some applications that install per-user - installing via Administrator wouldn't work for that. It's something we'd also need to consider.

@andrewtchilds
Copy link

@flcdrg Good point. We disable the built-in administrator account on our workstations, so I don't think it'd work for most corporate-managed machines.

@ferventcoder
Copy link
Member

Also added this so that folks are not in the dark when this occurs - nodejs/node#24000

@VinciShark
Copy link
Author

@pauby Looks like it comes form Node.js
After install the newest Node.js, the script's shortcut comes into start menu.

@Degats
Copy link

Degats commented Nov 6, 2018

Ignoring the fact that disabling UAC is in itself a monumentally terrible idea for security reasons, there are a few other issues:

  1. If AD Group Policy enforces the use of UAC, this has the potential to cause an infinite reboot, as the computer UAC policy will most likely be applied before the user logs in to run the startup script (haven't actually tested this, but may be another cause of the reboot problems people have been having especially when on AD, see Node v11.0 windows installation opting into chocolatey+boxstarter results in infinite reboots nodejs/node#23838 ).

  2. Following on from @andrewtchilds and @flcdrg if the user running the install does not have local admin privileges, the startup script will be installed in the profile of whatever account was used via UAC to elevate the install (assuming said profile exists, see later).
    This means that after reboot:

    • If the account with local admin rights has never been used to actually log on to the machine before, (very possible in an AD environment, especially as there is a GP setting that cleans up unused profiles after x days) the startup script may never have been installed as that account does not have a local profile
    • If the startup script was sucessfuly installed it will not actually run, as the profile it was installed to is not logged in.
    • It will be impossible to run anything elevated while a user without local admin privieges is logged in (because UAC is now disabled) until:
    • The account with local admin that was originally used to elevate the install must log in to the machine for the startup script to run and complete install/cleanup and reactivate UAC.
    • If the startup script was not installed because the local admin account's profile did not exist, another account with local admin privileges must log in to the machine to be able to manually re-enable UAC.
  3. As a systems admin, I consider any installer that cannot be run completely silently for automated installs to be fundamentally broken. Errors should be able to be reported via log files and exit codes. Reboots should be able to be suppressed and done at the user's convenience later (quite frankly, the only thing that should even require a reboot before installation can complete is a firmware/driver update).

  4. At least one of the reboots is forced. This prevents open programs prompting the user to save changes and therefore is likely to cause loss of work. I can think of no good reason for the reboot to be forced.

@pauby
Copy link
Member

pauby commented Nov 6, 2018

@Degats are you referring to Boxstarter being used with NodeJS or something else?

Ignoring the fact that disabling UAC is in itself a monumentally terrible idea for security reasons

Matt explained the history of this.

@Degats
Copy link

Degats commented Nov 6, 2018

@pauby The specific problems we've encountered I believe are when installing boxstarter as included in the Node installer and not actively using boxstarter. Some of the behaviour may be down to the Node installer, but I understand all the behaviour around UAC is to do with boxstarter.
However my comment was more to point out some additional side-effects of the process of disabling UAC that (for the most part) I haven't seen mentioned before (I admit I've only had a cursory scan of a couple of threads/issues).

I did read that explanation post before writing my comment, which is why I glossed over the security specifics, though I couldn't help mentioning it as it seems to be far too easy to end up stuck in an insecure state potentially without the user's knowledge - as my colleague would have this morning if his account had local admin privileges. As it was, he got stuck in unprivelaged userland and had to log on as a different user to fix the issue.
I kind of understand the reasoning behind it as a workaround (in 2006 at least), but IMO it should never have got near a public installer that may be run on production(dev) machines - whoever's decision that might have been.

Feel free to treat my previous post as an FYI, I just wanted to point out a few more things that may trip people up and that makes this IMO quite a serious issue.

@pauby
Copy link
Member

pauby commented Nov 6, 2018

@Degats I get exactly where you are coming from but I do hate the 'this should never have made it into a public installer' comment when we are looking at it in hindsight and not when it was done. That's in no way intended to start a flame war (is that term still used?) just me being honest. Matt has already admitted that if things were done today they would likely have been done differently. We are where we are. We either try and move forward or we keep saying how terrible it is.

Boxstarter was original put together to bootstraps boxes. OS on them, Boxstarter comes along and does it's thing. Packages installed. Lots of reboots. You go away, grab some (or lots of coffee) and come back and your machine is 95% configured.

In the case of NodeJs it's being used for something it wasn't originally designed for. It's being used to install NodeJs on machines where we have users who are working. Reboot happens. Things get disabled. Things get updated. User's environment is generally changed.

My opinion is that when you take something that was designed to A and get it to do B you're going to end up with issues. And this is where we are.

Now while I get this is Boxstarter doing all the work, again my personal opinion, Boxstarter is not to blame for this (we can argue about how it could do things better but I'm specifically referring to this issue). It's doing what it's being told to do. When installing NodeJs the users are being informed it's going to reboot so the users are informed. I get not everybody reads those boxes that pop up especially if you've installed umpteen previous versions of NodeJs and you're expecting nothing to change. In addition, as I said, it's being used to install software on somebody's workstation that is 'live' - not what it was built to do.

So I get the issues around NodeJs and I don't want to point the finger (as that doesn't actually get us anywhere).

Thanks for making us aware of the other issues around AD which are incredibly useful. Could I ask you to raise a separate issue around them so we can track them outwith of this issue?

@Degats
Copy link

Degats commented Nov 6, 2018

@pauby No worries, I'm not trying to argue either, just venting a little ;)

I'm honestly not sure who's (Node/boxstarter) doing what as part of the process, because I've not really dug into anything yet (only found the problem a few hours ago and followed likely looking google hits). I've personally not come across boxstarter before and barely touched Node, I just piped up here as this issue seemed to be the root cause as it were.

As you suggest, I'll raise a new issue regarding the AD stuff once I get a spare half hour to gather my thoughts.

@pauby
Copy link
Member

pauby commented Nov 6, 2018

@Degats No problem. I do absolutely get it.

And the issues that are being raised around NodeJS using Boxstarter are not being swept under the carpet. We are looking into them. We have been taken by surprise at the number of new issues around it since being used by NodeJS.

Thanks for raising that new issue. It's much appreciated.

@MPagel
Copy link

MPagel commented Nov 9, 2018

Number 2 (and 1) messed me up for at least a day recently (also with node.js setup - for ESlint configuration in Komodo). I was not at all expecting the reboot, and when my system came back up, I logged into my user account as normal. But then I couldn't install/modify anything, because I couldn't request elevated access for anything. It took me a while to figure out that I needed to log out of my main user, then log on under my admin credentials, wait for the post-install to wrap up, log out, then log back into my main (non-admin) user account.

Ignoring the fact that disabling UAC is in itself a monumentally terrible idea for security reasons, there are a few other issues:

1. If AD Group Policy enforces the use of UAC, this has the potential to cause an infinite reboot, as the computer UAC policy will most likely be applied before the user logs in to run the startup script (haven't actually tested this, but may be another cause of the reboot problems people have been having especially when on AD, see [nodejs/node#23838](https://github.com/nodejs/node/issues/23838) ).

2. Following on from @andrewtchilds and @flcdrg if the user running the install does not have local admin privileges, the startup script will be installed in the profile of whatever account was used via UAC to elevate the install (assuming said profile exists, see later).
   This means that after reboot:
   
   * If the account with local admin rights has never been used to actually log on to the machine before, (very possible in an AD environment, especially as there is a GP setting that cleans up unused profiles after x days) the startup script may never have been installed as that account does not have a local profile
   * If the startup script was sucessfuly installed it will not actually run, as the profile it was installed to is not logged in.
   * It will be impossible to run _anything_ elevated while a user without local admin privieges is logged in (because UAC is now disabled) until:
   * The account with local admin that was originally used to elevate the install must log in to the machine for the startup script to run and complete install/cleanup and reactivate UAC.
   * If the startup script was not installed because the local admin account's profile did not exist, another account with local admin privileges must log in to the machine to be able to manually re-enable UAC.

3. As a systems admin, I consider any installer that cannot be run completely silently for automated installs to be fundamentally broken. Errors should be able to be reported via log files and exit codes. Reboots should be able to be suppressed and done at the user's convenience later (quite frankly, the only thing that should even require a reboot before installation can complete is a firmware/driver update).

4. At least one of the reboots is forced. This prevents open programs prompting the user to save changes and therefore is likely to cause loss of work. I can think of no good reason for the reboot to be forced.

@SkoZombie
Copy link

Rebooting computers without a prompt or warning is really f*n stupid. There should ALWAYS be an opportunity for users to save any work that might be open.

I'll make sure the rest of the people in the company know not to install boxstarter.

@pauby
Copy link
Member

pauby commented Nov 14, 2018

@SkoZombie did you install Boxstarter? How did you install it?

@mwallner
Copy link
Member

Hey @SkoZombie

Rebooting computers without a prompt or warning is really f*n stupid.

Boxstarter is intended to be used for automating box setups.
In a typical setup you've got multiple packages (pieces of software) where some of them may ask for a reboot to complete the installation procedure.
You can't automate complex setups when you need a user to be present to "click ok" sometimes.

There should ALWAYS be an opportunity for users to save any work that might be open.

You probably shouldn't be installing software when there's someone actively working on that machine.

I'll make sure the rest of the people in the company know not to install boxstarter.

Your choice, but I'd start with checking your current workflow.

  • how did you get to a point that a computer suddenly restarted?
  • what commands did you use?
  • did you install node.js? / there are known issues with a couple of issues adressing them. (WIP)

@SuperFlue
Copy link

SuperFlue commented Nov 16, 2018

Hello,

Bumped into this because the devs in my workplace was using the Disable-UAC script as part of their setup and this was causing issues.

The more proper way (but still a security risk) is to not to disable UAC but set Windows to elevate without prompting.
In brief; in the disable UAC script change set the property "ConsentPromptBehaviorAdmin" to 0 instead.

This will cause Windows to allow privileged actions without prompting.

https://docs.microsoft.com/en-us/windows/security/identity-protection/user-account-control/user-account-control-group-policy-and-registry-key-settings#registry-key-settings

Then to be sure that you tasks are launched in an elevated shell you can do:
Start-process Powershell.exe -Arguments $yourargs -Verb runas

@pauby
Copy link
Member

pauby commented Nov 16, 2018

@SuperFlue This is something that makes a lot of sense for us and something we are considering working towards but it also a lot of work and we need to prioritise it. If you haven't already have a look at why we use UAC above

@SuperFlue
Copy link

SuperFlue commented Nov 16, 2018

A possible way is to disable prompting and then make the script self elevating?

Like this:

function Test-IsAdmin {
    ([Security.Principal.WindowsPrincipal] [Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole([Security.Principal.WindowsBuiltInRole] "544")
}

function CheckAdmin{
    if ((Test-IsAdmin)){
        "Script is runnin as elevated all good!"
    }
    elseif (!(Test-IsAdmin)){
        "Not running as admin trying to elevate"
        Start-Process Powershell -ArgumentList "-File $PSCommandPath -Noexit" -Verb Runas
    }
}

CheckAdmin

@pauby pauby added Invalid / Not Issue / No repro / Not Implementing No Response / Stale Used on issues when additional information is requested, and no response has been given and removed 0 - _Triaging Issue is accepted, but a milestone has yet to be added for the issue Invalid / Not Issue / No repro / Not Implementing labels Mar 19, 2019
@pauby
Copy link
Member

pauby commented Mar 19, 2019

This issue was started around NodeJS installs incorporating Boxstarter and the problems that followed. The NodeJS team have since removed Boxstarter. This issue is not 5 months old so going to close it. It can be reopened again if needed.

@pauby pauby closed this as completed Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Response / Stale Used on issues when additional information is requested, and no response has been given
Projects
None yet
Development

No branches or pull requests