Skip to content

Commit 45c25dc

Browse files
authored
[client] Clamp MSS on outbound traffic (#4735)
1 parent 679c58c commit 45c25dc

24 files changed

+804
-134
lines changed

client/firewall/create.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ import (
1515
)
1616

1717
// NewFirewall creates a firewall manager instance
18-
func NewFirewall(iface IFaceMapper, _ *statemanager.Manager, flowLogger nftypes.FlowLogger, disableServerRoutes bool) (firewall.Manager, error) {
18+
func NewFirewall(iface IFaceMapper, _ *statemanager.Manager, flowLogger nftypes.FlowLogger, disableServerRoutes bool, mtu uint16) (firewall.Manager, error) {
1919
if !iface.IsUserspaceBind() {
2020
return nil, fmt.Errorf("not implemented for this OS: %s", runtime.GOOS)
2121
}
2222

2323
// use userspace packet filtering firewall
24-
fm, err := uspfilter.Create(iface, disableServerRoutes, flowLogger)
24+
fm, err := uspfilter.Create(iface, disableServerRoutes, flowLogger, mtu)
2525
if err != nil {
2626
return nil, err
2727
}

client/firewall/create_linux.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ const SKIP_NFTABLES_ENV = "NB_SKIP_NFTABLES_CHECK"
3434
// FWType is the type for the firewall type
3535
type FWType int
3636

37-
func NewFirewall(iface IFaceMapper, stateManager *statemanager.Manager, flowLogger nftypes.FlowLogger, disableServerRoutes bool) (firewall.Manager, error) {
37+
func NewFirewall(iface IFaceMapper, stateManager *statemanager.Manager, flowLogger nftypes.FlowLogger, disableServerRoutes bool, mtu uint16) (firewall.Manager, error) {
3838
// on the linux system we try to user nftables or iptables
3939
// in any case, because we need to allow netbird interface traffic
4040
// so we use AllowNetbird traffic from these firewall managers
4141
// for the userspace packet filtering firewall
42-
fm, err := createNativeFirewall(iface, stateManager, disableServerRoutes)
42+
fm, err := createNativeFirewall(iface, stateManager, disableServerRoutes, mtu)
4343

4444
if !iface.IsUserspaceBind() {
4545
return fm, err
@@ -48,11 +48,11 @@ func NewFirewall(iface IFaceMapper, stateManager *statemanager.Manager, flowLogg
4848
if err != nil {
4949
log.Warnf("failed to create native firewall: %v. Proceeding with userspace", err)
5050
}
51-
return createUserspaceFirewall(iface, fm, disableServerRoutes, flowLogger)
51+
return createUserspaceFirewall(iface, fm, disableServerRoutes, flowLogger, mtu)
5252
}
5353

54-
func createNativeFirewall(iface IFaceMapper, stateManager *statemanager.Manager, routes bool) (firewall.Manager, error) {
55-
fm, err := createFW(iface)
54+
func createNativeFirewall(iface IFaceMapper, stateManager *statemanager.Manager, routes bool, mtu uint16) (firewall.Manager, error) {
55+
fm, err := createFW(iface, mtu)
5656
if err != nil {
5757
return nil, fmt.Errorf("create firewall: %s", err)
5858
}
@@ -64,26 +64,26 @@ func createNativeFirewall(iface IFaceMapper, stateManager *statemanager.Manager,
6464
return fm, nil
6565
}
6666

67-
func createFW(iface IFaceMapper) (firewall.Manager, error) {
67+
func createFW(iface IFaceMapper, mtu uint16) (firewall.Manager, error) {
6868
switch check() {
6969
case IPTABLES:
7070
log.Info("creating an iptables firewall manager")
71-
return nbiptables.Create(iface)
71+
return nbiptables.Create(iface, mtu)
7272
case NFTABLES:
7373
log.Info("creating an nftables firewall manager")
74-
return nbnftables.Create(iface)
74+
return nbnftables.Create(iface, mtu)
7575
default:
7676
log.Info("no firewall manager found, trying to use userspace packet filtering firewall")
7777
return nil, errors.New("no firewall manager found")
7878
}
7979
}
8080

81-
func createUserspaceFirewall(iface IFaceMapper, fm firewall.Manager, disableServerRoutes bool, flowLogger nftypes.FlowLogger) (firewall.Manager, error) {
81+
func createUserspaceFirewall(iface IFaceMapper, fm firewall.Manager, disableServerRoutes bool, flowLogger nftypes.FlowLogger, mtu uint16) (firewall.Manager, error) {
8282
var errUsp error
8383
if fm != nil {
84-
fm, errUsp = uspfilter.CreateWithNativeFirewall(iface, fm, disableServerRoutes, flowLogger)
84+
fm, errUsp = uspfilter.CreateWithNativeFirewall(iface, fm, disableServerRoutes, flowLogger, mtu)
8585
} else {
86-
fm, errUsp = uspfilter.Create(iface, disableServerRoutes, flowLogger)
86+
fm, errUsp = uspfilter.Create(iface, disableServerRoutes, flowLogger, mtu)
8787
}
8888

8989
if errUsp != nil {

client/firewall/iptables/manager_linux.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type iFaceMapper interface {
3636
}
3737

3838
// Create iptables firewall manager
39-
func Create(wgIface iFaceMapper) (*Manager, error) {
39+
func Create(wgIface iFaceMapper, mtu uint16) (*Manager, error) {
4040
iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
4141
if err != nil {
4242
return nil, fmt.Errorf("init iptables: %w", err)
@@ -47,7 +47,7 @@ func Create(wgIface iFaceMapper) (*Manager, error) {
4747
ipv4Client: iptablesClient,
4848
}
4949

50-
m.router, err = newRouter(iptablesClient, wgIface)
50+
m.router, err = newRouter(iptablesClient, wgIface, mtu)
5151
if err != nil {
5252
return nil, fmt.Errorf("create router: %w", err)
5353
}
@@ -66,6 +66,7 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error {
6666
NameStr: m.wgIface.Name(),
6767
WGAddress: m.wgIface.Address(),
6868
UserspaceBind: m.wgIface.IsUserspaceBind(),
69+
MTU: m.router.mtu,
6970
},
7071
}
7172
stateManager.RegisterState(state)

client/firewall/iptables/manager_linux_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/stretchr/testify/require"
1212

1313
fw "github.com/netbirdio/netbird/client/firewall/manager"
14+
"github.com/netbirdio/netbird/client/iface"
1415
"github.com/netbirdio/netbird/client/iface/wgaddr"
1516
)
1617

@@ -53,7 +54,7 @@ func TestIptablesManager(t *testing.T) {
5354
require.NoError(t, err)
5455

5556
// just check on the local interface
56-
manager, err := Create(ifaceMock)
57+
manager, err := Create(ifaceMock, iface.DefaultMTU)
5758
require.NoError(t, err)
5859
require.NoError(t, manager.Init(nil))
5960

@@ -114,7 +115,7 @@ func TestIptablesManagerDenyRules(t *testing.T) {
114115
ipv4Client, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
115116
require.NoError(t, err)
116117

117-
manager, err := Create(ifaceMock)
118+
manager, err := Create(ifaceMock, iface.DefaultMTU)
118119
require.NoError(t, err)
119120
require.NoError(t, manager.Init(nil))
120121

@@ -198,7 +199,7 @@ func TestIptablesManagerIPSet(t *testing.T) {
198199
}
199200

200201
// just check on the local interface
201-
manager, err := Create(mock)
202+
manager, err := Create(mock, iface.DefaultMTU)
202203
require.NoError(t, err)
203204
require.NoError(t, manager.Init(nil))
204205

@@ -264,7 +265,7 @@ func TestIptablesCreatePerformance(t *testing.T) {
264265
for _, testMax := range []int{10, 20, 30, 40, 50, 60, 70, 80, 90, 100, 200, 300, 400, 500, 600, 700, 800, 900, 1000} {
265266
t.Run(fmt.Sprintf("Testing %d rules", testMax), func(t *testing.T) {
266267
// just check on the local interface
267-
manager, err := Create(mock)
268+
manager, err := Create(mock, iface.DefaultMTU)
268269
require.NoError(t, err)
269270
require.NoError(t, manager.Init(nil))
270271
time.Sleep(time.Second)

client/firewall/iptables/router_linux.go

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,30 @@ const (
3030

3131
chainPOSTROUTING = "POSTROUTING"
3232
chainPREROUTING = "PREROUTING"
33+
chainFORWARD = "FORWARD"
3334
chainRTNAT = "NETBIRD-RT-NAT"
3435
chainRTFWDIN = "NETBIRD-RT-FWD-IN"
3536
chainRTFWDOUT = "NETBIRD-RT-FWD-OUT"
3637
chainRTPRE = "NETBIRD-RT-PRE"
3738
chainRTRDR = "NETBIRD-RT-RDR"
39+
chainRTMSSCLAMP = "NETBIRD-RT-MSSCLAMP"
3840
routingFinalForwardJump = "ACCEPT"
3941
routingFinalNatJump = "MASQUERADE"
4042

4143
jumpManglePre = "jump-mangle-pre"
4244
jumpNatPre = "jump-nat-pre"
4345
jumpNatPost = "jump-nat-post"
46+
jumpMSSClamp = "jump-mss-clamp"
4447
markManglePre = "mark-mangle-pre"
4548
markManglePost = "mark-mangle-post"
4649
matchSet = "--match-set"
4750

4851
dnatSuffix = "_dnat"
4952
snatSuffix = "_snat"
5053
fwdSuffix = "_fwd"
54+
55+
// ipTCPHeaderMinSize represents minimum IP (20) + TCP (20) header size for MSS calculation
56+
ipTCPHeaderMinSize = 40
5157
)
5258

5359
type ruleInfo struct {
@@ -77,16 +83,18 @@ type router struct {
7783
ipsetCounter *ipsetCounter
7884
wgIface iFaceMapper
7985
legacyManagement bool
86+
mtu uint16
8087

8188
stateManager *statemanager.Manager
8289
ipFwdState *ipfwdstate.IPForwardingState
8390
}
8491

85-
func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper) (*router, error) {
92+
func newRouter(iptablesClient *iptables.IPTables, wgIface iFaceMapper, mtu uint16) (*router, error) {
8693
r := &router{
8794
iptablesClient: iptablesClient,
8895
rules: make(map[string][]string),
8996
wgIface: wgIface,
97+
mtu: mtu,
9098
ipFwdState: ipfwdstate.NewIPForwardingState(),
9199
}
92100

@@ -392,6 +400,7 @@ func (r *router) cleanUpDefaultForwardRules() error {
392400
{chainRTPRE, tableMangle},
393401
{chainRTNAT, tableNat},
394402
{chainRTRDR, tableNat},
403+
{chainRTMSSCLAMP, tableMangle},
395404
} {
396405
ok, err := r.iptablesClient.ChainExists(chainInfo.table, chainInfo.chain)
397406
if err != nil {
@@ -416,6 +425,7 @@ func (r *router) createContainers() error {
416425
{chainRTPRE, tableMangle},
417426
{chainRTNAT, tableNat},
418427
{chainRTRDR, tableNat},
428+
{chainRTMSSCLAMP, tableMangle},
419429
} {
420430
if err := r.iptablesClient.NewChain(chainInfo.table, chainInfo.chain); err != nil {
421431
return fmt.Errorf("create chain %s in table %s: %w", chainInfo.chain, chainInfo.table, err)
@@ -438,6 +448,10 @@ func (r *router) createContainers() error {
438448
return fmt.Errorf("add jump rules: %w", err)
439449
}
440450

451+
if err := r.addMSSClampingRules(); err != nil {
452+
log.Errorf("failed to add MSS clamping rules: %s", err)
453+
}
454+
441455
return nil
442456
}
443457

@@ -518,6 +532,35 @@ func (r *router) addPostroutingRules() error {
518532
return nil
519533
}
520534

535+
// addMSSClampingRules adds MSS clamping rules to prevent fragmentation for forwarded traffic.
536+
// TODO: Add IPv6 support
537+
func (r *router) addMSSClampingRules() error {
538+
mss := r.mtu - ipTCPHeaderMinSize
539+
540+
// Add jump rule from FORWARD chain in mangle table to our custom chain
541+
jumpRule := []string{
542+
"-j", chainRTMSSCLAMP,
543+
}
544+
if err := r.iptablesClient.Insert(tableMangle, chainFORWARD, 1, jumpRule...); err != nil {
545+
return fmt.Errorf("add jump to MSS clamp chain: %w", err)
546+
}
547+
r.rules[jumpMSSClamp] = jumpRule
548+
549+
ruleOut := []string{
550+
"-o", r.wgIface.Name(),
551+
"-p", "tcp",
552+
"--tcp-flags", "SYN,RST", "SYN",
553+
"-j", "TCPMSS",
554+
"--set-mss", fmt.Sprintf("%d", mss),
555+
}
556+
if err := r.iptablesClient.Append(tableMangle, chainRTMSSCLAMP, ruleOut...); err != nil {
557+
return fmt.Errorf("add outbound MSS clamp rule: %w", err)
558+
}
559+
r.rules["mss-clamp-out"] = ruleOut
560+
561+
return nil
562+
}
563+
521564
func (r *router) insertEstablishedRule(chain string) error {
522565
establishedRule := getConntrackEstablished()
523566

@@ -558,7 +601,7 @@ func (r *router) addJumpRules() error {
558601
}
559602

560603
func (r *router) cleanJumpRules() error {
561-
for _, ruleKey := range []string{jumpNatPost, jumpManglePre, jumpNatPre} {
604+
for _, ruleKey := range []string{jumpNatPost, jumpManglePre, jumpNatPre, jumpMSSClamp} {
562605
if rule, exists := r.rules[ruleKey]; exists {
563606
var table, chain string
564607
switch ruleKey {
@@ -571,6 +614,9 @@ func (r *router) cleanJumpRules() error {
571614
case jumpNatPre:
572615
table = tableNat
573616
chain = chainPREROUTING
617+
case jumpMSSClamp:
618+
table = tableMangle
619+
chain = chainFORWARD
574620
default:
575621
return fmt.Errorf("unknown jump rule: %s", ruleKey)
576622
}

client/firewall/iptables/router_linux_test.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
firewall "github.com/netbirdio/netbird/client/firewall/manager"
1616
"github.com/netbirdio/netbird/client/firewall/test"
17+
"github.com/netbirdio/netbird/client/iface"
1718
nbnet "github.com/netbirdio/netbird/client/net"
1819
)
1920

@@ -30,15 +31,14 @@ func TestIptablesManager_RestoreOrCreateContainers(t *testing.T) {
3031
iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
3132
require.NoError(t, err, "failed to init iptables client")
3233

33-
manager, err := newRouter(iptablesClient, ifaceMock)
34+
manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
3435
require.NoError(t, err, "should return a valid iptables manager")
3536
require.NoError(t, manager.init(nil))
3637

3738
defer func() {
3839
assert.NoError(t, manager.Reset(), "shouldn't return error")
3940
}()
4041

41-
// Now 5 rules:
4242
// 1. established rule forward in
4343
// 2. estbalished rule forward out
4444
// 3. jump rule to POST nat chain
@@ -48,7 +48,9 @@ func TestIptablesManager_RestoreOrCreateContainers(t *testing.T) {
4848
// 7. static return masquerade rule
4949
// 8. mangle prerouting mark rule
5050
// 9. mangle postrouting mark rule
51-
require.Len(t, manager.rules, 9, "should have created rules map")
51+
// 10. jump rule to MSS clamping chain
52+
// 11. MSS clamping rule for outbound traffic
53+
require.Len(t, manager.rules, 11, "should have created rules map")
5254

5355
exists, err := manager.iptablesClient.Exists(tableNat, chainPOSTROUTING, "-j", chainRTNAT)
5456
require.NoError(t, err, "should be able to query the iptables %s table and %s chain", tableNat, chainPOSTROUTING)
@@ -82,7 +84,7 @@ func TestIptablesManager_AddNatRule(t *testing.T) {
8284
iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
8385
require.NoError(t, err, "failed to init iptables client")
8486

85-
manager, err := newRouter(iptablesClient, ifaceMock)
87+
manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
8688
require.NoError(t, err, "shouldn't return error")
8789
require.NoError(t, manager.init(nil))
8890

@@ -155,7 +157,7 @@ func TestIptablesManager_RemoveNatRule(t *testing.T) {
155157
t.Run(testCase.Name, func(t *testing.T) {
156158
iptablesClient, _ := iptables.NewWithProtocol(iptables.ProtocolIPv4)
157159

158-
manager, err := newRouter(iptablesClient, ifaceMock)
160+
manager, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
159161
require.NoError(t, err, "shouldn't return error")
160162
require.NoError(t, manager.init(nil))
161163
defer func() {
@@ -217,7 +219,7 @@ func TestRouter_AddRouteFiltering(t *testing.T) {
217219
iptablesClient, err := iptables.NewWithProtocol(iptables.ProtocolIPv4)
218220
require.NoError(t, err, "Failed to create iptables client")
219221

220-
r, err := newRouter(iptablesClient, ifaceMock)
222+
r, err := newRouter(iptablesClient, ifaceMock, iface.DefaultMTU)
221223
require.NoError(t, err, "Failed to create router manager")
222224
require.NoError(t, r.init(nil))
223225

client/firewall/iptables/state_linux.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import (
44
"fmt"
55
"sync"
66

7+
"github.com/netbirdio/netbird/client/iface"
78
"github.com/netbirdio/netbird/client/iface/wgaddr"
89
)
910

1011
type InterfaceState struct {
1112
NameStr string `json:"name"`
1213
WGAddress wgaddr.Address `json:"wg_address"`
1314
UserspaceBind bool `json:"userspace_bind"`
15+
MTU uint16 `json:"mtu"`
1416
}
1517

1618
func (i *InterfaceState) Name() string {
@@ -42,7 +44,11 @@ func (s *ShutdownState) Name() string {
4244
}
4345

4446
func (s *ShutdownState) Cleanup() error {
45-
ipt, err := Create(s.InterfaceState)
47+
mtu := s.InterfaceState.MTU
48+
if mtu == 0 {
49+
mtu = iface.DefaultMTU
50+
}
51+
ipt, err := Create(s.InterfaceState, mtu)
4652
if err != nil {
4753
return fmt.Errorf("create iptables manager: %w", err)
4854
}

client/firewall/nftables/manager_linux.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ type Manager struct {
4444
}
4545

4646
// Create nftables firewall manager
47-
func Create(wgIface iFaceMapper) (*Manager, error) {
47+
func Create(wgIface iFaceMapper, mtu uint16) (*Manager, error) {
4848
m := &Manager{
4949
rConn: &nftables.Conn{},
5050
wgIface: wgIface,
@@ -53,7 +53,7 @@ func Create(wgIface iFaceMapper) (*Manager, error) {
5353
workTable := &nftables.Table{Name: tableNameNetbird, Family: nftables.TableFamilyIPv4}
5454

5555
var err error
56-
m.router, err = newRouter(workTable, wgIface)
56+
m.router, err = newRouter(workTable, wgIface, mtu)
5757
if err != nil {
5858
return nil, fmt.Errorf("create router: %w", err)
5959
}
@@ -93,6 +93,7 @@ func (m *Manager) Init(stateManager *statemanager.Manager) error {
9393
NameStr: m.wgIface.Name(),
9494
WGAddress: m.wgIface.Address(),
9595
UserspaceBind: m.wgIface.IsUserspaceBind(),
96+
MTU: m.router.mtu,
9697
},
9798
}); err != nil {
9899
log.Errorf("failed to update state: %v", err)

0 commit comments

Comments
 (0)