On Mon, 5 Oct 2015 21:43:23 +0200 Steffan Karger <stef...@karger.me> wrote:
> Thanks. This version looks good to me, but the patch won't apply. > Most likely your MUA messed up line wrapping. Could you resend the > patch using 'git send-email', or alternatively as an attachment? > > -Steffan Hi, You're right, my mail client is wrapping lines once they go past 72 characters, so it broke the patch. I'm really surprised I haven't noticed that. This time I've used 'git format-patch', and included resulting file as a separate attachment (as you requested). I'm sorry for the delay, I was unavailable lately. Lukasz K.
From 35d268234fc65b75239b2dff8f9eb15c8c4a642a Mon Sep 17 00:00:00 2001
From: Lukasz Kutyla <movrax-...@cryptolab.net>
List-Post: openvpn-devel@lists.sourceforge.net
Date: Sat, 17 Oct 2015 21:15:15 +0200
Subject: [PATCH] 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-...@cryptolab.net>
---
src/openvpn/init.c | 31 ++++++++++++++++++-------------
src/openvpn/openvpn.h | 6 ++++--
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 5dd8781..c5c0ab6 100644
--- 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 4fab00b..3f1df6e 100644
--- 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;
};
--
2.6.1
pgpLQh5tz8zBS.pgp
Description: OpenPGP digital signature
