Page MenuHomeFreeBSD
Differential D38514

net/rospo: Update to 0.10.2
ClosedPublic
Actions

Authored by fuz on Feb 11 2023, 7:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 4 2025, 5:02 AM
Unknown Object (File)
Feb 1 2025, 1:01 PM
Unknown Object (File)
Feb 1 2025, 12:47 PM
Unknown Object (File)
Feb 1 2025, 9:59 AM
Unknown Object (File)
Jan 28 2025, 8:26 PM
Unknown Object (File)
Jan 21 2025, 9:05 PM
Unknown Object (File)
Jan 19 2025, 12:53 PM
Unknown Object (File)
Dec 3 2024, 4:54 AM
View All 76 Files
Subscribers
mat

Details

Reviewers
eduardo
flo
Commits
R11:f3c54c53e8a2: net/rospo: Update to 0.10.2
R11:b30c936dfa2d: net/rospo: Update to 0.10.2
Summary
net/rospo: Update to 0.10.2

A patch release.

Changelog: https://github.com/ferama/rospo/releases/tag/v0.10.2

MFH:		2023Q1
Test Plan

Tested on i386 amd64 FreeBSD 12.4 13.1. arm64 pending.
See http://fuz.su/~fuz/freebsd/batch1/

Diff Detail

Repository
R11 FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

fuz created this revision.Feb 11 2023, 7:14 PM
Herald added a subscriber: mat. · View Herald TranscriptFeb 11 2023, 7:14 PM
fuz requested review of this revision.Feb 11 2023, 7:14 PM
Harbormaster completed remote builds in B49696: Diff 116998.Feb 11 2023, 7:14 PM
flo accepted this revision.Feb 12 2023, 9:39 PM

Approved. FWIW, the sentence "A patch release" didn't immediately make sense to me. I'd use a couple of more words, like "fixes a couple of bugs in the 0.10 branch". I guess you want do describe why this will be MFH'ed?

This revision is now accepted and ready to land.Feb 12 2023, 9:39 PM
fuz added a comment.Feb 13 2023, 12:24 AM

“path release” means that this release mainly (or exclusive) fixes bugs. As per policy, bug fixes should be merged into the quarterly branch, which is why I have done so.

Closed by commit R11:b30c936dfa2d: net/rospo: Update to 0.10.2 (authored by fuz). · Explain WhyFeb 14 2023, 8:24 PM
This revision was automatically updated to reflect the committed changes.
fuz added a commit: R11:b30c936dfa2d: net/rospo: Update to 0.10.2.
fuz added a commit: R11:f3c54c53e8a2: net/rospo: Update to 0.10.2.Feb 15 2023, 7:28 PM

Revision Contents
Changeset List

PathSize
net/
rospo/
3 lines
14 lines

Diff 117237

View Options

net/rospo/Makefile

