Re: [tcpm] Q&C regarding tcpsecure-09 recommendations

["Anantha Ramaiah (ananth)" <[e-mail]>]

Andre,
    Pl see comments inline.  

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On 
> Behalf Of Andre Oppermann
> Sent: Sunday, June 01, 2008 3:18 PM
> To: [e-mail]
> Subject: [tcpm] Q&C regarding tcpsecure-09 recommendations
> 
> I'm reworking/refactoring/rewriting the FreeBSD TCP input 
> path (in a separate perforce tree at the moment) and in that 
> process I'm evaluating all assumptions and test them against 
> a close reading of all relevant RFCs.

Cool, thanks.

> 
> While reading and applying draft-ietf-tcpm-tcpsecure-09.txt 
> I've come across a couple of inconsistencies and issues that 
> require further clarification:
> 
> Section 3.2 Mitigation of Blind RST
> 
>   [Real world observation]
>   Some implementations send the RST not on rcv_nxt but on rcv_nxt-1 or
>   rcv_nxt+rcv_win.  Thus we accept three small windows of 
> rcv_nxt(+-1),
>   last_ack_sent(+-1) and rcv_nxt+rcv_win(+-1).
> 
>   Of course the challenge ACK should have the same effect at 
> the expense
>   of another round trip.  While these three (two in case of 
> rcv_nxt == last_ack_sent)
>   do increase the chances of a succesful attack (only every 
> third seq# has to
>   be probed plus the average hit rate is increased due to 
> having two/three
>   different places that match) real world behavior and 
> interop should be
>   considered and discussed.

After you sent the challenge ACK, are you saying that implementations
can still respond with the above values ? (rcvnxt -1 ) rcvnxt + rcvwin
(+-1) etc., ? OR are you saying that the seq# of RST with rcvnxt - 1
should be accepted [Some implementations do this to accept the RST of
the keepalive packet]

FWIW, section 8 (middlebox considerations talks about some corner cases)

Also the following table summarises (standard way) way to generate RST
's :

 * Generating a RST packet in response to an incoming packet.
 * According to TCP/IP Illustrated, Vol 2, page 994:
 *
 * +--------------+----------------------------------------------------+
 * |              |                 RST segment generated              |
 * |              +----------------+-----------------+-----------------|
 * | rcvd segment |      seq#      |      ack.field  |       flags     |
 * |===================================================================|
 * | Contains ACK | rcvd ack.field |          0      |      TH_RST     |
 * |--------------|----------------+-----------------+-----------------|
 * | ACK-less     |         0      | rcvd seq# + len | TH_RST | TH_ACK |
 * +--------------|----------------+-----------------+-----------------+
 *
 * The ACK flag should be set in all segments except when an initial SYN
 * is sent. So all RST packet will have 0 for ack number, without the
ACK
 * flag, except when the RST is in response to an SYN packet.
 */

> 
>   
> http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/netinet/tcp_inpu
> t.c#rev1.316
>   
> http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/netinet/tcp_inpu
> t.c#rev1.259
> 
> 
> Section 4.2 Mitigation of Blind SYN
> 
>   [Possible inconsistency in text]
>   The test in 4.2 eq. 1) is a little different.  In RFC793 it is:
> 
>    RCV.NXT =< SEG.SEQ < RCV.NXT+RCV.WND
> 
>   in tcpsecure it is described as one byte larger beyond the 
> real window:
> 
>    RCV.NXT <= SEG.SEQ <= RCV.NXT+RCV.WND.
> 
>   This seems to be incorrectly transcribed.

Good catch, I'll change it. I thought I had removed it before.

[I think I know the reason why this = got added, this is one of the
corner cases which we have fixed it long back, I'll see if I can
remember that and post it later, in any case I don't want to fix that in
this draft.]

> 
> 
> Section 5.2 Mitigation of Blind data injection attack
> 
>   [Clarification]
>   Data that doesn't satisfy the new check in first paragraph 
> MUST be discarded
>   silently. Before RFC793 used to require an ACK to be sent 
> back. This change
>   is not sufficiently explained and lacks an obvious rationale.

Good question, 793 implicitly tells that the following ACK range is
acceptable :
In other words an ACK value is acceptable if it is ((SND.UNA-(2^31-1))
<= SEG.ACK <= SND.NXT). 

Anything > SND.NXT above would solicit an ACK to be sent back, this is
simply an ACK being sent for something that has not yet been actually
sent. 

With this mitigation we are saying that the acceptability range becomes
:

((SND.UNA - MAX.SND.WND) <= SEG.ACK <= SND.NXT).

Since we haven't accepted this segment we drop this silently to
distinguish with the above case. This is whole point. (old ACK versus an
ACK ack'ing data that is unsent)

FWIW, this is been implemented and tested in some of the stacks and I
don't recall seeing any issues here.

I agree that generating an ACK back is also fine and shouldn't pose any
issues. 

I will think about this a bit more and will get back since my opinion is
either way is fine.


> 
>   The second related issue is where this check should be 
> placed relative to
>   RFC793 ordering?  Before the fifth check, last sentence 
> (page 72) "If the ACK
>   acks something not yet sent (SEG.ACK > SND.NXT) then send 
> an ACK, drop the
>   segment, and return." or right after it?  This has 

Should be before that, this is what we have tested with. Although I
can't think of on top of my head what would happen if we put after
that... 

> implications on the response
>   as tcpsecure requires to silently discard the segment.  If 
> it weren't for the
>   silent discard the new tcpsecure check could simply 
> replace/extend the fifth
>   check from RFC793.

Agreed. I think it is just a matter of perference and I think both
choices don't have any implications that I can see. 

> 
> 
> Feedback appreciated.

Thanks,
-Anantha
> 
> --
> Andre
> 
> [e-mail]
> 
> _______________________________________________
> tcpm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tcpm
> 
_______________________________________________
tcpm mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/tcpm