Skip to contentSkip to conversation

Change protocol private information

PATCH In review Pablo Neira 2 participants 2 messages Aug 5, 2005

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Change protocol private information

Hi Harald,

This is a resent. I'd like to discuss this a bit. Following the discusion left at:

[PATCH 4/7] Add ctnetlink_change_protoinfo

I think that users must be able to set the TCP state of a conntrack, otherwise if they create a conntrack via libnfnetlink_conntrack, the state will be set to NONE. If users aren't able to set the state, I'd consider that the library is kind of incomplete. I don't like the idea of kidding with stuff that could be compromising either, but I don't want to lose any features. So, I think that the solution is *remark* in the documentation of libnfnetlink_conntrack and the conntrack manpage that changing the protocol private information like the state could result in problems, I promise to do such thing.

net/ipv4/netfilter/ip_conntrack_proto_tcp.c +12 -0
Index: netfilter-2.6.14/net/ipv4/netfilter/ip_conntrack_proto_tcp.c
===================================================================
@@ -351,6 +351,17 @@
 nfattr_failure:
 	return -1;
 }
+
+static int nfattr_to_tcp(struct nfattr *tb[], struct ip_conntrack *ct)
+{
+	if (!tb[CTA_PROTOINFO_TCP_STATE-1])
+		return -EINVAL;
+
+	ct->proto.tcp.state = 
+		*(u_int8_t *)NFA_DATA(tb[CTA_PROTOINFO_TCP_STATE-1]);
+
+	return 0;
+}
 #endif
 
 static unsigned int get_conntrack_index(const struct tcphdr *tcph)
@@ -1121,6 +1132,7 @@
 #if defined(CONFIG_IP_NF_CONNTRACK_NETLINK) || \
     defined(CONFIG_IP_NF_CONNTRACK_NETLINK_MODULE)
 	.to_nfattr		= tcp_to_nfattr,
+	.from_nfattr		= nfattr_to_tcp,
 	.tuple_to_nfattr	= ip_ct_port_tuple_to_nfattr,
 	.nfattr_to_tuple	= ip_ct_port_nfattr_to_tuple,
 #endif
include/linux/netfilter_ipv4/ip_conntrack_protocol.h +2 -0
Index: netfilter-2.6.14/include/linux/netfilter_ipv4/ip_conntrack_protocol.h
===================================================================
@@ -52,6 +52,8 @@
 	int (*to_nfattr)(struct sk_buff *skb, struct nfattr *nfa,
 			 const struct ip_conntrack *ct);
 
+	int (*from_nfattr)(struct nfattr *tb[], struct ip_conntrack *ct);
+
 	int (*tuple_to_nfattr)(struct sk_buff *skb,
 			       const struct ip_conntrack_tuple *t);
 	int (*nfattr_to_tuple)(struct nfattr *tb[],
net/ipv4/netfilter/ip_conntrack_netlink.c +42 -0

60 lines not on this page · raw message

#1 lore raw
Quoted 4 lines · Pablo Neira

Hi Harald,

This is a resent. I'd like to discuss this a bit. Following the discusion left at:

I know it's an awful amount of overhead, but if we do this, we have to nest the protoinfo into another attribute.

layer4-specific attributes like CTA_PROTOINFO_TCP_STATE just don't belong in that list. CTA_PROTOINFO is fine, and then inside protoinfo every l4 helper can nest its own (private) nfattr's into that.

Please rework your patch and resubmit, thanks.

Signature · 5 lines
- Harald Welte <[email protected]>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie