linux-kernel - Re: [PATCH 2/3] net: netfilter: Add RFC-7597 Section 5.1 PSID support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210716151833.GD9904@breakpoint.cc>
Date:   Fri, 16 Jul 2021 17:18:33 +0200
From:   Florian Westphal <fw@...len.de>
To:     Cole Dishington <Cole.Dishington@...iedtelesis.co.nz>
Cc:     pablo@...filter.org, kadlec@...filter.org, fw@...len.de,
        davem@...emloft.net, kuba@...nel.org, shuah@...nel.org,
        linux-kernel@...r.kernel.org, netfilter-devel@...r.kernel.org,
        coreteam@...filter.org, netdev@...r.kernel.org,
        linux-kselftest@...r.kernel.org,
        Anthony Lineham <anthony.lineham@...iedtelesis.co.nz>,
        Scott Parlane <scott.parlane@...iedtelesis.co.nz>,
        Blair Steven <blair.steven@...iedtelesis.co.nz>
Subject: Re: [PATCH 2/3] net: netfilter: Add RFC-7597 Section 5.1 PSID support

Cole Dishington <Cole.Dishington@...iedtelesis.co.nz> wrote:
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 7de595ead06a..4a9448684504 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -195,13 +195,36 @@ static bool nf_nat_inet_in_range(const struct nf_conntrack_tuple *t,
>  static bool l4proto_in_range(const struct nf_conntrack_tuple *tuple,
>  			     enum nf_nat_manip_type maniptype,
>  			     const union nf_conntrack_man_proto *min,
> -			     const union nf_conntrack_man_proto *max)
> +			     const union nf_conntrack_man_proto *max,
> +			     const union nf_conntrack_man_proto *base,
> +			     bool is_psid)
>  {
>  	__be16 port;
> +	u16 psid, psid_mask, offset_mask;
> +
> +	/* In this case we are in PSID mode, avoid checking all ranges by computing bitmasks */
> +	if (is_psid) {
> +		u16 power_j = ntohs(max->all) - ntohs(min->all) + 1;
> +		u32 offset = ntohs(base->all);
> +		u16 power_a;
> +
> +		if (offset == 0)
> +			offset = 1 << 16;
> +
> +		power_a = (1 << 16) / offset;

Since the dividie is only needed nat setup and not for each packet I
think its ok.

> +	if (range->flags & NF_NAT_RANGE_PSID) {
> +		u16 base = ntohs(range->base_proto.all);
> +		u16 min =  ntohs(range->min_proto.all);
> +		u16 off = 0;
> +
> +		/* If offset=0, port range is in one contiguous block */
> +		if (base)
> +			off = prandom_u32() % (((1 << 16) / base) - 1);

Bases 32769 > gives 0 for the modulo value, so perhaps compute that
independently.

You could reject > 32769 in the iptables checkentry target.

Also, base of 21846 and above always give 0 result (% 1).

I don't know psid well enough to give a recommendation here.

If such inputs are nonsensical, just reject it when userspace asks for
this and add a 

if (WARN_ON_ONCE(base > bogus))
	return NF_DROP;

with s small coment explaining that xtables is supposed to not provide
such value.

Other than this I think its ok.

I still dislike the 'bool is_psid' in the nat core, but I can't find
a better solution.