[PATCH nf-next] netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

[PATCH nf-next] netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

[Subash Abhinov Kasiviswanathan]

conntrack defrag is needed only if some module like CONNTRACK or NAT
explicitly requests it. For plain forwarding scenarios, defrag is
not needed and can be skipped if NOTRACK is set in a rule.

Since conntrack defrag is currently higher priority than raw table,
setting NOTRACK is not sufficient. We need to introduce a lower
priority rule and deprecate the existing rule.

Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 include/uapi/linux/netfilter_ipv4.h | 3 ++-
 net/ipv4/netfilter/nf_defrag_ipv4.c | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/netfilter_ipv4.h b/include/uapi/linux/netfilter_ipv4.h
index e6b1a84..cedf7b4 100644
--- a/include/uapi/linux/netfilter_ipv4.h
+++ b/include/uapi/linux/netfilter_ipv4.h
@@ -57,9 +57,10 @@
 
 enum nf_ip_hook_priorities {
 	NF_IP_PRI_FIRST = INT_MIN,
-	NF_IP_PRI_CONNTRACK_DEFRAG = -400,
+	NF_IP_PRI_CONNTRACK_DEFRAG = -400, /* deprecated */
 	NF_IP_PRI_RAW = -300,
 	NF_IP_PRI_SELINUX_FIRST = -225,
+	NF_IP_PRI_CONNTRACK_DEFRAG_V1 = -210,
 	NF_IP_PRI_CONNTRACK = -200,
 	NF_IP_PRI_MANGLE = -150,
 	NF_IP_PRI_NAT_DST = -100,
diff --git a/net/ipv4/netfilter/nf_defrag_ipv4.c b/net/ipv4/netfilter/nf_defrag_ipv4.c
index 37fe1616..6496890 100644
--- a/net/ipv4/netfilter/nf_defrag_ipv4.c
+++ b/net/ipv4/netfilter/nf_defrag_ipv4.c
@@ -80,7 +80,7 @@ static unsigned int ipv4_conntrack_defrag(void *priv,
 #endif
 #endif
 	/* Gather fragments. */
-	if (ip_is_fragment(ip_hdr(skb))) {
+	if (skb->_nfct != IP_CT_UNTRACKED && ip_is_fragment(ip_hdr(skb))) {
 		enum ip_defrag_users user =
 			nf_ct_defrag_user(state->hook, skb);
 
@@ -95,13 +95,13 @@ static unsigned int ipv4_conntrack_defrag(void *priv,
 		.hook		= ipv4_conntrack_defrag,
 		.pf		= NFPROTO_IPV4,
 		.hooknum	= NF_INET_PRE_ROUTING,
-		.priority	= NF_IP_PRI_CONNTRACK_DEFRAG,
+		.priority	= NF_IP_PRI_CONNTRACK_DEFRAG_V1,
 	},
 	{
 		.hook           = ipv4_conntrack_defrag,
 		.pf             = NFPROTO_IPV4,
 		.hooknum        = NF_INET_LOCAL_OUT,
-		.priority       = NF_IP_PRI_CONNTRACK_DEFRAG,
+		.priority       = NF_IP_PRI_CONNTRACK_DEFRAG_V1,
 	},
 };
 
-- 
1.9.1

Re: [PATCH nf-next] netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

[Pablo Neira Ayuso]

Hi Subash,

Sorry it took a while, but I've been discussing this with Jozsef too,
he's now on Cc.

On Mon, Nov 13, 2017 at 06:37:26PM -0700, Subash Abhinov Kasiviswanathan wrote:
> conntrack defrag is needed only if some module like CONNTRACK or NAT
> explicitly requests it. For plain forwarding scenarios, defrag is
> not needed and can be skipped if NOTRACK is set in a rule.
> 
> Since conntrack defrag is currently higher priority than raw table,
> setting NOTRACK is not sufficient. We need to introduce a lower
> priority rule and deprecate the existing rule.
> 
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
> ---
>  include/uapi/linux/netfilter_ipv4.h | 3 ++-
>  net/ipv4/netfilter/nf_defrag_ipv4.c | 6 +++---
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/include/uapi/linux/netfilter_ipv4.h b/include/uapi/linux/netfilter_ipv4.h
> index e6b1a84..cedf7b4 100644
> --- a/include/uapi/linux/netfilter_ipv4.h
> +++ b/include/uapi/linux/netfilter_ipv4.h
> @@ -57,9 +57,10 @@
>  
>  enum nf_ip_hook_priorities {
>  	NF_IP_PRI_FIRST = INT_MIN,
> -	NF_IP_PRI_CONNTRACK_DEFRAG = -400,
> +	NF_IP_PRI_CONNTRACK_DEFRAG = -400, /* deprecated */
>  	NF_IP_PRI_RAW = -300,
>  	NF_IP_PRI_SELINUX_FIRST = -225,
> +	NF_IP_PRI_CONNTRACK_DEFRAG_V1 = -210,

Would it work for you if this is specific via global modparam? I'm
telling this because:

1) This is changing the default behaviour, which is always tricky.
2) This is already solved in nftables, so whatever solution that we
   apply, it should be iptables specific.

If modparam is fine, just placing a line into
/etc/modprobe.d/options.conf (or similar) should be good enough to
store that you're requesting raw hook registration before defrag.

Let me know,
Thanks!

Re: [PATCH nf-next] netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

[Subash Abhinov Kasiviswanathan]

> Would it work for you if this is specific via global modparam? I'm
> telling this because:
> 
> 1) This is changing the default behaviour, which is always tricky.
> 2) This is already solved in nftables, so whatever solution that we
>    apply, it should be iptables specific.
> 
> If modparam is fine, just placing a line into
> /etc/modprobe.d/options.conf (or similar) should be good enough to
> store that you're requesting raw hook registration before defrag.
> 
> Let me know,
> Thanks!

