Re: [iptables PATCH v3] iptables: accept lock file name at runtime

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

On Fri, Jul 17, 2020 at 10:39:40AM +0200, Giuseppe Scrivano wrote:
> allow users to override at runtime the lock file to use through the
> XTABLES_LOCKFILE environment variable.
> 
> It allows to use iptables when the user has granted enough
> capabilities (e.g. a user+network namespace) to configure the network
> but that lacks access to the XT_LOCK_NAME (by default placed under
> /run).
> 
> $ XTABLES_LOCKFILE=/tmp/xtables unshare -rn iptables ...
> 
> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
> ---
>  configure.ac           |  1 +
>  iptables/iptables.8.in |  8 ++++++++
>  iptables/xshared.c     | 11 ++++++++---
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 31a8bb26..d37752a2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -219,6 +219,7 @@ AC_SUBST([libxtables_vmajor])
>  
>  AC_DEFINE_UNQUOTED([XT_LOCK_NAME], "${xt_lock_name}",
>  	[Location of the iptables lock file])
> +AC_SUBST([XT_LOCK_NAME], "${xt_lock_name}")
>  
>  AC_CONFIG_FILES([Makefile extensions/GNUmakefile include/Makefile
>  	iptables/Makefile iptables/xtables.pc
> diff --git a/iptables/iptables.8.in b/iptables/iptables.8.in
> index 054564b3..999cf339 100644
> --- a/iptables/iptables.8.in
> +++ b/iptables/iptables.8.in
> @@ -397,6 +397,14 @@ corresponding to that rule's position in the chain.
>  \fB\-\-modprobe=\fP\fIcommand\fP
>  When adding or inserting rules into a chain, use \fIcommand\fP
>  to load any necessary modules (targets, match extensions, etc).
> +
> +.SH LOCK FILE
> +iptables uses the \fI@XT_LOCK_NAME@\fP file to take an exclusive lock at
> +launch.
> +
> +The \fBXTABLES_LOCKFILE\fP environment variable can be used to override
> +the default setting.
> +
>  .SH MATCH AND TARGET EXTENSIONS
>  .PP
>  iptables can use extended packet matching and target modules.
> diff --git a/iptables/xshared.c b/iptables/xshared.c
> index c1d1371a..7d97637f 100644
> --- a/iptables/xshared.c
> +++ b/iptables/xshared.c
> @@ -249,15 +249,20 @@ 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')

Probably remove the check for lock_file[0] == '\0'

Or is this intentional?

git grep getenv in iptables does not show any similar handling for
getenv().

Thanks.