Make stateful filtering configurable #357
Conversation
about stateful-filtering and snat-subnet-routes correlation
|
@coderabbitai review |
Actions PerformedReview triggered.
|
WalkthroughThe recent update to Tailscale introduces stateful packet filtering, enhancing network security by tracking connections and filtering traffic accordingly. This feature is configurable, allowing users to enable or disable it to support various networking needs, including basic site-to-site networking. The changes span several files, updating configurations, documentation, and translations to support this new functionality. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (2)
tailscale/DOCS.md (2)
Line range hint
20-20: Consider rephrasing for clarity and grammatical correctness.- You can also create an account during the add-on installation processes, however, it is nice to know where you need to go later on. + You can also create an account during the add-on installation process; however, it is helpful to know where you need to go later on.
Line range hint
201-201: Correct the infinitive form for grammatical accuracy.- This option lets you to specify a custom control server instead of the default + This option lets you specify a custom control server instead of the default
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- tailscale/DOCS.md (3 hunks)
- tailscale/config.yaml (1 hunks)
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (1 hunks)
- tailscale/translations/en.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- tailscale/config.yaml
Additional Context Used
LanguageTool (17)
tailscale/DOCS.md (17)
Near line 11: Consider a shorter alternative to avoid wordiness.
Context: ...om anywhere you are. ## Prerequisites In order to use this add-on, you'll need a Tailscal...
Near line 20: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...during the add-on installation processes, however, it is nice to know where you need to go...
Near line 61: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...You can't change them through the Web UI, because all the changes made there would be los...
Near line 61: Consider adding a comma.
Context: ...the Web UI, because all the changes made there would be lost when the add-on is restar...
Near line 178: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ... on only in case you are troubleshooting, because Tailscale's daemon is quite chatty. If ...
Near line 186: Loose punctuation mark.
Context: ...n issue. Possible values are: -trace: Show every detail, like all called inte...
Near line 187: Loose punctuation mark.
Context: ...all called internal functions. -debug: Shows detailed debug information. - `in...
Near line 188: Loose punctuation mark.
Context: ...ows detailed debug information. -info: Normal (usually) interesting events. - ...
Near line 189: Loose punctuation mark.
Context: ...(usually) interesting events. -notice: Normal but significant events. - `warni...
Near line 190: Loose punctuation mark.
Context: ...rmal but significant events. -warning: Exceptional occurrences that are not er...
Near line 191: Loose punctuation mark.
Context: ...currences that are not errors. -error: Runtime errors that do not require imme...
Near line 192: Loose punctuation mark.
Context: ... not require immediate action. -fatal: Something went terribly wrong. Add-on b...
Near line 201: Using the to-infinitive may not be correct in this context. Consider using the bare infinitive (without “to”) instead.
Context: ...n:login_serverThis option lets you to specify a custom control server instead of the ...
Near line 265: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...tworking][tailscale_info_site_to_site] (eg. to traverse multiple networks), you can...
Near line 332: Loose punctuation mark.
Context: ...ented based on the following: -MAJOR: Incompatible or major changes. - `MINOR...
Near line 333: Loose punctuation mark.
Context: ...Incompatible or major changes. -MINOR: Backwards-compatible new features and e...
Near line 334: Loose punctuation mark.
Context: ...new features and enhancements. -PATCH: Backwards-compatible bugfixes and packa...
Additional comments not posted (3)
tailscale/translations/en.yaml (1)
67-74: The translation entries forstateful_filteringare clear and consistent with the existing structure.tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (1)
58-65: The script correctly handles thestateful_filteringconfiguration, applying the appropriate Tailscale command-line options based on user settings.tailscale/DOCS.md (1)
77-77: The documentation forstateful_filteringis clear and informative, correctly explaining its functionality and default behavior.Also applies to: 271-282
| @@ -262,8 +263,23 @@ router, and this simplifies routing configuration. | |||
| When not set, this option is enabled by default. | |||
|
|
|||
| To support advanced [Site-to-site networking][tailscale_info_site_to_site] (eg. | |||
There was a problem hiding this comment.
Correct the abbreviation for consistency.
- to traverse multiple networks), you can disable this functionality, and execute steps 2 and 3 as described on [Site-to-site networking][tailscale_info_site_to_site] (eg. to traverse multiple networks), you can...
+ to traverse multiple networks), you can disable this functionality, and execute steps 2 and 3 as described on [Site-to-site networking][tailscale_info_site_to_site] (e.g., to traverse multiple networks), you can...Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| To support advanced [Site-to-site networking][tailscale_info_site_to_site] (eg. | |
| To support advanced [Site-to-site networking][tailscale_info_site_to_site] (e.g., to traverse multiple networks), you can disable this functionality, and execute steps 2 and 3 as described on [Site-to-site networking][tailscale_info_site_to_site] (e.g., to traverse multiple networks), you can... |
Proposed Changes
TS 1.66.x made
stateful-filteringdefault enabled, but disabling it is required for site-to-site networking to work properly.stateful-filteringis/can be independent fromsnat-subnet-routes, so a separate option is the right solution in my opinion.(The problem is identified by @elcajon in lmagyar#127)
Related Issues
Summary by CodeRabbit
New Features
stateful_filteringoption to enable or disable stateful packet filtering on certain nodes.Configuration Updates
stateful_filteringboolean field to the configuration schema.stateful_filteringoption.Networking Enhancements