Fix privilege drop if first connection attempt fails

OpenVPN does not drop privileges (UID/GID/chroot) as requested according
to the configuration file and/or passed arguments if the first connection
attempt is not established successfully, this also includes applying
SELinux context.
Signals and restarts are processed after "context.first_time" is set to
"false", which results in omitting entire privilege dropping block in
"do_uid_gid_chroot()" when successful connection is finally made
(everything is initialized correctly and said function is called), since
"context.first_time" is used as block entry condition.

We modify "do_uid_gid_chroot()" in such a way that allows us to drop
privileges even when first connection attempt was unsuccessful.

Signed-off-by: Lukasz Kutyla <movrax-dev@cryptolab.net>
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20151018103446.5fed9f97.movrax-dev@cryptolab.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10301
Acked-by: Steffan Karger <steffan.karger@fox-it.com>
Message-Id: <20151018103446.5fed9f97.movrax-dev@cryptolab.net
20151018103446.5fed9f97.movrax-dev@cryptolab.net>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10301
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 5dd87815b327dcc2cc5dad7da407af6750e27093..c5c0ab6dd9ed1b24d8a8d8ec7f23bb5af5e8ff1c 100644 (file)
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -950,31 +950,30 @@ do_uid_gid_chroot (struct context *c, bool no_delay)
   static const char why_not[] = "will be delayed because of --client, --pull, or --up-delay";
   struct context_0 *c0 = c->c0;
 
-  if (c->first_time && c0 && !c0->uid_gid_set)
+  if (c0 && !c0->uid_gid_chroot_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);
        }
 
-      /* set user and/or group that we want to setuid/setgid to */
-      if (no_delay)
+      /* set user and/or group if we want to setuid/setgid */
+      if (c0->uid_gid_specified)
        {
-         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) {
+           platform_group_set (&c0->platform_state_group);
+           platform_user_set (&c0->platform_state_user);
+         }
+         else if (c->first_time)
+           msg (M_INFO, "NOTE: UID/GID downgrade %s", why_not);
        }
 
 #ifdef ENABLE_MEMSTATS
-      if (c->options.memstats_fn)
+      if (c->first_time && c->options.memstats_fn)
        mstats_open(c->options.memstats_fn);
 #endif
 
@@ -993,10 +992,16 @@ do_uid_gid_chroot (struct context *c, bool no_delay)
            else
              msg (M_INFO, "setcon to '%s' succeeded", c->options.selinux_context);
          }
-         else
+         else if (c->first_time)
            msg (M_INFO, "NOTE: setcon %s", why_not);
        }
 #endif
+
+      /* Privileges are going to be dropped by now (if requested), be sure
+       * to prevent any future privilege dropping attempts from now on.
+       */
+      if (no_delay)
+       c0->uid_gid_chroot_set = true;
     }
 }
 

diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index 4fab00be0ac2681b5634b2651b2b9df489fec111..3f1df6ec44f7a0f221dd723ba7b66a3d910264ff 100644 (file)
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -130,13 +130,15 @@ struct context_persist
  *
  * Level 0 state is initialized once at program startup, and then remains
  * throughout the lifetime of the OpenVPN process.  This structure
- * contains information related to the process's PID, user, and group.
+ * contains information related to the process's PID, user, group, and
+ * privileges.
  */
 struct context_0
 {
   /* workspace for --user/--group */
   bool uid_gid_specified;
-  bool uid_gid_set;
+  /* helper which tells us whether we should keep trying to drop privileges */
+  bool uid_gid_chroot_set;
   struct platform_state_user platform_state_user;
   struct platform_state_group platform_state_group;
 };