Suricata

We've run into a situation where Suricata 5.0.3 ignores a TCP flow because the 3-way handshake starts with:

SYN (TSval=X, TSecr=0)                -->
SYN (retransmitted, TSval=Y, TSecr=0) -->
                                      <-- SYN, ACK (TSval=Z, TSecr=Y)

The TCP state machine only stores the original SYN's TSval in the session, so StateSynSentValidateTimestamp() says the timestamps are invalid when it sees the SYN-ACK.
We have a fix for this that updates the session's timestamp if Suricata sees another SYN with a newer timestamp:

diff --git a/src/stream-tcp.c b/src/stream-tcp.c
index a99ce68b6..8e218f460 100644
--- a/src/stream-tcp.c
+++ b/src/stream-tcp.c
@@ -1641,6 +1641,21 @@ static int StreamTcpPacketStateSynSent(ThreadVars *tv, Packet *p,
                     "ssn->client.last_ack %"PRIu32"", ssn,
                     ssn->client.isn, ssn->client.next_seq,
                     ssn->client.last_ack);
+        } else if (PKT_IS_TOSERVER(p)) {
+            /* This covers a subtle edge case where a SYN is retransmitted with a newer
+             * TSval. If the the session's last_ts value isn't updated, the SYN-ACK
+             * will fail timestamp validation and the flow will be lost.
+             */
+            if ((ssn->client.flags & STREAMTCP_STREAM_FLAG_TIMESTAMP) != 0) {
+                if (TCP_HAS_TS(p)) {
+                    SCLogDebug("SYN retransmitted with timestamp options on packet %"PRIu64", ssn %p.", p->pcap_cnt, ssn);
+                    uint32_t ts_val = TCP_GET_TSVAL(p);
+                    if (ssn->client.last_ts < ts_val) {
+                        ssn->client.last_ts = ts_val;
+                        ssn->client.last_pkt_ts = p->ts.tv_sec;
+                    }
+                }
+            }
         }

         /** \todo check if it's correct or set event */

We're wondering if this is a reasonable fix, or if upheaving some security precaution.