All of lore.kernel.org
 help / color / mirror / Atom feed
* [resend] Passive OS fingerprint xtables match.
@ 2009-05-11  9:53 Evgeniy Polyakov
  2009-05-27 16:28 ` Patrick McHardy
  0 siblings, 1 reply; 16+ messages in thread
From: Evgeniy Polyakov @ 2009-05-11  9:53 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

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

Hi.

Passive OS fingerprinting netfilter module allows to passively detect
remote OS and perform various netfilter actions based on that knowledge.
This module compares some data (WS, MSS, options and it's order, ttl, df
and others) from packets with SYN bit set with dynamically loaded OS
fingerprints.

Fingerprint matching rules can be downloaded from OpenBSD source tree
and loaded via netfilter netlink subsystem into the kernel via special
util found in archive.

Archive contains library file (also attached), which was shipped
with iptables extensions some time ago (at least when ipt_osf existed
in patch-o-matic).

Both patch and library (attached) incorporated all requested features
and extensions, namely nfnetlink implementation, codying style cleanups,
RCU issues and others. Attached patch was rebased against current
vanilla tree.

Fingerprints can be downloaded from
http://www.openbsd.org/cgi-bin/cvsweb/src/etc/pf.os

Example usage:
# modrpobe xt_osf
# ./nfnl_osf -f ./pf.os
-d switch removes fingerprints
# iptables -I INPUT -j ACCEPT -p tcp -m osf --genre Linux --log 0 --ttl 2

You will find something like this in the syslog:
Windows [2000:SP3:Windows XP Pro SP1, 2000 SP3]: 11.22.33.55:4024 -> 11.22.33.44:139 hops=4
Linux [2.5:]: 1.2.3.4:44448 -> 11.22.33.44:22 hops=4

Please consider for inclusion.
Thank you.

Passive OS fingerprint homepage (archives, examples):
http://www.ioremap.net/projects/osf

Signed-off-by: Evgeniy Polyakov <zbr@ioremap.net>

 include/linux/netfilter/nfnetlink.h |    3 +-
 include/linux/netfilter/xt_osf.h    |  119 +++++++++
 net/netfilter/Kconfig               |   13 +
 net/netfilter/Makefile              |    1 +
 net/netfilter/xt_osf.c              |  469 +++++++++++++++++++++++++++++++++++
 5 files changed, 604 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
index c600083..bb9c9ae 100644
--- a/include/linux/netfilter/nfnetlink.h
+++ b/include/linux/netfilter/nfnetlink.h
@@ -46,7 +46,8 @@ struct nfgenmsg {
 #define NFNL_SUBSYS_CTNETLINK_EXP	2
 #define NFNL_SUBSYS_QUEUE		3
 #define NFNL_SUBSYS_ULOG		4
-#define NFNL_SUBSYS_COUNT		5
+#define NFNL_SUBSYS_OSF			5
+#define NFNL_SUBSYS_COUNT		6
 
 #ifdef __KERNEL__
 
diff --git a/include/linux/netfilter/xt_osf.h b/include/linux/netfilter/xt_osf.h
new file mode 100644
index 0000000..11903a9
--- /dev/null
+++ b/include/linux/netfilter/xt_osf.h
@@ -0,0 +1,119 @@
+/*
+ * Copyright (c) 2003+ Evgeniy Polyakov <johnpol@2ka.mxt.ru>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#ifndef _XT_OSF_H
+#define _XT_OSF_H
+
+#define MAXGENRELEN		32
+#define MAXDETLEN		64
+
+#define XT_OSF_GENRE		(1<<0)
+#define	XT_OSF_TTL		(1<<1)
+#define XT_OSF_LOG		(1<<2)
+#define XT_OSF_UNUSED		(1<<3)
+#define XT_OSF_CONNECTOR	(1<<4)
+#define XT_OSF_INVERT		(1<<5)
+
+#define XT_OSF_LOGLEVEL_ALL	0
+#define XT_OSF_LOGLEVEL_FIRST	1
+#define XT_OSF_LOGLEVEL_ALL_KNOWN	2
+
+#define XT_OSF_TTL_TRUE		0	/* True ip and fingerprint TTL comparison */
+#define XT_OSF_TTL_LESS		1	/* Check if ip TTL is less than fingerprint one */
+#define XT_OSF_TTL_NOCHECK	2	/* Do not compare ip and fingerprint TTL at all */
+
+struct xt_osf_info {
+	char			genre[MAXGENRELEN];
+	__u32			len;
+	__u32			flags;
+	__u32			loglevel;
+	__u32			ttl;
+};
+
+/*
+ * Wildcard MSS (kind of).
+ * It is used to implement a state machine for the different wildcard values
+ * of the MSS and window sizes.
+ */
+struct xt_osf_wc {
+	__u32			wc;
+	__u32			val;
+};
+
+/*
+ * This struct represents IANA options
+ * http://www.iana.org/assignments/tcp-parameters
+ */
+struct xt_osf_opt {
+	__u16			kind, length;
+	struct xt_osf_wc	wc;
+};
+
+struct xt_osf_user_finger {
+	struct xt_osf_wc	wss;
+
+	__u8			ttl, df;
+	__u16			ss, mss;
+	__u16			opt_num;
+
+	char			genre[MAXGENRELEN];
+	char			version[MAXGENRELEN];
+	char			subtype[MAXGENRELEN];
+
+	/* MAX_IPOPTLEN is maximum if all options are NOPs or EOLs */
+	struct xt_osf_opt	opt[MAX_IPOPTLEN];
+};
+
+struct xt_osf_nlmsg {
+	struct xt_osf_user_finger	f;
+	struct iphdr		ip;
+	struct tcphdr		tcp;
+};
+
+/* Defines for IANA option kinds */
+
+enum iana_options {
+	OSFOPT_EOL = 0,		/* End of options */
+	OSFOPT_NOP, 		/* NOP */
+	OSFOPT_MSS, 		/* Maximum segment size */
+	OSFOPT_WSO, 		/* Window scale option */
+	OSFOPT_SACKP,		/* SACK permitted */
+	OSFOPT_SACK,		/* SACK */
+	OSFOPT_ECHO,
+	OSFOPT_ECHOREPLY,
+	OSFOPT_TS,		/* Timestamp option */
+	OSFOPT_POCP,		/* Partial Order Connection Permitted */
+	OSFOPT_POSP,		/* Partial Order Service Profile */
+
+	/* Others are not used in the current OSF */
+	OSFOPT_EMPTY = 255,
+};
+
+enum xt_osf_msg_types {
+	OSF_MSG_SETUP,
+	OSF_MSG_MAX,
+};
+
+enum xt_osf_attr_type {
+	OSF_ATTR_UNSPEC,
+	OSF_ATTR_FINGER,
+	OSF_ATTR_MAX,
+};
+
+#endif				/* _XT_OSF_H */
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 2329c5f..b0273f9 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -916,6 +916,19 @@ config NETFILTER_XT_MATCH_U32
 
 	  Details and examples are in the kernel module source.
 
+config NETFILTER_XT_MATCH_OSF
+	tristate '"osf" Passive OS fingerprint match'
+	depends on NETFILTER_ADVANCED
+	help
+	  This option selects the Passive OS Fingerprinting match module
+	  that allows to passively match the remote operating system by
+	  analyzing incoming TCP SYN packets.
+
+	  Rules and loading software can be downloaded from
+	  http://www.ioremap.net/projects/osf
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 endif # NETFILTER_XTABLES
 
 endmenu
diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile
index 6282060..49f62ee 100644
--- a/net/netfilter/Makefile
+++ b/net/netfilter/Makefile
@@ -77,6 +77,7 @@ obj-$(CONFIG_NETFILTER_XT_MATCH_LIMIT) += xt_limit.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_MAC) += xt_mac.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_MARK) += xt_mark.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_MULTIPORT) += xt_multiport.o
+obj-$(CONFIG_NETFILTER_XT_MATCH_OSF) += xt_osf.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_OWNER) += xt_owner.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_PHYSDEV) += xt_physdev.o
 obj-$(CONFIG_NETFILTER_XT_MATCH_PKTTYPE) += xt_pkttype.o
diff --git a/net/netfilter/xt_osf.c b/net/netfilter/xt_osf.c
new file mode 100644
index 0000000..71b5eac
--- /dev/null
+++ b/net/netfilter/xt_osf.c
@@ -0,0 +1,469 @@
+/*
+ * Copyright (c) 2003+ Evgeniy Polyakov <zbr@ioremap.net>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+
+#include <linux/connector.h>
+#include <linux/if.h>
+#include <linux/inetdevice.h>
+#include <linux/ip.h>
+#include <linux/list.h>
+#include <linux/percpu.h>
+#include <linux/rculist.h>
+#include <linux/smp.h>
+#include <linux/skbuff.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/tcp.h>
+#include <linux/types.h>
+
+#include <net/ip.h>
+#include <net/tcp.h>
+
+#include <linux/netfilter/nfnetlink.h>
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter/xt_osf.h>
+
+struct xt_osf_finger {
+	struct rcu_head			rcu_head;
+	struct list_head		finger_entry;
+	struct xt_osf_user_finger	finger;
+};
+
+enum osf_fmatch_states {
+	/* Packet does not match the fingerprint */
+	FMATCH_WRONG = 0,
+	/* Packet matches the fingerprint */
+	FMATCH_OK,
+	/* Options do not match the fingerprint, but header does */
+	FMATCH_OPT_WRONG,
+};
+
+struct xt_osf_finger_storage
+{
+	struct list_head		finger_list;
+	spinlock_t			finger_lock;
+};
+
+/*
+ * Indexed by dont-fragment bit.
+ * It is the only constant value in the fingerprint.
+ */
+struct xt_osf_finger_storage xt_osf_fingers[2];
+
+struct xt_osf_message {
+	struct cn_msg		cmsg;
+	struct xt_osf_nlmsg	nlmsg;
+};
+
+static const struct nla_policy xt_osf_policy[OSF_ATTR_MAX + 1] = {
+	[OSF_ATTR_FINGER]	= { .len = sizeof(struct xt_osf_user_finger) },
+};
+
+static void xt_osf_finger_free_rcu(struct rcu_head *rcu_head)
+{
+	struct xt_osf_finger *f = container_of(rcu_head, struct xt_osf_finger, rcu_head);
+
+	kfree(f);
+}
+
+static int xt_osf_setup_callback(struct sock *ctnl, struct sk_buff *skb,
+			struct nlmsghdr *nlh, struct nlattr *osf_attrs[])
+{
+	struct xt_osf_user_finger *f;
+	struct nfgenmsg *nfmsg = NLMSG_DATA(nlh);
+	u16 delete = ntohs(nfmsg->res_id);
+	struct xt_osf_finger *kf = NULL, *sf;
+	struct xt_osf_finger_storage *st;
+	int err;
+
+	if (!osf_attrs[OSF_ATTR_FINGER])
+		return -EINVAL;
+
+	f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
+	st = &xt_osf_fingers[!!f->df];
+
+	/*
+	 * If 'delete' is set to 0 then we add attached fingerprint,
+	 * otherwise remove, and in this case we do not need to allocate data.
+	 */
+	if (!delete) {
+		kf = kmalloc(sizeof(struct xt_osf_finger), GFP_KERNEL);
+		if (!kf)
+			return -ENOMEM;
+
+		memcpy(&kf->finger, f, sizeof(struct xt_osf_user_finger));
+	}
+
+	err = -ENOENT;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sf, &st->finger_list, finger_entry) {
+		if (memcmp(&sf->finger, f, sizeof(struct xt_osf_user_finger)))
+			continue;
+
+		if (delete) {
+			spin_lock_bh(&st->finger_lock);
+			list_del_rcu(&sf->finger_entry);
+			spin_unlock_bh(&st->finger_lock);
+			call_rcu(&sf->rcu_head, xt_osf_finger_free_rcu);
+		} else {
+			kfree(kf);
+			kf = NULL;
+		}
+
+		err = 0;
+		break;
+	}
+
+	if (kf) {
+		spin_lock_bh(&st->finger_lock);
+		list_add_tail_rcu(&kf->finger_entry, &st->finger_list);
+		spin_unlock_bh(&st->finger_lock);
+#if 0
+		printk(KERN_INFO "Added rule for %s:%s:%s.\n",
+			kf->finger.genre, kf->finger.version, kf->finger.subtype);
+#endif
+	}
+	rcu_read_unlock();
+
+	return 0;
+}
+
+static const struct nfnl_callback xt_osf_nfnetlink_callbacks[OSF_MSG_MAX] = {
+	[OSF_MSG_SETUP]	= {
+		.call		= xt_osf_setup_callback,
+		.attr_count	= OSF_ATTR_MAX,
+		.policy		= xt_osf_policy,
+	},
+};
+
+static const struct nfnetlink_subsystem xt_osf_nfnetlink = {
+	.name			= "osf",
+	.subsys_id		= NFNL_SUBSYS_OSF,
+	.cb_count		= OSF_MSG_MAX,
+	.cb			= xt_osf_nfnetlink_callbacks,
+};
+
+static inline int xt_osf_ttl(const struct sk_buff *skb, const struct xt_osf_info *info,
+			    unsigned char f_ttl)
+{
+	const struct iphdr *ip = ip_hdr(skb);
+
+	if (info->flags & XT_OSF_TTL) {
+		if (info->ttl == XT_OSF_TTL_TRUE)
+			return ip->ttl == f_ttl;
+		if (info->ttl == XT_OSF_TTL_NOCHECK)
+			return 1;
+		else if (ip->ttl <= f_ttl)
+			return 1;
+		else {
+			struct in_device *in_dev = in_dev_get(skb->dev);
+			int ret = 0;
+
+			for_ifa(in_dev) {
+				if (inet_ifa_match(ip->saddr, ifa)) {
+					ret = (ip->ttl == f_ttl);
+					break;
+				}
+			}
+			endfor_ifa(in_dev);
+
+			in_dev_put(in_dev);
+			return ret;
+		}
+	}
+
+	return ip->ttl == f_ttl;
+}
+
+static bool xt_osf_match_packet(const struct sk_buff *skb,
+		const struct xt_match_param *p)
+{
+	const struct xt_osf_info *info = p->matchinfo;
+	const struct iphdr *ip = ip_hdr(skb);
+	const struct tcphdr *tcp;
+	struct tcphdr _tcph;
+	int fmatch = FMATCH_WRONG, fcount = 0;
+	unsigned int optsize = 0, check_WSS = 0;
+	u16 window, totlen, mss = 0;
+	bool df;
+	const unsigned char *optp = NULL, *_optp = NULL;
+	unsigned char opts[MAX_IPOPTLEN];
+	const struct xt_osf_finger *kf;
+	const struct xt_osf_user_finger *f;
+	const struct xt_osf_finger_storage *st;
+
+	if (!info)
+		return false;
+
+	tcp = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(struct tcphdr), &_tcph);
+	if (!tcp)
+		return false;
+
+	if (!tcp->syn)
+		return false;
+
+	totlen = ntohs(ip->tot_len);
+	df = ntohs(ip->frag_off) & IP_DF;
+	window = ntohs(tcp->window);
+
+	if (tcp->doff * 4 > sizeof(struct tcphdr)) {
+		optsize = tcp->doff * 4 - sizeof(struct tcphdr);
+
+		if (optsize > sizeof(opts))
+			optsize = sizeof(opts);
+
+		_optp = optp = skb_header_pointer(skb, ip_hdrlen(skb) + sizeof(struct tcphdr),
+				optsize, opts);
+	}
+
+	st = &xt_osf_fingers[df];
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(kf, &st->finger_list, finger_entry) {
+		f = &kf->finger;
+
+		if (!(info->flags & XT_OSF_LOG) && strcmp(info->genre, f->genre))
+			continue;
+
+		optp = _optp;
+		fmatch = FMATCH_WRONG;
+
+		if (totlen == f->ss && xt_osf_ttl(skb, info, f->ttl)) {
+			int foptsize, optnum;
+
+			check_WSS = 0;
+
+			switch (f->wss.wc) {
+			case 0:
+				check_WSS = 0;
+				break;
+			case 'S':
+				check_WSS = 1;
+				break;
+			case 'T':
+				check_WSS = 2;
+				break;
+			case '%':
+				check_WSS = 3;
+				break;
+			default:
+				check_WSS = 4;
+				break;
+			}
+			if (check_WSS == 4)
+				continue;
+
+			/* Check options */
+
+			foptsize = 0;
+			for (optnum = 0; optnum < f->opt_num; ++optnum)
+				foptsize += f->opt[optnum].length;
+
+			if (foptsize > MAX_IPOPTLEN || optsize > MAX_IPOPTLEN || optsize != foptsize)
+				continue;
+
+			for (optnum = 0; optnum < f->opt_num; ++optnum) {
+				if (f->opt[optnum].kind == (*optp)) {
+					__u32 len = f->opt[optnum].length;
+					const __u8 *optend = optp + len;
+					int loop_cont = 0;
+
+					fmatch = FMATCH_OK;
+
+					switch (*optp) {
+					case OSFOPT_MSS:
+						mss = optp[3];
+						mss <<= 8;
+						mss |= optp[2];
+
+						mss = ntohs(mss);
+						break;
+					case OSFOPT_TS:
+						loop_cont = 1;
+						break;
+					}
+
+					optp = optend;
+				} else
+					fmatch = FMATCH_OPT_WRONG;
+
+				if (fmatch != FMATCH_OK)
+					break;
+			}
+
+			if (fmatch != FMATCH_OPT_WRONG) {
+				fmatch = FMATCH_WRONG;
+
+				switch (check_WSS) {
+				case 0:
+					if (f->wss.val == 0 || window == f->wss.val)
+						fmatch = FMATCH_OK;
+					break;
+				case 1:	/* MSS */
+#define SMART_MSS_1	1460
+#define SMART_MSS_2	1448
+					if (window == f->wss.val * mss ||
+					    window == f->wss.val * SMART_MSS_1 ||
+					    window == f->wss.val * SMART_MSS_2)
+						fmatch = FMATCH_OK;
+					break;
+				case 2:	/* MTU */
+					if (window == f->wss.val * (mss + 40) ||
+					    window == f->wss.val * (SMART_MSS_1 + 40) ||
+					    window == f->wss.val * (SMART_MSS_2 + 40))
+						fmatch = FMATCH_OK;
+					break;
+				case 3:	/* MOD */
+					if ((window % f->wss.val) == 0)
+						fmatch = FMATCH_OK;
+					break;
+				}
+			}
+
+			if (fmatch != FMATCH_OK)
+				continue;
+
+			fcount++;
+			if (info->flags & XT_OSF_LOG)
+				printk(KERN_INFO "%s [%s:%s] : "
+					"%pi4:%d -> %pi4:%d hops=%d\n",
+					f->genre, f->version, f->subtype,
+					&ip->saddr, ntohs(tcp->source),
+					&ip->daddr, ntohs(tcp->dest),
+					f->ttl - ip->ttl);
+
+			if ((info->flags & XT_OSF_LOG) &&
+			    info->loglevel == XT_OSF_LOGLEVEL_FIRST)
+				break;
+		}
+	}
+	rcu_read_unlock();
+
+	if (!fcount && (info->flags & (XT_OSF_LOG | XT_OSF_CONNECTOR))) {
+		unsigned int i;
+		struct xt_osf_user_finger fg;
+
+		memset(&fg, 0, sizeof(fg));
+#if 1
+		if (info->flags & XT_OSF_LOG) {
+			if (info->loglevel != XT_OSF_LOGLEVEL_ALL_KNOWN)
+				printk(KERN_INFO "Unknown: win: %u, mss: %u, "
+					"totlen: %u, df: %d, ttl: %u : ",
+					window, mss, totlen, df, ip->ttl);
+			else
+				printk(KERN_INFO "");
+			if (_optp) {
+				optp = _optp;
+				for (i = 0; i < optsize; i++)
+					printk("%02X ", optp[i]);
+			}
+
+			printk("%pi4:%u -> %pi4:%u\n",
+			     &ip->saddr, ntohs(tcp->source),
+			     &ip->daddr, ntohs(tcp->dest));
+		}
+#endif
+	}
+
+	if (fcount)
+		fmatch = FMATCH_OK;
+
+	return fmatch == FMATCH_OK;
+}
+
+static struct xt_match xt_osf_match = {
+	.name 		= "osf",
+	.revision	= 0,
+	.family		= NFPROTO_IPV4,
+	.proto		= IPPROTO_TCP,
+	.hooks      	= (1 << NF_INET_LOCAL_IN) | (1 << NF_INET_PRE_ROUTING) | (1 << NF_INET_FORWARD),
+	.match 		= xt_osf_match_packet,
+	.matchsize	= sizeof(struct xt_osf_info),
+	.me		= THIS_MODULE,
+};
+
+static int __init xt_osf_init(void)
+{
+	int err = -EINVAL;
+	int i;
+
+	for (i=0; i<ARRAY_SIZE(xt_osf_fingers); ++i) {
+		struct xt_osf_finger_storage *st = &xt_osf_fingers[i];
+
+		INIT_LIST_HEAD(&st->finger_list);
+		spin_lock_init(&st->finger_lock);
+	}
+
+	err = nfnetlink_subsys_register(&xt_osf_nfnetlink);
+	if (err < 0) {
+		printk(KERN_ERR "Failed (%d) to register OSF nsfnetlink helper.\n", err);
+		goto err_out_exit;
+	}
+
+	err = xt_register_match(&xt_osf_match);
+	if (err) {
+		printk(KERN_ERR "Failed (%d) to register OS fingerprint "
+				"matching module.\n", err);
+		goto err_out_remove;
+	}
+
+	printk(KERN_INFO "Started passive OS fingerprint matching module.\n");
+
+	return 0;
+
+err_out_remove:
+	nfnetlink_subsys_unregister(&xt_osf_nfnetlink);
+err_out_exit:
+	return err;
+}
+
+static void __exit xt_osf_fini(void)
+{
+	struct xt_osf_finger *f;
+	int i;
+
+	nfnetlink_subsys_unregister(&xt_osf_nfnetlink);
+	xt_unregister_match(&xt_osf_match);
+
+	rcu_read_lock();
+	for (i=0; i<ARRAY_SIZE(xt_osf_fingers); ++i) {
+		struct xt_osf_finger_storage *st = &xt_osf_fingers[i];
+
+		list_for_each_entry_rcu(f, &st->finger_list, finger_entry) {
+			list_del_rcu(&f->finger_entry);
+			call_rcu(&f->rcu_head, xt_osf_finger_free_rcu);
+		}
+	}
+	rcu_read_unlock();
+
+	rcu_barrier();
+
+	printk(KERN_INFO "Passive OS fingerprint matching module finished.\n");
+}
+
+module_init(xt_osf_init);
+module_exit(xt_osf_fini);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Evgeniy Polyakov <zbr@ioremap.net>");
+MODULE_DESCRIPTION("Passive OS fingerprint matching.");
+MODULE_ALIAS_NFNL_SUBSYS(NFNL_SUBSYS_OSF);

-- 
	Evgeniy Polyakov

[-- Attachment #2: libxt_osf.c --]
[-- Type: text/x-csrc, Size: 4751 bytes --]

/*
 * Copyright (c) 2003+ Evgeniy Polyakov <zbr@ioremap.net>
 *
 *
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation; either version 2 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 */

/*
 * xtables interface for OS fingerprint matching module.
 */

#include <stdio.h>
#include <netdb.h>
#include <string.h>
#include <stdlib.h>
#include <getopt.h>
#include <ctype.h>

#include <linux/types.h>

#include <xtables.h>

#include <netinet/ip.h>
#include <netinet/tcp.h>

#include "xt_osf.h"

static void osf_help(void)
{
	printf("OS fingerprint match options:\n"
		"--genre [!] string         Match a OS genre by passive fingerprinting.\n"
		"--ttl                  Use some TTL check extensions to determine OS:\n"
		"       0                       true ip and fingerprint TTL comparison. Works for LAN.\n"
		"       1                       check if ip TTL is less than fingerprint one. Works for global addresses.\n"
		"       2                       do not compare TTL at all. Allows to detect NMAP, but can produce false results.\n"
		"--log level            Log determined genres into dmesg even if they do not match desired one:\n"
		"       0                       log all matched or unknown signatures.\n"
		"       1                       log only first one.\n"
		"       2                       log all known matched signatures.\n"
		);
}


static const struct option osf_opts[] = {
	{ .name = "genre",	.has_arg = true, .val = '1' },
	{ .name = "ttl",	.has_arg = true, .val = '2' },
	{ .name = "log",	.has_arg = true, .val = '3' },
	{ .name = NULL }
};


static void osf_parse_string(const unsigned char *s, struct xt_osf_info *info)
{
	if (strlen(s) < MAXGENRELEN) 
		strcpy(info->genre, s);
	else 
		exit_error(PARAMETER_PROBLEM, "Genre string too long `%s' [%d], max=%d", 
				s, strlen(s), MAXGENRELEN);
}

static int osf_parse(int c, char **argv, int invert, unsigned int *flags,
      			const void *entry,
      			struct xt_entry_match **match)
{
	struct xt_osf_info *info = (struct xt_osf_info *)(*match)->data;
	
	switch(c) {
		case '1': /* --genre */
			if (*flags & XT_OSF_GENRE)
				exit_error(PARAMETER_PROBLEM, "Can't specify multiple genre parameter");
			check_inverse(optarg, &invert, &optind, 0);
			osf_parse_string(argv[optind-1], info);
			if (invert)
				info->flags |= XT_OSF_INVERT;
			info->len=strlen(info->genre);
			*flags |= XT_OSF_GENRE;
			break;
		case '2': /* --ttl */
			if (*flags & XT_OSF_TTL)
				exit_error(PARAMETER_PROBLEM, "Can't specify multiple ttl parameter");
			*flags |= XT_OSF_TTL;
			info->flags |= XT_OSF_TTL;
			if (!xtables_strtoui(argv[optind-1], NULL, &info->ttl, 0, 2))
				xtables_error(PARAMETER_PROBLEM, "TTL parameter is too big");
			break;
		case '3': /* --log */
			if (*flags & XT_OSF_LOG)
				exit_error(PARAMETER_PROBLEM, "Can't specify multiple log parameter");
			*flags |= XT_OSF_LOG;
			if (!xtables_strtoui(argv[optind-1], NULL, &info->loglevel, 0, 2))
				xtables_error(PARAMETER_PROBLEM, "Log level parameter is too big");
			info->flags |= XT_OSF_LOG;
			break;
		default:
			return 0;
	}

