Add a simple port filter to SIFTR

[rscheff]

Summary

SIFTR does not allow any kind of filtering, but captures every packet processed by the TCP stack.
Often, only a specific session or service is of interest, and doing the filtering in post-processing of the log adds to the overhead of SIFTR.

This adds a new sysctl net.inet.siftr.port_filter. When set to zero, all packets get captured as previously. If set to any other value, only packets where either the source or the destination ports match, are captured in the log file.

[brooks]

sys/netinet/siftr.c

322 ↗

(On Diff #53008)

Does it make sense to use SYSCTL_U16 and a uint16_t here to match the port number?

919 ↗

(On Diff #53008)

I'd tend to use an explicit != 0 since this isn't a bool.

[rscheff]

• let us not forget on IPv6
• have sysctl check the port number range directly

[rscheff]

• line wrap

[rscheff]

• line wrap6

[rscheff]

sys/netinet/siftr.c

322 ↗

(On Diff #53008)

I assume that also implies that range checking of the user provided value is then done by the sysctl framework? Absolutely, I have to admit, I didn't check all the various types available when fetching the patch.

[cc]

sys/netinet/siftr.c

322 ↗

(On Diff #53008)

With SYSCTL_U16, this will wraparound and cause unexpected storage. Prefer the SYSCTL_UINT.

ccui@FBSD11:~/siftr % sudo sysctl net.inet.siftr.port_filter=22
net.inet.siftr.port_filter: 0 -> 22
ccui@FBSD11:~/siftr % sudo sysctl net.inet.siftr.port_filter=65535
net.inet.siftr.port_filter: 22 -> 65535
ccui@FBSD11:~/siftr % sudo sysctl net.inet.siftr.port_filter=65536 << using SYSCTL_U16
net.inet.siftr.port_filter: 65535 -> 0
ccui@FBSD11:~/siftr % sudo sysctl net.inet.siftr.port_filter=65537
net.inet.siftr.port_filter: 0 -> 1
ccui@FBSD11:~/siftr % sudo sysctl net.inet.siftr.port_filter
net.inet.siftr.port_filter: 1

[brooks]

sys/netinet/siftr.c

322 ↗

(On Diff #53008)

Crappy kernel API design... The handler should reject over-long inputs, but it's probably unfixable at this date. I'm not sure I care about this case of user error...

[rscheff]

sys/netinet/siftr.c

322 ↗

(On Diff #53008)

Suggestion:

• The uint16_t would probably require 4 bytes due to alignment anyway; perhaps we use SYSCTL_INT, and log an error when set >65535 into the siftr log?

• Alternatively keep it as a uint16_t for siftr, but have our own SYSCTL_PROC handler with CTLINT when changing the value...

• Slightly preferred: keep it as SYSCTL_U16 and be done with it...

Whichever way, I believe having a light-weight port filtering available at the siftr level reduces the clutter in the log files enough to be worthwhile.

(Also, sysctl returns the actually set value already; if misconfigurations happen frequently, a check should be scripted when setting the filter value, that the set value is the expected value...)

[cc]

Looks good. Thanks.