Hello,
On Fri, May 24, 2024 at 01:50:25PM +1000, David Gwynne wrote:
> I don't know enough about TCP to say if this is 100% correct or not. My
> biggest question is in which TCP state will pf know what the initial
> sequence numbers are for the dst side of the connection, and if that matters
> for merging the states together.

    this happens in pf_tcp_track_full(). the sequence number for
    each peer gets initialized with the first packet seen
    from from particular peer. In typical situation it should
    be SYN from client and SYN+ACK from server. The related code
    reads as follows:

4889         /*
4890          * Sequence tracking algorithm from Guido van Rooij's paper:
4891          *   http://www.madison-gurkha.com/publications/tcp_filtering/
4892          *      tcp_filtering.ps
4893          */
4894
4895         orig_seq = seq = ntohl(th->th_seq);
4896         if (src->seqlo == 0) {
4897                 /* First packet from this end. Set its state */
4898
4899                 if (((*stp)->state_flags & PFSTATE_SCRUB_TCP || dst->scrub) &&
4900                     src->scrub == NULL) {
4901                         if (pf_normalize_tcp_init(pd, src)) {
4902                                 REASON_SET(reason, PFRES_MEMORY);
4903                                 return (PF_DROP);
4904                         }
4905                 }
4906
4907                 /* Deferred generation of sequence number modulator */
....
4945
4946                 src->seqlo = seq;
4947                 if (src->state < TCPS_SYN_SENT)
4948                         pf_set_protostate(*stp, psrc, TCPS_SYN_SENT);
4949
4950

    It seems to me that if peer (state.src, state.dst) is in SYN_SENT state
    (or beyond SYN_SENT state) then firewall has learned the sequence number.
>
> Either way, this is a definite improvement and I think it should go in.

    I will commit it the change.

thanks and
regards
sashan