Re: [PATCH iptables v2 2/2] iptables-restore: support acquiring the lock.

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Lorenzo,

On Thu, Mar 16, 2017 at 04:55:02PM +0900, Lorenzo Colitti wrote:
> Currently, ip[6]tables-restore does not perform any locking, so it
> is not safe to use concurrently with ip[6]tables.
> 
> This patch makes ip[6]tables-restore wait for the lock if -w
> was specified. Arguments to -w and -W are supported in the same
> was as they are in ip[6]tables.
> 
> The lock is not acquired on startup. Instead, it is acquired when
> a new table handle is created (on encountering '*') and released
> when the table is committed (COMMIT). This makes it possible to
> keep long-running iptables-restore processes in the background
> (for example, reading commands from a pipe opened by a system
> management daemon) and simultaneously run iptables commands.
> 
> If -w is not specified, then the command proceeds without taking
> the lock.
> 
> Tested as follows:
> 
> 1. Run iptables-restore -w, and check that iptables commands work
>    with or without -w.
> 2. Type "*filter" into the iptables-restore input. Verify that
>    a) ip[6]tables commands without -w fail with "another app is
>       currently holding the xtables lock...".
>    b) ip[6]tables commands with "-w 2" fail after 2 seconds.
>    c) ip[6]tables commands with "-w" hang until "COMMIT" is
>       typed into the iptables-restore window.
> 3. With the lock held by an ip6tables-restore process:
>      strace -e flock /tmp/iptables/sbin/iptables-restore -w 1 -W 100000
>    shows 11 calls to flock and fails.
> 4. Run an iptables-restore with -w and one without -w, and check:
>    a) Type "*filter" in the first and then the second, and the
>       second exits with an error.
>    b) Type "*filter" in the second and "*filter" "-S" "COMMIT"
>       into the first. The rules are listed only when the first
>       copy sees "COMMIT".

My concern with this is that one iptables-restore instance may
postpone any other iptables call indefinitely, by simply typing
"*filter" with no COMMIT ever.

The other instance can simply kill this process, as they would be both
running with CAP_NET_ADMIN though.