Merge pull request #902 from SirPhuttel/applyconfigecho

spoofcheck: Make use of go-nft's ApplyConfigEcho()

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/d2g/dhcp4server v0.0.0-20181031114812-7d4a0a7f59a5
 	github.com/godbus/dbus/v5 v5.1.0
 	github.com/mattn/go-shellwords v1.0.12
-	github.com/networkplumbing/go-nft v0.3.0
+	github.com/networkplumbing/go-nft v0.4.0
 	github.com/onsi/ginkgo/v2 v2.11.0
 	github.com/onsi/gomega v1.27.8
 	github.com/opencontainers/selinux v1.11.0

go.sum

@@ -486,8 +486,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
 github.com/ncw/swift v1.0.47/go.mod h1:23YIA4yWVnGwv2dQlN4bB7egfYX6YLn0Yo/S6zZO/ZM=
-github.com/networkplumbing/go-nft v0.3.0 h1:IIc6yHjN85KyJx21p3ZEsO0iBMYHNXux22rc9Q8TfFw=
-github.com/networkplumbing/go-nft v0.3.0/go.mod h1:HnnM+tYvlGAsMU7yoYwXEVLLiDW9gdMmb5HoGcwpuQs=
+github.com/networkplumbing/go-nft v0.4.0 h1:kExVMwXW48DOAukkBwyI16h4uhE5lN9iMvQd52lpTyU=
+github.com/networkplumbing/go-nft v0.4.0/go.mod h1:HnnM+tYvlGAsMU7yoYwXEVLLiDW9gdMmb5HoGcwpuQs=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=

pkg/link/spoofcheck.go

@@ -30,7 +30,7 @@ const (
 )
 
 type NftConfigurer interface {
-	Apply(*nft.Config) error
+	Apply(*nft.Config) (*nft.Config, error)
 	Read(filterCommands ...string) (*nft.Config, error)
 }
 
@@ -39,12 +39,16 @@ type SpoofChecker struct {
 	macAddress string
 	refID      string
 	configurer NftConfigurer
+	rulestore  *nft.Config
 }
 
 type defaultNftConfigurer struct{}
 
