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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.00.0902122045080.670@fbirervta.pbzchgretzou.qr>
Date:	Thu, 12 Feb 2009 21:12:05 +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 19:57, Evgeniy Polyakov wrote:
>>  +/**
>> >+ * Wildcard MSS (kind of).
>>    * @wc:	wild card what(?)
>>    * @val:	some value?
>> >+ */
>> >+struct ipt_osf_wc {
>> >+	__u32			wc;
>> >+	__u32			val;
>> >+};
>> 
>
>This is a weird wildcards implementation for the MSS value.
>They can be pure wildcards and several subtypes which form the window
>size and MSS state machine.

Good to know. Please add this explanation as a comment
in the next iteration.

>> >+	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'?
>
>I would be very surprised if Linux will ever run on weird arch where int
>is not 32 bits.

If GCC had a switch to compile with I16 or I64, I could test,
but it does not. 'long', as used in the netfilter includes,
already bit people in pre-2.6.19, and nowadays we have that compat
crap thing in place.. Just don't take any chance, and go the safe
route with uint32_t.

>> >+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.
>
>What's this? It does not exist in the net/ipv4/netfilter/Kconfig

net/netfilter/Kconfig: (e.g.)

config NETFILTER_XT_MATCH_COMMENT
        tristate  '"comment" match support'
        depends on NETFILTER_ADVANCED
        ---help---

>> >+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'?
>
>Although it is a bit, it is not really a boolean type.

The underlying implementation is not of importance here.
It behaves like a bool, and that's all what is needed.

>
>> >+			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.
>
>Hmmm, why is this needed? We dereference linear kernel pointer at
>proper offset (modulo of the option size).

What if optp is odd?

>> Do this function via
>> 	.proto = IPPROTO_TCP,
>> in the osf_match struct instead:
>
>Hmm, it is not used in the existing net/ipv4/netfilter modules.

net/ipv4/netfilter/ is about the most dusted place in Netfilter-land.
Take a grep in net/netfilter/ ;-)
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