From: Patrick McHardy <kaber@trash.net>
To: Evgeniy Polyakov <zbr@ioremap.net>
Cc: netdev@vger.kernel.org, David Miller <davem@davemloft.net>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Netfilter Development Mailinglist
<netfilter-devel@vger.kernel.org>,
Jan Engelhardt <jengelh@medozas.de>
Subject: Re: Passive OS fingerprint xtables match.
Date: Fri, 05 Jun 2009 13:54:15 +0200 [thread overview]
Message-ID: <4A290767.6080202@trash.net> (raw)
In-Reply-To: <20090604162212.GA24661@ioremap.net>
Evgeniy Polyakov wrote:
>
> This version switches from printk to nf_log_packet() interface,
> introduces separate add/remove commands for fingerprint processing,
> drops unused variables and includes, adds more comments and contains
> other misc cleanups.
Thanks, this looks much better already. A few final requests:
> +++ b/net/netfilter/xt_osf.c
> +
> +struct xt_osf_finger_storage {
> + struct list_head finger_list;
> +};
Without the lock the struct doesn't seem useful anymore. Any reason
for not removing it?
> +static int xt_osf_add_callback(struct sock *ctnl, struct sk_buff *skb,
> + struct nlmsghdr *nlh, struct nlattr *osf_attrs[])
> +{
> + struct xt_osf_user_finger *f;
> + struct xt_osf_finger *kf = NULL, *sf;
> + struct xt_osf_finger_storage *st;
> + int err = 0;
> +
> + if (!osf_attrs[OSF_ATTR_FINGER])
> + return -EINVAL;
> +
> + f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> + st = &xt_osf_fingers[!!f->df];
> +
> + kf = kmalloc(sizeof(struct xt_osf_finger), GFP_KERNEL);
> + if (!kf)
> + return -ENOMEM;
> +
> + memcpy(&kf->finger, f, sizeof(struct xt_osf_user_finger));
> +
> + 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;
> +
> + /*
> + * We are protected by nfnl mutex.
Which means we don't need RCU here (and for removal), right?
> + */
> + kfree(kf);
> + kf = NULL;
> +
> + err = -EEXIST;
> + break;
Please verify that the correct netlink flags are set to avoid
problems in case someone wants to add replacements later on.
This means addition always requires NLM_F_CREATE. NLM_F_REPLACE
is currently not supported, but EEXIST should only be returned
when NLM_F_EXCL is set.
> +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);
This code is only running inside RCU read side section.
You could use the non-refcounted variant.
> + 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);
tcp_optlen()?
> +
> + if (optsize > sizeof(opts))
> + optsize = sizeof(opts);
How can this happen? The doff field can only represent up to 40
bytes of option length.
> +
> + _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;
> +
> + /*
> + * Should not happen if userspace parser was written correctly.
> + */
> + if (f->wss.wc >= OSF_WSS_MAX)
> + 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;
> +
> + check_WSS = f->wss.wc;
> +
> + 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 OSF_WSS_PLAIN:
> + if (f->wss.val == 0 || window == f->wss.val)
> + fmatch = FMATCH_OK;
> + break;
> + case OSF_WSS_MSS:
> + /*
> + * Some smart modems decrease mangle MSS to
> + * SMART_MSS_2, so we check standard, decreased
> + * and the one provided in the fingerprint MSS
> + * values.
> + */
> +#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 OSF_WSS_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 OSF_WSS_MODULO:
> + if ((window % f->wss.val) == 0)
> + fmatch = FMATCH_OK;
> + break;
> + }
> + }
> +
> + if (fmatch != FMATCH_OK)
> + continue;
> +
> + fcount++;
> + /*
> + * Everyone uses NULL loginfo (well, everyone uses
> + * NULL for all but format and message fields), so
> + * I did not set it up either.
> + */
That comment doesn't provide much value IMO. A loginfo field is only
passed in by the various LOG targets.
> + if (info->flags & XT_OSF_LOG)
> + nf_log_packet(p->hooknum, 0, skb, p->in, p->out, NULL,
> + "%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)) {
> + unsigned int i;
> + struct xt_osf_user_finger fg;
> +
> + memset(&fg, 0, sizeof(fg));
> +
> + if (info->flags & XT_OSF_LOG) {
> + if (info->loglevel != XT_OSF_LOGLEVEL_ALL_KNOWN)
> + nf_log_packet(p->hooknum, 0, skb, p->in, p->out, NULL,
> + "window: %u, mss: %u, totlen: %u, df: %d, ttl: %u : ",
> + window, mss, totlen, df, ip->ttl);
nf_log_packet() can pass the entire packet to userspace. Header-field
extraction should be done by whatever is logging in userspace. This
looks very much like debugging anyways, where is the actual message?
> + if (_optp) {
> + optp = _optp;
> + for (i = 0; i < optsize; i++)
> + nf_log_packet(p->hooknum, 0, skb, p->in, p->out, NULL,
> + "%02X ", optp[i]);
Same here and other logging calls.
> + }
> +
> + nf_log_packet(p->hooknum, 0, skb, p->in, p->out, NULL,
> + "%pi4:%u -> %pi4:%u\n",
> + &ip->saddr, ntohs(tcp->source),
> + &ip->daddr, ntohs(tcp->dest));
> + }
> + }
> +
> + 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);
> + }
> +
> + 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");
Please no messages on successful module load. Or at least
not when statically built, but preferrably not at all.
> +
> + 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");
Even less on unload :)
> +}
> +
> +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);
>
>
next prev parent reply other threads:[~2009-06-05 11:54 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-06-04 16:22 Passive OS fingerprint xtables match Evgeniy Polyakov
2009-06-05 11:54 ` Patrick McHardy [this message]
2009-06-05 13:10 ` Jan Engelhardt
2009-06-05 13:30 ` Patrick McHardy
2009-06-05 13:44 ` Jan Engelhardt
2009-06-07 15:12 ` Evgeniy Polyakov
-- strict thread matches above, loose matches on Subject: below --
2009-06-07 15:17 Evgeniy Polyakov
2009-06-08 15:06 ` Patrick McHardy
2009-06-08 17:25 ` Evgeniy Polyakov
2009-03-26 14:14 Evgeniy Polyakov
2009-03-26 14:18 ` Patrick McHardy
2009-03-26 14:59 ` Evgeniy Polyakov
2009-03-26 15:08 ` Patrick McHardy
2009-03-26 15:41 ` Evgeniy Polyakov
2009-03-26 15:47 ` Patrick McHardy
2009-03-30 6:20 ` Evgeniy Polyakov
2009-05-01 20:15 ` Evgeniy Polyakov
2009-03-10 15:13 Evgeniy Polyakov
2009-03-10 16:01 ` Evgeniy Polyakov
2009-03-10 16:07 ` Jan Engelhardt
2009-03-11 21:43 ` Evgeniy Polyakov
2009-03-10 21:01 ` Jesper Dangaard Brouer
2009-03-10 21:54 ` Evgeniy Polyakov
2009-03-16 14:40 ` Patrick McHardy
2009-03-11 9:54 ` Pablo Neira Ayuso
2009-03-11 10:00 ` Evgeniy Polyakov
2009-03-16 14:42 ` Patrick McHardy
2009-02-12 17:12 Evgeniy Polyakov
2009-02-12 17:42 ` Paul E. McKenney
2009-02-12 17:51 ` Evgeniy Polyakov
2009-02-12 20:41 ` Paul E. McKenney
2009-02-18 14:55 ` Patrick McHardy
2009-02-12 18:22 ` Jan Engelhardt
2009-02-12 18:57 ` Evgeniy Polyakov
2009-02-12 20:12 ` Jan Engelhardt
2009-02-13 13:03 ` Evgeniy Polyakov
2009-02-13 13:51 ` Jan Engelhardt
2009-02-13 14:22 ` Evgeniy Polyakov
2009-02-13 14:41 ` Jan Engelhardt
2009-02-15 17:32 ` Evgeniy Polyakov
2009-02-18 15:02 ` Patrick McHardy
2009-02-18 15:07 ` Evgeniy Polyakov
2009-02-18 15:30 ` Jan Engelhardt
2009-02-19 11:56 ` Evgeniy Polyakov
2009-02-18 15:00 ` Patrick McHardy
2009-02-18 15:28 ` Jan Engelhardt
2009-02-18 15:14 ` Patrick McHardy
2009-01-29 17:20 Evgeniy Polyakov
2009-01-30 1:05 ` Paul E. McKenney
2009-02-09 16:09 ` Patrick McHardy
2009-02-13 12:49 ` Evgeniy Polyakov
2009-01-27 22:55 Evgeniy Polyakov
2009-01-29 3:36 ` Paul E. McKenney
2009-01-29 15:03 ` Evgeniy Polyakov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A290767.6080202@trash.net \
--to=kaber@trash.net \
--cc=davem@davemloft.net \
--cc=jengelh@medozas.de \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=paulmck@linux.vnet.ibm.com \
--cc=zbr@ioremap.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.