-func (dnc defaultNftConfigurer) Apply(cfg *nft.Config) error {
-	return nft.ApplyConfig(cfg)
+func (dnc defaultNftConfigurer) Apply(cfg *nft.Config) (*nft.Config, error) {
+	const timeout = 55 * time.Second
+	ctxWithTimeout, cancelFunc := context.WithTimeout(context.Background(), timeout)
+	defer cancelFunc()
+	return nft.ApplyConfigEcho(ctxWithTimeout, cfg)
 }
 
 func (dnc defaultNftConfigurer) Read(filterCommands ...string) (*nft.Config, error) {
@@ -59,7 +63,7 @@ func NewSpoofChecker(iface, macAddress, refID string) *SpoofChecker {
 }
 
 func NewSpoofCheckerWithConfigurer(iface, macAddress, refID string, configurer NftConfigurer) *SpoofChecker {
-	return &SpoofChecker{iface, macAddress, refID, configurer}
+	return &SpoofChecker{iface, macAddress, refID, configurer, nil}
 }
 
 // Setup applies nftables configuration to restrict traffic
@@ -88,7 +92,7 @@ func (sc *SpoofChecker) Setup() error {
 	macChain := sc.macChain(ifaceChain.Name)
 	baseConfig.AddChain(macChain)
 
-	if err := sc.configurer.Apply(baseConfig); err != nil {
+	if _, err := sc.configurer.Apply(baseConfig); err != nil {
 		return fmt.Errorf("failed to setup spoof-check: %v", err)
 	}
 
@@ -102,45 +106,59 @@ func (sc *SpoofChecker) Setup() error {
 	rulesConfig.AddRule(sc.matchMacRule(macChain.Name))
 	rulesConfig.AddRule(sc.dropRule(macChain.Name))
 
-	if err := sc.configurer.Apply(rulesConfig); err != nil {
+	rulestore, err := sc.configurer.Apply(rulesConfig)
+	if err != nil {
 		return fmt.Errorf("failed to setup spoof-check: %v", err)
 	}
+	sc.rulestore = rulestore
 
 	return nil
 }
 
+func (sc *SpoofChecker) findPreroutingRule(ruleToFind *schema.Rule) ([]*schema.Rule, error) {
+	ruleset := sc.rulestore
+	if ruleset == nil {
+		chain, err := sc.configurer.Read(listChainBridgeNatPrerouting()...)
+		if err != nil {
+			return nil, err
+		}
+		ruleset = chain
+	}
+	return ruleset.LookupRule(ruleToFind), nil
+}
+
 // Teardown removes the interface and mac-address specific chains and their rules.
 // The table and base-chain are expected to survive while the base-chain rule that matches the
 // interface is removed.
 func (sc *SpoofChecker) Teardown() error {
 	ifaceChain := sc.ifaceChain()
-	currentConfig, ifaceMatchRuleErr := sc.configurer.Read(listChainBridgeNatPrerouting()...)
-	if ifaceMatchRuleErr == nil {
 	expectedRuleToFind := sc.matchIfaceJumpToChainRule(preRoutingBaseChainName, ifaceChain.Name)
 	// It is safer to exclude the statement matching, avoiding cases where a current statement includes
 	// additional default entries (e.g. counters).
 	ruleToFindExcludingStatements := *expectedRuleToFind
 	ruleToFindExcludingStatements.Expr = nil
-		rules := currentConfig.LookupRule(&ruleToFindExcludingStatements)
-		if len(rules) > 0 {
+
+	rules, ifaceMatchRuleErr := sc.findPreroutingRule(&ruleToFindExcludingStatements)
+	if ifaceMatchRuleErr == nil && len(rules) > 0 {
 		c := nft.NewConfig()
 		for _, rule := range rules {
 			c.DeleteRule(rule)
 		}
-			if err := sc.configurer.Apply(c); err != nil {
+		if _, err := sc.configurer.Apply(c); err != nil {
 			ifaceMatchRuleErr = fmt.Errorf("failed to delete iface match rule: %v", err)
 		}
+		// Drop the cache, it should contain deleted rule(s) now
+		sc.rulestore = nil
 	} else {
 		fmt.Fprintf(os.Stderr, "spoofcheck/teardown: unable to detect iface match rule for deletion: %+v", expectedRuleToFind)
 	}
-	}
 
 	regularChainsConfig := nft.NewConfig()
 	regularChainsConfig.DeleteChain(ifaceChain)
 	regularChainsConfig.DeleteChain(sc.macChain(ifaceChain.Name))
 
 	var regularChainsErr error
-	if err := sc.configurer.Apply(regularChainsConfig); err != nil {
+	if _, err := sc.configurer.Apply(regularChainsConfig); err != nil {
 		regularChainsErr = fmt.Errorf("failed to delete regular chains: %v", err)
 	}
 

pkg/link/spoofcheck_test.go

@@ -113,6 +113,25 @@ var _ = Describe("spoofcheck", func() {
 			)))
 		})
 	})
+
+	Context("echo", func() {
+		It("succeeds, no read called", func() {
+			c := configurerStub{}
+			sc := link.NewSpoofCheckerWithConfigurer(iface, mac, id, &c)
+			Expect(sc.Setup()).To(Succeed())
+			Expect(sc.Teardown()).To(Succeed())
+			Expect(c.readCalled).To(BeFalse())
+		})
+
+		It("succeeds, fall back to config read", func() {
+			c := configurerStub{applyReturnNil: true}
+			sc := link.NewSpoofCheckerWithConfigurer(iface, mac, id, &c)
+			Expect(sc.Setup()).To(Succeed())
+			c.readConfig = c.applyConfig[0]
+			Expect(sc.Teardown()).To(Succeed())
+			Expect(c.readCalled).To(BeTrue())
+		})
+	})
 })
 
 func assertExpectedRegularChainsDeletionInTeardownConfig(action configurerStub) {
@@ -274,21 +293,28 @@ type configurerStub struct {
 	failFirstApplyConfig  bool
 	failSecondApplyConfig bool
 	failReadConfig        bool
+
+	applyReturnNil bool
+	readCalled     bool
 }
 
-func (a *configurerStub) Apply(c *nft.Config) error {
+func (a *configurerStub) Apply(c *nft.Config) (*nft.Config, error) {
 	a.applyCounter++
 	if a.failFirstApplyConfig && a.applyCounter == 1 {
-		return fmt.Errorf(errorFirstApplyText)
+		return nil, fmt.Errorf(errorFirstApplyText)
 	}
 	if a.failSecondApplyConfig && a.applyCounter == 2 {
-		return fmt.Errorf(errorSecondApplyText)
+		return nil, fmt.Errorf(errorSecondApplyText)
 	}
 	a.applyConfig = append(a.applyConfig, c)
-	return nil
+	if a.applyReturnNil {
+		return nil, nil
+	}
+	return c, nil
 }
 
 func (a *configurerStub) Read(_ ...string) (*nft.Config, error) {
+	a.readCalled = true
 	if a.failReadConfig {
 		return nil, fmt.Errorf(errorReadText)
 	}

vendor/github.com/networkplumbing/go-nft/nft/config.go

@@ -67,3 +67,10 @@ func ApplyConfig(c *Config) error {
 func ApplyConfigContext(ctx context.Context, c *Config) error {
 	return nftexec.ApplyConfig(ctx, c)
 }
+
+// ApplyConfigEcho applies the given nftables config on the system, echoing
+// back the added elements with their assigned handles
+// The system is expected to have the `nft` executable deployed and nftables enabled in the kernel.
+func ApplyConfigEcho(ctx context.Context, c *Config) (*Config, error) {
+	return nftexec.ApplyConfigEcho(ctx, c)
+}

vendor/github.com/networkplumbing/go-nft/nft/exec/exec.go

@@ -23,8 +23,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io/ioutil"
-	"os"
 	"os/exec"
 	"strings"
 
@@ -33,22 +31,24 @@ import (
 
 const (
 	cmdBin     = "nft"
+	cmdHandle  = "-a"
+	cmdEcho    = "-e"
 	cmdFile    = "-f"
 	cmdJSON    = "-j"
 	cmdList    = "list"
 	cmdRuleset = "ruleset"
+	cmdStdin   = "-"
 )
 
 // ReadConfig loads the nftables configuration from the system and
 // returns it as a nftables config structure.
 // The system is expected to have the `nft` executable deployed and nftables enabled in the kernel.
 func ReadConfig(ctx context.Context, filterCommands ...string) (*nftconfig.Config, error) {
-
 	whatToList := cmdRuleset
 	if len(filterCommands) > 0 {
 		whatToList = strings.Join(filterCommands, " ")
 	}
-	stdout, err := execCommand(ctx, cmdJSON, cmdList, whatToList)
+	stdout, err := execCommand(ctx, nil, cmdJSON, cmdList, whatToList)
 	if err != nil {
 		return nil, err
 	}
@@ -69,38 +69,52 @@ func ApplyConfig(ctx context.Context, c *nftconfig.Config) error {
 		return err
 	}
 
-	tmpFile, err := ioutil.TempFile(os.TempDir(), "spoofcheck-")
-	if err != nil {
-		return fmt.Errorf("failed to create temporary file: %v", err)
-	}
-	defer os.Remove(tmpFile.Name())
-
-	if _, err = tmpFile.Write(data); err != nil {
-		return fmt.Errorf("failed to write to temporary file: %v", err)
-	}
-
-	if err := tmpFile.Close(); err != nil {
-		return fmt.Errorf("failed to close temporary file: %v", err)
-	}
-
-	if _, err := execCommand(ctx, cmdJSON, cmdFile, tmpFile.Name()); err != nil {
+	if _, err := execCommand(ctx, data, cmdJSON, cmdFile, cmdStdin); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func execCommand(ctx context.Context, args ...string) (*bytes.Buffer, error) {
+// ApplyConfigEcho applies the given nftables config on the system, echoing
+// back the added elements with their assigned handles
+// The system is expected to have the `nft` executable deployed and nftables enabled in the kernel.
+func ApplyConfigEcho(ctx context.Context, c *nftconfig.Config) (*nftconfig.Config, error) {
+	data, err := c.ToJSON()
+	if err != nil {
+		return nil, err
+	}
+
+	stdout, err := execCommand(ctx, data, cmdHandle, cmdEcho, cmdJSON, cmdFile, cmdStdin)
+	if err != nil {
+		return nil, err
+	}
+
+	config := nftconfig.New()
+	if err := config.FromJSON(stdout.Bytes()); err != nil {
+		return nil, fmt.Errorf("failed to parse echo: %v", err)
+	}
+
+	return config, nil
+}
+
+func execCommand(ctx context.Context, input []byte, args ...string) (*bytes.Buffer, error) {
 	cmd := exec.CommandContext(ctx, cmdBin, args...)
 
 	var stdout, stderr bytes.Buffer
 	cmd.Stderr = &stderr
 	cmd.Stdout = &stdout
 
+	if input != nil {
+		var stdin bytes.Buffer
+		stdin.Write(input)
+		cmd.Stdin = &stdin
+	}
+
 	if err := cmd.Run(); err != nil {
 		return nil, fmt.Errorf(
-			"failed to execute %s %s: %v stdout:'%s' stderr:'%s'",
-			cmd.Path, strings.Join(cmd.Args, " "), err, stdout.String(), stderr.String(),
+			"failed to execute %s %s: %v stdin:'%s' stdout:'%s' stderr:'%s'",
+			cmd.Path, strings.Join(cmd.Args, " "), err, string(input), stdout.String(), stderr.String(),
 		)
 	}
 

vendor/modules.txt

@@ -103,7 +103,7 @@ github.com/google/pprof/profile
 # github.com/mattn/go-shellwords v1.0.12
 ## explicit; go 1.13
 github.com/mattn/go-shellwords
-# github.com/networkplumbing/go-nft v0.3.0
+# github.com/networkplumbing/go-nft v0.4.0
 ## explicit; go 1.16
 github.com/networkplumbing/go-nft/nft
 github.com/networkplumbing/go-nft/nft/config