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: cannot run setup64.exe without admin privileges (even if renamed foo.exe)


On Oct 16 02:18, Shaddy Baddah wrote:
> On 15/10/13 23:22, Corinna Vinschen wrote:
> >On Oct 15 21:21, Shaddy Baddah wrote:
> >>On 15/10/13 20:08, Corinna Vinschen wrote:
> >>>[...]
> >thanks for letting us know and your patch.  I had a look and it looks
> >good for a start.  You just call the CheckTokenMembership function,
> >though.  The problem is, you won't know if the process has been started
> >by a non-admin or by an admin without elevation.  So you always call
> >ShellExecute if setup is started without admin rights, for non-admins
> >and non-elevated admins alike, unless the --no-admin option is given.
> 
> Yes that is deliberate. Initially I coded this for how I viewed was
> desirable, that being that it would use CheckTokenMembership to see if
> it was elevated (or just run as Administrator on XP). If it was not, it
> would attempt to elevate using "runas" flag. If that was rejected, it
> would carry on. The --no-admin was still used as per the patch
> submitted to avoid infinite loop just in-case ShellExecuteEx didn't
> honour "runas" and just ran setup all over again with no Administrator
> rights.
> 
> However I sensed from previous discussions that really it was desirable
> to steer users towards running as Admin/elevated privilege. Unless they
> really knew what they were doing. So much so that building a custom
> setup.exe was being recommended without much discussion of an
> alternative... until now.
> 
> So this patch tries to be as backwards compatible as possible. That is,
> unless the user has already explicitly elevated privilege (context
> menu -> Run as ...), setup.exe forces an attempt. And fails if it does
> not elevate. The forced attempt can be avoided by explicitly specifying
> --no-admin.

Yeah, that makes sense.

> The patch I provided doesn't match exactly the behaviour of setup on
> various systems at the moment, in the following ways:
> 
> * I don't know how, and did not bother too much about emulating the
>   "access denied" when the user rejects the privilege elevation.

I don't think that much of a problem.  If the user refuses the elevation,
setup won't run.  That's as much message as one needs, I think.

> * Under x86 (32bit), where the user has performed the rename magic that
>   normally circumvents the UAC Installer Detection Technology, setup
>   will still behave as if they hadn't and continue to try to elevate.
>   IMO this is minor, but if we were going for 100% compatibility, we
>   could check the exe filename against the same list that cygports uses,
>   and avoid the privilege elevation attempt:
> 
> /usr/share/cygport/lib/src_postinst.cygpart:1112:       find * -type
> f -executable -a \( -name '*instal*.exe' -o -name '*patch*.exe' -o
> -name '*setup*.exe' -o -name '*update*.exe' \) -print0 | \

I don't know if that's really needed anymore.  Your patch provides a
nice workaround, the --no-admin option.

> * Under XP (32 bit... I don't know about any other archs (if they

XP 64 bit exists :)

>   exist?) or other XP like editions (server???)), for a
>   non-Administrator the attempt to elevate privilege is also an extra
>   behaviour that wouldn't have been normally encountered. Again, it
>   may have been easy to detect that and do something about it too.

XP and Server 2003 don't know the concept anyway, so calling GetVersion
and skipping the entire elevation code if the result shows we're running
on a pre-Vista system should do it.  Something like this:

  if (LOWORD (GetVersion ()) >= _WIN32_WINNT_VISTA)
    {
      ...do the elevate thingy...
    }

> Another bit of polish this patch needs is a effective way to close off
> logging once and for all when we are about to call ShellExecuteEx.
> Putting in the line:
> 
> +          l->msg.clear();
> 
> staved off the doubling up of logging when LogFile::exit was being
> called. But unfortunately a bunch of newlines where being logged
> instead. I never got around to knocking all that on the head.

Hmm, that should be fixable.  AFAICS, LogFile::log_save doesn't
check if tstr is longer than 0:

  if (tstr[strlen (tstr) - 1] != '\n')
    f->write ("\n", 1);

Does it help to write the NL only if there was some non-empty string?

  if (strlen (tstr) > 0 && tstr[strlen (tstr) - 1] != '\n')
    f->write ("\n", 1);

?

> >Is that what we want?  Or should the process only be elevated when
> >started by a non-elevated admin as I proposed.  I'm not sure, really.
> 
> If I understand right, you were proposing that if the user was not an
> Admin at all, then just let them run as they were.
> 
> As I wrote earlier, it seemed from discussion that it was desirable (if
> not pseudo policy) that regular Cygwin users run with privilege
> elevated. And knowing of various features built into setup that require
> elevated privilege, eg. replace on boot, I can see why that is.

Yes, as I said, that makes sense.  I think your approach is fine.

> But I am happy to be guided either way. I can help out too, but as you
> can see by the latency in my communication/implementation, I am not
> a pillar of reliability :-(

There's no immediate pressure, I think.  Feel free to submit your
patch when you think you're ready, or if you want some testing.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: pgpzPcRJaG6H8.pgp
Description: PGP signature


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