Hello,
I did test similar rules on my system
OpenBSD 7.2-current (GENERIC.MP) #978: Sun Jan 22 11:41:04 MST 2023
these are my rules:
set skip on lo
block return # block stateless traffic
pass out log # establish keep-state
pass in on iwn0 proto tcp from 192.168.2.0/24 to self port 22 \
keep state (source-track rule, max-src-conn 3)
this is my test terminal on remote host:
router$ for i in `seq 5` ; do nc 192.168.2.175 22 & done
[1] 32472
[2] 97453
[3] 7192
[4] 50386
[5] 57517
router$ SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
SSH-2.0-OpenSSH_9.1
there are three SSH version strings which indicates
I got 3 working connection of 5 attempts.
I'm not sure why it seems to work for me (ssh) while
it is broken for you (http). I'll give it a try with
webserver.
On Wed, Feb 08, 2023 at 04:34:55PM -0600, joshua stein wrote:
</snip>
> diff --git sys/net/pf.c sys/net/pf.c
> index 8cb1326a160..89703feab12 100644
> --- sys/net/pf.c
> +++ sys/net/pf.c
> @@ -481,12 +481,10 @@ pf_src_connlimit(struct pf_state **stp)
> if ((sn = pf_get_src_node((*stp), PF_SN_NONE)) == NULL)
> return (0);
>
> - sn->conn++;
> - (*stp)->src.tcp_est = 1;
> pf_add_threshold(&sn->conn_rate);
>
> if ((*stp)->rule.ptr->max_src_conn &&
> - (*stp)->rule.ptr->max_src_conn < sn->conn) {
> + sn->conn >= (*stp)->rule.ptr->max_src_conn) {
> pf_status.lcounters[LCNT_SRCCONN]++;
> bad++;
> }
> @@ -497,8 +495,11 @@ pf_src_connlimit(struct pf_state **stp)
> bad++;
> }
>
> - if (!bad)
> + if (!bad) {
> + sn->conn++;
> + (*stp)->src.tcp_est = 1;
> return (0);
> + }
>
> if ((*stp)->rule.ptr->overload_tbl) {
> struct pfr_addr p;
it seems to me the change to pf_src_connlimit() does
not alter behavior. I think change to pf_src_connlimit()
can be dropped.
> @@ -4919,14 +4920,14 @@ pf_tcp_track_full(struct pf_pdesc *pd, struct
> pf_state **stp, u_short *reason,
> pf_set_protostate(*stp, psrc, TCPS_CLOSING);
> if (th->th_flags & TH_ACK) {
> if (dst->state == TCPS_SYN_SENT) {
> - pf_set_protostate(*stp, pdst,
> - TCPS_ESTABLISHED);
> - if (src->state == TCPS_ESTABLISHED &&
> + if (src->state == TCPS_SYN_SENT &&
> !SLIST_EMPTY(&(*stp)->src_nodes) &&
> pf_src_connlimit(stp)) {
> REASON_SET(reason, PFRES_SRCLIMIT);
> return (PF_DROP);
> }
> + pf_set_protostate(*stp, pdst,
> + TCPS_ESTABLISHED);
> } else if (dst->state == TCPS_CLOSING)
> pf_set_protostate(*stp, pdst,
> TCPS_FIN_WAIT_2);
>
If I understand things right then in current pf we check conn limit
when we see acknowledgment of 3rd client's ACK sent by server. Not sure
if I sound clear here so let me draw little diagram:
SYN ----> sent by client moves state to
TCPS_SYN_SENT : TCPS_LISTEN/TCPS_CLOSED (1)
SYN | ACK <---- sent by server moves state to
TCPS_SYN_SENT : TCPS_SYN_SENT (2)
ACK ----> 3rd ack sent by client move state to:
TCPS_ESTABLISHED : TCPS_SYN_SENT (3)
ACK <---- server acknowledges client's 3rd ack moves state to
TCPS_ESTABLISHED : TCPS_ESTABLISHED (4)
currently we do conn limit check in step (4). Your change moves this
to earlier step (3) (given I understand things right here).
It's awfully early here I need sleep on this.
can you give it a try with your slightly modified diff? just drop
changes to pf_src_connlimit() and keep those in pf_tcp_track_full() which
I believe is the only relevant part.
thanks and
regards
sashan
