Hello,

On Sun, 30 Aug 2015 22:37:49 +0200
Gert Doering <g...@greenie.muc.de> wrote:

> Hi,
> 
> On Sun, Aug 30, 2015 at 04:25:34PM +0200, Lukasz K. wrote:
> > -  if (c->first_time && c0 && !c0->uid_gid_set)
> > +  if (c0 && !c0->uid_gid_set)
> >      {
> >        /* chroot if requested */
> >        if (c->options.chroot_dir)
> >     {
> >       if (no_delay)
> >         platform_chroot (c->options.chroot_dir);
> > -     else
> > +     else if (c->first_time)
> >         msg (M_INFO, "NOTE: chroot %s", why_not);
> >     }
> 
> Looking a bit more closely at this one.  What happens if you do not
> set uid/gid, but *do* set --chroot?  Will it then execute the chroot
> call every single time (since effectively the first_time check is
> removed, and c0->uid_git_set will never be set)?
> 
> Or am I misreading this?
> 
> gert
> 

As I've mentioned in my first message:
"[...] even when UID/GID are not specified ("context_0.uid_gid_set" is
set to "true" one way or another) [...]"

To see this it's necessary to take a better look at
"do_uid_gid_chroot()" code.
This is original, unmodified part of code from git (I have stripped out
original comments and added my own in places worth to look at with
explanations):
---
[...]
  /* "c0->uid_gid_set" is controlling whether to enter this block
   * regardless of "c->first_time" presence
   */
  if (c->first_time && c0 && !c0->uid_gid_set)
    {
      if (c->options.chroot_dir)
        {
          if (no_delay)
            platform_chroot (c->options.chroot_dir);
          else
            msg (M_INFO, "NOTE: chroot %s", why_not);
        }

      /* There's no explicit check for UID/GID presence, so it will
       * always enter this block when everything has been set and
       * "no_delay" is passed as "true", even when there's no
       * UID/GID change (only chroot etc.).
       */
      if (no_delay)
        {
          /* "platform_*_set()" is internally checking on it's own
           * whether UID/GID has been specified.
           */
          platform_group_set (&c0->platform_state_group);
          platform_user_set (&c0->platform_state_user);
          /* "c0->uid_gid_set" is flipped to "true", which means that
           * from now on "c0 && !c0->uid_gid_set" expression from the
           * beginning will return "false", which in turn will omit the
           * entire block next time like "c->first_time == false" would
           */
          c0->uid_gid_set = true;
        }
      else if (c0->uid_gid_specified)
        {
          msg (M_INFO, "NOTE: UID/GID downgrade %s", why_not);
        }
[...]
--

External calls are made only when "no_delay" is passed as "true", and
when this happens "c0->uid_gid_set" is always set to "true".
The only exception is memstat (where it's not bound to "no_delay"
checks) and call to "msg()" with "NOTE:":
---
[...]
#ifdef ENABLE_MEMSTATS
      if (c->options.memstats_fn)
        mstats_open(c->options.memstats_fn);
#endif
[...]
--
That's why it gets a "first_time" check as this is something that could
be called more than once.

User will also keep receiving appropriate "NOTE:" messages (depending on
which privileges are going to be dropped) on each reconnect until
"no_delay" is passed as "true" (everything is initialized and ready to
actually do privilege dropping if requested, and where
"c0->uid_gid_set" is set to "true"):
---
[...]
          if (no_delay)
            platform_chroot (c->options.chroot_dir);
          else
            msg (M_INFO, "NOTE: chroot %s", why_not);
[...]
      if (no_delay)
        {
          platform_group_set (&c0->platform_state_group);
          platform_user_set (&c0->platform_state_user);
          c0->uid_gid_set = true;
        }
      else if (c0->uid_gid_specified)
        {
          msg (M_INFO, "NOTE: UID/GID downgrade %s", why_not);
        }
[...]
          if (no_delay) {
            if (-1 == setcon (c->options.selinux_context))
              msg (M_ERR, "setcon to '%s' failed; is /proc
accessible?", c->options.selinux_context);
            else
              msg (M_INFO, "setcon to '%s' succeeded",
c->options.selinux_context);
          }
          else
            msg (M_INFO, "NOTE: setcon %s", why_not);
[...]
--
And this is something that second patch also targets (with the use of
"first_time").

This logic hasn't changed since 2.1.3 (this is the oldest release I've
tested).
In 2.1.3/2.2.1 there's no memstat section though, it looks like it has
been added in 2.3.x. There are some minor naming changes between those
versions too.

It's worth to note that the use of "context_0.uid_gid_set" seems to be
limited to said function only.

Lukasz K.

Attachment: pgpIVpFgPtdjb.pgp
Description: OpenPGP digital signature

Reply via email to