verify_superpriv always fails #81

Closed
tadokoro opened this Issue Mar 11, 2014 · 1 comment

Comments

Projects
None yet
3 participants
@tadokoro
Contributor

tadokoro commented Mar 11, 2014

The verify_superpriv function in mod/config.c looks current->flags directly,
but I think it would not be appropriate for privilege check.
It would be correct to use capable(CAP_SYS_ADMIN) (or CAP_NET_ADMIN) insted.

In fact, the verify_superpriv always fails in my debian.

# jool -z --minMTU6=1500 
ERR1000: Operation not permitted (System error -28)
# dmesg | tail -n 1
[85541.977775] Administrative privileges required: 512

And the following patch seems to work correctly for me,
it succeeds when I'm a root and it fails when I'm a normal user.

diff --git a/mod/config.c b/mod/config.c
index e6bc9fb..7e1da2c 100644
--- a/mod/config.c
+++ b/mod/config.c
@@ -86,7 +86,7 @@ static int respond_ack(struct nlmsghdr *nl_hdr_in)

 static int verify_superpriv(struct request_hdr *nat64_hdr)
 {
-       if (!(current->flags & PF_SUPERPRIV)) {
+       if (!capable(CAP_SYS_ADMIN)) {
                log_warning("Administrative privileges required: %d", nat64_hdr->operation);
                return -EPERM;
        }
# jool -z --minMTU6=1500 
Value changed successfully.
@ydahhrk

This comment has been minimized.

Show comment
Hide comment
@ydahhrk

ydahhrk Mar 15, 2014

Member

Here's a slight update:

You're correct. We will upload your changes on monday so the code can be seen available before the release.
We plan to use CAP_NET_ADMIN instead of CAP_SYS_ADMIN, unless someone has reasons to disagree.

Thank you

Member

ydahhrk commented Mar 15, 2014

Here's a slight update:

You're correct. We will upload your changes on monday so the code can be seen available before the release.
We plan to use CAP_NET_ADMIN instead of CAP_SYS_ADMIN, unless someone has reasons to disagree.

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment