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

[Andre Oppermann <[e-mail]>]

Anantha Ramaiah (ananth) wrote:
> Andre,
>     Pl see comments inline.  

Likewise.

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

This +-1 extension is done to accept the initial RST w/o having to send
a challenge ACK.  It has been observed by my fellow FreeBSD developer
Qing Li that certain system send the RST to weird off-by-one places in
either direction.

A surprisingly large number of RSTs are generated by socket closes of
connections that actually were in use shortly before.  Depending on the
exact timing of the connection teardown the RST is not sent in response
to some incoming segment but from the tcpcb state to inform the other
end of the immediate termination of this connection.

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

Would be great to hear about the corner case.

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

I can only partly follow you here.  My interpretation of the text in RFC793
tells us to send back a challenge-ACK to re-synchronize the connection.
Not to ACK something not yet 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.

I don't doubt the extended check at all.  I'm only concerned about its
relationship to the ACK check in RFC793 (section 3.9, page 72, fifth check,
ESTABLISHED STATE, first paragraph, last sentence) in two ways:

  a) tcp-secure section 5.2 essentially makes this check just tighter with
     the addition of the lower bound of (SND.UNA - MAX.SND.WND).  Instead
     of being another check (with a as of yet unclear ordering relative
     to RFC793) this one could simply replace the one from RFC793.  Would
     make the text and application of the check more clear.

  b) the change from a challenge-ACK (as my interpretation of RFC793) into
     a silent drop.

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

Then it should only test for the lower bound, not the upper one.
Otherwise the check from RFC793 would never trigger again.  Everything
is already filtered.

  5a) (SND.UNA - MAX.SND.WND) <= SEG.ACK <= SND.NXT  [tcp-secure 5.2]

  5b) SEG.ACK <= SND.NXT  [RFC793, expression changed from negative to positive]

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

My vote goes to replacing the fifth check of RFC793.  Whether to silently
ignore or sending a challenge-ACK I'm still undecided.

-- 
Andre

[e-mail]

_______________________________________________
tcpm mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/tcpm