diff mbox

[resent,iputils,01-07] setuid/capabilities fixups

Message ID khaapr$ipr$1@ger.gmane.org
State Not Applicable, archived
Delegated to: David Miller
Headers show

Commit Message

Yuriy M. Kaminskiy March 7, 2013, 3:13 p.m. UTC 
Yuriy Kaminskiy wrote:
> Run ping, look at /proc/`pidof ping`/status -> oops (capabilities are not
> [permanently] dropped, some of uids are not dropped, etc). Fix assorted issues
> with setuid and capabilities drop. Limited testing only, please review/check
> carefully.
diff mbox

Patch

From 4298d3af86b881ef7abb61a36f83a610fc8bb772 Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <yumkam@gmail.com>
Date: Wed, 2 Jan 2013 03:08:30 +0400
Subject: [PATCH 7/7] ninfod: fix capabilities setting

1) -u option failed to change real uid too (likely leaving it as root);
2) it failed to drop saved uid;
---
 ninfod/ninfod.c |   58 +++++++++++++++++-------------------------------------
 1 files changed, 18 insertions(+), 40 deletions(-)

diff --git a/ninfod/ninfod.c b/ninfod/ninfod.c
index f1db977..7f6a2fa 100644
--- a/ninfod/ninfod.c
+++ b/ninfod/ninfod.c
@@ -497,16 +497,28 @@  static void do_daemonize(void)
 /* --------- */
 #ifdef HAVE_LIBCAP
 static const cap_value_t cap_net_raw = CAP_NET_RAW;
-static const cap_value_t cap_setuid =  CAP_SETUID; 
-static cap_flag_value_t cap_ok;
-#else
-static uid_t euid;
 #endif
 
 static void limit_capabilities(void)
 {
 #ifdef HAVE_LIBCAP
 	cap_t cap_p, cap_cur_p;
+	cap_flag_value_t cap_ok;
+
+	if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
+		DEBUG(LOG_ERR, "prctl: %s\n", strerror(errno));
+		exit(-1);
+	}
+
+	if (setuid(opt_u ? opt_u : getuid()) < 0) {
+		DEBUG(LOG_ERR, "setuid: %s\n", strerror(errno));
+		exit(-1);
+	}
+
+	if (prctl(PR_SET_KEEPCAPS, 0) < 0) {
+		DEBUG(LOG_ERR, "prctl: %s\n", strerror(errno));
+		exit(-1);
+	}
 
 	cap_p = cap_init();
 	if (!cap_p) {
@@ -520,32 +532,20 @@  static void limit_capabilities(void)
 		exit(-1);
         }
 
-	/* net_raw + setuid / net_raw */
 	cap_get_flag(cap_cur_p, CAP_NET_RAW, CAP_PERMITTED, &cap_ok);
 	if (cap_ok != CAP_CLEAR) {
 		cap_set_flag(cap_p, CAP_PERMITTED, 1, &cap_net_raw, CAP_SET);
 		cap_set_flag(cap_p, CAP_EFFECTIVE, 1, &cap_net_raw, CAP_SET);
 	}
 
-	cap_get_flag(cap_cur_p, CAP_SETUID, CAP_PERMITTED, &cap_ok);
-	if (cap_ok != CAP_CLEAR)
-		cap_set_flag(cap_p, CAP_PERMITTED, 1, &cap_setuid, CAP_SET);
-
 	if (cap_set_proc(cap_p) < 0) {
 		DEBUG(LOG_ERR, "cap_set_proc: %s\n", strerror(errno));
 		if (errno != EPERM)
 			exit(-1);
 	}
 
-	if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
-		DEBUG(LOG_ERR, "prctl: %s\n", strerror(errno));
-		exit(-1);
-	}
-
 	cap_free(cap_cur_p);
 	cap_free(cap_p);
-#else
-	euid = geteuid();
 #endif
 }
 
@@ -560,28 +560,6 @@  static void drop_capabilities(void)
 		exit(-1);
 	}
 
-	/* setuid / setuid */
-	if (cap_ok != CAP_CLEAR) {
-		cap_set_flag(cap_p, CAP_PERMITTED, 1, &cap_setuid, CAP_SET);
-		cap_set_flag(cap_p, CAP_EFFECTIVE, 1, &cap_setuid, CAP_SET);
-
-		if (cap_set_proc(cap_p) < 0) {
-			DEBUG(LOG_ERR, "cap_set_proc: %s\n", strerror(errno));
-			exit(-1);
-		}
-	}
-
-	if (seteuid(opt_u ? opt_u : getuid()) < 0) {
-		DEBUG(LOG_ERR, "setuid: %s\n", strerror(errno));
-		exit(-1);
-	}
-
-	if (prctl(PR_SET_KEEPCAPS, 0) < 0) {
-		DEBUG(LOG_ERR, "prctl: %s\n", strerror(errno));
-		exit(-1);
-	}
-
-	cap_clear(cap_p);
 	if (cap_set_proc(cap_p) < 0) {
 		DEBUG(LOG_ERR, "cap_set_proc: %s\n", strerror(errno));
 		exit(-1);
@@ -667,14 +645,14 @@  int main (int argc, char **argv)
 	appname = argv[0];
 	set_logfile();
 
+	parse_args(argc, argv);
+
 	limit_capabilities();
 
 	sock = open_sock();
 	if (sock < 0)
 		sock_errno = errno;
 
-	parse_args(argc, argv);
-
 	drop_capabilities();
 
 	if (opt_h || opt_v)
-- 
1.7.6.3