	return 1;
}

static void osf_final_check(unsigned int flags)
{
	if (!flags)
		exit_error(PARAMETER_PROBLEM, "OS fingerprint match: You must specify `--genre'");
}

static void osf_print(const void *ip, const struct xt_entry_match *match, int numeric)
{
	const struct xt_osf_info *info = (const struct xt_osf_info*) match->data;

	printf("OS fingerprint match %s%s ", (info->flags & XT_OSF_INVERT) ? "! " : "", info->genre);
}

static void osf_save(const void *ip, const struct xt_entry_match *match)
{
	const struct xt_osf_info *info = (const struct xt_osf_info*) match->data;

	printf("--genre %s%s ", (info->flags & XT_OSF_INVERT) ? "! ": "", info->genre);
}

static struct xtables_match osf_match = {
	.name		= "osf",
	.version	= XTABLES_VERSION,
	.size		= XT_ALIGN(sizeof(struct xt_osf_info)),
	.userspacesize	= XT_ALIGN(sizeof(struct xt_osf_info)),
	.help		= osf_help,
	.parse		= osf_parse,
	.print		= osf_print,
	.final_check	= osf_final_check,
	.save		= osf_save,
	.extra_opts	= osf_opts,
	.family		= NFPROTO_IPV4
};

void _init(void)
{
	xtables_register_match(&osf_match);
}

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-05-11  9:53 [resend] Passive OS fingerprint xtables match Evgeniy Polyakov
@ 2009-05-27 16:28 ` Patrick McHardy
  2009-05-29  8:59   ` Evgeniy Polyakov
  2009-06-04 11:37   ` Evgeniy Polyakov
  0 siblings, 2 replies; 16+ messages in thread
From: Patrick McHardy @ 2009-05-27 16:28 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Evgeniy Polyakov wrote:
> Example usage:
> # modrpobe xt_osf
> # ./nfnl_osf -f ./pf.os
> -d switch removes fingerprints
> # iptables -I INPUT -j ACCEPT -p tcp -m osf --genre Linux --log 0 --ttl 2
> 
> You will find something like this in the syslog:
> Windows [2000:SP3:Windows XP Pro SP1, 2000 SP3]: 11.22.33.55:4024 -> 11.22.33.44:139 hops=4
> Linux [2.5:]: 1.2.3.4:44448 -> 11.22.33.44:22 hops=4

Please convert this to use nf_log_packet().

> diff --git a/include/linux/netfilter/nfnetlink.h b/include/linux/netfilter/nfnetlink.h
> index c600083..bb9c9ae 100644
> --- a/include/linux/netfilter/nfnetlink.h
> +++ b/include/linux/netfilter/nfnetlink.h
> @@ -46,7 +46,8 @@ struct nfgenmsg {
>  #define NFNL_SUBSYS_CTNETLINK_EXP	2
>  #define NFNL_SUBSYS_QUEUE		3
>  #define NFNL_SUBSYS_ULOG		4
> -#define NFNL_SUBSYS_COUNT		5
> +#define NFNL_SUBSYS_OSF			5
> +#define NFNL_SUBSYS_COUNT		6
>  
>  #ifdef __KERNEL__
>  
> diff --git a/include/linux/netfilter/xt_osf.h b/include/linux/netfilter/xt_osf.h
> new file mode 100644
> index 0000000..11903a9
> --- /dev/null
> +++ b/include/linux/netfilter/xt_osf.h
> +#ifndef _XT_OSF_H
> +#define _XT_OSF_H
> +
> +#define MAXGENRELEN		32
> +#define MAXDETLEN		64

^ Unused

> +
> +#define XT_OSF_GENRE		(1<<0)
> +#define	XT_OSF_TTL		(1<<1)
> +#define XT_OSF_LOG		(1<<2)
> +#define XT_OSF_UNUSED		(1<<3)

^ Unused? :)

> +#define XT_OSF_CONNECTOR	(1<<4)
> +#define XT_OSF_INVERT		(1<<5)
> +
> +#define XT_OSF_LOGLEVEL_ALL	0
> +#define XT_OSF_LOGLEVEL_FIRST	1
> +#define XT_OSF_LOGLEVEL_ALL_KNOWN	2

What does this do?

> +#define XT_OSF_TTL_TRUE		0	/* True ip and fingerprint TTL comparison */
> +#define XT_OSF_TTL_LESS		1	/* Check if ip TTL is less than fingerprint one */
> +#define XT_OSF_TTL_NOCHECK	2	/* Do not compare ip and fingerprint TTL at all */

These seem redundant - having neither of TRUE or LESS seems
equivalent to NOCHECK. Perhaps thats the reason why its not
used at all :) Looking at the code, "TRUE" would be better
named as "EQUAL".

> +struct xt_osf_info {
> +	char			genre[MAXGENRELEN];
> +	__u32			len;
> +	__u32			flags;
> +	__u32			loglevel;
> +	__u32			ttl;
> +};

Unless you're really really sure that this is not going to
change, please use netlink attributes. Similar for the other
ABI structures.

> +
> +/*
> + * Wildcard MSS (kind of).
> + * It is used to implement a state machine for the different wildcard values
> + * of the MSS and window sizes.
> + */
> +struct xt_osf_wc {
> +	__u32			wc;
> +	__u32			val;
> +};
> +
> +/*
> + * This struct represents IANA options
> + * http://www.iana.org/assignments/tcp-parameters
> + */
> +struct xt_osf_opt {
> +	__u16			kind, length;
> +	struct xt_osf_wc	wc;
> +};
> +
> +struct xt_osf_user_finger {
> +	struct xt_osf_wc	wss;
> +
> +	__u8			ttl, df;
> +	__u16			ss, mss;
> +	__u16			opt_num;
> +
> +	char			genre[MAXGENRELEN];
> +	char			version[MAXGENRELEN];
> +	char			subtype[MAXGENRELEN];
> +
> +	/* MAX_IPOPTLEN is maximum if all options are NOPs or EOLs */
> +	struct xt_osf_opt	opt[MAX_IPOPTLEN];

This really looks like you should use nested attributes.

> +};
> +
> +struct xt_osf_nlmsg {
> +	struct xt_osf_user_finger	f;
> +	struct iphdr		ip;
> +	struct tcphdr		tcp;
> +};
> +
> +/* Defines for IANA option kinds */
> +
> +enum iana_options {
> +	OSFOPT_EOL = 0,		/* End of options */
> +	OSFOPT_NOP, 		/* NOP */
> +	OSFOPT_MSS, 		/* Maximum segment size */
> +	OSFOPT_WSO, 		/* Window scale option */
> +	OSFOPT_SACKP,		/* SACK permitted */
> +	OSFOPT_SACK,		/* SACK */
> +	OSFOPT_ECHO,
> +	OSFOPT_ECHOREPLY,
> +	OSFOPT_TS,		/* Timestamp option */
> +	OSFOPT_POCP,		/* Partial Order Connection Permitted */
> +	OSFOPT_POSP,		/* Partial Order Service Profile */
> +
> +	/* Others are not used in the current OSF */
> +	OSFOPT_EMPTY = 255,
> +};

Why do we need to duplicate these?

> +
> +enum xt_osf_msg_types {
> +	OSF_MSG_SETUP,
> +	OSF_MSG_MAX,
> +};
> +
> +enum xt_osf_attr_type {
> +	OSF_ATTR_UNSPEC,
> +	OSF_ATTR_FINGER,
> +	OSF_ATTR_MAX,
> +};
> +
> +#endif				/* _XT_OSF_H */
> diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
> index 2329c5f..b0273f9 100644
> --- a/net/netfilter/Kconfig
> +++ b/net/netfilter/Kconfig
> @@ -916,6 +916,19 @@ config NETFILTER_XT_MATCH_U32
>  
>  	  Details and examples are in the kernel module source.
>  
> +config NETFILTER_XT_MATCH_OSF
> +	tristate '"osf" Passive OS fingerprint match'
> +	depends on NETFILTER_ADVANCED

&& NFNETLINK

> --- /dev/null
> +++ b/net/netfilter/xt_osf.c
> @@ -0,0 +1,469 @@
> +/*
> + * Copyright (c) 2003+ Evgeniy Polyakov <zbr@ioremap.net>
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +
> +#include <linux/connector.h>

Not needed. The remaining ones look like some (percpu?) could be
removed as well.

> +#include <linux/if.h>
> +#include <linux/inetdevice.h>
> +#include <linux/ip.h>
> +#include <linux/list.h>
> +#include <linux/percpu.h>
> +#include <linux/rculist.h>
> +#include <linux/smp.h>
> +#include <linux/skbuff.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/tcp.h>
> +#include <linux/types.h>
> +
> +#include <net/ip.h>
> +#include <net/tcp.h>
> +
> +#include <linux/netfilter/nfnetlink.h>
> +#include <linux/netfilter/x_tables.h>
> +#include <linux/netfilter/xt_osf.h>
> +
> +struct xt_osf_finger {
> +	struct rcu_head			rcu_head;
> +	struct list_head		finger_entry;
> +	struct xt_osf_user_finger	finger;
> +};
> +
> +enum osf_fmatch_states {
> +	/* Packet does not match the fingerprint */
> +	FMATCH_WRONG = 0,
> +	/* Packet matches the fingerprint */
> +	FMATCH_OK,
> +	/* Options do not match the fingerprint, but header does */
> +	FMATCH_OPT_WRONG,
> +};
> +
> +struct xt_osf_finger_storage
> +{

Please place the opening bracket consistently with the other
structure definitions.

> +	struct list_head		finger_list;
> +	spinlock_t			finger_lock;
> +};
> +
> +/*
> + * Indexed by dont-fragment bit.
> + * It is the only constant value in the fingerprint.
> + */
> +struct xt_osf_finger_storage xt_osf_fingers[2];

static

> +
> +struct xt_osf_message {
> +	struct cn_msg		cmsg;
> +	struct xt_osf_nlmsg	nlmsg;
> +};

Unused.

> +static int xt_osf_setup_callback(struct sock *ctnl, struct sk_buff *skb,
> +			struct nlmsghdr *nlh, struct nlattr *osf_attrs[])
> +{
> +	struct xt_osf_user_finger *f;
> +	struct nfgenmsg *nfmsg = NLMSG_DATA(nlh);
> +	u16 delete = ntohs(nfmsg->res_id);

This looks like abuse, we use message types to distinguish between
additions and deletions, alternative NLM_F_REPLACE.

> +	struct xt_osf_finger *kf = NULL, *sf;
> +	struct xt_osf_finger_storage *st;
> +	int err;
> +
> +	if (!osf_attrs[OSF_ATTR_FINGER])
> +		return -EINVAL;
> +
> +	f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> +	st = &xt_osf_fingers[!!f->df];
> +
> +	/*
> +	 * If 'delete' is set to 0 then we add attached fingerprint,
> +	 * otherwise remove, and in this case we do not need to allocate data.
> +	 */
> +	if (!delete) {
> +		kf = kmalloc(sizeof(struct xt_osf_finger), GFP_KERNEL);
> +		if (!kf)
> +			return -ENOMEM;
> +
> +		memcpy(&kf->finger, f, sizeof(struct xt_osf_user_finger));
> +	}
> +
> +	err = -ENOENT;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(sf, &st->finger_list, finger_entry) {
> +		if (memcmp(&sf->finger, f, sizeof(struct xt_osf_user_finger)))
> +			continue;
> +
> +		if (delete) {
> +			spin_lock_bh(&st->finger_lock);

This lock looks useless, all changes are done in netlink context under
the nfnl mutex.

> +			list_del_rcu(&sf->finger_entry);
> +			spin_unlock_bh(&st->finger_lock);
> +			call_rcu(&sf->rcu_head, xt_osf_finger_free_rcu);
> +		} else {
> +			kfree(kf);
> +			kf = NULL;
> +		}
> +
> +		err = 0;
> +		break;
> +	}
> +
> +	if (kf) {
> +		spin_lock_bh(&st->finger_lock);
> +		list_add_tail_rcu(&kf->finger_entry, &st->finger_list);
> +		spin_unlock_bh(&st->finger_lock);
> +#if 0
> +		printk(KERN_INFO "Added rule for %s:%s:%s.\n",
> +			kf->finger.genre, kf->finger.version, kf->finger.subtype);
> +#endif
> +	}
> +	rcu_read_unlock();
> +
> +	return 0;
> +}
> +
> +static const struct nfnl_callback xt_osf_nfnetlink_callbacks[OSF_MSG_MAX] = {
> +	[OSF_MSG_SETUP]	= {
> +		.call		= xt_osf_setup_callback,
> +		.attr_count	= OSF_ATTR_MAX,
> +		.policy		= xt_osf_policy,
> +	},
> +};
> +
> +static const struct nfnetlink_subsystem xt_osf_nfnetlink = {
> +	.name			= "osf",
> +	.subsys_id		= NFNL_SUBSYS_OSF,
> +	.cb_count		= OSF_MSG_MAX,
> +	.cb			= xt_osf_nfnetlink_callbacks,
> +};
> +
> +static inline int xt_osf_ttl(const struct sk_buff *skb, const struct xt_osf_info *info,
> +			    unsigned char f_ttl)
> +{
> +	const struct iphdr *ip = ip_hdr(skb);
> +
> +	if (info->flags & XT_OSF_TTL) {
> +		if (info->ttl == XT_OSF_TTL_TRUE)
> +			return ip->ttl == f_ttl;
> +		if (info->ttl == XT_OSF_TTL_NOCHECK)
> +			return 1;
> +		else if (ip->ttl <= f_ttl)
> +			return 1;
> +		else {
> +			struct in_device *in_dev = in_dev_get(skb->dev);
> +			int ret = 0;
> +
> +			for_ifa(in_dev) {
> +				if (inet_ifa_match(ip->saddr, ifa)) {
> +					ret = (ip->ttl == f_ttl);
> +					break;
> +				}
> +			}
> +			endfor_ifa(in_dev);
> +
> +			in_dev_put(in_dev);
> +			return ret;
> +		}
> +	}
> +
> +	return ip->ttl == f_ttl;
> +}
> +
> +static bool xt_osf_match_packet(const struct sk_buff *skb,
> +		const struct xt_match_param *p)
> +{
> +	const struct xt_osf_info *info = p->matchinfo;
> +	const struct iphdr *ip = ip_hdr(skb);
> +	const struct tcphdr *tcp;
> +	struct tcphdr _tcph;
> +	int fmatch = FMATCH_WRONG, fcount = 0;
> +	unsigned int optsize = 0, check_WSS = 0;
> +	u16 window, totlen, mss = 0;
> +	bool df;
> +	const unsigned char *optp = NULL, *_optp = NULL;
> +	unsigned char opts[MAX_IPOPTLEN];
> +	const struct xt_osf_finger *kf;
> +	const struct xt_osf_user_finger *f;
> +	const struct xt_osf_finger_storage *st;
> +
> +	if (!info)
> +		return false;
> +
> +	tcp = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(struct tcphdr), &_tcph);
> +	if (!tcp)
> +		return false;
> +
> +	if (!tcp->syn)
> +		return false;
> +
> +	totlen = ntohs(ip->tot_len);
> +	df = ntohs(ip->frag_off) & IP_DF;
> +	window = ntohs(tcp->window);
> +
> +	if (tcp->doff * 4 > sizeof(struct tcphdr)) {
> +		optsize = tcp->doff * 4 - sizeof(struct tcphdr);
> +
> +		if (optsize > sizeof(opts))
> +			optsize = sizeof(opts);
> +
> +		_optp = optp = skb_header_pointer(skb, ip_hdrlen(skb) + sizeof(struct tcphdr),

Please break the line at 80 characters

> +				optsize, opts);
> +	}
> +
> +	st = &xt_osf_fingers[df];
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(kf, &st->finger_list, finger_entry) {
> +		f = &kf->finger;
> +
> +		if (!(info->flags & XT_OSF_LOG) && strcmp(info->genre, f->genre))
> +			continue;
> +
> +		optp = _optp;
> +		fmatch = FMATCH_WRONG;
> +
> +		if (totlen == f->ss && xt_osf_ttl(skb, info, f->ttl)) {
> +			int foptsize, optnum;
> +
> +			check_WSS = 0;
> +
> +			switch (f->wss.wc) {
> +			case 0:
> +				check_WSS = 0;
> +				break;
> +			case 'S':
> +				check_WSS = 1;
> +				break;
> +			case 'T':
> +				check_WSS = 2;
> +				break;
> +			case '%':
> +				check_WSS = 3;
> +				break;
> +			default:
> +				check_WSS = 4;
> +				break;
> +			}

This is really pushing my taste-buds. Whatever this does, please at
use symbolic constants so the reader at least has a chance to understand
it.

> +			if (check_WSS == 4)
> +				continue;
> +
> +			/* Check options */
> +
> +			foptsize = 0;
> +			for (optnum = 0; optnum < f->opt_num; ++optnum)
> +				foptsize += f->opt[optnum].length;
> +
> +			if (foptsize > MAX_IPOPTLEN || optsize > MAX_IPOPTLEN || optsize != foptsize)
> +				continue;
> +
> +			for (optnum = 0; optnum < f->opt_num; ++optnum) {
> +				if (f->opt[optnum].kind == (*optp)) {
> +					__u32 len = f->opt[optnum].length;
> +					const __u8 *optend = optp + len;
> +					int loop_cont = 0;
> +
> +					fmatch = FMATCH_OK;
> +
> +					switch (*optp) {
> +					case OSFOPT_MSS:
> +						mss = optp[3];
> +						mss <<= 8;
> +						mss |= optp[2];
> +
> +						mss = ntohs(mss);
> +						break;
> +					case OSFOPT_TS:
> +						loop_cont = 1;
> +						break;
> +					}
> +
> +					optp = optend;
> +				} else
> +					fmatch = FMATCH_OPT_WRONG;
> +
> +				if (fmatch != FMATCH_OK)
> +					break;
> +			}
> +
> +			if (fmatch != FMATCH_OPT_WRONG) {
> +				fmatch = FMATCH_WRONG;
> +
> +				switch (check_WSS) {
> +				case 0:
> +					if (f->wss.val == 0 || window == f->wss.val)
> +						fmatch = FMATCH_OK;
> +					break;
> +				case 1:	/* MSS */
> +#define SMART_MSS_1	1460
> +#define SMART_MSS_2	1448

Sigh. This entire function is completely unreadable and full of
unexplained magic. I'll stop here, please clean this before
resubmitting.

> +					if (window == f->wss.val * mss ||
> +					    window == f->wss.val * SMART_MSS_1 ||
> +					    window == f->wss.val * SMART_MSS_2)
> +						fmatch = FMATCH_OK;
> +					break;
> +				case 2:	/* MTU */
> +					if (window == f->wss.val * (mss + 40) ||
> +					    window == f->wss.val * (SMART_MSS_1 + 40) ||
> +					    window == f->wss.val * (SMART_MSS_2 + 40))
> +						fmatch = FMATCH_OK;
> +					break;
> +				case 3:	/* MOD */
> +					if ((window % f->wss.val) == 0)
> +						fmatch = FMATCH_OK;
> +					break;
> +				}
> +			}
> +
> +			if (fmatch != FMATCH_OK)
> +				continue;
> +
> +			fcount++;
> +			if (info->flags & XT_OSF_LOG)
> +				printk(KERN_INFO "%s [%s:%s] : "
> +					"%pi4:%d -> %pi4:%d hops=%d\n",
> +					f->genre, f->version, f->subtype,
> +					&ip->saddr, ntohs(tcp->source),
> +					&ip->daddr, ntohs(tcp->dest),
> +					f->ttl - ip->ttl);
> +
> +			if ((info->flags & XT_OSF_LOG) &&
> +			    info->loglevel == XT_OSF_LOGLEVEL_FIRST)
> +				break;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	if (!fcount && (info->flags & (XT_OSF_LOG | XT_OSF_CONNECTOR))) {
> +		unsigned int i;
> +		struct xt_osf_user_finger fg;
> +
> +		memset(&fg, 0, sizeof(fg));
> +#if 1
> +		if (info->flags & XT_OSF_LOG) {
> +			if (info->loglevel != XT_OSF_LOGLEVEL_ALL_KNOWN)
> +				printk(KERN_INFO "Unknown: win: %u, mss: %u, "
> +					"totlen: %u, df: %d, ttl: %u : ",
> +					window, mss, totlen, df, ip->ttl);
> +			else
> +				printk(KERN_INFO "");
> +			if (_optp) {
> +				optp = _optp;
> +				for (i = 0; i < optsize; i++)
> +					printk("%02X ", optp[i]);
> +			}
> +
> +			printk("%pi4:%u -> %pi4:%u\n",
> +			     &ip->saddr, ntohs(tcp->source),
> +			     &ip->daddr, ntohs(tcp->dest));
> +		}
> +#endif
> +	}
> +
> +	if (fcount)
> +		fmatch = FMATCH_OK;
> +
> +	return fmatch == FMATCH_OK;
> +}
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-05-27 16:28 ` Patrick McHardy
@ 2009-05-29  8:59   ` Evgeniy Polyakov
  2009-05-29  9:49     ` Jan Engelhardt
  2009-06-02 12:40     ` Patrick McHardy
  2009-06-04 11:37   ` Evgeniy Polyakov
  1 sibling, 2 replies; 16+ messages in thread
