Date: Wed, 20 Dec 2000 15:51:18 +0100 From: Jesper Skriver <jesper@skriver.dk> To: Don Lewis <Don.Lewis@tsc.tdk.com> Cc: Kris Kennaway <kris@FreeBSD.ORG>, Poul-Henning Kamp <phk@critter.freebsd.dk>, security-officer@FreeBSD.ORG, cvs-all@FreeBSD.ORG, freebsd-net@FreeBSD.ORG Subject: Re: what to do now ? Was: cvs commit: src/sys/netinet ip_icmp.c tcp_subr.c tcp_var.h Message-ID: <20001220155118.N81814@skriver.dk> In-Reply-To: <200012201046.CAA19456@salsa.gv.tsc.tdk.com>; from Don.Lewis@tsc.tdk.com on Wed, Dec 20, 2000 at 02:46:21AM -0800 References: <20001218182600.C1856@skriver.dk> <20001219222730.A29741@skriver.dk> <200012201046.CAA19456@salsa.gv.tsc.tdk.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Dec 20, 2000 at 02:46:21AM -0800, Don Lewis wrote:
> } It has the following functionality.
> }
> } - If the sysctl net.inet.tcp.icmp_admin_prohib_like_rst == 1 (default)
> } it enables the below.
> } - When a ICMP administrative prohibited is recieved, it check is the
> } IP header attached to the ICMP packet has any options set, if it has
> } it ignores it. The reason for this is, if any options is set the extra
> } 8 bytes is no longer the first 8 bytes from the TCP header, source/
> } destination ports and sequence number, which we need to find the
> } right TCP session.
>
> According to Stevens, we should get the first 8 bytes of the TCP header
> even if there are options on the ICMP packet. We would have to be
> careful to do sanity checking in this case, as well as guard against
> unaligned accesses to the TCP header data.
I'll read more on this, for now I think it's a improvement to ignore all
packets with IP options, then we can improve it later by handling
packets with options too.
> } @@ -714,6 +715,15 @@
> } (lport && inp->inp_lport != lport) ||
> } (laddr.s_addr && inp->inp_laddr.s_addr != laddr.s_addr) ||
> } (fport && inp->inp_fport != fport)) {
> } + inp = inp->inp_list.le_next;
> } + continue;
>
> Wouldn't it be more cleaner (gets rid of the loop) and more efficient (if
> we're getting blasted with ICMP messages) to use in_pcblookup_hash()?
I didn't change the loop, but I'll have a look at this code, to see if
we can improve it, but again, to get moving, I'd like to commit this,
and leave this for a later improvement, ok ?
> } + }
> } + /*
> } + * If tcp_sequence is set, then skip sessions where
> } + * the sequence number is not one of a unacknowledged
> } + * packet.
> } + */
> } + if ((tcp_sequence) && (tcp_seq_vs_sess(inp, tcp_sequence) == 0)) {
> } inp = inp->inp_list.le_next;
> } continue;
>
> We should pass in an extra flag to indicate if tcp_sequence is valid, since
> it can legally be zero.
Ack, will do.
> We should also bail out if the sequence check fails,
> since it isn't possible for there to be another connection with the same
> src/srcport/dst/dstport, so there is no sense in continuing the search.
That is was we do right ?
First we check if src/dst ip address and port numbers match, if not we
bail out, so if we reach the above check we know these match, then we
check for tcp sequence number, if this doesn't match we bail out.
/Jesper
--
Jesper Skriver, jesper(at)skriver(dot)dk - CCIE #5456
Work: Network manager @ AS3292 (Tele Danmark DataNetworks)
Private: Geek @ AS2109 (A much smaller network ;-)
One Unix to rule them all, One Resolver to find them,
One IP to bring them all and in the zone to bind them.
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20001220155118.N81814>