Skip to content

tcpmss clamping in egress policies #161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
chbmuc wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

tcpmss clamping in egress policies #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
21b22fa
first draft for tcpmss clamping in egress policies
chbmuc Jul 11, 2023
d485950
fix existing unit tests
chbmuc Jul 11, 2023
6d7673c
allow policies without port + small fix
chbmuc Jul 11, 2023
c557ddf
omit comments if unused in template
chbmuc Jul 11, 2023
7b329d8
Merge branch 'master' into tcpmss
majst01 Aug 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/v1/clusterwidenetworkpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ type EgressRule struct {
// ToFQDNs rules can't contain To rules.
// +optional
ToFQDNs []FQDNSelector `json:"toFQDNs,omitempty"`

// Maximum MSS size for outgoing traffic to the egress destination
// If specified nftables rules for TCP-MSS clamping will be created.
// If none specified no rule will be applied.
// +optional
TcpMss *uint16 `json:"tcpmss,omitempty"`
}

// FQDNSelector describes rules for matching DNS names.
Expand Down
5 changes: 5 additions & 0 deletions api/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions config/crd/bases/metal-stack.io_clusterwidenetworkpolicies.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ spec:
type: string
type: object
type: array
tcpmss:
description: Maximum MSS size for outgoing traffic to the egress
destination If specified nftables rules for TCP-MSS clamping
will be created. If none specified no rule will be applied.
type: integer
to:
description: List of destinations for outgoing traffic of a
cluster for this rule. Items in this list are combined using
Expand Down
1 change: 1 addition & 0 deletions pkg/nftables/firewall.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ type nftablesRules []string
type forwardingRules struct {
Ingress nftablesRules
Egress nftablesRules
TcpMss nftablesRules
}

