[PATCH] fix missing attribute in DCCP

[PATCH] fix missing attribute in DCCP

[Pablo Neira Ayuso]

Hi Patrick,

While finishing the DCCP support (I think it's the last to finish
the conntrack-tools), I notice that there's one field that is
not exported to user-space via ctnetlink.

This patch adds a couple of missing attributes that allows me to
set the role in DCCP connections, otherwise the ctnetlink support
is not of much help to be used by conntrackd.

This can be done with only one attribute, so we can have just one
CTA_PROTOINFO_DCCP_ROLE_ORIGINAL and set the reply as the opposite.
However, this may look at bit inconsistent. I don't really mind
the way this is done, just tell me I'll do it :), but I need this
to complete DCCP support.

BTW, I think that this can be qualified as bugfix for 2.6.30-rc
since the current support is not of any help to create
conntrack entries with DCCP.

---

Pablo Neira Ayuso (1):
      netfilter: conntrack: add missing role attributes for DCCP


 include/linux/netfilter/nfnetlink_conntrack.h |    2 ++
 net/netfilter/nf_conntrack_proto_dccp.c       |   13 ++++++++++++-
 2 files changed, 14 insertions(+), 1 deletions(-)

[PATCH] netfilter: conntrack: add missing role attributes for DCCP

[Pablo Neira Ayuso]

This patch adds missing role attributes to the DCCP type, otherwise
the creation of entries is not of any use.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---

 include/linux/netfilter/nfnetlink_conntrack.h |    2 ++
 net/netfilter/nf_conntrack_proto_dccp.c       |   13 ++++++++++++-
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_conntrack.h b/include/linux/netfilter/nfnetlink_conntrack.h
index 29fe9ea..4f967e2 100644
--- a/include/linux/netfilter/nfnetlink_conntrack.h
+++ b/include/linux/netfilter/nfnetlink_conntrack.h
@@ -100,6 +100,8 @@ enum ctattr_protoinfo_tcp {
 enum ctattr_protoinfo_dccp {
 	CTA_PROTOINFO_DCCP_UNSPEC,
 	CTA_PROTOINFO_DCCP_STATE,
+	CTA_PROTOINFO_DCCP_ROLE_ORIGINAL,
+	CTA_PROTOINFO_DCCP_ROLE_REPLY,
 	__CTA_PROTOINFO_DCCP_MAX,
 };
 #define CTA_PROTOINFO_DCCP_MAX (__CTA_PROTOINFO_DCCP_MAX - 1)
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index 50dac8d..eb851e3 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -633,6 +633,10 @@ static int dccp_to_nlattr(struct sk_buff *skb, struct nlattr *nla,
 	if (!nest_parms)
 		goto nla_put_failure;
 	NLA_PUT_U8(skb, CTA_PROTOINFO_DCCP_STATE, ct->proto.dccp.state);
+	NLA_PUT_U8(skb, CTA_PROTOINFO_DCCP_ROLE_ORIGINAL,
+		   ct->proto.dccp.role[IP_CT_DIR_ORIGINAL]);
+	NLA_PUT_U8(skb, CTA_PROTOINFO_DCCP_ROLE_REPLY,
+		   ct->proto.dccp.role[IP_CT_DIR_REPLY]);
 	nla_nest_end(skb, nest_parms);
 	read_unlock_bh(&dccp_lock);
 	return 0;
@@ -661,11 +665,18 @@ static int nlattr_to_dccp(struct nlattr *cda[], struct nf_conn *ct)
 		return err;
 
 	if (!tb[CTA_PROTOINFO_DCCP_STATE] ||
-	    nla_get_u8(tb[CTA_PROTOINFO_DCCP_STATE]) >= CT_DCCP_IGNORE)
+	    !tb[CTA_PROTOINFO_DCCP_ROLE_ORIGINAL] ||
+	    !tb[CTA_PROTOINFO_DCCP_ROLE_REPLY] ||
+	    nla_get_u8(tb[CTA_PROTOINFO_DCCP_STATE]) >= CT_DCCP_IGNORE) {
 		return -EINVAL;
+	}
 
 	write_lock_bh(&dccp_lock);
 	ct->proto.dccp.state = nla_get_u8(tb[CTA_PROTOINFO_DCCP_STATE]);
+	ct->proto.dccp.role[IP_CT_DIR_ORIGINAL] =
+		nla_get_u8(tb[CTA_PROTOINFO_DCCP_ROLE_ORIGINAL]);
+	ct->proto.dccp.role[IP_CT_DIR_REPLY] =
+		nla_get_u8(tb[CTA_PROTOINFO_DCCP_ROLE_REPLY]);
 	write_unlock_bh(&dccp_lock);
 	return 0;
 }

Re: [PATCH] fix missing attribute in DCCP

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Hi Patrick,
> 
> While finishing the DCCP support (I think it's the last to finish
> the conntrack-tools), I notice that there's one field that is
> not exported to user-space via ctnetlink.
> 
> This patch adds a couple of missing attributes that allows me to
> set the role in DCCP connections, otherwise the ctnetlink support
> is not of much help to be used by conntrackd.
> 
> This can be done with only one attribute, so we can have just one
> CTA_PROTOINFO_DCCP_ROLE_ORIGINAL and set the reply as the opposite.
> However, this may look at bit inconsistent. I don't really mind
> the way this is done, just tell me I'll do it :), but I need this
> to complete DCCP support.

Indeed, this looks like an unnecessary way to do something wrong for
userspace. Since the roles are always the opposite in each direction,
we might as well change the kernel to only keep track of the role
in the original direction and use !role for the reply direction.

> +	ct->proto.dccp.role[IP_CT_DIR_ORIGINAL] =
> +		nla_get_u8(tb[CTA_PROTOINFO_DCCP_ROLE_ORIGINAL]);
> +	ct->proto.dccp.role[IP_CT_DIR_REPLY] =
> +		nla_get_u8(tb[CTA_PROTOINFO_DCCP_ROLE_REPLY]);

Validation please, incorrect values might crash the kernel.