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

[Shaddy Baddah <lithium-cygwin@shaddybaddah.name>]

Hi,

On Nov 08 02:23, Christopher Faylor wrote:
> 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.

Done. Don't mention it. I'm delighted to give back to the Cygwin
community and thank you all for what I feel is one of the best, if not
the best open source project and community.

I'll stick around too and help if there is any issues with the patch.

I am also hoping to find time in the near future to ITP a couple of
packages.

-- 
Regards,
Shaddy