Hi Pablo

Can you explain a bit more about the /etc/modprobe.d/ option and how
it would be configured for this? /etc/modprobe.d/ doesnt exist on
Android based Linux systems so it might be a problem for me.

Would it be an acceptable solution to create a kernel config for this
particular feature instead?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH nf-next] netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

[Pablo Neira Ayuso]

On Sat, Dec 09, 2017 at 07:06:14PM -0700, Subash Abhinov Kasiviswanathan wrote:
> >Would it work for you if this is specific via global modparam? I'm
> >telling this because:
> >
> >1) This is changing the default behaviour, which is always tricky.
> >2) This is already solved in nftables, so whatever solution that we
> >   apply, it should be iptables specific.
> >
> >If modparam is fine, just placing a line into
> >/etc/modprobe.d/options.conf (or similar) should be good enough to
> >store that you're requesting raw hook registration before defrag.
> >
> >Let me know,
> >Thanks!
> 
> Hi Pablo
> 
> Can you explain a bit more about the /etc/modprobe.d/ option and how
> it would be configured for this? /etc/modprobe.d/ doesnt exist on
> Android based Linux systems so it might be a problem for me.
> 
> Would it be an acceptable solution to create a kernel config for this
> particular feature instead?

I'm actually refering to module_param(), that is specified at modprobe
time. Such parameter would set an alternative hook priority for the
raw table, ie. before the defrag hook. I guess there must be a way to
store these module parameters in Android, so whenever modprobe is
invoked, either explicitly or via module autoload, this module
parameter is passed to the iptable_raw module.

Re: [PATCH nf-next] netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

[Subash Abhinov Kasiviswanathan]

> I'm actually refering to module_param(), that is specified at modprobe
> time. Such parameter would set an alternative hook priority for the
> raw table, ie. before the defrag hook. I guess there must be a way to
> store these module parameters in Android, so whenever modprobe is
> invoked, either explicitly or via module autoload, this module
> parameter is passed to the iptable_raw module.

Thanks for the clarification Pablo. I'll try this and send a new patch.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project