add guardrails and cleanup

tools/istio-iptables/pkg/builder/iptables_builder_impl.go

@@ -314,6 +314,14 @@ func (rb *IptablesRuleBuilder) buildCleanupRulesRestore(rules []*Rule) string {
 	return rb.constructIptablesRestoreContents(tableRulesMap)
 }
 
+func (rb *IptablesRuleBuilder) buildGuardrails() []*Rule {
+	rules := make([]*Rule, 0)
+	rb.insertInternal(&rules, iptableslog.UndefinedCommand, constants.INPUT, constants.FILTER, 1, "-p", "tcp", "-j", "DROP")
+	rb.insertInternal(&rules, iptableslog.UndefinedCommand, constants.FORWARD, constants.FILTER, 1, "-p", "tcp", "-j", "DROP")
+	rb.insertInternal(&rules, iptableslog.UndefinedCommand, constants.OUTPUT, constants.FILTER, 1, "-p", "tcp", "-j", "DROP")
+	return rules
+}
+
 func (rb *IptablesRuleBuilder) BuildV4() [][]string {
 	return rb.buildRules(rb.rules.rulesv4)
 }
@@ -338,12 +346,34 @@ func (rb *IptablesRuleBuilder) BuildCheckV6() [][]string {
 	return rb.buildCheckRules(rb.rules.rulesv6)
 }
 
