| Message ID | 20200715065152.4172896-1-gscrivan@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [iptables,v2] iptables: accept lock file name at runtime | expand |
Hi, On Wed, Jul 15, 2020 at 08:51:52AM +0200, Giuseppe Scrivano wrote: > allow users to override at runtime the lock file to use through the > XTABLES_LOCKFILE environment variable. > > It allows using iptables from a network namespace owned by an user > that has no write access to XT_LOCK_NAME (by default under /run), and > without setting up a new mount namespace. This sentence appears overly complicated to me. Isn't the problem just that XT_LOCK_NAME may not be writeable? That "user that has no write access" is typically root anyway as iptables doesn't support being called by non-privileged UIDs. > $ XTABLES_LOCKFILE=/tmp/xtables unshare -rn iptables ... > > Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> > --- > iptables/xshared.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) Could you please update the man page as well? Unless you clarify why this should be a hidden feature, of course. :) Cheers, Phil
Hi Phil, thanks for the review. Phil Sutter <phil@nwl.cc> writes: > Hi, > > On Wed, Jul 15, 2020 at 08:51:52AM +0200, Giuseppe Scrivano wrote: >> allow users to override at runtime the lock file to use through the >> XTABLES_LOCKFILE environment variable. >> >> It allows using iptables from a network namespace owned by an user >> that has no write access to XT_LOCK_NAME (by default under /run), and >> without setting up a new mount namespace. > > This sentence appears overly complicated to me. Isn't the problem just > that XT_LOCK_NAME may not be writeable? That "user that has no write > access" is typically root anyway as iptables doesn't support being > called by non-privileged UIDs. I'll rephrase it but it is really about the user not having access to the lock file. Without involving user namespaces, a simple reproducer for the issue can be: $ caps="cap_net_admin,cap_net_raw,cap_setpcap,cap_setuid,cap_setgid" $ capsh --caps="$caps"+eip --keep=1 --gid=1000 --uid=1000 \ --addamb="$caps" \ -- -c "iptables -F ..." iptables seems to work fine even if the user is not running as root, as long as enough capabilities are granted. >> $ XTABLES_LOCKFILE=/tmp/xtables unshare -rn iptables ... >> >> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> >> --- >> iptables/xshared.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) > > Could you please update the man page as well? Unless you clarify why > this should be a hidden feature, of course. :) sure, I'll send a v3 shortly. Giuseppe
diff --git a/iptables/xshared.c b/iptables/xshared.c index c1d1371a..caf1dfcc 100644 --- a/iptables/xshared.c +++ b/iptables/xshared.c @@ -249,12 +249,17 @@ void xs_init_match(struct xtables_match *match) static int xtables_lock(int wait, struct timeval *wait_interval) { struct timeval time_left, wait_time; + const char *lock_file; int fd, i = 0; time_left.tv_sec = wait; time_left.tv_usec = 0; - fd = open(XT_LOCK_NAME, O_CREAT, 0600); + lock_file = getenv("XTABLES_LOCKFILE"); + if (lock_file == NULL || lock_file[0] == '\0') + lock_file = XT_LOCK_NAME; + + fd = open(lock_file, O_CREAT, 0600); if (fd < 0) { fprintf(stderr, "Fatal: can't open lock file %s: %s\n", XT_LOCK_NAME, strerror(errno));
allow users to override at runtime the lock file to use through the XTABLES_LOCKFILE environment variable. It allows using iptables from a network namespace owned by an user that has no write access to XT_LOCK_NAME (by default under /run), and without setting up a new mount namespace. $ XTABLES_LOCKFILE=/tmp/xtables unshare -rn iptables ... Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com> --- iptables/xshared.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)