On Mon, May 19, 2025 at 12:24:59PM +0000, Hu Ruinan wrote:
> >Description:
> When a TCP connection is in the ESTABLISHED state, OpenBSD's TCP
> implementation incorrectly accepts a packet with an acknowledgment
> number (ACK) smaller than expected and containing the FIN+ACK flags.
> In my test, I sent such a FIN+ACK packet with a smaller ACK value and
> OpenBSD accepted it, responded with FIN+ACK.
I can confirm this behavior.
> According to RFC 9293, if the ACK is a duplicate, it can be ignored. If the
> ACK acks something not yet sent, then send an ACK, drop the segment, and
> return.
I guess you refer to this section of the RFC. Qouting the relevant
path and conditions.
3. Functional Specification
3.10. Event Processing
3.10.7. SEGMENT ARRIVES
3.10.7.4. Other States
Fifth, check the ACK field:
- if the ACK bit is on,
o RFC 5961 [9], Section 5 describes a potential blind data
injection attack, and mitigation that implementations MAY
choose to include (MAY-12). TCP stacks that implement RFC
5961 MUST add an input check that the ACK value is
acceptable only if it is in the range of ((SND.UNA -
MAX.SND.WND) =< SEG.ACK =< SND.NXT). All incoming segments
whose ACK value doesn't satisfy the above condition MUST be
discarded and an ACK sent back. The new state variable
MAX.SND.WND is defined as the largest window that the local
sender has ever received from its peer (subject to window
scaling) or may be hard-coded to a maximum permissible
window value. When the ACK value is acceptable, the per-
state processing below applies:
o ESTABLISHED STATE
+ If SND.UNA < SEG.ACK =< SND.NXT, then set SND.UNA <-
SEG.ACK. Any segments on the retransmission queue that
are thereby entirely acknowledged are removed. Users
should receive positive acknowledgments for buffers that
have been SENT and fully acknowledged (i.e., SEND buffer
should be returned with "ok" response). If the ACK is a
duplicate (SEG.ACK =< SND.UNA), it can be ignored. If
the ACK acks something not yet sent (SEG.ACK > SND.NXT),
then send an ACK, drop the segment, and return.
Eighth, check the FIN bit:
The question is, whether our behavior is illegal.
Before we do "Eighth, check the FIN bit" we must comply to "Fifth,
check the ACK field".
There the hard requirement to drop the ACK packet is ((SND.UNA -
MAX.SND.WND) =< SEG.ACK =< SND.NXT). In your script you use
ack=ack_ack-5. As 5 is less than MAX.SND.WND the condition is not
met.
The part (SEG.ACK > SND.NXT) does not apply, as your ack number is
too small. The text "not yet sent" refers to some potential data
in the future, not never existing data in the past.
So the case you refer to is (SEG.ACK =< SND.UNA). It says "it can
be ignored". Note that "can" is not "MUST". And what does "it"
mean? The acknowledgement or the whole packet that carries the
additional FIN?
So while our behavior is questionable, I think it is not illegal.
Usually I am very carefull here, as every change has a high risk.
Interoperability may suffer or even DoS is possible when sending
too many ACK packets. And just changing some line in tcp_input()
can have unexpected consequences for other corner cases.
Do you see a clear violation of the RFC?
Do you see a potential DoS by not being strict enough?
What would be the benefit if we would ignore your ACK packet?
Thanks for testing our TCP stack and searching for corner cases.
bluhm
