netdev - Re: [RFC] sctp/tcp: Question -- ICMPv4 length check (not) redundant?

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 28 Jul 2008 13:27:46 -0400
From:	Vlad Yasevich <vladislav.yasevich@...com>
To:	Gerrit Renker <gerrit@....abdn.ac.uk>,
	Vlad Yasevich <vladislav.yasevich@...com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [RFC] sctp/tcp: Question -- ICMPv4 length check (not) redundant?

Gerrit Renker wrote:
> | >  * only TCP/SCTP seem to have a proper per-protocol "payload too short" test;
> | 
> | Hm..  In the standard case, these do seem to be redundant since 8 bytes are required
> | by ICMP spec.
> | 
> | >  * for DCCP, the work is actually doubled since 
> | >    - first the ICMP handler tests for minimally 8 bytes, 
> | >    - then the DCCP error handler tests for required minimum of 12 bytes.
> | 
> | DCCP and any other protocol that requires more error data should check for it in
> | its own handler.  8 bytes should be guaranteed to such handler.
> | 
> | What am I missing?
> | 
> As per last message, please disregard the patch suggestion made at the
> beginning of the thread.
> 
> In TCP, the 8 bytes happen to be enough for doing sequence number checks. Other
> protocols have different header lengths and semantics. Thus doing the checks
> at the transport layer makes more sense than in the ICMP handler.
> 
> RFC 1122 is almost 20 years old, from a time before IPcomp, SCTP, or DCCP.

So the suggestion really is then to remove the length check icmp_unreach()?

Because as it stands right now, the protocol error handler will not be invoked
if we don't have the iphdr + 8 bytes worth of data.  That's is actually a requirement
from the ICMP rfc 792.

Seems that not including those 8 bytes is a violation of that spec.  So, icmp_unreach()
should keep current code and jump to out_err upon failure.

Each upper layer protocol that requires additional space, should validate the existence
of that additional space and handle it appropriately.

It doesn't matter that IPcomp header is only 4 bytes.  ICMP error requires ihlen + 8 bytes and 
at least that much should be provided  Otherwise, it a malformed ICMP packet.

-vlad

--
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