This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]