[Openvpn-devel] Don't clear capability bounding set on capng_change_id

Message ID 20230118142428.162-1-timo@rothenpieler.org
State Accepted
Headers show
Series [Openvpn-devel] Don't clear capability bounding set on capng_change_id | expand

Commit Message

Timo Rothenpieler Jan. 18, 2023, 2:24 p.m. UTC
The bounding set being empty will overpower the likes of su/sudo
and will make it impossible for any child processes to ever gain
additional privileges again.

This fixes https://github.com/OpenVPN/openvpn/issues/220

Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
---
 src/openvpn/platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Gert Doering Jan. 19, 2023, 8:14 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Thanks for so quickly coming to help with investigating the issue and
providing a fix :-)

I have not tested this myself, but the code change is trivial, and the
positive effects have been verified in the GH issue.  I have tested
that "running a DCO p2mp instance as user nobody" still works (it does).

Your patch has been applied to the master and release/2.6 branch.

commit d8523119b95db55d2c101b8364ce7e9d0d0f6f3a (master)
commit 99a098e13a427e72ade5ef2812b7ea342ea64aa6 (release/2.6)
Author: Timo Rothenpieler
Date:   Wed Jan 18 15:24:28 2023 +0100

     Don't clear capability bounding set on capng_change_id

     Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20230118142428.162-1-timo@rothenpieler.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26048.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 1b5fa9ad..580c4cb8 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -246,7 +246,7 @@  platform_user_group_set(const struct platform_state_user *user_state,
     /* Change to new UID/GID.
      * capng_change_id() internally calls capng_apply() to apply prepared capabilities.
      */
-    res = capng_change_id(new_uid, new_gid, CAPNG_DROP_SUPP_GRP | CAPNG_CLEAR_BOUNDING);
+    res = capng_change_id(new_uid, new_gid, CAPNG_DROP_SUPP_GRP);
     if (res == -4 || res == -6)
     {
         /* -4 and -6 mean failure of setuid/gid respectively.