Skip to content
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.

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

tcpmss clamping in egress policies #161

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
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

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

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
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
23 changes: 18 additions & 5 deletions pkg/nftables/networkpolicy.go
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,18 @@ 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 {
chbmuc marked this conversation as resolved.
Show resolved Hide resolved
tcpmss = append(tcpmss, fmt.Sprintf("%s dport { %s } tcp flags syn tcp option maxseg size set %d", rb.baseout, strings.Join(tcpPorts, ", "), e.TcpMss))
tcpmss = append(tcpmss, fmt.Sprintf("%s sport { %s } tcp flags syn tcp option maxseg size set %d", rb.basein, strings.Join(tcpPorts, ", "), 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
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
5 changes: 5 additions & 0 deletions pkg/nftables/nftables.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ table inet firewall {
{{ . }}
{{- end }}

# TCP-MSS clamping
chbmuc marked this conversation as resolved.
Show resolved Hide resolved
{{- range .ForwardingRules.TcpMss }}
{{ . }}
{{- 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
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: 2 additions & 0 deletions pkg/nftables/test_data/more-rules.nftable.v4
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ table inet firewall {
# rate limits
meta iifname "eth0" limit rate over 10 mbytes/second counter name drop_ratelimit drop

# TCP-MSS clamping

# 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
2 changes: 2 additions & 0 deletions pkg/nftables/test_data/sets.nftable.v4
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ table inet firewall {
# rate limits
meta iifname "eth0" limit rate over 10 mbytes/second counter name drop_ratelimit drop

# TCP-MSS clamping

# 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
2 changes: 2 additions & 0 deletions pkg/nftables/test_data/simple.nftable.v4
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ table inet firewall {
# rate limits
meta iifname "eth0" limit rate over 10 mbytes/second counter name drop_ratelimit drop

# TCP-MSS clamping

# 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
2 changes: 2 additions & 0 deletions pkg/nftables/test_data/validated.nftable.v4
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ table inet firewall {

# rate limits

# TCP-MSS clamping

# 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