Details

Reviewers

None

Group Reviewers

network

Commits

rG540c8cd7adc5: pf: support 'return' for SCTP
rGd1bc1e9e1ae0: pf: support 'return' for SCTP

Summary

Send an SCTP Abort message if we're refusing a connection, just like we
send a RST for TCP.

MFC after: 3 weeks
Sponsored by: Orange Business Services

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 52481
Build 49372: arc lint + arc unit

Event Timeline

kp created this revision.Jul 4 2023, 7:11 AM

Herald added subscribers: glebius, melifaro, farrokhi and 2 others. · View Herald TranscriptJul 4 2023, 7:11 AM

kp requested review of this revision.Jul 4 2023, 7:11 AM

kp added a parent revision: D40863: pf tests: basic SCTP connection test.

Harbormaster completed remote builds in B52414: Diff 124145.Jul 4 2023, 7:12 AM

kp added a child revision: D40865: pf tests: test SCTP 'return'.Jul 4 2023, 7:12 AM

I think you need to add some checks here according to RFC 9260:

Don't send the ABORT, if the verification tag is not zero.
Don't send the ABORT, if the INIT chunk is not the only chunk in the packet.

I also would not send an ABORT, if the initiate tag is 0.

In D40864#930026, @tuexen wrote:

I think you need to add some checks here according to RFC 9260:

Don't send the ABORT, if the verification tag is not zero.

Okay, that's something I'll add to pf_scan_sctp() (and a different patch), as part of the normalisation code.
That'd also mean we do those checks (and drop packets violating them) even if there's no rule with 'return' in play.

Don't send the ABORT, if the INIT chunk is not the only chunk in the packet.

That too is one where I think we need to do in pf_scap_sctp().

I also would not send an ABORT, if the initiate tag is 0.

Same. And I may as well check inbound/outbound streams and initial window sizes too.

rebase

Harbormaster completed remote builds in B52448: Diff 124222.Jul 6 2023, 4:35 PM

I've added those extra checks in D40862.

Re-add accidentally removed break

Harbormaster completed remote builds in B52450: Diff 124224.Jul 6 2023, 4:38 PM

tuexen added inline comments.Jul 6 2023, 7:51 PM

sys/netpfil/pf/pf.c
2966	Why should pf only support SCTP when running on a kernel with SCTP support? Isn't end point functionality and middlebox functionality independent of each other?
3052	You could use `sctp_calculate_cksum()`. That is always available. It is also used by `ipfw`...
3183	Why this `#if`? At least `ipfw` supports SCTP no matter SCTP is enabled in the kernel.
3185	Sorry for the question, I have no experience with pf. Is it possible the trigger the sending of an SCTP packet with an ABORT chunk in response to arbitrary packets? `ipfw` sort of support this, not yet at the level I want, but the basic functionality is there. The reason I'm asking: If pf does support this, you need so set the T-bit in some cases and I don't see code for this right now.

kp added inline comments.Jul 7 2023, 8:02 AM

sys/netpfil/pf/pf.c
2966	It is, but I followed the example of the existing SCTP checksum code in pf_route(), and didn't know about sctp_calculate_cksum(). I'll use that everywhere instead, because that's clearly better. Thanks!
3185	The ABORT will basically only be sent in response to an INIT packet on a port (or from a host or ... ) we don't want to allow traffic to. (And then only if 'return' is set on the relevant rule.)