stream: RST no longer acks all data

[victorjulien]

Since forever (1578ef1) a valid RST would update the internal last_ack representation to include all unack'd data. This was originally done to make sure the unACK'd data was inspected/processed at flow timeout.

It was observed however, that if GAPs existed in this unACK'd data, a GAP could be reported in the stats and a GAP event would be raised. This doesn't make sense, as missing segments in the unACK'd part of the stream are completely normal. Segments simply do not all arrive in order.

It turns out that the original behavior of updating last_ack to include all unACK'd data is no longer needed. Both raw stream inspection and app-layer updates will include the unACK'd data on stream timeout.

Since the GAP detection uses last_ack to determine GAPs, not moving last_ack addresses the GAP false positives.

Ticket: #7422.

SV_BRANCH=OISF/suricata-verify#2154

[codecov]

Codecov Report

Attention: Patch coverage is 85.71429% with 4 lines in your changes missing coverage. Please review.

Project coverage is 83.14%. Comparing base (287d836) to head (3fe31ad).
Report is 7 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #12186       +/-   ##
===========================================
+ Coverage   49.98%   83.14%   +33.15%     
===========================================
  Files         912      912               
  Lines      257066   257045       -21     
===========================================
+ Hits       128503   213719    +85216     
+ Misses     128563    43326    -85237     

Flag	Coverage Δ
fuzzcorpus	60.94% <85.71%> (-0.04%)	⬇️
livemode	19.41% <0.00%> (+<0.01%)	⬆️
pcap	44.39% <71.42%> (-0.01%)	⬇️
suricata-verify	62.70% <71.42%> (-0.02%)	⬇️
unittests	59.18% <0.00%> (+50.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[suricata-qa]

Information:

ERROR: QA failed on SURI_TLPW1_files_sha256.

field	baseline	test	%
	SURI_TLPR1_stats_chk
.flow.end.tcp_liberal	13588	12288	90.43%
.app_layer.error.http.parser	700	561	80.14%
.app_layer.error.tls.gap	1718	1649	95.98%

Pipeline 23616

[victorjulien]

@ct0br0 I have the pcaps, no action needed at this point.

[victorjulien]

After investigating the missing hashes, my conclusion is that the behavior in this PR is correct. They all seem to have the following pattern:

1. unACK'd data in flight, with some GAPs (some segments missing)
2. RST comes in, last_ack doesn't cover all in flight segments

In master, the RST will auto-ACK all data, which means we're now sending this incomplete data to the app-layer parser. The parsers get confused and GAPs are counted. The HTTP parser specifically, uses some of the post-GAP data from the in-flight section to create new complete nonsense transaction, with no URI, etc. It's it this nonsense tx that is treated like a file, and for which the hashes are reported missing for this PR.

Since the issues are with TLPW1, I'm crafting some additional SV tests to show the issue.