diff --git a/go.mod b/go.mod index 9ded6e9de..1237ff35d 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.3.1-0.20230602131639-734b9ba38567 github.com/onsi/ginkgo/v2 v2.9.2 github.com/onsi/gomega v1.27.6 github.com/opencontainers/selinux v1.11.0 diff --git a/go.sum b/go.sum index bbba7ba84..d9bab99c1 100644 --- a/go.sum +++ b/go.sum @@ -488,6 +488,8 @@ github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+ 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.3.1-0.20230602131639-734b9ba38567 h1:eh5vjjPoYcrhQpMFR98BA/1FG7oQpK+vBeVSdAWvTyQ= +github.com/networkplumbing/go-nft v0.3.1-0.20230602131639-734b9ba38567/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..84fb55758 100644 --- a/pkg/link/spoofcheck_test.go +++ b/pkg/link/spoofcheck_test.go @@ -112,6 +112,26 @@ var _ = Describe("spoofcheck", func() { errorFirstApplyText, errorSecondApplyText, ))) }) + + }) + + 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()) + }) }) }) @@ -274,21 +294,29 @@ 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..72c6b396d 100644 --- a/vendor/github.com/networkplumbing/go-nft/nft/exec/exec.go +++ b/vendor/github.com/networkplumbing/go-nft/nft/exec/exec.go @@ -33,6 +33,8 @@ import ( const ( cmdBin = "nft" + cmdHandle = "-a" + cmdEcho = "-e" cmdFile = "-f" cmdJSON = "-j" cmdList = "list" @@ -90,6 +92,42 @@ func ApplyConfig(ctx context.Context, c *nftconfig.Config) error { 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 + } + + tmpFile, err := ioutil.TempFile(os.TempDir(), "spoofcheck-") + if err != nil { + return nil, fmt.Errorf("failed to create temporary file: %v", err) + } + defer os.Remove(tmpFile.Name()) + + if _, err = tmpFile.Write(data); err != nil { + return nil, fmt.Errorf("failed to write to temporary file: %v", err) + } + + if err := tmpFile.Close(); err != nil { + return nil, fmt.Errorf("failed to close temporary file: %v", err) + } + + stdout, err := execCommand(ctx, cmdHandle, cmdEcho, cmdJSON, cmdFile, tmpFile.Name()) + 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, args ...string) (*bytes.Buffer, error) { cmd := exec.CommandContext(ctx, cmdBin, args...) diff --git a/vendor/modules.txt b/vendor/modules.txt index d4127a753..770ac7d14 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.3.1-0.20230602131639-734b9ba38567 ## explicit; go 1.16 github.com/networkplumbing/go-nft/nft github.com/networkplumbing/go-nft/nft/config