| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 12 Feb 2009 19:22:25 +0100 (CET)
From: Jan Engelhardt <jengelh@...ozas.de>
To: Evgeniy Polyakov <zbr@...emap.net>
cc: Patrick McHardy <kaber@...sh.net>, netdev@...r.kernel.org,
David Miller <davem@...emloft.net>,
"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Netfilter Development Mailinglist
<netfilter-devel@...r.kernel.org>
Subject: Re: Passive OS fingerprint xtables match.
On Thursday 2009-02-12 18:12, Evgeniy Polyakov wrote:
>
>You will find something like this in the syslog:
>ipt_osf: Windows [2000:SP3:Windows XP Pro SP1, 2000 SP3]: 11.22.33.55:4024 -> 11.22.33.44:139
>
>Passive OS fingerprint homepage (archives, examples):
>http://www.ioremap.net/projects/osf
>diff --git a/include/linux/netfilter_ipv4/ipt_osf.h b/include/linux/netfilter_ipv4/ipt_osf.h
>new file mode 100644
>index 0000000..c2d2c7a
>--- /dev/null
>+++ b/include/linux/netfilter_ipv4/ipt_osf.h
Much of this I think should be upgraded to the xt_ prefixing
already, even if it does not do IPv6 yet.
>+struct ipt_osf_info {
>+ char genre[MAXGENRELEN];
>+ __u32 len;
>+ __u32 flags;
>+ __u32 loglevel;
>+ __u32 ttl;
>+};
I do not think you need an entire u32 for loglevel and ttl.
Could there at least be some description as to what the
following fields are?
+/**
>+ * Wildcard MSS (kind of).
* @wc: wild card what(?)
* @val: some value?
>+ */
>+struct ipt_osf_wc {
>+ __u32 wc;
>+ __u32 val;
>+};
>+struct ipt_osf_user_finger {
>+ struct ipt_osf_wc wss;
>+
>+ __u8 ttl, df;
>+ __u16 ss, mss;
>+ int opt_num;
>+
>+ char genre[MAXGENRELEN];
>+ char version[MAXGENRELEN];
>+ char subtype[MAXGENRELEN];
>+
>+ /* MAX_IPOPTLEN is maximum if all options are NOPs or EOLs */
>+ struct ipt_osf_opt opt[MAX_IPOPTLEN];
>+};
Are you sure it is safe to use arch-dependent types like 'int'?
>+/* Defines for IANA option kinds */
>+
>+#define OSFOPT_EOL 0 /* End of options */
>+#define OSFOPT_NOP 1 /* NOP */
>+#define OSFOPT_MSS 2 /* Maximum segment size */
>+#define OSFOPT_WSO 3 /* Window scale option */
>+#define OSFOPT_SACKP 4 /* SACK permitted */
>+#define OSFOPT_SACK 5 /* SACK */
>+#define OSFOPT_ECHO 6
>+#define OSFOPT_ECHOREPLY 7
>+#define OSFOPT_TS 8 /* Timestamp option */
>+#define OSFOPT_POCP 9 /* Partial Order Connection Permitted */
>+#define OSFOPT_POSP 10 /* Partial Order Service Profile */
>+#define OSFOPT_EMPTY 255
>+/* Others are not used in current OSF */
Wrapping this into an enum seems to look nicer.
>+#ifdef __KERNEL__
>+
>+#include <linux/list.h>
>+#include <net/tcp.h>
>+
>+struct ipt_osf_finger {
>+ struct rcu_head rcu_head;
>+ struct list_head finger_entry;
>+ struct ipt_osf_user_finger finger;
>+};
>+
>+#endif /* __KERNEL__ */
>+
>+#endif /* _IPT_OSF_H */
Move this section to the .c file.
>+config IP_NF_MATCH_OSF
>+ tristate '"osf" match support'
>+ depends on NETFILTER_ADVANCED && CONNECTOR
>+ help
>+ Passive OS fingerprint matching module.
>+ You should download and install rule loading software from
>+ http://www.ioremap.net/projects/osf
>+
>+ To compile it as a module, choose M here. If unsure, say N.
>+
Please do use NETFILTER_XT_ and its section.
>--- /dev/null
>+++ b/net/ipv4/netfilter/ipt_osf.c
>+
>+static inline int ipt_osf_ttl(const struct sk_buff *skb, struct ipt_osf_info *info,
>+ unsigned char f_ttl)
>+{
>+ struct iphdr *ip = ip_hdr(skb);
>+
>+ if (info->flags & IPT_OSF_TTL) {
>+ if (info->ttl == IPT_OSF_TTL_TRUE)
>+ return (ip->ttl == f_ttl);
>+ if (info->ttl == IPT_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 ipt_osf_match_packet(const struct sk_buff *skb,
>+ const struct xt_match_param *p)
>+{
>+ struct ipt_osf_info *info = (struct ipt_osf_info *)p->matchinfo;
>+ struct iphdr *ip;
>+ struct tcphdr _tcph, *tcp;
>+ int fmatch = FMATCH_WRONG, fcount = 0;
>+ unsigned int optsize = 0, check_WSS = 0;
>+ u16 window, totlen, mss = 0;
>+ unsigned char df, *optp = NULL, *_optp = NULL;
Maybe this should be 'bool df'?
>+ unsigned char opts[MAX_IPOPTLEN];
>+ struct ipt_osf_finger *kf;
>+ struct ipt_osf_user_finger *f;
>+ struct ipt_osf_finger_storage *st;
>+
>+ if (!info)
>+ return 0;
Use 'false' - the function returns bool.
>+
>+ ip = ip_hdr(skb);
>+ if (!ip)
>+ return 0;
>+
>+ tcp = skb_header_pointer(skb, ip->ihl * 4, sizeof(struct tcphdr), &_tcph);
Use ip_hdrlen(skb) instead of ihl*4.
>+ df = ((ntohs(ip->frag_off) & IP_DF) ? 1 : 0);
Together with bool df, it's just df = ntohs(ip_frag_off) & IP_DF;
>+ st = &ipt_osf_fingers[!!df];
And the !! becomes redundant.
>+ for (optnum = 0; optnum < f->opt_num; ++optnum) {
>+ if (f->opt[optnum].kind == (*optp)) {
>+ __u32 len = f->opt[optnum].length;
>+ __u8 *optend = optp + len;
>+ int loop_cont = 0;
>+
>+ fmatch = FMATCH_OK;
>+
>+ switch (*optp) {
>+ case OSFOPT_MSS:
>+ mss = ntohs(*(u16 *)(optp + 2));
This needs get_unaligned(), in case someone sends a malicious packet.
>+ if (info->flags & IPT_OSF_LOG)
>+ printk(KERN_INFO "%s [%s:%s] : "
>+ "%u.%u.%u.%u:%u -> %u.%u.%u.%u:%u hops=%d\n",
>+ f->genre, f->version, f->subtype,
>+ NIPQUAD(ip->saddr), ntohs(tcp->source),
>+ NIPQUAD(ip->daddr), ntohs(tcp->dest),
>+ f->ttl - ip->ttl);
Dave told me NIPQUAD is slated for ripout. Though, I have not
looked if there is already a new format specifier for v4 addresses.
>+ return (fmatch == FMATCH_OK) ? 1 : 0;
Just fmatch == FMATCH_OK is sufficient for bool-returning functions.
>+static bool
>+ipt_osf_checkentry(const struct xt_mtchk_param *m)
>+{
>+ struct ipt_ip *ip = (struct ipt_ip *)m->entryinfo;
>+
>+ if (ip->proto != IPPROTO_TCP)
>+ return false;
>+
>+ return true;
>+}
Do this function via
.proto = IPPROTO_TCP,
in the osf_match struct instead:
>+static struct xt_match ipt_osf_match = {
>+ .name = "osf",
>+ .revision = 0,
>+ .family = AF_INET,
.family = NFPROTO_IPV4
>+ .hooks = (1 << NF_INET_LOCAL_IN) | (1 << NF_INET_PRE_ROUTING),
>+ .match = ipt_osf_match_packet,
>+ .checkentry = ipt_osf_checkentry,
>+ .matchsize = sizeof(struct ipt_osf_info),
>+ .me = THIS_MODULE,
>+};
>+ if (kf) {
>+ spin_lock_bh(&st->finger_lock);
>+ list_add_tail_rcu(&kf->finger_entry, &st->finger_list);
>+ spin_unlock_bh(&st->finger_lock);
>+
>+ printk("%s: added rule for %s:%s:%s.\n", __func__, kf->finger.genre, kf->finger.version, kf->finger.subtype);
Missing verbosity level.
>+static void __devexit ipt_osf_fini(void)
Why __devinit and __devexit, should not this be __init/__exit?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists