Re: [GOLDSTAR] Re: [PATCH] setup: allow running as non-admin

[Christopher Faylor <cgf-use-the-mailinglist-please@cygwin.com>]

On Thu, Nov 07, 2013 at 02:15:21PM +0100, Corinna Vinschen wrote:
>Hi Shaddy,
>
>On Nov  7 11:39, Shaddy Baddah wrote:
>> 2013-11-06  Shaddy Baddah <lithium-cygwin at shaddybaddah dot name>
>> 
>> 	* LogFile.cc (LogFile::flushAll): New function to flush log all logging to
>> 	files without exiting (as LogFile::exit does).
>> 	* LogFile.h: Declare new method closeAll.
>> 	* main.cc (NoAdminOption): Add new CLI options -B/--no-admin. This option
>> 	allows the user to suppress privilege elevation (in tandem with
>> 	"asInvoker" requestedExecutionLevel changes to exe manifests).
>> 	(WinMain): check if setup run with Administrator privilege and if the
>> 	NoAdminOption has not been specified, attempt to elevate privilege to an
>> 	Administrator via WINAPI ShellExecuteEx().
>> 	* setup.exe.manifest: Add requestedExecutionLevel of asInvoker to allow
>> 	suppression of privilege elevation.
>> 	* setup64.exe.manifest: Modify requestedExecutionLevel from
>> 	requireAdministrator to asInvoker to allow suppression of privilege
>> 	elevation. Continuity of privilege elevation attempt on startup is
>> 	implemented by main.cc changes to WinMain().
>> 	* win32.cc (NTSecurity::isRunAsAdmin): New function to allow main.cc to
>> 	check if setup.exe has been run with privilege elevated to Administrator
>> 	level.
>> 	* win32.h: Declare new method isRunAsAdmin.
>
>Thanks a lot for this patch.  I applied it with a few minor tweaks.
>First of all, this comment was a bit misleading now, given that the
>code doesn't run on pre-Vista anyway:
>
>> +		// Note, this is necessary to avoid an infinite loop.
>> +		// The understanding is that pre-Vista, the runas verb will not
>> +		// result in a privilege elevated process. Therefore we need to
>> +		// indicate to the forked process that it should be happy with
>> +		// whatever privileges it is run with.
>> +		std::string command_line_cs (command_line);
>> +		command_line_cs += " -";
>> +		command_line_cs += NoAdminOption.shortOption();
>> +		sei.lpParameters = command_line_cs.c_str ();
>
>I shortened the comment to a simple one-liner:
>
>              // Avoid another isRunAsAdmin check in the child.
>
>I also added a small change for the sake of starting setup from the
>command line.  While the log to the logfiles has been stopped, the
>log to stdout persist up to the call of theLog->exit.  I added a 
>bit of code to stop printing
>
>  Ending cygwin install
>  
>if the elevation was successful.  In that case the stdout log now prints
>
>  note: Hand installation over to elevated child process.
>
>
>Thanks again for this patch, it's highly appreciated and is worth
>a gold star, I think.
>
>Chris, do your worst ;)

The new setup's are installed.  Shaddy, do you want to respond to the
Cygwin ML thread and tell them that you've fixed the problem?

Thanks again for doing this.

cgf