diff --git a/go.mod b/go.mod index 306a80950..07dd5f936 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 984be4152..089d3248a 100644 --- a/go.sum +++ b/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= diff --git a/pkg/link/spoofcheck.go b/pkg/link/spoofcheck.go index f22a67534..de9d04c24 100644 --- a/pkg/link/spoofcheck.go +++ b/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,37 +106,51 @@ 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 { - c := nft.NewConfig() - for _, rule := range rules { - c.DeleteRule(rule) - } - if err := sc.configurer.Apply(c); err != nil { - ifaceMatchRuleErr = fmt.Errorf("failed to delete iface match rule: %v", err) - } - } else { - fmt.Fprintf(os.Stderr, "spoofcheck/teardown: unable to detect iface match rule for deletion: %+v", expectedRuleToFind) + 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, 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 { + 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() @@ -140,7 +158,7 @@ func (sc *SpoofChecker) Teardown() error { 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) } diff --git a/pkg/link/spoofcheck_test.go b/pkg/link/spoofcheck_test.go index f26aa6a62..50899b694 100644 --- a/pkg/link/spoofcheck_test.go +++ b/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) } diff --git a/vendor/github.com/networkplumbing/go-nft/nft/config.go b/vendor/github.com/networkplumbing/go-nft/nft/config.go index 27ad5edb7..c16ffc127 100644 --- a/vendor/github.com/networkplumbing/go-nft/nft/config.go +++ b/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) +} diff --git a/vendor/github.com/networkplumbing/go-nft/nft/exec/exec.go b/vendor/github.com/networkplumbing/go-nft/nft/exec/exec.go index 9ce3d181f..ad0d1b719 100644 --- a/vendor/github.com/networkplumbing/go-nft/nft/exec/exec.go +++ b/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) + if _, err := execCommand(ctx, data, cmdJSON, cmdFile, cmdStdin); err != nil { + return err } - defer os.Remove(tmpFile.Name()) - if _, err = tmpFile.Write(data); err != nil { - return fmt.Errorf("failed to write to temporary file: %v", err) + return nil +} + +// 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 } - if err := tmpFile.Close(); err != nil { - return fmt.Errorf("failed to close temporary file: %v", err) + stdout, err := execCommand(ctx, data, cmdHandle, cmdEcho, cmdJSON, cmdFile, cmdStdin) + if err != nil { + return nil, err } - if _, err := execCommand(ctx, cmdJSON, cmdFile, tmpFile.Name()); err != nil { - return err + config := nftconfig.New() + if err := config.FromJSON(stdout.Bytes()); err != nil { + return nil, fmt.Errorf("failed to parse echo: %v", err) } - return nil + return config, nil } -func execCommand(ctx context.Context, args ...string) (*bytes.Buffer, error) { +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(), ) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 0e14e56eb..5ce66b0a4 100644 --- a/vendor/modules.txt +++ b/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