// NewDefaultFirewall creates a new default nftables firewall.
Expand Down
28 changes: 23 additions & 5 deletions pkg/nftables/networkpolicy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@ import (
type ruleBase struct {
comment string
base []string
baseout []string
basein []string
}

// clusterwideNetworkPolicyRules generates nftables rules for a clusterwidenetworkpolicy
func clusterwideNetworkPolicyRules(
cache FQDNCache,
np firewallv1.ClusterwideNetworkPolicy,
logAcceptedConnections bool,
) (ingress nftablesRules, egress nftablesRules, updated firewallv1.ClusterwideNetworkPolicy) {
) (ingress nftablesRules, egress nftablesRules, tcpmss nftablesRules, updated firewallv1.ClusterwideNetworkPolicy) {
updated = np

if len(np.Spec.Egress) > 0 {
egress, updated = clusterwideNetworkPolicyEgressRules(cache, np, logAcceptedConnections)
egress, tcpmss, updated = clusterwideNetworkPolicyEgressRules(cache, np, logAcceptedConnections)
}
if len(np.Spec.Ingress) > 0 {
ingress = append(ingress, clusterwideNetworkPolicyIngressRules(np, logAcceptedConnections)...)
Expand Down Expand Up @@ -64,22 +66,29 @@ func clusterwideNetworkPolicyEgressRules(
cache FQDNCache,
np firewallv1.ClusterwideNetworkPolicy,
logAcceptedConnections bool,
) (rules nftablesRules, updated firewallv1.ClusterwideNetworkPolicy) {
) (rules nftablesRules, tcpmss nftablesRules, updated firewallv1.ClusterwideNetworkPolicy) {
for _, e := range np.Spec.Egress {
tcpPorts, udpPorts := calculatePorts(e.Ports)

ruleBases := []ruleBase{}
if len(e.To) > 0 {
allow, except := clusterwideNetworkPolicyEgressToRules(e)
rb := []string{"ip saddr == @cluster_prefixes"}
rbmssout := []string{""}
rbmssin := []string{""}
if len(except) > 0 {
rb = append(rb, fmt.Sprintf("ip daddr != { %s }", strings.Join(except, ", ")))
rbmssout = append(rb, fmt.Sprintf("ip daddr != { %s }", strings.Join(except, ", ")))
rbmssin = append(rb, fmt.Sprintf("ip saddr != { %s }", strings.Join(except, ", ")))
}
if len(allow) > 0 {
if allow[0] != "0.0.0.0/0" {
rb = append(rb, fmt.Sprintf("ip daddr { %s }", strings.Join(allow, ", ")))
rbmssout = append(rb, fmt.Sprintf("ip daddr { %s }", strings.Join(allow, ", ")))
rbmssin = append(rb, fmt.Sprintf("ip saddr { %s }", strings.Join(allow, ", ")))
}
}
ruleBases = append(ruleBases, ruleBase{base: rb})
ruleBases = append(ruleBases, ruleBase{base: rb, baseout: rbmssin, basein: rbmssout})
} else if len(e.ToFQDNs) > 0 && cache.IsInitialized() {
// Generate allow rules based on DNS selectors
rbs, u := clusterwideNetworkPolicyEgressToFQDNRules(cache, np.Status.FQDNState, e)
Expand All @@ -91,14 +100,23 @@ func clusterwideNetworkPolicyEgressRules(
for _, rb := range ruleBases {
if len(tcpPorts) > 0 {
rules = append(rules, assembleDestinationPortRule(rb.base, "tcp", tcpPorts, logAcceptedConnections, comment+" tcp"+rb.comment))
if e.TcpMss != nil {
tcpmss = append(tcpmss, fmt.Sprintf("%s tcp dport { %s } tcp flags syn tcp option maxseg size set %d", rb.baseout, strings.Join(tcpPorts, ", "), e.TcpMss))
tcpmss = append(tcpmss, fmt.Sprintf("%s tcp sport { %s } tcp flags syn tcp option maxseg size set %d", rb.basein, strings.Join(tcpPorts, ", "), e.TcpMss))
}
} else {
if e.TcpMss != nil {
tcpmss = append(tcpmss, fmt.Sprintf("%s tcp flags syn tcp option maxseg size set %d", rb.baseout, e.TcpMss))
tcpmss = append(tcpmss, fmt.Sprintf("%s tcp flags syn tcp option maxseg size set %d", rb.basein, e.TcpMss))
}
}
if len(udpPorts) > 0 {
rules = append(rules, assembleDestinationPortRule(rb.base, "udp", udpPorts, logAcceptedConnections, comment+" udp"+rb.comment))
}
}
}

return uniqueSorted(rules), np
return uniqueSorted(rules), uniqueSorted(tcpmss), np
}

func clusterwideNetworkPolicyEgressToRules(e firewallv1.EgressRule) (allow, except []string) {
Expand Down
29 changes: 25 additions & 4 deletions pkg/nftables/networkpolicy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ func TestClusterwideNetworkPolicyRules(t *testing.T) {
type want struct {
ingress nftablesRules
egress nftablesRules
tcpmss nftablesRules
ingressAL nftablesRules
egressAL nftablesRules
tcpmssAL nftablesRules
}

tests := []struct {
Expand Down Expand Up @@ -98,6 +100,7 @@ func TestClusterwideNetworkPolicyRules(t *testing.T) {
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53, 443-448 } counter accept comment "accept traffic for np tcp"`,
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`,
},
tcpmss: nftablesRules{},
ingressAL: nftablesRules{
`ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } log prefix "nftables-firewall-accepted: " limit rate 10/second`,
`ip saddr != { 1.1.0.1 } ip saddr { 1.1.0.0/24 } tcp dport { 80, 443-448 } counter accept comment "accept traffic for k8s network policy tcp"`,
Expand All @@ -108,27 +111,34 @@ func TestClusterwideNetworkPolicyRules(t *testing.T) {
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second`,
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`,
},
tcpmssAL: nftablesRules{},
},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
ingress, egress, _ := clusterwideNetworkPolicyRules(nil, tt.input, false)
ingress, egress, tcpmss, _ := clusterwideNetworkPolicyRules(nil, tt.input, false)
if !cmp.Equal(ingress, tt.want.ingress) {
t.Errorf("clusterwideNetworkPolicyRules() ingress diff: %v", cmp.Diff(ingress, tt.want.ingress))
}
if !cmp.Equal(egress, tt.want.egress) {
t.Errorf("clusterwideNetworkPolicyRules() egress diff: %v", cmp.Diff(egress, tt.want.egress))
}
if !cmp.Equal(tcpmss, tt.want.tcpmss) {
t.Errorf("clusterwideNetworkPolicyRules() tcpmss diff: %v", cmp.Diff(tcpmss, tt.want.tcpmss))
}

ingressAL, egressAL, _ := clusterwideNetworkPolicyRules(nil, tt.input, true)
ingressAL, egressAL, tcpmssAL, _ := clusterwideNetworkPolicyRules(nil, tt.input, true)
if !cmp.Equal(ingressAL, tt.want.ingressAL) {
t.Errorf("clusterwideNetworkPolicyRules() ingress with accessLog diff: %v", cmp.Diff(ingressAL, tt.want.ingressAL))
}
if !cmp.Equal(egressAL, tt.want.egressAL) {
t.Errorf("clusterwideNetworkPolicyRules() egress with accessLog diff: %v", cmp.Diff(egressAL, tt.want.egressAL))
}
if !cmp.Equal(tcpmssAL, tt.want.tcpmssAL) {
t.Errorf("clusterwideNetworkPolicyRules() tcpmss with accessLog diff: %v", cmp.Diff(egressAL, tt.want.egressAL))
}
})
}
}
Expand All @@ -139,7 +149,9 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) {

type want struct {
egress nftablesRules
tcpmss nftablesRules
egressAL nftablesRules
tcpmssAL nftablesRules
}

tests := []struct {
Expand Down Expand Up @@ -183,12 +195,14 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) {
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } counter accept comment "accept traffic for np tcp"`,
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`,
},
tcpmss: nftablesRules{},
egressAL: nftablesRules{
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second`,
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } tcp dport { 53 } counter accept comment "accept traffic for np tcp"`,
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } log prefix "nftables-firewall-accepted: " limit rate 10/second`,
`ip saddr == @cluster_prefixes ip daddr != { 1.1.0.1 } ip daddr { 1.1.0.0/24, 1.1.1.0/24 } udp dport { 53 } counter accept comment "accept traffic for np udp"`,
},
tcpmssAL: nftablesRules{},
},
},
{
Expand Down Expand Up @@ -240,6 +254,7 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) {
`ip saddr == @cluster_prefixes ip6 daddr @test2 tcp dport { 53 } counter accept comment "accept traffic for np tcp, fqdn: *.test.com"`,
`ip saddr == @cluster_prefixes ip6 daddr @test2 udp dport { 53 } counter accept comment "accept traffic for np udp, fqdn: *.test.com"`,
},
tcpmss: nftablesRules{},
},
},
}
Expand All @@ -253,17 +268,23 @@ func TestClusterwideNetworkPolicyEgressRules(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
tt.record(fqdnCache)
if len(tt.want.egress) > 0 {
egress, _ := clusterwideNetworkPolicyEgressRules(fqdnCache, tt.input, false)
egress, tcpmss, _ := clusterwideNetworkPolicyEgressRules(fqdnCache, tt.input, false)
if !cmp.Equal(egress, tt.want.egress) {
t.Errorf("clusterwideNetworkPolicyEgressRules() diff: %v", cmp.Diff(egress, tt.want.egress))
}
if !cmp.Equal(tcpmss, tt.want.tcpmss) {
t.Errorf("clusterwideNetworkPolicyEgressRules() diff: %v", cmp.Diff(tcpmss, tt.want.tcpmss))
}
}

if len(tt.want.egressAL) > 0 {
egressAL, _ := clusterwideNetworkPolicyEgressRules(fqdnCache, tt.input, true)
egressAL, tcpmssAL, _ := clusterwideNetworkPolicyEgressRules(fqdnCache, tt.input, true)
if !cmp.Equal(egressAL, tt.want.egressAL) {
t.Errorf("clusterwideNetworkPolicyEgressRules() with accessLog diff: %v", cmp.Diff(egressAL, tt.want.egressAL))
}
if !cmp.Equal(tcpmssAL, tt.want.tcpmssAL) {
t.Errorf("clusterwideNetworkPolicyEgressRules() with accessLog diff: %v", cmp.Diff(tcpmssAL, tt.want.tcpmssAL))
}
}
})
}
Expand Down
9 changes: 7 additions & 2 deletions pkg/nftables/nftables.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,17 @@ table inet firewall {
# network traffic accounting for internal traffic
ip saddr @internal_prefixes oifname {"vlan{{ .PrivateVrfID }}", "vrf{{ .PrivateVrfID }}"} counter name internal_in comment "count internal traffic incomming"
ip daddr @internal_prefixes iifname {"vlan{{ .PrivateVrfID }}", "vrf{{ .PrivateVrfID }}"} counter name internal_out comment "count internal traffic outgoing"

{{ if gt (len .RateLimitRules) 0 }}
# rate limits
{{- range .RateLimitRules }}
{{ . }}
{{- end }}

{{ end }}{{ if gt (len .ForwardingRules.TcpMss) 0 }}
Copy link
Contributor

@majst01 majst01 Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{ end }}{{ if gt (len .ForwardingRules.TcpMss) 0 }}
{{- end }}
{{- if gt (len .ForwardingRules.TcpMss) 0 }}

This should do the trick to ommit the newline

# TCP-MSS clamping
{{- range .ForwardingRules.TcpMss }}
{{ . }}
{{- end }}
{{ end }}
# state dependent rules
ct state established,related counter accept comment "accept established connections"
ct state invalid counter drop comment "drop packets with invalid ct state"
Expand Down
6 changes: 4 additions & 2 deletions pkg/nftables/rendering.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,17 @@ type firewallRenderingData struct {
}

func newFirewallRenderingData(f *Firewall) (*firewallRenderingData, error) {
ingress, egress := nftablesRules{}, nftablesRules{}
ingress, egress, tcpmss := nftablesRules{}, nftablesRules{}, nftablesRules{}
for ind, np := range f.clusterwideNetworkPolicies.Items {
err := np.Spec.Validate()
if err != nil {
continue
}

i, e, u := clusterwideNetworkPolicyRules(f.cache, np, f.logAcceptedConnections)
i, e, t, u := clusterwideNetworkPolicyRules(f.cache, np, f.logAcceptedConnections)
ingress = append(ingress, i...)
egress = append(egress, e...)
tcpmss = append(tcpmss, t...)
f.clusterwideNetworkPolicies.Items[ind] = u
}

Expand All @@ -54,6 +55,7 @@ func newFirewallRenderingData(f *Firewall) (*firewallRenderingData, error) {
ForwardingRules: forwardingRules{
Ingress: ingress,
Egress: egress,
TcpMss: tcpmss,
},
RateLimitRules: rateLimitRules(f),
SnatRules: snatRules,
Expand Down
2 changes: 0 additions & 2 deletions pkg/nftables/test_data/validated.nftable.v4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ table inet firewall {
ip saddr @internal_prefixes oifname {"vlan42", "vrf42"} counter name internal_in comment "count internal traffic incomming"
ip daddr @internal_prefixes iifname {"vlan42", "vrf42"} counter name internal_out comment "count internal traffic outgoing"

# rate limits

# state dependent rules
ct state established,related counter accept comment "accept established connections"
ct state invalid counter drop comment "drop packets with invalid ct state"
Expand Down
Loading