unkin/tomswall
4
0
Fork 0
Code Issues 5 Pull Requests 3 Actions Packages Projects Releases 4 Wiki Activity

Rate-limit log sites with LOGLIMIT #33

Merged
benvin merged 3 commits from benvin/loglimit into main 2026-10-04 18:19:06 +11:00
Conversation 3 Commits 3 Files Changed 7
+273 -19
unkin-agent commented 2026-10-04 15:52:42 +11:00
Member
Copy Link
Copy Source

Log sites have no rate limit, so a flood of dropped or logged traffic floods the journal. Shorewall caps this with LOGLIMIT, which migrate ignored.

  • add log_limit setting (rate/unit[:burst]); unset keeps logging every hit
  • migrate LOGLIMIT, falling back to a global limit with a warning for per-address s:/d: forms
  • put a limit before every log; a logged rule with an action splits into a limited log rule plus the unlogged rule, as shorewall does
Log sites have no rate limit, so a flood of dropped or logged traffic floods the journal. Shorewall caps this with LOGLIMIT, which migrate ignored. - add `log_limit` setting (`rate/unit[:burst]`); unset keeps logging every hit - migrate LOGLIMIT, falling back to a global limit with a warning for per-address `s:`/`d:` forms - put a limit before every log; a logged rule with an action splits into a limited log rule plus the unlogged rule, as shorewall does
unkin-agent added 1 commit 2026-10-04 15:52:42 +11:00
Rate-limit log sites with shorewall LOGLIMIT
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
2e8d51759d
unkin-agent commented 2026-10-04 15:54:02 +11:00
Author
Member
Copy Link
Copy Source
  • internal/nftables/compiler.go:89 — limitLogs splits at the log expr, but applyRuleExtras appends mark/user/time/ratelimit/connlimit/setmark AFTER the log, so the log-only rule drops them: a rule with mark/ratelimit and log logs every matching packet regardless of mark/user/time, and a log action with extras leaves an unlogged no-op rule → copy the extras (everything between the log and the verdict) into the log rule too, or run limitLogs before applyRuleExtras and have the extras applied to both halves; drop ratelimit/connlimit/setmark from the log half (limiter/side-effect must not run twice); add a test with Mark + RateLimit + Log.
  • internal/nftables/compiler_test.go:3158 — TestCompile_LogLimit covers no rule with extras (Mark/User/Time/RateLimit/ConnLimit), DNAT/SNAT logs or non-input chains → add cases so the above is caught.
- internal/nftables/compiler.go:89 — limitLogs splits at the log expr, but applyRuleExtras appends mark/user/time/ratelimit/connlimit/setmark AFTER the log, so the log-only rule drops them: a rule with `mark`/`ratelimit` and `log` logs every matching packet regardless of mark/user/time, and a `log` action with extras leaves an unlogged no-op rule → copy the extras (everything between the log and the verdict) into the log rule too, or run limitLogs before applyRuleExtras and have the extras applied to both halves; drop ratelimit/connlimit/setmark from the log half (limiter/side-effect must not run twice); add a test with Mark + RateLimit + Log. - internal/nftables/compiler_test.go:3158 — TestCompile_LogLimit covers no rule with extras (Mark/User/Time/RateLimit/ConnLimit), DNAT/SNAT logs or non-input chains → add cases so the above is caught.
unkin-agent added 1 commit 2026-10-04 15:56:25 +11:00
Keep rule match extras on the limited log rule
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
ci/woodpecker/pr/test Pipeline was successful
Details
be391ed385
unkin-agent commented 2026-10-04 15:57:40 +11:00
Author
Member
Copy Link
Copy Source
  • internal/shorewall/convert.go:145 — shorewall LOGLIMIT allows a name ([s:|d:][name:]rate/unit[:burst]); only s:/d: is stripped, so name:1/sec:5 (or s:name:...) is written to config and then rejected by the logLimitRe check at config/config.go:128, so migrate emits a config that fails to load → strip an optional leading name: too (same as parseRateLimit does), or fall back with the same warning, and add a convert test for it.
- internal/shorewall/convert.go:145 — shorewall LOGLIMIT allows a name (`[s:|d:][name:]rate/unit[:burst]`); only `s:`/`d:` is stripped, so `name:1/sec:5` (or `s:name:...`) is written to config and then rejected by the `logLimitRe` check at config/config.go:128, so migrate emits a config that fails to load → strip an optional leading `name:` too (same as parseRateLimit does), or fall back with the same warning, and add a convert test for it.
unkin-agent added 1 commit 2026-10-04 15:58:15 +11:00
Drop the name from named shorewall LOGLIMIT values
ci/woodpecker/pr/test Pipeline was successful
Details
ci/woodpecker/pr/build Pipeline was successful
Details
ci/woodpecker/pr/pre-commit Pipeline was successful
Details
15ab32431d
unkin-agent commented 2026-10-04 15:59:41 +11:00
Author
Member
Copy Link
Copy Source

No findings.

No findings.
benvin merged commit 2832dd0e7d into main 2026-10-04 18:19:06 +11:00
benvin deleted branch benvin/loglimit 2026-10-04 18:19:07 +11:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
benvin (BenVincent) unkin-agent (Unkin Agent) unkinben (Ben Vincent)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: unkin/tomswall#33
Delete Branch "benvin/loglimit"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?