[PATCH] ctnetlink: Make expect events work

[PATCH] ctnetlink: Make expect events work

[Marcus Sundberg]

[-- Attachment #1: Type: text/plain, Size: 319 bytes --]

Hi,

expectation events are improperly tagged in the current ctnetlink
code, this patch makes them work.

//Marcus
-- 
---------------------------------------+--------------------------
   Marcus Sundberg <marcus@ingate.com>  | Firewalls with SIP & NAT
  Software Developer, Ingate Systems AB |  http://www.ingate.com/

[-- Attachment #2: ctnetlink-expect-event.diff --]
[-- Type: text/x-patch, Size: 631 bytes --]

[NETFILTER] ctnetlink: Make expect events work

The ctnetlink expectation events should use the NFNL_SUBSYS_CTNETLINK_EXP
subsystem, not NFNL_SUBSYS_CTNETLINK.

Signed-off-by: Marcus Sundberg <marcus@ingate.com>

--- linux-2.6.15/net/ipv4/netfilter/ip_conntrack_netlink.c	2006/01/12 18:59:57	1.1
+++ linux-2.6.15/net/ipv4/netfilter/ip_conntrack_netlink.c	2006/01/12 19:00:03
@@ -1223,7 +1223,7 @@ static int ctnetlink_expect_event(struct
 
 	b = skb->tail;
 
-	type |= NFNL_SUBSYS_CTNETLINK << 8;
+	type |= NFNL_SUBSYS_CTNETLINK_EXP << 8;
 	nlh   = NLMSG_PUT(skb, 0, 0, type, sizeof(struct nfgenmsg));
 	nfmsg = NLMSG_DATA(nlh);
 

Re: [PATCH] ctnetlink: Make expect events work

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 334 bytes --]

Marcus Sundberg wrote:
> [NETFILTER] ctnetlink: Make expect events work
> 
> The ctnetlink expectation events should use the NFNL_SUBSYS_CTNETLINK_EXP
> subsystem, not NFNL_SUBSYS_CTNETLINK.

Thanks, applied with the same change in nfnetlink_conntrack.
For future patches please also check if bugs are present in
nfnetlink_conntrack.

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1678 bytes --]

[NETFILTER]: ctnetlink: Fix subsystem used for expectation events

The ctnetlink expectation events should use the NFNL_SUBSYS_CTNETLINK_EXP
subsystem, not NFNL_SUBSYS_CTNETLINK.

Signed-off-by: Marcus Sundberg <marcus@ingate.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 078ff587a861c7d2499990ca46766c030bae5ecc
tree 0064472eb1373f544ac136d3cd35194b6493a437
parent d98b092ef1a291d2fd31cbf1152bd16cca2e925d
author Marcus Sundberg <marcus@ingate.com> Thu, 12 Jan 2006 22:00:52 +0100
committer Patrick McHardy <kaber@trash.net> Thu, 12 Jan 2006 22:00:52 +0100

 net/ipv4/netfilter/ip_conntrack_netlink.c |    2 +-
 net/netfilter/nf_conntrack_netlink.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/ip_conntrack_netlink.c b/net/ipv4/netfilter/ip_conntrack_netlink.c
index c9ebbe0..b62518b 100644
--- a/net/ipv4/netfilter/ip_conntrack_netlink.c
+++ b/net/ipv4/netfilter/ip_conntrack_netlink.c
@@ -1216,7 +1216,7 @@ static int ctnetlink_expect_event(struct
 
 	b = skb->tail;
 
-	type |= NFNL_SUBSYS_CTNETLINK << 8;
+	type |= NFNL_SUBSYS_CTNETLINK_EXP << 8;
 	nlh   = NLMSG_PUT(skb, 0, 0, type, sizeof(struct nfgenmsg));
 	nfmsg = NLMSG_DATA(nlh);
 
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 73ab16b..e98d00c 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1232,7 +1232,7 @@ static int ctnetlink_expect_event(struct
 
 	b = skb->tail;
 
-	type |= NFNL_SUBSYS_CTNETLINK << 8;
+	type |= NFNL_SUBSYS_CTNETLINK_EXP << 8;
 	nlh   = NLMSG_PUT(skb, 0, 0, type, sizeof(struct nfgenmsg));
 	nfmsg = NLMSG_DATA(nlh);
 

Re: [PATCH] ctnetlink: Make expect events work

[Marcus Sundberg]

Patrick McHardy wrote:

> Marcus Sundberg wrote:
> 
>> [NETFILTER] ctnetlink: Make expect events work
>>
>> The ctnetlink expectation events should use the NFNL_SUBSYS_CTNETLINK_EXP
>> subsystem, not NFNL_SUBSYS_CTNETLINK.
> 
> Thanks, applied with the same change in nfnetlink_conntrack.
> For future patches please also check if bugs are present in
> nfnetlink_conntrack.

Thanks for the reminder, didn't even think about nfnetlink_conntrack.

//Marcus
-- 
---------------------------------------+--------------------------
  Marcus Sundberg <marcus@ingate.com>  | Firewalls with SIP & NAT
 Software Developer, Ingate Systems AB |  http://www.ingate.com/

Re: [PATCH] ctnetlink: Make expect events work

[Pablo Neira Ayuso]

Marcus Sundberg wrote:
> expectation events are improperly tagged in the current ctnetlink
> code, this patch makes them work.

Please, be more accurate with your descriptions. The expectation events
are working just fine since NFNL_SUBSYS_CTNETLINK and
NFNL_SUBSYS_CTNETLINK_EXP messages are handled similarly by
libnetfilter_conntrack.

I agree that it's improperly tagged, so I like the change, but it's not
a fix and it shouldn't go to -stable, it's a cleanup. Thanks!

-- 
Pablo

Re: [PATCH] ctnetlink: Make expect events work

[Marcus Sundberg]

Pablo Neira Ayuso wrote:
> Marcus Sundberg wrote:
> 
>>expectation events are improperly tagged in the current ctnetlink
>>code, this patch makes them work.
> 
> Please, be more accurate with your descriptions. The expectation events
> are working just fine since NFNL_SUBSYS_CTNETLINK and
> NFNL_SUBSYS_CTNETLINK_EXP messages are handled similarly by
> libnetfilter_conntrack.
> 
> I agree that it's improperly tagged, so I like the change, but it's not
> a fix and it shouldn't go to -stable, it's a cleanup. Thanks!

Hi,

I disagree with that. libnetfilter_conntrack happens to work because
it uses a different handler callback depending on what kind of data
it *expects* to receive, and lacks a check for what kind of data it
actually *is* receiving.

Without properly checking the subsystem there is no way of *knowing*
whether type 0 and attribute 1 in a netlink reply means IPCTNL_MSG_CT_NEW
and CTA_TUPLE_ORIG or if they mean IPCTNL_MSG_EXP_NEW and CTA_EXPECT_MASTER.
You can only guess based on what netlink requests you have previously
sent to the kernel and your knowledge about current kernel internals.

It may work, but it's not the way to make robust APIs.

//Marcus
-- 
---------------------------------------+--------------------------
   Marcus Sundberg <marcus@ingate.com>  | Firewalls with SIP & NAT
  Software Developer, Ingate Systems AB |  http://www.ingate.com/