-func (rb *IptablesRuleBuilder) BuildCleanupV4Restore() string {
-	return rb.buildCleanupRulesRestore(rb.rules.rulesv4)
+func (rb *IptablesRuleBuilder) BuildGuardrails() [][]string {
+	rules := rb.buildGuardrails()
+	output := make([][]string, 0)
+	for _, r := range rules {
+		cmd := append([]string{"-t", r.table}, r.params...)
+		output = append(output, cmd)
+	}
+	return output
+}
+
+func (rb *IptablesRuleBuilder) BuildCheckGuardrails() [][]string {
+	rules := checkRules(rb.buildGuardrails())
+	output := make([][]string, 0)
+	for _, r := range rules {
+		cmd := append([]string{"-t", r.table}, r.params...)
+		output = append(output, cmd)
+	}
+	return output
 }
 
-func (rb *IptablesRuleBuilder) BuildCleanupV6Restore() string {
-	return rb.buildCleanupRulesRestore(rb.rules.rulesv6)
+func (rb *IptablesRuleBuilder) BuildCleanupGuardrails() [][]string {
+	rules := reverseRules(rb.buildGuardrails())
+	output := make([][]string, 0)
+	for _, r := range rules {
+		cmd := append([]string{"-t", r.table}, r.params...)
+		output = append(output, cmd)
+	}
+	return output
 }
 
 func (rb *IptablesRuleBuilder) constructIptablesRestoreContents(tableRulesMap map[string][]string) string {

tools/istio-iptables/pkg/capture/run.go

@@ -779,7 +779,7 @@ func (cfg *IptablesConfigurator) getStateFromSave(data string) map[string]map[st
 		result[defaultTable] = make(map[string][]string)
 	}
 
-	flagRegex := regexp.MustCompile(`-{1,2}([^\s]+)(?:\s+([^-\s]+))?`)
+	flagRegex := regexp.MustCompile(`-([^\s]+)(?:\s+([^-\s]+))?`)
 
 	table := ""
 
@@ -809,9 +809,6 @@ func (cfg *IptablesConfigurator) getStateFromSave(data string) map[string]map[st
 		matches := flagRegex.FindAllStringSubmatch(line, -1)
 		flagsAndValues := []ParsedCmd{}
 		for _, match := range matches {
-			if match[1] == "m" || match[1] == "module" {
-				continue
-			}
 			toAdd := ParsedCmd{}
 			toAdd.flag = match[1]
 			if len(match) > 2 && match[2] != "" {
@@ -820,7 +817,7 @@ func (cfg *IptablesConfigurator) getStateFromSave(data string) map[string]map[st
 			flagsAndValues = append(flagsAndValues, toAdd)
 		}
 		for _, el := range flagsAndValues {
-			if el.flag == "A" || el.flag == "append" || el.flag == "-I" || el.flag == "insert" {
+			if el.flag == "A" || el.flag == "-append" || el.flag == "I" || el.flag == "-insert" {
 				chain = el.value
 				break
 			}
@@ -852,26 +849,29 @@ func (cfg *IptablesConfigurator) getStateFromSave(data string) map[string]map[st
 func (cfg *IptablesConfigurator) VerifyRerunStatus(iptVer, ipt6Ver *dep.IptablesVersion) (bool, bool) {
 	applyRequired := false
 	residueFound := false
+
 check_loop:
-	for _, pair := range []struct {
+	for _, ipCfg := range []struct {
 		ver        *dep.IptablesVersion
 		expected   string
 		checkRules [][]string
 	}{
 		{iptVer, cfg.ruleBuilder.BuildV4Restore(), cfg.ruleBuilder.BuildCheckV4()},
 		{ipt6Ver, cfg.ruleBuilder.BuildV6Restore(), cfg.ruleBuilder.BuildCheckV6()},
 	} {
-		output, err := cfg.ext.RunWithOutput(constants.IPTablesSave, pair.ver, nil)
+		output, err := cfg.ext.RunWithOutput(constants.IPTablesSave, ipCfg.ver, nil)
 		if err == nil {
 			currentState := cfg.getStateFromSave(output.String())
 			for _, value := range currentState {
-				residueFound = len(value) != 0
 				if residueFound {
-					break check_loop
+					break
 				}
+				residueFound = len(value) != 0
 			}
-
-			expectedState := cfg.getStateFromSave(pair.expected)
+			if !residueFound {
+				continue
+			}
+			expectedState := cfg.getStateFromSave(ipCfg.expected)
 			if len(currentState) != len(expectedState) {
 				applyRequired = true
 				break
@@ -890,7 +890,7 @@ check_loop:
 					}
 				}
 			}
-			err = cfg.executeIptablesCommands(pair.ver, pair.checkRules)
+			err = cfg.executeIptablesCommands(ipCfg.ver, ipCfg.checkRules)
 			if err != nil {
 				applyRequired = true
 				break
@@ -903,18 +903,48 @@ check_loop:
 	if !residueFound {
 		return false, true
 	}
+
+	if residueFound && !applyRequired {
+		log.Info("Found compatible iptables rules/chains, no additional changes are needed")
+	} else if residueFound {
+		log.Info("Found residue of old iptables rules/chains, cleanup is needed")
+	}
+
 	return residueFound, applyRequired
 }
 
 func (cfg *IptablesConfigurator) executeCommands(iptVer, ipt6Ver *dep.IptablesVersion) error {
+	guardrails := false
+	defer func() {
+		if guardrails {
+			log.Info("Removing guardrails")
+			guardrailsCleanup := cfg.ruleBuilder.BuildCleanupGuardrails()
+			cfg.executeIptablesCommands(iptVer, guardrailsCleanup)
+			cfg.executeIptablesCommands(ipt6Ver, guardrailsCleanup)
+		}
+	}()
+
 	residueFound, applyRequired := cfg.VerifyRerunStatus(iptVer, ipt6Ver)
-	if residueFound && !applyRequired {
-		log.Info("Found equivalent iptables rules, no additional changes are needed")
-	}
 
 	// Cleanup Step
 	if (residueFound && applyRequired) || cfg.cfg.CleanupOnly {
-		log.Info("Performing cleanup of iptables")
+		// Apply safety guardrails if not there
+		if residueFound {
+			log.Info("Setting up guardrails")
+			guardrailsCleanup := cfg.ruleBuilder.BuildCleanupGuardrails()
+			guardrailsRules := cfg.ruleBuilder.BuildGuardrails()
+			cfg.tryExecuteIptablesCommands(iptVer, guardrailsCleanup)
+			cfg.tryExecuteIptablesCommands(ipt6Ver, guardrailsCleanup)
+			if err := cfg.executeIptablesCommands(iptVer, guardrailsRules); err != nil {
+				return err
+			}
+			guardrails = true
+			if err := cfg.executeIptablesCommands(ipt6Ver, guardrailsRules); err != nil {
+				return err
+			}
+		}
+		// Remove old iptables
+		log.Info("Performing cleanup of existing iptables")
 		cfg.tryExecuteIptablesCommands(iptVer, cfg.ruleBuilder.BuildCleanupV4())
 		cfg.tryExecuteIptablesCommands(ipt6Ver, cfg.ruleBuilder.BuildCleanupV6())
 	}

tools/istio-iptables/pkg/capture/run_test.go

@@ -15,7 +15,6 @@
 package capture
 
 import (
-	"fmt"
 	"net/netip"
 	"path/filepath"
 	"reflect"
@@ -385,7 +384,6 @@ func TestIdempotentRerun(t *testing.T) {
 				cfg.CleanupOnly = true
 				iptConfigurator := NewIptablesConfigurator(cfg, ext)
 				assert.NoError(t, iptConfigurator.Run())
-				fmt.Println("Veryfing status")
 				residueFound, applyRequired := iptConfigurator.VerifyRerunStatus(&iptVer, &ipt6Ver)
 				assert.Equal(t, residueFound, false)
 				assert.Equal(t, applyRequired, true)

tools/istio-iptables/pkg/dependencies/implementation.go

@@ -218,15 +218,15 @@ func transformToXTablesErrorMessage(stderr string, err error) string {
 
 // Run runs a command
 func (r *RealDependencies) Run(cmd constants.IptablesCmd, iptVer *IptablesVersion, stdin io.ReadSeeker, args ...string) error {
-	return r.executeXTables(cmd, iptVer, false, false, stdin, args...)
+	return r.executeXTables(cmd, iptVer, false, stdin, args...)
 }
 
 // Run runs a command and returns stdout
 func (r *RealDependencies) RunWithOutput(cmd constants.IptablesCmd, iptVer *IptablesVersion, stdin io.ReadSeeker, args ...string) (*bytes.Buffer, error) {
-	return r.executeXTablesWithOutput(cmd, iptVer, false, false, stdin, args...)
+	return r.executeXTablesWithOutput(cmd, iptVer, false, stdin, args...)
 }
 
 // RunQuietlyAndIgnore runs a command quietly and ignores errors
 func (r *RealDependencies) RunQuietlyAndIgnore(cmd constants.IptablesCmd, iptVer *IptablesVersion, stdin io.ReadSeeker, args ...string) {
-	_ = r.executeXTables(cmd, iptVer, true, true, stdin, args...)
+	_ = r.executeXTables(cmd, iptVer, true, stdin, args...)
 }

tools/istio-iptables/pkg/dependencies/implementation_linux.go

@@ -204,7 +204,7 @@ func mount(src, dst string) error {
 }
 
 func (r *RealDependencies) executeXTablesWithOutput(cmd constants.IptablesCmd, iptVer *IptablesVersion,
-	ignoreErrors bool, quiet bool, stdin io.ReadSeeker, args ...string,
+	ignoreErrors bool, stdin io.ReadSeeker, args ...string,
 ) (*bytes.Buffer, error) {
 	mode := "without lock"
 	stdout := &bytes.Buffer{}
@@ -254,25 +254,15 @@ func (r *RealDependencies) executeXTablesWithOutput(cmd constants.IptablesCmd, i
 			c = exec.Command(cmdBin, args...)
 		}
 	}
-	if !quiet {
-		log.Infof("Running command (%s): %s %s", mode, cmdBin, strings.Join(args, " "))
-	}
+	log.Infof("Running command (%s): %s %s", mode, cmdBin, strings.Join(args, " "))
 
 	c.Stdout = stdout
 	c.Stderr = stderr
 	c.Stdin = stdin
 	err := run(c)
-	if !quiet {
-		if len(stdout.String()) != 0 {
-			log.Infof("Command output: \n%v", stdout.String())
-		}
-		if err != nil {
-			log.Errorf("Command error output: %v", stderr.String())
-		}
+	if len(stdout.String()) != 0 {
+		log.Infof("Command output: \n%v", stdout.String())
 	}
-	//if err != nil {
-	//	log.Errorf("Command error output: %v", stderr.String())
-	//}
 
 	// TODO Check naming and redirection logic
 	if (err != nil || len(stderr.String()) != 0) && !ignoreErrors {
@@ -289,7 +279,7 @@ func (r *RealDependencies) executeXTablesWithOutput(cmd constants.IptablesCmd, i
 	return stdout, err
 }
 
-func (r *RealDependencies) executeXTables(cmd constants.IptablesCmd, iptVer *IptablesVersion, ignoreErrors bool, quiet bool, stdin io.ReadSeeker, args ...string) error {
-	_, err := r.executeXTablesWithOutput(cmd, iptVer, ignoreErrors, false, stdin, args...)
+func (r *RealDependencies) executeXTables(cmd constants.IptablesCmd, iptVer *IptablesVersion, ignoreErrors bool, stdin io.ReadSeeker, args ...string) error {
+	_, err := r.executeXTablesWithOutput(cmd, iptVer, ignoreErrors, stdin, args...)
 	return err
 }