libnetfilter_conntrack checks for (getuid() == 0)

libnetfilter_conntrack checks for (getuid() == 0)

[Sebastian Hagen]

I'm in the process of writing a program that depends on
libnetfilter_conntrack (currently using the current version, that is svn
revision 6663), and have run into an annoyance.
Obviously interfacing with the ip_conntrack_netlink module requires elevated
privileges; I'm not quite certain what the required set of required
privileges for initializing the socket is, but after that CAP_NET_ADMIN is
definitely sufficient for using dump_conntrack_table().

None of these operations, afaict, really require the process to have an uid
of 0. Unfortunately libnetfilter_conntrack checks for that anyway,
specifically in nfct_event_conntrack() and nfct_event_expectation(). The
specific code is in both cases:

if (getuid() != 0)
   return -EPERM;

The actual useful code of these functions appears to me to be a strict
subset of that of dump_conntrack_table(); so since dump_conntrack_table()
continues to work with only CAP_NET_ADMIN, so should nfct_event_conntrack()
and nfct_event_expectation().
Additionally, if one does drop CAP_NET_ADMIN from the effective capability
set, dump_conntrack_table() will return the error correctly.

IMHO, these explicit checks for getuid() == 0...
a) are wrong as they prevent the library user from dropping 'privileges'
(uid==0 isn't strictly a privilege, but considering the file ownership on
many systems, it might as well be) they really should be able to drop

b) allow false negatives as uid 0 processes don't necessarily have CAP_NET_ADMIN

c) are afaict completely useless in any event, since nfnl_listen() will fail
correctly in the absence of CAP_NET_ADMIN

...and should therefore be removed.

Please do correct me if I'm mistaken about any of this.
If I'm not, should I make a patch for this? Since the fix would simply
consist of removing the four mentioned lines from the source, doing that
would be trivial.

Sebastian Hagen

Re: libnetfilter_conntrack checks for (getuid() == 0)

[Patrick McHardy]

Sebastian Hagen wrote:
> I'm in the process of writing a program that depends on
> libnetfilter_conntrack (currently using the current version, that is svn
> revision 6663), and have run into an annoyance.
> Obviously interfacing with the ip_conntrack_netlink module requires elevated
> privileges; I'm not quite certain what the required set of required
> privileges for initializing the socket is, but after that CAP_NET_ADMIN is
> definitely sufficient for using dump_conntrack_table().
> 
> None of these operations, afaict, really require the process to have an uid
> of 0. Unfortunately libnetfilter_conntrack checks for that anyway,
> specifically in nfct_event_conntrack() and nfct_event_expectation(). The
> specific code is in both cases:
> 
> if (getuid() != 0)
>    return -EPERM;
> 
> The actual useful code of these functions appears to me to be a strict
> subset of that of dump_conntrack_table(); so since dump_conntrack_table()
> continues to work with only CAP_NET_ADMIN, so should nfct_event_conntrack()
> and nfct_event_expectation().
> Additionally, if one does drop CAP_NET_ADMIN from the effective capability
> set, dump_conntrack_table() will return the error correctly.
> 
> IMHO, these explicit checks for getuid() == 0...
> a) are wrong as they prevent the library user from dropping 'privileges'
> (uid==0 isn't strictly a privilege, but considering the file ownership on
> many systems, it might as well be) they really should be able to drop
> 
> b) allow false negatives as uid 0 processes don't necessarily have CAP_NET_ADMIN
> 
> c) are afaict completely useless in any event, since nfnl_listen() will fail
> correctly in the absence of CAP_NET_ADMIN
> 
> ...and should therefore be removed.

Fully agreed.

> Please do correct me if I'm mistaken about any of this.
> If I'm not, should I make a patch for this? Since the fix would simply
> consist of removing the four mentioned lines from the source, doing that
> would be trivial.


Please send a patch.

Re: libnetfilter_conntrack checks for (getuid() == 0)

[Sebastian Hagen]

[-- Attachment #1: Type: text/plain, Size: 96 bytes --]

Patrick McHardy wrote:
> Please send a patch.
Done, and attached to this mail.

Sebastian Hagen

[-- Attachment #2: libnetfilter_conntrack_getuid.patch --]
[-- Type: text/x-patch, Size: 784 bytes --]

--- libnetfilter_conntrack/src/libnetfilter_conntrack.c	2006-05-24 19:39:29.000000000 +0200
+++ libnetfilter_conntrack_patched/src/libnetfilter_conntrack.c	2006-08-15 19:41:25.000000000 +0200
@@ -1125,12 +1125,6 @@
 
 int nfct_event_conntrack(struct nfct_handle *cth)
 {
-	/*
-	 * You need to be root to listen to conntrack events
-	 */
-	if (getuid() != 0)
-		return -EPERM;
-	
 	cth->handler = nfct_conntrack_netlink_handler;
 	return nfnl_listen(cth->nfnlh, &callback_handler, cth);
 }
@@ -1298,12 +1292,6 @@
 
 int nfct_event_expectation(struct nfct_handle *cth)
 {
-	/*
-	 * You need to be root to listen to conntrack events
-	 */
-	if (getuid() != 0)
-		return -EPERM;
-
 	cth->handler = nfct_expect_netlink_handler;
 	return nfnl_listen(cth->nfnlh, &callback_handler, cth);
 }

Re: libnetfilter_conntrack checks for (getuid() == 0)

[Patrick McHardy]

Sebastian Hagen wrote:
> Patrick McHardy wrote:
> 
>>Please send a patch.
> 
> Done, and attached to this mail.

Thanks, applied.