vyos-1x icon indicating copy to clipboard operation
vyos-1x copied to clipboard

firewall: T4709: fix firewall MSS clamping issues

Open initramfs opened this issue 3 years ago • 4 comments
trafficstars

Change Summary

  • ~Fixes TCP MSS clamping by converting the relevant rules to native nftables.~
  • Fiixes the MSS clamping range for both IPv4 and IPv6.
  • Backports the clamp-mss-to-pmtu option to equuleus

Types of changes

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Code style update (formatting, renaming)
  • [ ] Refactoring (no functional changes)
  • [ ] Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • [ ] Other (please describe):

Related Task(s)

  • https://phabricator.vyos.net/T4709

Component(s) name

firewall

Proposed changes

  1. ~We refactor the MSS clamping rule from iptables to nftables to fix the conversion issue.~
  2. We fix the MSS clamping range by raising the maximum to 65535 octets. We also move the arbitrary minimum of 500 octets to 536 for IPv4 to conform with RFC9293 and RFC791. We reduce the minimum MSS for IPv6 from 1280 to 1220 in accordance to RFC9293 and RFC8200.
  3. Since we've already already converted the relevant iptables to nftables we can easily implement the clamp-mss-to-pmtu option, as such we add the ability to specify that option to use the PMTU instead of a hard-coded MSS value.

How to test

Create an example firewall configuration such as:

firewall {
    options {
        interface eth0 {
            adjust-mss6 1220
        }
    }
}

and validate that commit succeeds.

Additionally when using the clamp-mss-to-pmtu option, validate the generated rule looks of the form:

oifname "eth0" tcp flags & (syn | rst) == syn counter tcp option maxseg size set rt mtu

Checklist:

  • [x] I have read the CONTRIBUTING document
  • [x] I have linked this PR to one or more Phabricator Task(s)
  • [ ] I have run the components SMOKETESTS if applicable
  • [x] My commit headlines contain a valid Task id
  • [x] My change requires a change to the documentation
  • [x] I have updated the documentation accordingly

initramfs avatar Sep 24 '22 19:09 initramfs

Please kote, VyOS 1.3 does kot use nftables, it is all based on iptables

c-po avatar Sep 24 '22 20:09 c-po

Maybe I'm wrong but it seems like the NAT components are using native nftables, in fact I followed the code in src/conf_mode/nat.py and template in data/templates/firewall/nftables-nat.tmpl when writing changes for this PR.

Also stated in the phabricator task, it's all nftables under the hood anyway (from debian's use of iptables-nft).

initramfs avatar Sep 24 '22 20:09 initramfs

nat.py is in the repos but not used in production in 1.3. The XML/node.def files are deleted during make so noone calls nat.py - VyOS 1.3 uses vyatta-cfg-nat package. Only VyOS 1.4 uses the new NAT implementation.

c-po avatar Sep 25 '22 05:09 c-po

@c-po It seems I was wrong about the iptables version of the MSS clamp not working (I accidentally broke it whilst fixing something else and attributed it to iptables/nftables). I've updated the phabricator task to indicate as such.

As such I've removed the conversion to nftables and only left the MSS range fixes and the dynamic PMTU clamping options in this PR.

initramfs avatar Sep 26 '22 03:09 initramfs