From: Evgeniy Polyakov @ 2009-05-29  8:59 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Hi Patrick.

Thanks for your comments.

On Wed, May 27, 2009 at 06:28:16PM +0200, Patrick McHardy (kaber@trash.net) wrote:
> >You will find something like this in the syslog:
> >Windows [2000:SP3:Windows XP Pro SP1, 2000 SP3]: 11.22.33.55:4024 -> 
> >11.22.33.44:139 hops=4
> >Linux [2.5:]: 1.2.3.4:44448 -> 11.22.33.44:22 hops=4
> 
> Please convert this to use nf_log_packet().

Ok, will use.

> >--- /dev/null
> >+++ b/include/linux/netfilter/xt_osf.h
> >+#ifndef _XT_OSF_H
> >+#define _XT_OSF_H
> >+
> >+#define MAXGENRELEN		32
> >+#define MAXDETLEN		64
> 
> ^ Unused
> 
> >+
> >+#define XT_OSF_GENRE		(1<<0)
> >+#define	XT_OSF_TTL		(1<<1)
> >+#define XT_OSF_LOG		(1<<2)
> >+#define XT_OSF_UNUSED		(1<<3)
> 
> ^ Unused? :)

I will drop those constants.

> >+#define XT_OSF_CONNECTOR	(1<<4)
> >+#define XT_OSF_INVERT		(1<<5)
> >+
> >+#define XT_OSF_LOGLEVEL_ALL	0
> >+#define XT_OSF_LOGLEVEL_FIRST	1
> >+#define XT_OSF_LOGLEVEL_ALL_KNOWN	2
> 
> What does this do?

