From: Yuriy Kaminskiy <yumkam@gmail.com>
To: netdev@vger.kernel.org
Subject: [iputils][patch 01-07] setuid/capabilities fixups
Date: Wed, 09 Jan 2013 23:41:00 +0400 [thread overview]
Message-ID: <kckh4a$taf$1@ger.gmane.org> (raw)
[-- Attachment #1: Type: text/plain, Size: 243 bytes --]
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.
[-- Attachment #2: 0001-arping-ping-ping6-traceroute2-clockdiff-drop-fsuid-a.patch --]
[-- Type: text/x-diff, Size: 2800 bytes --]
>From bc01c19df465bec369ded83bf48b069c6ad462d2 Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <yumkam@gmail.com>
Date: Wed, 2 Jan 2013 01:43:01 +0400
Subject: [PATCH 1/7] arping, ping, ping6, traceroute2, clockdiff: drop fsuid
at start
Prevent ignoring netfilter -m owner checks. With this patch ping
correctly blocked by iptables rules, e.g.:
$ sudo iptables -I OUTPUT -m owner --uid-owner $UID -j DROP
$ ping www.google.com
---
arping.c | 9 +++++++++
clockdiff.c | 8 ++++++++
ping_common.c | 9 +++++++++
traceroute6.c | 8 ++++++++
4 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/arping.c b/arping.c
index 35408c1..bdf81e9 100644
--- a/arping.c
+++ b/arping.c
@@ -27,6 +27,9 @@
#include <sys/prctl.h>
#include <sys/capability.h>
#endif
+#ifdef __linux__
+#include <sys/fsuid.h>
+#endif
#include <netdb.h>
#include <unistd.h>
@@ -229,6 +232,12 @@ int modify_capability_raw(int on)
perror("arping: setuid");
return -1;
}
+#ifdef __linux__
+ if (on) {
+ /* FIXME: error handling? setfsuid() have weird return code */
+ setfsuid(getuid());
+ }
+#endif
#endif
return 0;
}
diff --git a/clockdiff.c b/clockdiff.c
index 7c1ea1b..f12da2d 100644
--- a/clockdiff.c
+++ b/clockdiff.c
@@ -3,6 +3,9 @@
#include <sys/param.h>
#include <stdio.h>
#include <unistd.h>
+#ifdef __linux__
+#include <sys/fsuid.h>
+#endif
#include <stdlib.h>
#include <math.h>
#include <string.h>
@@ -561,6 +564,11 @@ main(int argc, char *argv[])
usage();
}
+#ifdef __linux__
+ // FIXME: error handling? setfsuid() have weird return code
+ setfsuid(getuid());
+#endif
+
sock_raw = socket(AF_INET, SOCK_RAW, IPPROTO_ICMP);
s_errno = errno;
diff --git a/ping_common.c b/ping_common.c
index 7f82851..12c87a4 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -2,6 +2,9 @@
#include <ctype.h>
#include <sched.h>
#include <math.h>
+#ifdef __linux__
+#include <sys/fsuid.h>
+#endif
int options;
@@ -175,6 +178,12 @@ int modify_capability(int on)
perror("seteuid");
return -1;
}
+#ifdef __linux__
+ if (on) {
+ /* FIXME: error handling? setfsuid() have weird return code */
+ setfsuid(uid);
+ }
+#endif
return 0;
}
diff --git a/traceroute6.c b/traceroute6.c
index 0538d4b..a14ddb6 100644
--- a/traceroute6.c
+++ b/traceroute6.c
@@ -266,6 +266,9 @@ char copyright[] =
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#ifdef __linux__
+#include <sys/fsuid.h>
+#endif
#include "SNAPSHOT.h"
@@ -343,6 +346,11 @@ int main(int argc, char *argv[])
int ch, i, on, probe, seq, tos, ttl;
int socket_errno;
+#ifdef __linux__
+ // FIXME: error handling? setfsuid() have weird return code
+ setfsuid(getuid());
+#endif
+
icmp_sock = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
socket_errno = errno;
--
1.7.6.3
[-- Attachment #3: 0002-ping-permanently-drop-capabilities-before-entering-m.patch --]
[-- Type: text/x-diff, Size: 554 bytes --]
>From da09261219322ec5116f016a858f11f83902deb9 Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <yumkam@gmail.com>
Date: Wed, 2 Jan 2013 01:44:49 +0400
Subject: [PATCH 2/7] ping: permanently drop capabilities before entering main
loop
---
ping.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/ping.c b/ping.c
index c0366cd..7ac1217 100644
--- a/ping.c
+++ b/ping.c
@@ -587,6 +587,8 @@ main(int argc, char **argv)
setup(icmp_sock);
+ drop_capabilities();
+
main_loop(icmp_sock, packet, packlen);
}
--
1.7.6.3
[-- Attachment #4: 0003-arping-use-seteuid-for-temporal-uid-changes.patch --]
[-- Type: text/x-diff, Size: 609 bytes --]
>From c4b6600fe72e169c64e47f85a973484053f23861 Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <yumkam@gmail.com>
Date: Wed, 2 Jan 2013 01:46:44 +0400
Subject: [PATCH 3/7] arping: use seteuid for temporal uid changes
---
arping.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arping.c b/arping.c
index bdf81e9..a35fafd 100644
--- a/arping.c
+++ b/arping.c
@@ -228,7 +228,7 @@ int modify_capability_raw(int on)
cap_free(cap_p);
#else
- if (setuid(on ? euid : getuid())) {
+ if (seteuid(on ? euid : getuid())) {
perror("arping: setuid");
return -1;
}
--
1.7.6.3
[-- Attachment #5: 0004-arping-ping_common-reset-euid-before-permanent-drop.patch --]
[-- Type: text/x-diff, Size: 1025 bytes --]
>From d8f54230f344c92f2120230dc24ac9c5d6672da9 Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <yumkam@gmail.com>
Date: Wed, 2 Jan 2013 01:53:39 +0400
Subject: [PATCH 4/7] arping, ping_common: reset euid before permanent drop
setuid drop saved uid only if euid is 0
---
arping.c | 4 ++++
ping_common.c | 4 ++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arping.c b/arping.c
index a35fafd..0033f33 100644
--- a/arping.c
+++ b/arping.c
@@ -269,6 +269,10 @@ void drop_capabilities(void)
cap_free(cap_p);
#else
+ if (seteuid(euid)) {
+ perror("arping: setuid");
+ return -1;
+ }
if (setuid(getuid()) < 0) {
perror("arping: setuid");
exit(-1);
diff --git a/ping_common.c b/ping_common.c
index 12c87a4..39b2c74 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -199,6 +199,10 @@ void drop_capabilities(void)
}
cap_free(cap);
#else
+ if (seteuid(euid)) {
+ perror("seteuid");
+ exit(-1);
+ }
if (setuid(getuid())) {
perror("ping: setuid");
exit(-1);
--
1.7.6.3
[-- Attachment #6: 0005-ping-ping6-arping-clockdiff-Fix-CAP_SETUID-setuid-in.patch --]
[-- Type: text/x-diff, Size: 2783 bytes --]
>From 00f63e1afc9d07e9793574304ef34a5bb54629b7 Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <yumkam@gmail.com>
Date: Wed, 2 Jan 2013 01:56:07 +0400
Subject: [PATCH 5/7] ping, ping6, arping, clockdiff: Fix CAP_SETUID <->
setuid() interaction
setuid() only drops saved uid if process have CAP_SETUID.
Drop capabilities only after setuid().
---
arping.c | 30 +++++++++++++++---------------
clockdiff.c | 8 +++++---
ping_common.c | 30 +++++++++++++++---------------
3 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/arping.c b/arping.c
index 0033f33..3c02abf 100644
--- a/arping.c
+++ b/arping.c
@@ -161,6 +161,21 @@ void limit_capabilities(void)
#ifdef CAPABILITIES
cap_t cap_p;
+ if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
+ perror("arping: prctl");
+ exit(-1);
+ }
+
+ if (setuid(getuid()) < 0) {
+ perror("arping: setuid");
+ exit(-1);
+ }
+
+ if (prctl(PR_SET_KEEPCAPS, 0) < 0) {
+ perror("arping: prctl");
+ exit(-1);
+ }
+
cap_p = cap_get_proc();
if (!cap_p) {
perror("arping: cap_get_proc");
@@ -184,21 +199,6 @@ void limit_capabilities(void)
}
}
- if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
- perror("arping: prctl");
- exit(-1);
- }
-
- if (setuid(getuid()) < 0) {
- perror("arping: setuid");
- exit(-1);
- }
-
- if (prctl(PR_SET_KEEPCAPS, 0) < 0) {
- perror("arping: prctl");
- exit(-1);
- }
-
cap_free(cap_p);
#else
euid = geteuid();
diff --git a/clockdiff.c b/clockdiff.c
index f12da2d..540366d 100644
--- a/clockdiff.c
+++ b/clockdiff.c
@@ -536,6 +536,11 @@ usage() {
}
void drop_rights(void) {
+ if (setuid(getuid())) {
+ perror("clockdiff: setuid");
+ exit(-1);
+ }
+ {
#ifdef CAPABILITIES
cap_t caps = cap_init();
if (cap_set_proc(caps)) {
@@ -544,9 +549,6 @@ void drop_rights(void) {
}
cap_free(caps);
#endif
- if (setuid(getuid())) {
- perror("clockdiff: setuid");
- exit(-1);
}
}
diff --git a/ping_common.c b/ping_common.c
index 39b2c74..08c8582 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -80,6 +80,21 @@ void limit_capabilities(void)
cap_t cap_p;
cap_flag_value_t cap_ok;
+ if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
+ perror("ping: prctl");
+ exit(-1);
+ }
+
+ if (setuid(getuid()) < 0) {
+ perror("setuid");
+ exit(-1);
+ }
+
+ if (prctl(PR_SET_KEEPCAPS, 0) < 0) {
+ perror("ping: prctl");
+ exit(-1);
+ }
+
cap_cur_p = cap_get_proc();
if (!cap_cur_p) {
perror("ping: cap_get_proc");
@@ -109,21 +124,6 @@ void limit_capabilities(void)
exit(-1);
}
- if (prctl(PR_SET_KEEPCAPS, 1) < 0) {
- perror("ping: prctl");
- exit(-1);
- }
-
- if (setuid(getuid()) < 0) {
- perror("setuid");
- exit(-1);
- }
-
- if (prctl(PR_SET_KEEPCAPS, 0) < 0) {
- perror("ping: prctl");
- exit(-1);
- }
-
cap_free(cap_p);
cap_free(cap_cur_p);
#endif
--
1.7.6.3
[-- Attachment #7: 0006-ninfod-use-u-without-capabilities-too.patch --]
[-- Type: text/x-diff, Size: 662 bytes --]
>From 32ad925cba9aca3513561c7655c9f61642a341ca Mon Sep 17 00:00:00 2001
From: "Yuriy M. Kaminskiy" <yumkam@gmail.com>
Date: Wed, 2 Jan 2013 02:57:15 +0400
Subject: [PATCH 6/7] ninfod: use -u without capabilities too
---
ninfod/ninfod.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/ninfod/ninfod.c b/ninfod/ninfod.c
index d1b99d9..0f54da3 100644
--- a/ninfod/ninfod.c
+++ b/ninfod/ninfod.c
@@ -561,7 +561,7 @@ static void drop_capabilities(void)
cap_free(cap_p);
#else
- if (setuid(getuid()) < 0) {
+ if (setuid(opt_u ? opt_u : getuid()) < 0) {
DEBUG(LOG_ERR, "setuid: %s\n", strerror(errno));
exit(-1);
}
--
1.7.6.3
[-- Attachment #8: 0007-ninfod-fix-capabilities-setting.patch --]
[-- Type: text/x-diff, Size: 3169 bytes --]
>From ab1b55fd572c1e28f762ef4feff911053c5eb1fd 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 0f54da3..3e24c18 100644
--- a/ninfod/ninfod.c
+++ b/ninfod/ninfod.c
@@ -469,16 +469,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) {
@@ -492,32 +504,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
}
@@ -532,28 +532,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);
@@ -637,14 +615,14 @@ int main (int argc, char **argv)
appname = argv[0];
+ 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
reply other threads:[~2013-01-09 19:44 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='kckh4a$taf$1@ger.gmane.org' \
--to=yumkam@gmail.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.