Skip to content

Commit 85dc587

Browse files
huntergregoryhuntergregory
committed
fix: avoid segfault by only listing one chain
Signed-off-by: Hunter Gregory <[email protected]>
1 parent 334d8c0 commit 85dc587

File tree

4 files changed

+104
-83
lines changed

4 files changed

+104
-83
lines changed

‎npm/pkg/dataplane/policies/chain-management_linux.go‎

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ var (
9191
util.IptablesJumpFlag,
9292
util.IptablesAzureChain,
9393
}
94+
95+
listChainArgs = []string{util.IptablesWaitFlag, util.IptablesDefaultWaitTime, util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag, util.IptablesListFlag}
96+
listHintChainArgs = append(listChainArgs, "KUBE-IPTABLES-HINT")
97+
listCanaryChainArgs = append(listChainArgs, "KUBE-KUBELET-CANARY")
9498
)
9599

96100
type exitErrorInfo struct {
@@ -246,46 +250,23 @@ func (pMgr *PolicyManager) bootupAfterDetectAndCleanup() error {
246250
// detectIptablesVersion sets the global iptables variable to nft if detected or legacy if detected.
247251
// NPM will crash if it fails to detect either.
248252
// This global variable is referenced in all iptables related functions.
253+
// NPM should use the same iptables version as kube-proxy.
254+
// kube-proxy creates an iptables chain as a hint for which version it uses.
255+
// For more details, see: https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode
249256
func (pMgr *PolicyManager) detectIptablesVersion() error {
250-
klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle")
251-
cmd := pMgr.ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle")
252-
output, nftErr := cmd.CombinedOutput()
253-
if nftErr != nil {
254-
msg := "failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %s"
255-
metrics.SendErrorLogAndMetric(util.IptmID, msg, nftErr.Error())
256-
} else if strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") {
257-
msg := "detected iptables version on first attempt. found KUBE chains in nft iptables. NPM will use iptables-nft"
258-
klog.Info(msg)
259-
metrics.SendLog(util.IptmID, msg, metrics.DonotPrint)
257+
klog.Info("first attempt detecting iptables version. looking for hint/canary chain in iptables-nft")
258+
if pMgr.hintOrCanaryChainExist(util.IptablesNft) {
260259
util.SetIptablesToNft()
261260
return nil
262261
}
263262

264-
klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle")
265-
lCmd := pMgr.ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle")
266-
loutput, legacyErr := lCmd.CombinedOutput()
267-
if legacyErr != nil {
268-
msg := "failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %s"
269-
metrics.SendErrorLogAndMetric(util.IptmID, msg, legacyErr.Error())
270-
} else {
271-
if strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") {
272-
msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy"
273-
klog.Info(msg)
274-
metrics.SendLog(util.IptmID, msg, metrics.DonotPrint)
275-
util.SetIptablesToLegacy()
276-
return nil
277-
} else if nftErr != nil {
278-
msg := "NPM will use iptables-nft. iptables-nft-save failed earlier, but iptables-legacy-save didn't have KUBE chains"
279-
klog.Info(msg)
280-
metrics.SendLog(util.IptmID, msg, metrics.DonotPrint)
281-
util.SetIptablesToNft()
282-
return nil
283-
}
263+
klog.Info("second attempt detecting iptables version. looking for hint/canary chain in iptables-legacy")
264+
if pMgr.hintOrCanaryChainExist(util.IptablesLegacy) {
265+
util.SetIptablesToLegacy()
266+
return nil
284267
}
285268

286-
// we are here if either:
287-
// 1. both nft and legacy save commands failed
288-
// 2. both nft and legacy save commands didn't have KUBE chains
269+
// at this point, chains do not exist in iptables legacy/nft and/or iptables commands have failed for other reasons
289270
klog.Info("third attempt detecting iptables version. getting kernel version")
290271
var majorVersion int
291272
var versionError error
@@ -315,6 +296,31 @@ func (pMgr *PolicyManager) detectIptablesVersion() error {
315296
return nil
316297
}
317298

299+
func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool {
300+
// hint chain should exist since k8s 1.24 (see https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode)
301+
hintCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listHintChainArgs...)
302+
_, hintErr := hintCmd.CombinedOutput()
303+
if hintErr != nil {
304+
klog.Infof("failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error())
305+
metrics.SendErrorLogAndMetric(util.IptmID, "failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error())
306+
} else {
307+
metrics.SendLog(util.IptmID, fmt.Sprintf("found hint chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint)
308+
return true
309+
}
310+
311+
// check for canary chain
312+
canaryCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listCanaryChainArgs...)
313+
_, canaryErr := canaryCmd.CombinedOutput()
314+
if canaryErr != nil {
315+
klog.Infof("failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error())
316+
metrics.SendErrorLogAndMetric(util.IptmID, "failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error())
317+
return false
318+
}
319+
320+
metrics.SendLog(util.IptmID, fmt.Sprintf("found canary chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint)
321+
return true
322+
}
323+
318324
// clenaupOtherIptablesChains cleans up legacy tables if using nft and vice versa.
319325
// It will only return an error if it fails to delete a jump rule and flush the AZURE-NPM chain (see comment about #3088 below).
320326
// Cleanup logic:

‎npm/pkg/dataplane/policies/chain-management_linux_test.go‎

Lines changed: 62 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -909,58 +909,69 @@ func TestDetectIptablesVersion(t *testing.T) {
909909

910910
tests := []args{
911911
{
912-
name: "iptables-nft-save returns kube chains",
912+
name: "nft has hint chain",
913913
calls: []testutils.TestCmd{
914914
{
915-
Cmd: []string{"iptables-nft-save", "-t", "mangle"},
916-
Stdout: iptablesSaveMangleOutput,
915+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
916+
ExitCode: 0,
917917
},
918918
},
919919
expectedErr: false,
920920
expectedIptablesVersion: util.IptablesNft,
921921
},
922922
{
923-
name: "iptables-save returns kube chains",
923+
name: "nft has only canary chain",
924924
calls: []testutils.TestCmd{
925925
{
926-
Cmd: []string{"iptables-nft-save", "-t", "mangle"},
927-
Stdout: "",
926+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
927+
ExitCode: 1,
928928
},
929929
{
930-
Cmd: []string{"iptables-save", "-t", "mangle"},
931-
Stdout: iptablesSaveMangleOutput,
930+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
931+
ExitCode: 0,
932932
},
933933
},
934934
expectedErr: false,
935-
expectedIptablesVersion: util.IptablesLegacy,
935+
expectedIptablesVersion: util.IptablesNft,
936936
},
937937
{
938-
name: "iptables-nft-save and iptables-save both fail: kernel version >= 5",
939-
kernelVersion: 5,
938+
name: "legacy has hint chain",
940939
calls: []testutils.TestCmd{
941940
{
942-
Cmd: []string{"iptables-nft-save", "-t", "mangle"},
941+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
943942
ExitCode: 1,
944943
},
945944
{
946-
Cmd: []string{"iptables-save", "-t", "mangle"},
945+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
947946
ExitCode: 1,
948947
},
948+
{
949+
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
950+
ExitCode: 0,
951+
},
949952
},
950953
expectedErr: false,
951-
expectedIptablesVersion: util.IptablesNft,
954+
expectedIptablesVersion: util.IptablesLegacy,
952955
},
953956
{
954-
name: "no kube chains: kernel version >= 5",
957+
name: "nft and legacy both fail: kernel version >= 5",
955958
kernelVersion: 5,
956959
calls: []testutils.TestCmd{
957960
{
958-
Cmd: []string{"iptables-nft-save", "-t", "mangle"},
959-
Stdout: "",
961+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
962+
ExitCode: 2,
960963
},
961964
{
962-
Cmd: []string{"iptables-save", "-t", "mangle"},
963-
Stdout: "",
965+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
966+
ExitCode: 2,
967+
},
968+
{
969+
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
970+
ExitCode: 2,
971+
},
972+
{
973+
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
974+
ExitCode: 2,
964975
},
965976
},
966977
expectedErr: false,
@@ -971,12 +982,20 @@ func TestDetectIptablesVersion(t *testing.T) {
971982
kernelVersion: 4,
972983
calls: []testutils.TestCmd{
973984
{
974-
Cmd: []string{"iptables-nft-save", "-t", "mangle"},
975-
Stdout: "",
985+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
986+
ExitCode: 1,
976987
},
977988
{
978-
Cmd: []string{"iptables-save", "-t", "mangle"},
979-
Stdout: "",
989+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
990+
ExitCode: 1,
991+
},
992+
{
993+
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
994+
ExitCode: 1,
995+
},
996+
{
997+
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
998+
ExitCode: 1,
980999
},
9811000
},
9821001
expectedErr: false,
@@ -987,12 +1006,20 @@ func TestDetectIptablesVersion(t *testing.T) {
9871006
kernelVersionErr: fmt.Errorf("kernel version error"),
9881007
calls: []testutils.TestCmd{
9891008
{
990-
Cmd: []string{"iptables-nft-save", "-t", "mangle"},
991-
Stdout: "",
1009+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
1010+
ExitCode: 1,
1011+
},
1012+
{
1013+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
1014+
ExitCode: 1,
1015+
},
1016+
{
1017+
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
1018+
ExitCode: 1,
9921019
},
9931020
{
994-
Cmd: []string{"iptables-save", "-t", "mangle"},
995-
Stdout: "",
1021+
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
1022+
ExitCode: 1,
9961023
},
9971024
},
9981025
expectedErr: true,
@@ -1001,7 +1028,8 @@ func TestDetectIptablesVersion(t *testing.T) {
10011028

10021029
for _, tt := range tests {
10031030
tt := tt
1004-
if tt.name != "no kube chains: kernel version is empty" {
1031+
1032+
if tt.name != "no kube chains: kernel version error" {
10051033
continue
10061034
}
10071035

@@ -1012,11 +1040,12 @@ func TestDetectIptablesVersion(t *testing.T) {
10121040
ioshim := common.NewMockIOShim(tt.calls)
10131041
defer ioshim.VerifyCalls(t, tt.calls)
10141042
cfg := &PolicyManagerCfg{
1015-
debug: true,
1016-
debugKernelVersion: tt.kernelVersion,
1017-
NodeIP: "6.7.8.9",
1018-
PolicyMode: IPSetPolicyMode,
1019-
PlaceAzureChainFirst: util.PlaceAzureChainFirst,
1043+
debug: true,
1044+
debugKernelVersion: tt.kernelVersion,
1045+
debugKernelVersionErr: tt.kernelVersionErr,
1046+
NodeIP: "6.7.8.9",
1047+
PolicyMode: IPSetPolicyMode,
1048+
PlaceAzureChainFirst: util.PlaceAzureChainFirst,
10201049
}
10211050
pMgr := NewPolicyManager(ioshim, cfg)
10221051
err := pMgr.detectIptablesVersion()

‎npm/pkg/dataplane/policies/testutils_linux.go‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,6 @@ import (
77
testutils "github.com/Azure/azure-container-networking/test/utils"
88
)
99

10-
const iptablesSaveMangleOutput = `# Generated by iptables-save v1.8.4 on Thu Jan 7 17:00:00 2021
11-
*mangle
12-
:PREROUTING ACCEPT [0:0]
13-
:INPUT ACCEPT [0:0]
14-
:FORWARD ACCEPT [0:0]
15-
:OUTPUT ACCEPT [0:0]
16-
:POSTROUTING ACCEPT [0:0]
17-
:KUBE-IPTABLES-HINT - [0:0]
18-
:KUBE-KUBELET-CANARY - [0:0]
19-
:KUBE-PROXY-CANARY - [0:0]
20-
-A FORWARD -d 168.63.129.16/32 -p tcp -m tcp --dport 32526 -j DROP
21-
-A FORWARD -d 168.63.129.16/32 -p tcp -m tcp --dport 80 -j DROP
22-
COMMIT
23-
`
24-
2510
var (
2611
fakeIPTablesRestoreCommand = testutils.TestCmd{Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}}
2712
fakeIPTablesRestoreFailureCommand = testutils.TestCmd{Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"}, ExitCode: 1}
@@ -68,8 +53,8 @@ func GetBootupTestCalls() []testutils.TestCmd {
6853
bootUp := []testutils.TestCmd{
6954
// detect iptables version to be nft
7055
{
71-
Cmd: []string{"iptables-nft-save", "-t", "mangle"},
72-
Stdout: iptablesSaveMangleOutput,
56+
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
57+
ExitCode: 0,
7358
},
7459
// legacy clean up
7560
{Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"}, ExitCode: 2}, //nolint // AZURE-NPM chain didn't exist

‎npm/util/const.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ const (
8383
IptablesEstablishedState string = "ESTABLISHED"
8484
IptablesNewState string = "NEW"
8585
IptablesFilterTable string = "filter"
86+
IptablesMangleTable string = "mangle"
8687
IptablesCommentModuleFlag string = "comment"
8788
IptablesCommentFlag string = "--comment"
8889
IptablesAddCommentFlag

0 commit comments

Comments
 (0)