There are 3 log levels - dump all matched entries, only the first
matched and dump unknown packet data if needed.

> >+#define XT_OSF_TTL_TRUE		0	/* True ip and fingerprint 
> >TTL comparison */
> >+#define XT_OSF_TTL_LESS		1	/* Check if ip TTL is less 
> >than fingerprint one */
> >+#define XT_OSF_TTL_NOCHECK	2	/* Do not compare ip and fingerprint 
> >TTL at all */
> 
> These seem redundant - having neither of TRUE or LESS seems
> equivalent to NOCHECK. Perhaps thats the reason why its not
> used at all :) Looking at the code, "TRUE" would be better
> named as "EQUAL".

There are only three types of TTL check - equal (for true), less than
fingerprint one and when no check is performed at all. NOCHECK is
actually used, but LESS one does not have a special check, but a default
clause when neither TRUE or NOCHECK is specified.

> >+struct xt_osf_info {
> >+	char			genre[MAXGENRELEN];
> >+	__u32			len;
> >+	__u32			flags;
> >+	__u32			loglevel;
> >+	__u32			ttl;
> >+};
> 
> Unless you're really really sure that this is not going to
> change, please use netlink attributes. Similar for the other
> ABI structures.

It was not changed for the last 5 years, maybe even more.
There are no other fields in the tcp header to match against :)

> >+struct xt_osf_user_finger {
> >+	struct xt_osf_wc	wss;
> >+
> >+	__u8			ttl, df;
> >+	__u16			ss, mss;
> >+	__u16			opt_num;
> >+
> >+	char			genre[MAXGENRELEN];
> >+	char			version[MAXGENRELEN];
> >+	char			subtype[MAXGENRELEN];
> >+
> >+	/* MAX_IPOPTLEN is maximum if all options are NOPs or EOLs */
> >+	struct xt_osf_opt	opt[MAX_IPOPTLEN];
> 
> This really looks like you should use nested attributes.

This will be an unneded complication - we should provide an option
sequence, and maximum number of options is strickly determined by
the protocol specification. How does having separate attributes for each
option simplify the process?

> >+/* Defines for IANA option kinds */
> >+
> >+enum iana_options {
> >+	OSFOPT_EOL = 0,		/* End of options */
> >+	OSFOPT_NOP, 		/* NOP */
> >+	OSFOPT_MSS, 		/* Maximum segment size */
> >+	OSFOPT_WSO, 		/* Window scale option */
> >+	OSFOPT_SACKP,		/* SACK permitted */
> >+	OSFOPT_SACK,		/* SACK */
> >+	OSFOPT_ECHO,
> >+	OSFOPT_ECHOREPLY,
> >+	OSFOPT_TS,		/* Timestamp option */
> >+	OSFOPT_POCP,		/* Partial Order Connection Permitted */
> >+	OSFOPT_POSP,		/* Partial Order Service Profile */
> >+
> >+	/* Others are not used in the current OSF */
> >+	OSFOPT_EMPTY = 255,
> >+};
> 
> Why do we need to duplicate these?

Why duplicate? It is the only place of the constants describing used
option numbers. include/net/tcp.h does not have 'partial order' options
in particular.

> >+config NETFILTER_XT_MATCH_OSF
> >+	tristate '"osf" Passive OS fingerprint match'
> >+	depends on NETFILTER_ADVANCED
> 
> && NFNETLINK

Will add.

> >--- /dev/null
> >+++ b/net/netfilter/xt_osf.c
> >@@ -0,0 +1,469 @@
> >+/*
> >+ * Copyright (c) 2003+ Evgeniy Polyakov <zbr@ioremap.net>
> >+ *
> >+ *
> >+ * This program is free software; you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License as published by
> >+ * the Free Software Foundation; either version 2 of the License, or
> >+ * (at your option) any later version.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >+ * GNU General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program; if not, write to the Free Software
> >+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> >+ */
> >+
> >+#include <linux/module.h>
> >+#include <linux/kernel.h>
> >+
> >+#include <linux/connector.h>
> 
> Not needed. The remaining ones look like some (percpu?) could be
> removed as well.

Yup.

> >+struct xt_osf_finger_storage
> >+{
> 
> Please place the opening bracket consistently with the other
> structure definitions.

I.e. always on the new line? :)

> >+	struct list_head		finger_list;
> >+	spinlock_t			finger_lock;
> >+};
> >+
> >+/*
> >+ * Indexed by dont-fragment bit.
> >+ * It is the only constant value in the fingerprint.
> >+ */
> >+struct xt_osf_finger_storage xt_osf_fingers[2];
> 
> static

Ok.

