diff --git a/internal/firewall/cmd_matcher_test.go b/internal/firewall/cmd_matcher_test.go index 5d6e767a..c0e57e8c 100644 --- a/internal/firewall/cmd_matcher_test.go +++ b/internal/firewall/cmd_matcher_test.go @@ -48,7 +48,7 @@ func (cm *cmdMatcher) String() string { return fmt.Sprintf("path %s, argument regular expressions %v", cm.path, cm.argsRegex) } -func newCmdMatcher(path string, argsRegex ...string) *cmdMatcher { //nolint:unparam +func newCmdMatcher(path string, argsRegex ...string) *cmdMatcher { argsRegexp := make([]*regexp.Regexp, len(argsRegex)) for i, argRegex := range argsRegex { argsRegexp[i] = regexp.MustCompile(argRegex) diff --git a/internal/firewall/support.go b/internal/firewall/support.go index acb82e80..3147b275 100644 --- a/internal/firewall/support.go +++ b/internal/firewall/support.go @@ -6,6 +6,7 @@ import ( "fmt" "math/rand" "os/exec" + "sort" "strings" "github.com/qdm12/golibs/command" @@ -20,7 +21,7 @@ var ( func checkIptablesSupport(ctx context.Context, runner command.Runner, iptablesPathsToTry ...string) (iptablesPath string, err error) { - var lastUnsupportedMessage string + iptablesPathToUnsupportedMessage := make(map[string]string, len(iptablesPathsToTry)) for _, pathToTest := range iptablesPathsToTry { ok, unsupportedMessage, err := testIptablesPath(ctx, pathToTest, runner) if err != nil { @@ -29,17 +30,35 @@ func checkIptablesSupport(ctx context.Context, runner command.Runner, iptablesPath = pathToTest break } - - lastUnsupportedMessage = unsupportedMessage + iptablesPathToUnsupportedMessage[pathToTest] = unsupportedMessage } - if iptablesPath == "" { // all iptables to try failed - return "", fmt.Errorf("%w: from %s: last error is: %s", - ErrIPTablesNotSupported, strings.Join(iptablesPathsToTry, ", "), - lastUnsupportedMessage) + if iptablesPath != "" { + // some paths may be unsupported but that does not matter + // since we found one working. + return iptablesPath, nil } - return iptablesPath, nil + allArePermissionDenied := true + allUnsupportedMessages := make(sort.StringSlice, 0, len(iptablesPathToUnsupportedMessage)) + for iptablesPath, unsupportedMessage := range iptablesPathToUnsupportedMessage { + if !isPermissionDenied(unsupportedMessage) { + allArePermissionDenied = false + } + unsupportedMessage = iptablesPath + ": " + unsupportedMessage + allUnsupportedMessages = append(allUnsupportedMessages, unsupportedMessage) + } + + allUnsupportedMessages.Sort() // predictable order for tests + + if allArePermissionDenied { + // If the error is related to a denied permission for all iptables path, + // return an error describing what to do from an end-user perspective. + return "", fmt.Errorf("%w: %s", ErrNetAdminMissing, strings.Join(allUnsupportedMessages, "; ")) + } + + return "", fmt.Errorf("%w: errors encountered are: %s", + ErrIPTablesNotSupported, strings.Join(allUnsupportedMessages, "; ")) } func testIptablesPath(ctx context.Context, path string, @@ -56,14 +75,6 @@ func testIptablesPath(ctx context.Context, path string, "-A", "OUTPUT", "-o", testInterfaceName, "-j", "DROP") output, err := runner.Run(cmd) if err != nil { - if isPermissionDenied(output) { - // If the error is related to a denied permission, - // return an error describing what to do from an end-user - // perspective. This is a critical error and likely - // applies to all iptables. - criticalErr = fmt.Errorf("%w: %s", ErrNetAdminMissing, output) - return false, "", criticalErr - } unsupportedMessage = fmt.Sprintf("%s (%s)", output, err) return false, unsupportedMessage, nil } @@ -84,10 +95,6 @@ func testIptablesPath(ctx context.Context, path string, cmd = exec.CommandContext(ctx, path, "-L", "INPUT") output, err = runner.Run(cmd) if err != nil { - if isPermissionDenied(output) { - criticalErr = fmt.Errorf("%w: %s", ErrNetAdminMissing, output) - return false, "", criticalErr - } unsupportedMessage = fmt.Sprintf("%s (%s)", output, err) return false, unsupportedMessage, nil } @@ -109,10 +116,6 @@ func testIptablesPath(ctx context.Context, path string, cmd = exec.CommandContext(ctx, path, "--policy", "INPUT", inputPolicy) output, err = runner.Run(cmd) if err != nil { - if isPermissionDenied(output) { - criticalErr = fmt.Errorf("%w: %s", ErrNetAdminMissing, output) - return false, "", criticalErr - } unsupportedMessage = fmt.Sprintf("%s (%s)", output, err) return false, unsupportedMessage, nil } diff --git a/internal/firewall/support_test.go b/internal/firewall/support_test.go index eed5da0b..25128ec9 100644 --- a/internal/firewall/support_test.go +++ b/internal/firewall/support_test.go @@ -8,10 +8,130 @@ import ( "github.com/golang/mock/gomock" "github.com/qdm12/golibs/command" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) //go:generate mockgen -destination=runner_mock_test.go -package $GOPACKAGE github.com/qdm12/golibs/command Runner +func newAppendTestRuleMatcher(path string) *cmdMatcher { + return newCmdMatcher(path, + "^-A$", "^OUTPUT$", "^-o$", "^[a-z0-9]{15}$", + "^-j$", "^DROP$") +} + +func newDeleteTestRuleMatcher(path string) *cmdMatcher { + return newCmdMatcher(path, + "^-D$", "^OUTPUT$", "^-o$", "^[a-z0-9]{15}$", + "^-j$", "^DROP$") +} + +func newListInputRulesMatcher(path string) *cmdMatcher { + return newCmdMatcher(path, + "^-L$", "^INPUT$") +} + +func newSetPolicyMatcher(path, inputPolicy string) *cmdMatcher { //nolint:unparam + return newCmdMatcher(path, + "^--policy$", "^INPUT$", "^"+inputPolicy+"$") +} + +func Test_checkIptablesSupport(t *testing.T) { + t.Parallel() + + ctx := context.Background() + errDummy := errors.New("exit code 4") + const inputPolicy = "ACCEPT" + + testCases := map[string]struct { + buildRunner func(ctrl *gomock.Controller) command.Runner + iptablesPathsToTry []string + iptablesPath string + errSentinel error + errMessage string + }{ + "critical error when checking": { + buildRunner: func(ctrl *gomock.Controller) command.Runner { + runner := NewMockRunner(ctrl) + runner.EXPECT().Run(newAppendTestRuleMatcher("path1")). + Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher("path1")). + Return("output", errDummy) + return runner + }, + iptablesPathsToTry: []string{"path1", "path2"}, + errSentinel: ErrTestRuleCleanup, + errMessage: "for path1: failed cleaning up test rule: " + + "output (exit code 4)", + }, + "found valid path": { + buildRunner: func(ctrl *gomock.Controller) command.Runner { + runner := NewMockRunner(ctrl) + runner.EXPECT().Run(newAppendTestRuleMatcher("path1")). + Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher("path1")). + Return("", nil) + runner.EXPECT().Run(newListInputRulesMatcher("path1")). + Return("Chain INPUT (policy "+inputPolicy+")", nil) + runner.EXPECT().Run(newSetPolicyMatcher("path1", inputPolicy)). + Return("", nil) + return runner + }, + iptablesPathsToTry: []string{"path1", "path2"}, + iptablesPath: "path1", + }, + "all permission denied": { + buildRunner: func(ctrl *gomock.Controller) command.Runner { + runner := NewMockRunner(ctrl) + runner.EXPECT().Run(newAppendTestRuleMatcher("path1")). + Return("Permission denied (you must be root) more context", errDummy) + runner.EXPECT().Run(newAppendTestRuleMatcher("path2")). + Return("context: Permission denied (you must be root)", errDummy) + return runner + }, + iptablesPathsToTry: []string{"path1", "path2"}, + errSentinel: ErrNetAdminMissing, + errMessage: "NET_ADMIN capability is missing: " + + "path1: Permission denied (you must be root) more context (exit code 4); " + + "path2: context: Permission denied (you must be root) (exit code 4)", + }, + "no valid path": { + buildRunner: func(ctrl *gomock.Controller) command.Runner { + runner := NewMockRunner(ctrl) + runner.EXPECT().Run(newAppendTestRuleMatcher("path1")). + Return("output 1", errDummy) + runner.EXPECT().Run(newAppendTestRuleMatcher("path2")). + Return("output 2", errDummy) + return runner + }, + iptablesPathsToTry: []string{"path1", "path2"}, + errSentinel: ErrIPTablesNotSupported, + errMessage: "no iptables supported found: " + + "errors encountered are: " + + "path1: output 1 (exit code 4); " + + "path2: output 2 (exit code 4)", + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + + runner := testCase.buildRunner(ctrl) + + iptablesPath, err := + checkIptablesSupport(ctx, runner, testCase.iptablesPathsToTry...) + + require.ErrorIs(t, err, testCase.errSentinel) + if testCase.errSentinel != nil { + assert.EqualError(t, err, testCase.errMessage) + } + assert.Equal(t, testCase.iptablesPath, iptablesPath) + }) + } +} + func Test_testIptablesPath(t *testing.T) { t.Parallel() @@ -20,17 +140,6 @@ func Test_testIptablesPath(t *testing.T) { errDummy := errors.New("exit code 4") const inputPolicy = "ACCEPT" - appendTestRuleMatcher := newCmdMatcher(path, - "^-A$", "^OUTPUT$", "^-o$", "^[a-z0-9]{15}$", - "^-j$", "^DROP$") - deleteTestRuleMatcher := newCmdMatcher(path, - "^-D$", "^OUTPUT$", "^-o$", "^[a-z0-9]{15}$", - "^-j$", "^DROP$") - listInputRulesMatcher := newCmdMatcher(path, - "^-L$", "^INPUT$") - setPolicyMatcher := newCmdMatcher(path, - "^--policy$", "^INPUT$", "^"+inputPolicy+"$") - testCases := map[string]struct { buildRunner func(ctrl *gomock.Controller) command.Runner ok bool @@ -41,18 +150,16 @@ func Test_testIptablesPath(t *testing.T) { "append test rule permission denied": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)). Return("Permission denied (you must be root)", errDummy) return runner }, - criticalErrWrapped: ErrNetAdminMissing, - criticalErrMessage: "NET_ADMIN capability is missing: " + - "Permission denied (you must be root)", + unsupportedMessage: "Permission denied (you must be root) (exit code 4)", }, "append test rule unsupported": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)). Return("some output", errDummy) return runner }, @@ -61,8 +168,8 @@ func Test_testIptablesPath(t *testing.T) { "remove test rule error": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(deleteTestRuleMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher(path)). Return("some output", errDummy) return runner }, @@ -72,22 +179,20 @@ func Test_testIptablesPath(t *testing.T) { "list input rules permission denied": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(deleteTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(listInputRulesMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newListInputRulesMatcher(path)). Return("Permission denied (you must be root)", errDummy) return runner }, - criticalErrWrapped: ErrNetAdminMissing, - criticalErrMessage: "NET_ADMIN capability is missing: " + - "Permission denied (you must be root)", + unsupportedMessage: "Permission denied (you must be root) (exit code 4)", }, "list input rules unsupported": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(deleteTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(listInputRulesMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newListInputRulesMatcher(path)). Return("some output", errDummy) return runner }, @@ -96,9 +201,9 @@ func Test_testIptablesPath(t *testing.T) { "list input rules no policy": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(deleteTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(listInputRulesMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newListInputRulesMatcher(path)). Return("some\noutput", nil) return runner }, @@ -108,26 +213,24 @@ func Test_testIptablesPath(t *testing.T) { "set policy permission denied": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(deleteTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(listInputRulesMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newListInputRulesMatcher(path)). Return("\nChain INPUT (policy "+inputPolicy+")\nxx\n", nil) - runner.EXPECT().Run(setPolicyMatcher). + runner.EXPECT().Run(newSetPolicyMatcher(path, inputPolicy)). Return("Permission denied (you must be root)", errDummy) return runner }, - criticalErrWrapped: ErrNetAdminMissing, - criticalErrMessage: "NET_ADMIN capability is missing: " + - "Permission denied (you must be root)", + unsupportedMessage: "Permission denied (you must be root) (exit code 4)", }, "set policy unsupported": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(deleteTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(listInputRulesMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newListInputRulesMatcher(path)). Return("\nChain INPUT (policy "+inputPolicy+")\nxx\n", nil) - runner.EXPECT().Run(setPolicyMatcher). + runner.EXPECT().Run(newSetPolicyMatcher(path, inputPolicy)). Return("some output", errDummy) return runner }, @@ -136,11 +239,12 @@ func Test_testIptablesPath(t *testing.T) { "success": { buildRunner: func(ctrl *gomock.Controller) command.Runner { runner := NewMockRunner(ctrl) - runner.EXPECT().Run(appendTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(deleteTestRuleMatcher).Return("", nil) - runner.EXPECT().Run(listInputRulesMatcher). + runner.EXPECT().Run(newAppendTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newDeleteTestRuleMatcher(path)).Return("", nil) + runner.EXPECT().Run(newListInputRulesMatcher(path)). Return("\nChain INPUT (policy "+inputPolicy+")\nxx\n", nil) - runner.EXPECT().Run(setPolicyMatcher).Return("some output", nil) + runner.EXPECT().Run(newSetPolicyMatcher(path, inputPolicy)). + Return("some output", nil) return runner }, ok: true,