Show First 20 Lines • Show All 196 Lines • ▼ Show 20 LinesSYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_syn, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_syn), 0, &VNET_NAME(tcp_insecure_syn), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting SYN packets"); "Follow RFC793 instead of RFC5961 criteria for accepting SYN packets");
VNET_DEFINE(int, tcp_insecure_rst) = 0; VNET_DEFINE(int, tcp_insecure_rst) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_rst), 0, &VNET_NAME(tcp_insecure_rst), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting RST packets"); "Follow RFC793 instead of RFC5961 criteria for accepting RST packets");
VNET_DEFINE(int, tcp_insecure_ack) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_ack, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_ack), 0,
"Follow RFC793 criteria for validating SEG.ACK");
VNET_DEFINE(int, tcp_recvspace) = 1024*64; VNET_DEFINE(int, tcp_recvspace) = 1024*64;
#define V_tcp_recvspace VNET(tcp_recvspace) #define V_tcp_recvspace VNET(tcp_recvspace)
SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size"); &VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size");
VNET_DEFINE(int, tcp_do_autorcvbuf) = 1; VNET_DEFINE(int, tcp_do_autorcvbuf) = 1;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_do_autorcvbuf), 0, &VNET_NAME(tcp_do_autorcvbuf), 0,
▲ Show 20 Lines • Show All 2,018 Lines • ▼ Show 20 Lines if (V_tcp_insecure_syn &&
/* Send challenge ACK. */ /* Send challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt, tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt,
tp->snd_nxt, TH_ACK); tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt; tp->last_ack_sent = tp->rcv_nxt;
m = NULL; m = NULL;
} }
goto drop; goto drop;
} }
ccUnsubmitted
Done
Inline Actions

At the server side (listening side), I am thinking if we move the secure ACK check here and above the TOF_TS check, it would save cycles on checking these timestamps.

cc: At the server side (listening side), I am thinking if we move the secure ACK check here and…
tuexenAuthorUnsubmitted
Done
Inline Actions

This patch is only about improving the input validation, not about changing the sequence of checks.
Please note that RFC 7323 requires to perform the timestamp checks before the SEG.ACK number checks.

tuexen: This patch is only about improving the input validation, not about changing the sequence of…
ccUnsubmitted
Done
Inline Actions

I see, in the section 5.3. Basic PAWS Algorithm from RFC7323 claims Also, PAWS processing MUST take precedence over the regular TCP acceptability check (Section 3.3 in [RFC0793]), which is performed after verification of the received Timestamps option...

cc: I see, in the section `5.3. Basic PAWS Algorithm` from RFC7323 claims `Also, PAWS processing…
/* /*
* RFC 1323 PAWS: If we have a timestamp reply on this segment * RFC 1323 PAWS: If we have a timestamp reply on this segment
* and it's less than ts_recent, drop it. * and it's less than ts_recent, drop it.
*/ */
if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent && if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent &&
TSTMP_LT(to.to_tsval, tp->ts_recent)) { TSTMP_LT(to.to_tsval, tp->ts_recent)) {
/* Check to see if ts_recent is over 24 days old. */ /* Check to see if ts_recent is over 24 days old. */
if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) { if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) {
▲ Show 20 Lines • Show All 185 Lines • ▼ Show 20 Lines if (tp->t_state == TCPS_SYN_RECEIVED ||
goto dropafterack; goto dropafterack;
else else
goto drop; goto drop;
} }
/* /*
* Ack processing. * Ack processing.
*/ */
if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) {
ccUnsubmitted
Done
Inline Actions

Can use TCP_MAXWIN instead of 65535.

cc: Can use `TCP_MAXWIN` instead of `65535`.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed in 37b3e6a660f551ceda412e2115748f3b97b2e615, since this review has already been committed.

tuexen: Fixed in [[ https://cgit.FreeBSD.org/src/commit/?id=37b3e6a660f551ceda412e2115748f3b97b2e615 |…
/* Checking SEG.ACK against ISS is definitely redundant. */
tp->t_flags2 |= TF2_NO_ISS_CHECK;
ccUnsubmitted
Done
Inline Actions

code style: As there are two lines, recommend using a pair of {} for readability.

cc: code style: As there are two lines, recommend using a pair of `{}` for readability.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed.

tuexen: Fixed.
}
if (!V_tcp_insecure_ack) {
tcp_seq seq_min;
bool ghost_ack_check;
if (tp->t_flags2 & TF2_NO_ISS_CHECK) {
/* Check for too old ACKs (RFC 5961, Section 5.2). */
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
} else {
if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) {
/* Checking for ghost ACKs is stricter. */
seq_min = tp->iss + 1;
ghost_ack_check = true;
} else {
/*
* Checking for too old ACKs (RFC 5961,
* Section 5.2) is stricter.
*/
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
}
}
if (SEQ_LT(th->th_ack, seq_min)) {
if (ghost_ack_check)
TCPSTAT_INC(tcps_rcvghostack);
else
TCPSTAT_INC(tcps_rcvacktooold);
/* Send a challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m,
tp->rcv_nxt, tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt;
m = NULL;
goto drop;
}
}
switch (tp->t_state) { switch (tp->t_state) {
/* /*
* In SYN_RECEIVED state, the ack ACKs our SYN, so enter * In SYN_RECEIVED state, the ack ACKs our SYN, so enter
* ESTABLISHED state and continue processing. * ESTABLISHED state and continue processing.
* The ACK was checked above. * The ACK was checked above.
*/ */
case TCPS_SYN_RECEIVED: case TCPS_SYN_RECEIVED:
▲ Show 20 Lines • Show All 1,746 Lines • Show Last 20 Lines
View Options

net/rospo/distinfo

Show First 20 Lines • Show All 196 Lines • ▼ Show 20 LinesSYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_syn, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_syn), 0, &VNET_NAME(tcp_insecure_syn), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting SYN packets"); "Follow RFC793 instead of RFC5961 criteria for accepting SYN packets");
VNET_DEFINE(int, tcp_insecure_rst) = 0; VNET_DEFINE(int, tcp_insecure_rst) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_rst, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_rst), 0, &VNET_NAME(tcp_insecure_rst), 0,
"Follow RFC793 instead of RFC5961 criteria for accepting RST packets"); "Follow RFC793 instead of RFC5961 criteria for accepting RST packets");
VNET_DEFINE(int, tcp_insecure_ack) = 0;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, insecure_ack, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_insecure_ack), 0,
"Follow RFC793 criteria for validating SEG.ACK");
VNET_DEFINE(int, tcp_recvspace) = 1024*64; VNET_DEFINE(int, tcp_recvspace) = 1024*64;
#define V_tcp_recvspace VNET(tcp_recvspace) #define V_tcp_recvspace VNET(tcp_recvspace)
SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, TCPCTL_RECVSPACE, recvspace, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size"); &VNET_NAME(tcp_recvspace), 0, "Initial receive socket buffer size");
VNET_DEFINE(int, tcp_do_autorcvbuf) = 1; VNET_DEFINE(int, tcp_do_autorcvbuf) = 1;
SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW, SYSCTL_INT(_net_inet_tcp, OID_AUTO, recvbuf_auto, CTLFLAG_VNET | CTLFLAG_RW,
&VNET_NAME(tcp_do_autorcvbuf), 0, &VNET_NAME(tcp_do_autorcvbuf), 0,
▲ Show 20 Lines • Show All 2,018 Lines • ▼ Show 20 Lines if (V_tcp_insecure_syn &&
/* Send challenge ACK. */ /* Send challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt, tcp_respond(tp, mtod(m, void *), th, m, tp->rcv_nxt,
tp->snd_nxt, TH_ACK); tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt; tp->last_ack_sent = tp->rcv_nxt;
m = NULL; m = NULL;
} }
goto drop; goto drop;
} }
ccUnsubmitted
Done
Inline Actions

At the server side (listening side), I am thinking if we move the secure ACK check here and above the TOF_TS check, it would save cycles on checking these timestamps.

cc: At the server side (listening side), I am thinking if we move the secure ACK check here and…
tuexenAuthorUnsubmitted
Done
Inline Actions

This patch is only about improving the input validation, not about changing the sequence of checks.
Please note that RFC 7323 requires to perform the timestamp checks before the SEG.ACK number checks.

tuexen: This patch is only about improving the input validation, not about changing the sequence of…
ccUnsubmitted
Done
Inline Actions

I see, in the section 5.3. Basic PAWS Algorithm from RFC7323 claims Also, PAWS processing MUST take precedence over the regular TCP acceptability check (Section 3.3 in [RFC0793]), which is performed after verification of the received Timestamps option...

cc: I see, in the section `5.3. Basic PAWS Algorithm` from RFC7323 claims `Also, PAWS processing…
/* /*
* RFC 1323 PAWS: If we have a timestamp reply on this segment * RFC 1323 PAWS: If we have a timestamp reply on this segment
* and it's less than ts_recent, drop it. * and it's less than ts_recent, drop it.
*/ */
if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent && if ((to.to_flags & TOF_TS) != 0 && tp->ts_recent &&
TSTMP_LT(to.to_tsval, tp->ts_recent)) { TSTMP_LT(to.to_tsval, tp->ts_recent)) {
/* Check to see if ts_recent is over 24 days old. */ /* Check to see if ts_recent is over 24 days old. */
if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) { if (tcp_ts_getticks() - tp->ts_recent_age > TCP_PAWS_IDLE) {
▲ Show 20 Lines • Show All 185 Lines • ▼ Show 20 Lines if (tp->t_state == TCPS_SYN_RECEIVED ||
goto dropafterack; goto dropafterack;
else else
goto drop; goto drop;
} }
/* /*
* Ack processing. * Ack processing.
*/ */
if (SEQ_GEQ(tp->snd_una, tp->iss + (65535 << tp->snd_scale))) {
ccUnsubmitted
Done
Inline Actions

Can use TCP_MAXWIN instead of 65535.

cc: Can use `TCP_MAXWIN` instead of `65535`.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed in 37b3e6a660f551ceda412e2115748f3b97b2e615, since this review has already been committed.

tuexen: Fixed in [[ https://cgit.FreeBSD.org/src/commit/?id=37b3e6a660f551ceda412e2115748f3b97b2e615 |…
/* Checking SEG.ACK against ISS is definitely redundant. */
tp->t_flags2 |= TF2_NO_ISS_CHECK;
ccUnsubmitted
Done
Inline Actions

code style: As there are two lines, recommend using a pair of {} for readability.

cc: code style: As there are two lines, recommend using a pair of `{}` for readability.
tuexenAuthorUnsubmitted
Done
Inline Actions

Fixed.

tuexen: Fixed.
}
if (!V_tcp_insecure_ack) {
tcp_seq seq_min;
bool ghost_ack_check;
if (tp->t_flags2 & TF2_NO_ISS_CHECK) {
/* Check for too old ACKs (RFC 5961, Section 5.2). */
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
} else {
if (SEQ_GT(tp->iss + 1, tp->snd_una - tp->max_sndwnd)) {
/* Checking for ghost ACKs is stricter. */
seq_min = tp->iss + 1;
ghost_ack_check = true;
} else {
/*
* Checking for too old ACKs (RFC 5961,
* Section 5.2) is stricter.
*/
seq_min = tp->snd_una - tp->max_sndwnd;
ghost_ack_check = false;
}
}
if (SEQ_LT(th->th_ack, seq_min)) {
if (ghost_ack_check)
TCPSTAT_INC(tcps_rcvghostack);
else
TCPSTAT_INC(tcps_rcvacktooold);
/* Send a challenge ACK. */
tcp_respond(tp, mtod(m, void *), th, m,
tp->rcv_nxt, tp->snd_nxt, TH_ACK);
tp->last_ack_sent = tp->rcv_nxt;
m = NULL;
goto drop;
}
}
switch (tp->t_state) { switch (tp->t_state) {
/* /*
* In SYN_RECEIVED state, the ack ACKs our SYN, so enter * In SYN_RECEIVED state, the ack ACKs our SYN, so enter
* ESTABLISHED state and continue processing. * ESTABLISHED state and continue processing.
* The ACK was checked above. * The ACK was checked above.
*/ */
case TCPS_SYN_RECEIVED: case TCPS_SYN_RECEIVED:
▲ Show 20 Lines • Show All 1,746 Lines • Show Last 20 Lines