> >+
> >+struct xt_osf_message {
> >+	struct cn_msg		cmsg;
> >+	struct xt_osf_nlmsg	nlmsg;
> >+};
> 
> Unused.

Yes.

> >+static int xt_osf_setup_callback(struct sock *ctnl, struct sk_buff *skb,
> >+			struct nlmsghdr *nlh, struct nlattr *osf_attrs[])
> >+{
> >+	struct xt_osf_user_finger *f;
> >+	struct nfgenmsg *nfmsg = NLMSG_DATA(nlh);
> >+	u16 delete = ntohs(nfmsg->res_id);
> 
> This looks like abuse, we use message types to distinguish between
> additions and deletions, alternative NLM_F_REPLACE.

Why to introduce the whole new callback function and attribute when the
only difference is to add or remove a processed entry?

> >+	struct xt_osf_finger *kf = NULL, *sf;
> >+	struct xt_osf_finger_storage *st;
> >+	int err;
> >+
> >+	if (!osf_attrs[OSF_ATTR_FINGER])
> >+		return -EINVAL;
> >+
> >+	f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> >+	st = &xt_osf_fingers[!!f->df];
> >+
> >+	/*
> >+	 * If 'delete' is set to 0 then we add attached fingerprint,
> >+	 * otherwise remove, and in this case we do not need to allocate 
> >data.
> >+	 */
> >+	if (!delete) {
> >+		kf = kmalloc(sizeof(struct xt_osf_finger), GFP_KERNEL);
> >+		if (!kf)
> >+			return -ENOMEM;
> >+
> >+		memcpy(&kf->finger, f, sizeof(struct xt_osf_user_finger));
> >+	}
> >+
> >+	err = -ENOENT;
> >+
> >+	rcu_read_lock();
> >+	list_for_each_entry_rcu(sf, &st->finger_list, finger_entry) {
> >+		if (memcmp(&sf->finger, f, sizeof(struct 
> >xt_osf_user_finger)))
> >+			continue;
> >+
> >+		if (delete) {
> >+			spin_lock_bh(&st->finger_lock);
> 
> This lock looks useless, all changes are done in netlink context under
> the nfnl mutex.

Agree.

> >+			list_del_rcu(&sf->finger_entry);
> >+			spin_unlock_bh(&st->finger_lock);
> >+			call_rcu(&sf->rcu_head, xt_osf_finger_free_rcu);
> >+		} else {
> >+			kfree(kf);
> >+			kf = NULL;
> >+		}
> >+
> >+		err = 0;
> >+		break;
> >+	}
> >+
> >+	if (kf) {
> >+		spin_lock_bh(&st->finger_lock);

Here too.

> >+		list_add_tail_rcu(&kf->finger_entry, &st->finger_list);
> >+		spin_unlock_bh(&st->finger_lock);
> >+#if 0
> >+		printk(KERN_INFO "Added rule for %s:%s:%s.\n",
> >+			kf->finger.genre, kf->finger.version, 
> >kf->finger.subtype);
> >+#endif
> >+	}
> >+	rcu_read_unlock();
> >+
> >+	return 0;
> >+}

...

