NICMx / Jool

SIIT and NAT64 for Linux
GNU General Public License v2.0
320 stars 66 forks source link

See if we can safely make `jool stats` not require CAP_NET_ADMIN / root #350

Closed cooperlees closed 3 years ago

cooperlees commented 3 years ago

Today to read stats you need to be root OR give the process the CAP_NET_ADMIN Linux capability.

I believe we could make the jool binary smart enough to know it does not need this to read netlink data.

I am not sure if this makes sense for iptables or in future nftables (have done no research here as I only run via netfilter).

cooperlees commented 3 years ago

A quick no compile test shows we could possibly do something like this:

diff --git a/src/mod/common/nl/nl_common.c b/src/mod/common/nl/nl_common.c
index 6aa7dac3..d5f6468c 100644
--- a/src/mod/common/nl/nl_common.c
+++ b/src/mod/common/nl/nl_common.c
@@ -56,12 +56,14 @@ static int validate_version(struct joolnlhdr *hdr)
        return -EINVAL;
 }

-int request_handle_start(struct genl_info *info, xlator_type xt, struct xlator *jool)
+int request_handle_start(
+       struct genl_info *info, xlator_type xt, struct xlator *jool, bool require_net_admin=true
+)
 {
        struct joolnlhdr *hdr;
        int error;

-       if (!capable(CAP_NET_ADMIN)) {
+       if (require_net_admin && !capable(CAP_NET_ADMIN)) {
                log_err("CAP_NET_ADMIN capability required. (Maybe try su or sudo?)");
                return -EPERM;
        }
diff --git a/src/mod/common/nl/nl_common.h b/src/mod/common/nl/nl_common.h
index b70b038b..dc437f5a 100644
--- a/src/mod/common/nl/nl_common.h
+++ b/src/mod/common/nl/nl_common.h
@@ -8,7 +8,7 @@
 char *get_iname(struct genl_info *info);
 struct joolnlhdr *get_jool_hdr(struct genl_info *info);

-int request_handle_start(struct genl_info *info, xlator_type xt, struct xlator *jool);
+int request_handle_start(struct genl_info *info, xlator_type xt, struct xlator *jool, bool require_net_admin);
 void request_handle_end(struct xlator *jool);

 #endif /* SRC_MOD_COMMON_NL_COMMON_H_ */
diff --git a/src/mod/common/nl/stats.c b/src/mod/common/nl/stats.c
index 5786449b..95399048 100644
--- a/src/mod/common/nl/stats.c
+++ b/src/mod/common/nl/stats.c
@@ -15,7 +15,7 @@ int handle_stats_foreach(struct sk_buff *skb, struct genl_info *info)
        unsigned int written;
        int error;

-       error = request_handle_start(info, XT_ANY, &jool);
+       error = request_handle_start(info, XT_ANY, &jool, false);
        if (error)
                return jresponse_send_simple(NULL, info, error);

I'm sure there is more to it.

cooperlees commented 3 years ago

Sweet - Thanks!

Will add a comment stating this in my projects documentation and keep an eye out for this version to hit Ubuntu (my home routers).