> >+static bool xt_osf_match_packet(const struct sk_buff *skb,
> >+		const struct xt_match_param *p)
> >+{
> >+	const struct xt_osf_info *info = p->matchinfo;
> >+	const struct iphdr *ip = ip_hdr(skb);
> >+	const struct tcphdr *tcp;
> >+	struct tcphdr _tcph;
> >+	int fmatch = FMATCH_WRONG, fcount = 0;
> >+	unsigned int optsize = 0, check_WSS = 0;
> >+	u16 window, totlen, mss = 0;
> >+	bool df;
> >+	const unsigned char *optp = NULL, *_optp = NULL;
> >+	unsigned char opts[MAX_IPOPTLEN];
> >+	const struct xt_osf_finger *kf;
> >+	const struct xt_osf_user_finger *f;
> >+	const struct xt_osf_finger_storage *st;
> >+
> >+	if (!info)
> >+		return false;
> >+
> >+	tcp = skb_header_pointer(skb, ip_hdrlen(skb), sizeof(struct tcphdr), 
> >&_tcph);
> >+	if (!tcp)
> >+		return false;
> >+
> >+	if (!tcp->syn)
> >+		return false;
> >+
> >+	totlen = ntohs(ip->tot_len);
> >+	df = ntohs(ip->frag_off) & IP_DF;
> >+	window = ntohs(tcp->window);
> >+
> >+	if (tcp->doff * 4 > sizeof(struct tcphdr)) {
> >+		optsize = tcp->doff * 4 - sizeof(struct tcphdr);
> >+
> >+		if (optsize > sizeof(opts))
> >+			optsize = sizeof(opts);
> >+
> >+		_optp = optp = skb_header_pointer(skb, ip_hdrlen(skb) + 
> >sizeof(struct tcphdr),
> 
> Please break the line at 80 characters

Will do.

> >+				optsize, opts);
> >+	}
> >+
> >+	st = &xt_osf_fingers[df];
> >+
> >+	rcu_read_lock();
> >+	list_for_each_entry_rcu(kf, &st->finger_list, finger_entry) {
> >+		f = &kf->finger;
> >+
> >+		if (!(info->flags & XT_OSF_LOG) && strcmp(info->genre, 
> >f->genre))
> >+			continue;
> >+
> >+		optp = _optp;
> >+		fmatch = FMATCH_WRONG;
> >+
> >+		if (totlen == f->ss && xt_osf_ttl(skb, info, f->ttl)) {
> >+			int foptsize, optnum;
> >+
> >+			check_WSS = 0;
> >+
> >+			switch (f->wss.wc) {
> >+			case 0:
> >+				check_WSS = 0;
> >+				break;
> >+			case 'S':
> >+				check_WSS = 1;
> >+				break;
> >+			case 'T':
> >+				check_WSS = 2;
> >+				break;
> >+			case '%':
> >+				check_WSS = 3;
> >+				break;
> >+			default:
> >+				check_WSS = 4;
> >+				break;
> >+			}
> 
> This is really pushing my taste-buds. Whatever this does, please at
> use symbolic constants so the reader at least has a chance to understand
> it.

That's a bit unlcear window size processing state machine.
It links together knowledge about window-size, mss, mtu dependancy on
plain size numbers and modulo operations (like window size is multiple
of x bytes).

> >+			if (check_WSS == 4)
> >+				continue;
> >+
> >+			/* Check options */
> >+
> >+			foptsize = 0;
> >+			for (optnum = 0; optnum < f->opt_num; ++optnum)
> >+				foptsize += f->opt[optnum].length;
> >+
> >+			if (foptsize > MAX_IPOPTLEN || optsize > 
> >MAX_IPOPTLEN || optsize != foptsize)
> >+				continue;
> >+
> >+			for (optnum = 0; optnum < f->opt_num; ++optnum) {
> >+				if (f->opt[optnum].kind == (*optp)) {
> >+					__u32 len = f->opt[optnum].length;
> >+					const __u8 *optend = optp + len;
> >+					int loop_cont = 0;
> >+
> >+					fmatch = FMATCH_OK;
> >+
> >+					switch (*optp) {
> >+					case OSFOPT_MSS:
> >+						mss = optp[3];
> >+						mss <<= 8;
> >+						mss |= optp[2];
> >+
> >+						mss = ntohs(mss);
> >+						break;
> >+					case OSFOPT_TS:
> >+						loop_cont = 1;
> >+						break;
> >+					}
> >+
> >+					optp = optend;
> >+				} else
> >+					fmatch = FMATCH_OPT_WRONG;
> >+
> >+				if (fmatch != FMATCH_OK)
> >+					break;
> >+			}
> >+
> >+			if (fmatch != FMATCH_OPT_WRONG) {
> >+				fmatch = FMATCH_WRONG;
> >+
> >+				switch (check_WSS) {
> >+				case 0:
> >+					if (f->wss.val == 0 || window == 
> >f->wss.val)
> >+						fmatch = FMATCH_OK;
> >+					break;
> >+				case 1:	/* MSS */
> >+#define SMART_MSS_1	1460
> >+#define SMART_MSS_2	1448
> 
> Sigh. This entire function is completely unreadable and full of
> unexplained magic. I'll stop here, please clean this before
> resubmitting.

This is a special hack for special modems, which can decrease mss, and
since there is no common knowledge on how to differentiate them, there
is a check against different types.

This function is a core processing state machine, which checks received
packet parameters and compares them against prebuilt set of rules, which
include not only simple equal/not-equal, but also a bit more complex,
like multiple of (various parameter, mss, window-size, plain number).

-- 
	Evgeniy Polyakov

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-05-29  8:59   ` Evgeniy Polyakov
@ 2009-05-29  9:49     ` Jan Engelhardt
  2009-05-29 10:20       ` Evgeniy Polyakov
  2009-06-02 12:40     ` Patrick McHardy
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2009-05-29  9:49 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: Patrick McHardy, netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist


On Friday 2009-05-29 10:59, Evgeniy Polyakov wrote:
>> >+/* Defines for IANA option kinds */
>> >+
>> >+enum iana_options {
>> >+	OSFOPT_EOL = 0,		/* End of options */
>> >+	OSFOPT_NOP, 		/* NOP */
>> >+	OSFOPT_MSS, 		/* Maximum segment size */
>> >+	OSFOPT_WSO, 		/* Window scale option */
>> >+	OSFOPT_SACKP,		/* SACK permitted */
>> >+	OSFOPT_SACK,		/* SACK */
>> >+	OSFOPT_ECHO,
>> >+	OSFOPT_ECHOREPLY,
>> >+	OSFOPT_TS,		/* Timestamp option */
>> >+	OSFOPT_POCP,		/* Partial Order Connection Permitted */
>> >+	OSFOPT_POSP,		/* Partial Order Service Profile */
>> >+
>> >+	/* Others are not used in the current OSF */
>> >+	OSFOPT_EMPTY = 255,
>> >+};
>> 
>> Why do we need to duplicate these?
>
>Why duplicate? It is the only place of the constants describing used
>option numbers. include/net/tcp.h does not have 'partial order' options
>in particular.

Then you do one of these:

1. add TCPOPT_POCP/POSP to tcp.h or
2. define it locally:
#include <net/tcp.h>
enum {
	TCPOPT_POCP = 9,
	TCPOPT_POSP = 10,
};

>> >+config NETFILTER_XT_MATCH_OSF
>> >+	tristate '"osf" Passive OS fingerprint match'
>> >+	depends on NETFILTER_ADVANCED
>> 
>> && NFNETLINK
>
>Will add.

Does it really need to depend on nfnetlink? Even if I just want to
have it dumped to syslog?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-05-29  9:49     ` Jan Engelhardt
@ 2009-05-29 10:20       ` Evgeniy Polyakov
  0 siblings, 0 replies; 16+ messages in thread
From: Evgeniy Polyakov @ 2009-05-29 10:20 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Patrick McHardy, netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist

Hi Jan.

On Fri, May 29, 2009 at 11:49:51AM +0200, Jan Engelhardt (jengelh@medozas.de) wrote:
> >> >+config NETFILTER_XT_MATCH_OSF
> >> >+	tristate '"osf" Passive OS fingerprint match'
> >> >+	depends on NETFILTER_ADVANCED
> >> 
> >> && NFNETLINK
> >
> >Will add.
> 
> Does it really need to depend on nfnetlink? Even if I just want to
> have it dumped to syslog?

OSF is configured by nfnetlink now.

-- 
	Evgeniy Polyakov

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-05-29  8:59   ` Evgeniy Polyakov
  2009-05-29  9:49     ` Jan Engelhardt
@ 2009-06-02 12:40     ` Patrick McHardy
  1 sibling, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2009-06-02 12:40 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Evgeniy Polyakov wrote:
> Hi Patrick.
> 
> Thanks for your comments.
> 
> On Wed, May 27, 2009 at 06:28:16PM +0200, Patrick McHardy (kaber@trash.net) wrote:
>>> +#define XT_OSF_TTL_TRUE		0	/* True ip and fingerprint 
>>> TTL comparison */
>>> +#define XT_OSF_TTL_LESS		1	/* Check if ip TTL is less 
>>> than fingerprint one */
>>> +#define XT_OSF_TTL_NOCHECK	2	/* Do not compare ip and fingerprint 
>>> TTL at all */
>> These seem redundant - having neither of TRUE or LESS seems
>> equivalent to NOCHECK. Perhaps thats the reason why its not
>> used at all :) Looking at the code, "TRUE" would be better
>> named as "EQUAL".
> 
> There are only three types of TTL check - equal (for true), less than
> fingerprint one and when no check is performed at all. NOCHECK is
> actually used, but LESS one does not have a special check, but a default
> clause when neither TRUE or NOCHECK is specified.

OK, thanks for the explanation.

>>> +struct xt_osf_user_finger {
>>> +	struct xt_osf_wc	wss;
>>> +
>>> +	__u8			ttl, df;
>>> +	__u16			ss, mss;
>>> +	__u16			opt_num;
>>> +
>>> +	char			genre[MAXGENRELEN];
>>> +	char			version[MAXGENRELEN];
>>> +	char			subtype[MAXGENRELEN];
>>> +
>>> +	/* MAX_IPOPTLEN is maximum if all options are NOPs or EOLs */
>>> +	struct xt_osf_opt	opt[MAX_IPOPTLEN];
>> This really looks like you should use nested attributes.
> 
> This will be an unneded complication - we should provide an option
> sequence, and maximum number of options is strickly determined by
> the protocol specification. How does having separate attributes for each
> option simplify the process?

It doesn't, but it provides more flexibility which might make things
easier in case someone decides to add IPv6 support.

>>> +/* Defines for IANA option kinds */
>>> +
>>> +enum iana_options {
>>> +	OSFOPT_EOL = 0,		/* End of options */
>>> +	OSFOPT_NOP, 		/* NOP */
>>> +	OSFOPT_MSS, 		/* Maximum segment size */
>>> +	OSFOPT_WSO, 		/* Window scale option */
>>> +	OSFOPT_SACKP,		/* SACK permitted */
>>> +	OSFOPT_SACK,		/* SACK */
>>> +	OSFOPT_ECHO,
>>> +	OSFOPT_ECHOREPLY,
>>> +	OSFOPT_TS,		/* Timestamp option */
>>> +	OSFOPT_POCP,		/* Partial Order Connection Permitted */
>>> +	OSFOPT_POSP,		/* Partial Order Service Profile */
>>> +
>>> +	/* Others are not used in the current OSF */
>>> +	OSFOPT_EMPTY = 255,
>>> +};
>> Why do we need to duplicate these?
> 
> Why duplicate? It is the only place of the constants describing used
> option numbers. include/net/tcp.h does not have 'partial order' options
> in particular.

Indeed, I noticed this after sending my mail.

>>> +struct xt_osf_finger_storage
>>> +{
>> Please place the opening bracket consistently with the other
>> structure definitions.
> 
> I.e. always on the new line? :)

Just keep it consistent within your file, though I personally
prefer to keep it inconsistent with some of the other netfilter
files and not use a new line :)

>>> +static int xt_osf_setup_callback(struct sock *ctnl, struct sk_buff *skb,
>>> +			struct nlmsghdr *nlh, struct nlattr *osf_attrs[])
>>> +{
>>> +	struct xt_osf_user_finger *f;
>>> +	struct nfgenmsg *nfmsg = NLMSG_DATA(nlh);
>>> +	u16 delete = ntohs(nfmsg->res_id);
>> This looks like abuse, we use message types to distinguish between
>> additions and deletions, alternative NLM_F_REPLACE.
> 
> Why to introduce the whole new callback function and attribute when the
> only difference is to add or remove a processed entry?

Sticking to the defined semantics avoids the need for special-casing
in generic code like libnl. A new function also doesn't seem like a
loss at all, the only common part between addition and removal appears
to be the iteration.

>>> +		if (totlen == f->ss && xt_osf_ttl(skb, info, f->ttl)) {
>>> +			int foptsize, optnum;
>>> +
>>> +			check_WSS = 0;
>>> +
>>> +			switch (f->wss.wc) {
>>> +			case 0:
>>> +				check_WSS = 0;
>>> +				break;
>>> +			case 'S':
>>> +				check_WSS = 1;
>>> +				break;
>>> +			case 'T':
>>> +				check_WSS = 2;
>>> +				break;
>>> +			case '%':
>>> +				check_WSS = 3;
>>> +				break;
>>> +			default:
>>> +				check_WSS = 4;
>>> +				break;
>>> +			}
>> This is really pushing my taste-buds. Whatever this does, please at
>> use symbolic constants so the reader at least has a chance to understand
>> it.
> 
> That's a bit unlcear window size processing state machine.
> It links together knowledge about window-size, mss, mtu dependancy on
> plain size numbers and modulo operations (like window size is multiple
> of x bytes).

It very much reminds me of iptables userspace option parsing :|
Please at least use symbolic names for the different cases.
Why does it have to be mapped in the kernel at all? The raw value
of f->wss.wc doesn't seem to be used anywhere else.

>>> +#define SMART_MSS_1	1460
>>> +#define SMART_MSS_2	1448
>> Sigh. This entire function is completely unreadable and full of
>> unexplained magic. I'll stop here, please clean this before
>> resubmitting.
> 
> This is a special hack for special modems, which can decrease mss, and
> since there is no common knowledge on how to differentiate them, there
> is a check against different types.

Please explain what exactly it does in a comment. Would it make
sense to have the exact values supplied by userspace?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-05-27 16:28 ` Patrick McHardy
  2009-05-29  8:59   ` Evgeniy Polyakov
@ 2009-06-04 11:37   ` Evgeniy Polyakov
  2009-06-04 11:53     ` Patrick McHardy
  1 sibling, 1 reply; 16+ messages in thread
From: Evgeniy Polyakov @ 2009-06-04 11:37 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Hi.

On Wed, May 27, 2009 at 06:28:16PM +0200, Patrick McHardy (kaber@trash.net) wrote:
> >You will find something like this in the syslog:
> >Windows [2000:SP3:Windows XP Pro SP1, 2000 SP3]: 11.22.33.55:4024 -> 
> >11.22.33.44:139 hops=4
> >Linux [2.5:]: 1.2.3.4:44448 -> 11.22.33.44:22 hops=4
> 
> Please convert this to use nf_log_packet().

That's hard - there is no hook number in the match function, so we do
not really know if it is forward, input or prerouting.

In the version I'm about to send I fixed all rised issues except
multiple attributes - if there will be a need to extend attribute list
we can always switch them into new attribute except current all-in-one.

-- 
	Evgeniy Polyakov

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 11:37   ` Evgeniy Polyakov
@ 2009-06-04 11:53     ` Patrick McHardy
  2009-06-04 12:07       ` Evgeniy Polyakov
  0 siblings, 1 reply; 16+ messages in thread
From: Patrick McHardy @ 2009-06-04 11:53 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Evgeniy Polyakov wrote:
> Hi.
> 
> On Wed, May 27, 2009 at 06:28:16PM +0200, Patrick McHardy (kaber@trash.net) wrote:
>>> You will find something like this in the syslog:
>>> Windows [2000:SP3:Windows XP Pro SP1, 2000 SP3]: 11.22.33.55:4024 -> 
>>> 11.22.33.44:139 hops=4
>>> Linux [2.5:]: 1.2.3.4:44448 -> 11.22.33.44:22 hops=4
>> Please convert this to use nf_log_packet().
> 
> That's hard - there is no hook number in the match function, so we do
> not really know if it is forward, input or prerouting.

This is really needed, spamming the ring buffer is not a good option.

I'd say just add the hook number to xt_match_param. Its a bit
inconsistent anyways that we're handing it to checkentry for
validation, but not to the match function.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 11:53     ` Patrick McHardy
@ 2009-06-04 12:07       ` Evgeniy Polyakov
  2009-06-04 12:11         ` Patrick McHardy
  0 siblings, 1 reply; 16+ messages in thread
From: Evgeniy Polyakov @ 2009-06-04 12:07 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

On Thu, Jun 04, 2009 at 01:53:05PM +0200, Patrick McHardy (kaber@trash.net) wrote:
> >That's hard - there is no hook number in the match function, so we do
> >not really know if it is forward, input or prerouting.
> 
> This is really needed, spamming the ring buffer is not a good option.
> 
> I'd say just add the hook number to xt_match_param. Its a bit
> inconsistent anyways that we're handing it to checkentry for
> validation, but not to the match function.

Doesn't checkentry receive a mask of all possible hooks? There is still
no per-packet hook number. Although we can always use INPUT hook since
its the most widely used one. And drop a comment about this abuse.

-- 
	Evgeniy Polyakov

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 12:07       ` Evgeniy Polyakov
@ 2009-06-04 12:11         ` Patrick McHardy
  2009-06-04 13:11           ` Evgeniy Polyakov
  0 siblings, 1 reply; 16+ messages in thread
From: Patrick McHardy @ 2009-06-04 12:11 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Evgeniy Polyakov wrote:
> On Thu, Jun 04, 2009 at 01:53:05PM +0200, Patrick McHardy (kaber@trash.net) wrote:
>>> That's hard - there is no hook number in the match function, so we do
>>> not really know if it is forward, input or prerouting.
>> This is really needed, spamming the ring buffer is not a good option.
>>
>> I'd say just add the hook number to xt_match_param. Its a bit
>> inconsistent anyways that we're handing it to checkentry for
>> validation, but not to the match function.
> 
> Doesn't checkentry receive a mask of all possible hooks? There is still
> no per-packet hook number. Although we can always use INPUT hook since
> its the most widely used one. And drop a comment about this abuse.

Thats not what I meant. struct xt_match_param is passed to the
->match() callbacks from *t_do_table(). This is where you can
add the real hook number to have it available in ->match().

(Forgot to mention earlier: please in a seperate patch and adjusting
  all *tables copies)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 12:11         ` Patrick McHardy
@ 2009-06-04 13:11           ` Evgeniy Polyakov
  2009-06-04 13:16             ` Patrick McHardy
  0 siblings, 1 reply; 16+ messages in thread
From: Evgeniy Polyakov @ 2009-06-04 13:11 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

On Thu, Jun 04, 2009 at 02:11:24PM +0200, Patrick McHardy (kaber@trash.net) wrote:
> Thats not what I meant. struct xt_match_param is passed to the
> ->match() callbacks from *t_do_table(). This is where you can
> add the real hook number to have it available in ->match().
> 
> (Forgot to mention earlier: please in a seperate patch and adjusting
>  all *tables copies)

Kind of this (for ipv4 only so far, also reorderd a field to fill the
gap):

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 7b1a652..65a08ae 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -184,9 +184,10 @@ struct xt_counters_info
  * @matchinfo:	per-match data
  * @fragoff:	packet is a fragment, this is the data offset
  * @thoff:	position of transport header relative to skb->data
- * @hotdrop:	drop packet if we had inspection problems
+ * @hook:	hook number given packet came from
  * @family:	Actual NFPROTO_* through which the function is invoked
  * 		(helpful when match->family == NFPROTO_UNSPEC)
+ * @hotdrop:	drop packet if we had inspection problems
  */
 struct xt_match_param {
 	const struct net_device *in, *out;
@@ -194,8 +195,9 @@ struct xt_match_param {
 	const void *matchinfo;
 	int fragoff;
 	unsigned int thoff;
-	bool *hotdrop;
+	unsigned int hooknum;
 	u_int8_t family;
+	bool *hotdrop;
 };
 
 /**
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 810c0b6..385e4a0 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -335,7 +335,7 @@ ipt_do_table(struct sk_buff *skb,
 	mtpar.in      = tgpar.in  = in;
 	mtpar.out     = tgpar.out = out;
 	mtpar.family  = tgpar.family = NFPROTO_IPV4;
-	tgpar.hooknum = hook;
+	mtpar.hooknum = tgpar.hooknum = hook;
 
 	IP_NF_ASSERT(table->valid_hooks & (1 << hook));
 

-- 
	Evgeniy Polyakov

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 13:11           ` Evgeniy Polyakov
@ 2009-06-04 13:16             ` Patrick McHardy
  2009-06-04 13:30               ` Jan Engelhardt
  2009-06-04 14:50               ` Evgeniy Polyakov
  0 siblings, 2 replies; 16+ messages in thread
From: Patrick McHardy @ 2009-06-04 13:16 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Evgeniy Polyakov wrote:
> On Thu, Jun 04, 2009 at 02:11:24PM +0200, Patrick McHardy (kaber@trash.net) wrote:
>> Thats not what I meant. struct xt_match_param is passed to the
>> ->match() callbacks from *t_do_table(). This is where you can
>> add the real hook number to have it available in ->match().
>>
>> (Forgot to mention earlier: please in a seperate patch and adjusting
>>  all *tables copies)
> 
> Kind of this (for ipv4 only so far, also reorderd a field to fill the
> gap):

Exactly. But please verify that by reordering, you're not moving
the more commonly used members out of the first cacheline.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 13:16             ` Patrick McHardy
@ 2009-06-04 13:30               ` Jan Engelhardt
  2009-06-04 13:35                 ` Patrick McHardy
  2009-06-04 14:50               ` Evgeniy Polyakov
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2009-06-04 13:30 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Evgeniy Polyakov, netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist


On Thursday 2009-06-04 15:16, Patrick McHardy wrote:
> Evgeniy Polyakov wrote:
>> On Thu, Jun 04, 2009 at 02:11:24PM +0200, Patrick McHardy (kaber@trash.net)
>> wrote:
>>> Thats not what I meant. struct xt_match_param is passed to the
>>> ->match() callbacks from *t_do_table(). This is where you can
>>> add the real hook number to have it available in ->match().
>>>
>>> (Forgot to mention earlier: please in a seperate patch and adjusting
>>> all *tables copies)
>>
>> Kind of this (for ipv4 only so far, also reorderd a field to fill the
>> gap):
>
> Exactly. But please verify that by reordering, you're not moving
> the more commonly used members out of the first cacheline.
>
I am not sure the struct was ordered for optimized cacheline performance
beforehand either.

* par->in, par->out is only rarely used (think of xt_physdev, besides
  ipt_do_table itself);
* par->match similarly (xt_hashlimit)
* par->matchinfo, though showing more grep results, is usually copied to
  the stack by means of struct foo_target_info *info = par->matchinfo;
etc.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 13:30               ` Jan Engelhardt
@ 2009-06-04 13:35                 ` Patrick McHardy
  0 siblings, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2009-06-04 13:35 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Evgeniy Polyakov, netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist

Jan Engelhardt wrote:
> On Thursday 2009-06-04 15:16, Patrick McHardy wrote:
>> Evgeniy Polyakov wrote:
>>> On Thu, Jun 04, 2009 at 02:11:24PM +0200, Patrick McHardy (kaber@trash.net)
>>> wrote:
>>>> Thats not what I meant. struct xt_match_param is passed to the
>>>> ->match() callbacks from *t_do_table(). This is where you can
>>>> add the real hook number to have it available in ->match().
>>>>
>>>> (Forgot to mention earlier: please in a seperate patch and adjusting
>>>> all *tables copies)
>>> Kind of this (for ipv4 only so far, also reorderd a field to fill the
>>> gap):
>> Exactly. But please verify that by reordering, you're not moving
>> the more commonly used members out of the first cacheline.
>>
> I am not sure the struct was ordered for optimized cacheline performance
> beforehand either.
> 
> * par->in, par->out is only rarely used (think of xt_physdev, besides
>   ipt_do_table itself);
> * par->match similarly (xt_hashlimit)
> * par->matchinfo, though showing more grep results, is usually copied to
>   the stack by means of struct foo_target_info *info = par->matchinfo;
> etc.

Probably not (you ought to know :)). Just want to make sure if
it by accident had a good layout to not make it worse for this.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 13:16             ` Patrick McHardy
  2009-06-04 13:30               ` Jan Engelhardt
@ 2009-06-04 14:50               ` Evgeniy Polyakov
  2009-06-04 14:55                 ` Patrick McHardy
  1 sibling, 1 reply; 16+ messages in thread
From: Evgeniy Polyakov @ 2009-06-04 14:50 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

On Thu, Jun 04, 2009 at 03:16:17PM +0200, Patrick McHardy (kaber@trash.net) wrote:
> Exactly. But please verify that by reordering, you're not moving
> the more commonly used members out of the first cacheline.

That's what I ended up with. arptable does not use match entries. Did I
miss others?

As of cache line alignment - I moved the only writable member at the end
of the structure, so it should even improve things a bit, although I
really do not think this will ever be noticeble.

--

Added hook number into match extension parameter structure.

Signed-off-by: Evgeniy Polyakov <zbr@ioremap.net>

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 7b1a652..65a08ae 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -184,9 +184,10 @@ struct xt_counters_info
  * @matchinfo:	per-match data
  * @fragoff:	packet is a fragment, this is the data offset
  * @thoff:	position of transport header relative to skb->data
- * @hotdrop:	drop packet if we had inspection problems
+ * @hook:	hook number given packet came from
  * @family:	Actual NFPROTO_* through which the function is invoked
  * 		(helpful when match->family == NFPROTO_UNSPEC)
+ * @hotdrop:	drop packet if we had inspection problems
  */
 struct xt_match_param {
 	const struct net_device *in, *out;
@@ -194,8 +195,9 @@ struct xt_match_param {
 	const void *matchinfo;
 	int fragoff;
 	unsigned int thoff;
-	bool *hotdrop;
+	unsigned int hooknum;
 	u_int8_t family;
+	bool *hotdrop;
 };
 
 /**
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 820252a..db2540e 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -164,7 +164,7 @@ unsigned int ebt_do_table (unsigned int hook, struct sk_buff *skb,
 	mtpar.in      = tgpar.in  = in;
 	mtpar.out     = tgpar.out = out;
 	mtpar.hotdrop = &hotdrop;
-	tgpar.hooknum = hook;
+	mtpar.hooknum = tgpar.hooknum = hook;
 
 	read_lock_bh(&table->lock);
 	private = table->private;
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 810c0b6..385e4a0 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -335,7 +335,7 @@ ipt_do_table(struct sk_buff *skb,
 	mtpar.in      = tgpar.in  = in;
 	mtpar.out     = tgpar.out = out;
 	mtpar.family  = tgpar.family = NFPROTO_IPV4;
-	tgpar.hooknum = hook;
+	mtpar.hooknum = tgpar.hooknum = hook;
 
 	IP_NF_ASSERT(table->valid_hooks & (1 << hook));
 
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index 800ae85..d603ae4 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -361,7 +361,7 @@ ip6t_do_table(struct sk_buff *skb,
 	mtpar.in      = tgpar.in  = in;
 	mtpar.out     = tgpar.out = out;
 	mtpar.family  = tgpar.family = NFPROTO_IPV6;
-	tgpar.hooknum = hook;
+	mtpar.hooknum = tgpar.hooknum = hook;
 
 	IP_NF_ASSERT(table->valid_hooks & (1 << hook));
 


-- 
	Evgeniy Polyakov

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [resend] Passive OS fingerprint xtables match.
  2009-06-04 14:50               ` Evgeniy Polyakov
@ 2009-06-04 14:55                 ` Patrick McHardy
  0 siblings, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2009-06-04 14:55 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: netdev, David Miller, Paul E. McKenney,
	Netfilter Development Mailinglist, Jan Engelhardt

Evgeniy Polyakov wrote:
> Added hook number into match extension parameter structure.

Applied, thanks.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2009-06-04 14:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-11  9:53 [resend] Passive OS fingerprint xtables match Evgeniy Polyakov
2009-05-27 16:28 ` Patrick McHardy
2009-05-29  8:59   ` Evgeniy Polyakov
2009-05-29  9:49     ` Jan Engelhardt
2009-05-29 10:20       ` Evgeniy Polyakov
2009-06-02 12:40     ` Patrick McHardy
2009-06-04 11:37   ` Evgeniy Polyakov
2009-06-04 11:53     ` Patrick McHardy
2009-06-04 12:07       ` Evgeniy Polyakov
2009-06-04 12:11         ` Patrick McHardy
2009-06-04 13:11           ` Evgeniy Polyakov
2009-06-04 13:16             ` Patrick McHardy
2009-06-04 13:30               ` Jan Engelhardt
2009-06-04 13:35                 ` Patrick McHardy
2009-06-04 14:50               ` Evgeniy Polyakov
2009-06-04 14:55                 ` Patrick McHardy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.