From 0f424e93d7b74b0474bc0e1d654e239d2122abb8 Mon Sep 17 00:00:00 2001 From: HynoR <20227709+HynoR@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:51:39 +0800 Subject: [PATCH 1/6] refactor(firewall): add read-only provider detection Three callers only need the provider name: the boot replay, the historical firewall_type migration and the forwarding adapter factory. They each built a full filter client and read Name() off it. DetectProvider reports the name using the same selection order NewFirewallClient uses (firewalld+ufw conflict, then firewalld, ufw, iptables) without constructing anything, and NewFirewallClient is now that detection plus a name-to-client switch, so an unsupported provider is rejected in one place. --- agent/app/service/forwarding.go | 4 +- agent/init/firewall/firewall.go | 5 +- agent/init/migration/migrations/init.go | 4 +- agent/utils/firewall/client.go | 26 +++---- agent/utils/firewall/provider.go | 30 ++++++++ agent/utils/firewall/provider_test.go | 93 +++++++++++++++++++++++++ 6 files changed, 142 insertions(+), 20 deletions(-) create mode 100644 agent/utils/firewall/provider.go create mode 100644 agent/utils/firewall/provider_test.go diff --git a/agent/app/service/forwarding.go b/agent/app/service/forwarding.go index 28023d9f9990..23da154a6eca 100644 --- a/agent/app/service/forwarding.go +++ b/agent/app/service/forwarding.go @@ -35,11 +35,11 @@ func NewIForwardingService() IForwardingService { } func newForwardingAdapter() (forwardClient.Adapter, error) { - client, err := firewall.NewFirewallClient() + provider, err := firewall.DetectProvider() if err != nil { return nil, err } - return forwardClient.NewAdapter(client.Name()) + return forwardClient.NewAdapter(provider) } func (s *ForwardingService) LoadBaseInfo() (dto.FirewallBaseInfo, error) { diff --git a/agent/init/firewall/firewall.go b/agent/init/firewall/firewall.go index e9c27aed45a1..87556c89c6d1 100644 --- a/agent/init/firewall/firewall.go +++ b/agent/init/firewall/firewall.go @@ -19,17 +19,16 @@ func Init() { } InitPingStatus() global.LOG.Info("initializing firewall settings...") - client, err := firewall.NewFirewallClient() + provider, err := firewall.DetectProvider() if err != nil { return } - clientName := client.Name() if err := service.NewIForwardingService().Replay(); err != nil { global.LOG.Errorf("replay forwarding rules failed, err: %v", err) return } - if clientName != "iptables" { + if provider != "iptables" { return } settingRepo := repo.NewISettingRepo() diff --git a/agent/init/migration/migrations/init.go b/agent/init/migration/migrations/init.go index 26a95f4327e9..6f0eef86b507 100644 --- a/agent/init/migration/migrations/init.go +++ b/agent/init/migration/migrations/init.go @@ -919,9 +919,9 @@ var AddIptablesFilterRuleTable = &gormigrate.Migration{ _ = tx.Where("1 = 1").Find(&firewalls).Error firewallType := "" - client, err := firewall.NewFirewallClient() + provider, err := firewall.DetectProvider() if err == nil { - firewallType = client.Name() + firewallType = provider } for _, item := range firewalls { if err := tx.Model(&model.Firewall{}). diff --git a/agent/utils/firewall/client.go b/agent/utils/firewall/client.go index 442808981516..e3fa0d6187c6 100644 --- a/agent/utils/firewall/client.go +++ b/agent/utils/firewall/client.go @@ -31,24 +31,24 @@ type FilterClient interface { } func NewFirewallClient() (FilterClient, error) { - firewalld := cmd.Which("firewalld") - ufw := cmd.Which("ufw") - - if firewalld && ufw { - return nil, errors.New("It is detected that the system has both firewalld and ufw services. To avoid conflicts, please uninstall and try again!") + provider, err := DetectProvider() + if err != nil { + return nil, err } - if firewalld { + return newClientByName(provider) +} + +func newClientByName(name string) (FilterClient, error) { + switch name { + case "firewalld": return client.NewFirewalld() - } - if ufw { + case "ufw": return client.NewUfw() - } - - iptables := cmd.Which("iptables") - if iptables { + case "iptables": return client.NewIptables() + default: + return nil, errors.New("unsupported firewall provider: " + name) } - return nil, errors.New("No system firewall service detected (firewalld/ufw/iptables), please check and try again!") } func LoadPingStatus() string { diff --git a/agent/utils/firewall/provider.go b/agent/utils/firewall/provider.go new file mode 100644 index 000000000000..a59b670fee68 --- /dev/null +++ b/agent/utils/firewall/provider.go @@ -0,0 +1,30 @@ +package firewall + +import ( + "errors" + + "github.com/1Panel-dev/1Panel/agent/utils/cmd" +) + +// DetectProvider reuses the NewFirewallClient selection order without building a +// client, for the callers that only need the provider name: +// firewalld+ufw conflict -> firewalld -> ufw -> iptables. +func DetectProvider() (string, error) { + return resolveProviderFromPresence(cmd.Which("firewalld"), cmd.Which("ufw"), cmd.Which("iptables")) +} + +func resolveProviderFromPresence(firewalld, ufw, iptables bool) (string, error) { + if firewalld && ufw { + return "", errors.New("It is detected that the system has both firewalld and ufw services. To avoid conflicts, please uninstall and try again!") + } + if firewalld { + return "firewalld", nil + } + if ufw { + return "ufw", nil + } + if iptables { + return "iptables", nil + } + return "", errors.New("No system firewall service detected (firewalld/ufw/iptables), please check and try again!") +} diff --git a/agent/utils/firewall/provider_test.go b/agent/utils/firewall/provider_test.go new file mode 100644 index 000000000000..deb3a43ccba1 --- /dev/null +++ b/agent/utils/firewall/provider_test.go @@ -0,0 +1,93 @@ +package firewall + +import ( + "strings" + "testing" +) + +func TestResolveProviderFromPresence(t *testing.T) { + tests := []struct { + name string + firewalld bool + ufw bool + iptables bool + want string + wantErr string + }{ + { + name: "firewalld preferred", + firewalld: true, + want: "firewalld", + }, + { + name: "ufw", + ufw: true, + want: "ufw", + }, + { + name: "iptables", + iptables: true, + want: "iptables", + }, + { + name: "conflict", + firewalld: true, + ufw: true, + wantErr: "both firewalld and ufw", + }, + { + name: "none", + wantErr: "No system firewall service detected", + }, + { + name: "firewalld wins over iptables", + firewalld: true, + iptables: true, + want: "firewalld", + }, + { + name: "ufw wins over iptables", + ufw: true, + iptables: true, + want: "ufw", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := resolveProviderFromPresence(tt.firewalld, tt.ufw, tt.iptables) + if tt.wantErr != "" { + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErr) + } + if got != "" { + t.Fatalf("failed detection must not name a provider, got %q", got) + } + return + } + if err != nil { + t.Fatal(err) + } + if got != tt.want { + t.Fatalf("got %q want %q", got, tt.want) + } + }) + } +} + +func TestNewClientByNameRejectsUnknownProvider(t *testing.T) { + if _, err := newClientByName("unknown"); err == nil { + t.Fatal("unknown provider must not build a filter client") + } + for _, name := range []string{"ufw", "firewalld", "iptables"} { + client, err := newClientByName(name) + if err != nil { + t.Fatal(err) + } + if client.Name() != name { + t.Fatalf("got client %q want %q", client.Name(), name) + } + } +} From e3a40880ceeae8e6cee5e6a5f55f7bbf406a7f1c Mon Sep 17 00:00:00 2001 From: HynoR <20227709+HynoR@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:53:00 +0800 Subject: [PATCH 2/6] refactor(firewall): push rule expansion into provider clients One logical rule from the API can mean several native operations, and how it splits depends entirely on the provider: ufw applies a port list or range with ":" but records it with "-", drops the protocol so tcp/udp stays a single rule and shows an empty source as "Anywhere"; firewalld and iptables multiply protocols by ports by sources; only iptables owns a chain. Until now the service switched on client.Name() to decide all of that. FilterClient gains ExpandPortRule/ApplyPortUnit and ExpandAddressRule/ ApplyAddressUnit. Expansion is a pure function that runs no command and returns the operations in order, each carrying what to apply, what to record and which chain owns it - the applied and the recorded shape are not always the same. The service now runs one loop over those units for every provider and no longer knows what a rich rule is, so RichRules leaves the interface. The command argument construction inside each client is factored into pure builders so the argv can be asserted without running anything. Address expansion is the same for all three providers apart from the chain, so it stays a single shared helper. --- agent/app/service/firewall.go | 173 +++++------------------ agent/utils/firewall/client.go | 12 +- agent/utils/firewall/client/firewalld.go | 55 +++++-- agent/utils/firewall/client/iptables.go | 111 ++++++++++----- agent/utils/firewall/client/rule.go | 131 +++++++++++++++++ agent/utils/firewall/client/ufw.go | 110 +++++++++----- 6 files changed, 367 insertions(+), 225 deletions(-) create mode 100644 agent/utils/firewall/client/rule.go diff --git a/agent/app/service/firewall.go b/agent/app/service/firewall.go index ab46a5245511..16b3ff267d05 100644 --- a/agent/app/service/firewall.go +++ b/agent/app/service/firewall.go @@ -151,7 +151,9 @@ func (u *FirewallService) SearchWithPage(req dto.RuleSearch) (int64, interface{} } } - go u.cleanUnUsedData(client) + if req.Type == "port" || req.Type == "address" { + go u.cleanUnUsedData(client) + } return int64(total), backDatas, nil } @@ -224,88 +226,28 @@ func (u *FirewallService) OperatePortRule(req dto.PortRuleOperate, reload bool) if err != nil { return err } - if len(req.Chain) == 0 && client.Name() == "iptables" { - req.Chain = iptables.Chain1PanelBasic - } - protos := strings.Split(req.Protocol, "/") - itemAddress := splitFirewallRuleAddresses(req.Address) - - if client.Name() == "ufw" { - if strings.Contains(req.Port, ",") || strings.Contains(req.Port, "-") { - for _, proto := range protos { - for _, addr := range itemAddress { - if len(addr) == 0 { - addr = "Anywhere" - } - req.Address = addr - req.Port = strings.ReplaceAll(req.Port, "-", ":") - req.Protocol = proto - if err := u.operatePort(client, req); err != nil { - return err - } - req.Port = strings.ReplaceAll(req.Port, ":", "-") - if err := u.addPortRecord(req); err != nil { - return err - } - } - } - return nil - } - for _, addr := range itemAddress { - if len(addr) == 0 { - addr = "Anywhere" - } - if req.Protocol == "tcp/udp" { - req.Protocol = "" - } - req.Address = addr - if err := u.operatePort(client, req); err != nil { - return err - } - if len(req.Protocol) == 0 { - req.Protocol = "tcp/udp" - } - if err := u.addPortRecord(req); err != nil { - return err - } - } - return nil - } + return u.operatePortRuleWithClient(client, req, reload) +} - itemPorts := req.Port - for _, proto := range protos { - if strings.Contains(req.Port, "-") { - for _, addr := range itemAddress { - req.Protocol = proto - req.Address = addr - if err := u.operatePort(client, req); err != nil { - return err - } - if err := u.addPortRecord(req); err != nil { - return err - } - } - } else { - ports := strings.Split(itemPorts, ",") - for _, port := range ports { - if len(port) == 0 { - continue - } - for _, addr := range itemAddress { - req.Address = addr - req.Port = port - req.Protocol = proto - if err := u.operatePort(client, req); err != nil { - return err - } - if err := u.addPortRecord(req); err != nil { - return err - } - } - } +func (u *FirewallService) operatePortRuleWithClient(client firewall.FilterClient, req dto.PortRuleOperate, reload bool) error { + var rule fireClient.FireInfo + if err := copier.Copy(&rule, &req); err != nil { + return err + } + for _, unit := range client.ExpandPortRule(rule) { + if err := client.ApplyPortUnit(unit, req.Operation); err != nil { + return err + } + record := req + record.Chain = unit.Chain + record.Address = unit.Record.Address + record.Port = unit.Record.Port + record.Protocol = unit.Record.Protocol + record.Strategy = unit.Record.Strategy + if err := u.addPortRecord(record); err != nil { + return err } } - if reload { return client.Reload() } @@ -317,26 +259,21 @@ func (u *FirewallService) OperateAddressRule(req dto.AddrRuleOperate, reload boo if err != nil { return err } - chain := "" - if client.Name() == "iptables" { - chain = iptables.Chain1PanelBasic - } - var fireInfo fireClient.FireInfo - if err := copier.Copy(&fireInfo, &req); err != nil { + return u.operateAddressRuleWithClient(client, req, reload) +} + +func (u *FirewallService) operateAddressRuleWithClient(client firewall.FilterClient, req dto.AddrRuleOperate, reload bool) error { + var rule fireClient.FireInfo + if err := copier.Copy(&rule, &req); err != nil { return err } - - addressList := strings.Split(req.Address, ",") - for i := 0; i < len(addressList); i++ { - if len(addressList[i]) == 0 { - continue - } - fireInfo.Address = addressList[i] - if err := client.RichRules(fireInfo, req.Operation); err != nil { + for _, unit := range client.ExpandAddressRule(rule) { + if err := client.ApplyAddressUnit(unit, req.Operation); err != nil { return err } - req.Address = addressList[i] - if err := u.addAddressRecord(chain, req); err != nil { + record := req + record.Address = unit.Apply.Address + if err := u.addAddressRecord(unit.Chain, record); err != nil { return err } } @@ -413,6 +350,10 @@ func OperateFirewallPort(oldPorts, newPorts []int) error { if err != nil { return err } + return operateFirewallPorts(client, oldPorts, newPorts) +} + +func operateFirewallPorts(client firewall.FilterClient, oldPorts, newPorts []int) error { for _, port := range newPorts { if err := client.Port(fireClient.FireInfo{Port: strconv.Itoa(port), Protocol: "tcp", Strategy: "accept"}, "add"); err != nil { return err @@ -426,46 +367,6 @@ func OperateFirewallPort(oldPorts, newPorts []int) error { return client.Reload() } -func (u *FirewallService) operatePort(client firewall.FilterClient, req dto.PortRuleOperate) error { - var fireInfo fireClient.FireInfo - if err := copier.Copy(&fireInfo, &req); err != nil { - return err - } - fireInfo.Address = normalizeFirewallRuleAddress(fireInfo.Address) - - if client.Name() == "ufw" { - if len(fireInfo.Address) != 0 && !strings.EqualFold(fireInfo.Address, "Anywhere") { - return client.RichRules(fireInfo, req.Operation) - } - return client.Port(fireInfo, req.Operation) - } - - if len(fireInfo.Address) != 0 || fireInfo.Strategy == "drop" { - return client.RichRules(fireInfo, req.Operation) - } - return client.Port(fireInfo, req.Operation) -} - -func splitFirewallRuleAddresses(address string) []string { - parts := strings.Split(strings.TrimSuffix(address, ","), ",") - addresses := make([]string, 0, len(parts)) - for _, part := range parts { - addresses = append(addresses, normalizeFirewallRuleAddress(part)) - } - if len(addresses) == 0 { - return []string{""} - } - return addresses -} - -func normalizeFirewallRuleAddress(address string) string { - address = strings.TrimSpace(address) - if strings.EqualFold(address, "Anywhere") { - return "" - } - return address -} - type portOfApp struct { AppName string HttpPort string diff --git a/agent/utils/firewall/client.go b/agent/utils/firewall/client.go index e3fa0d6187c6..bf6b5aff0914 100644 --- a/agent/utils/firewall/client.go +++ b/agent/utils/firewall/client.go @@ -12,8 +12,8 @@ import ( "github.com/1Panel-dev/1Panel/agent/utils/firewall/client" ) -// FilterClient is the filter capability surface; port forwarding lives in its -// own adapter and is no longer reachable from here. +// FilterClient is the filter capability surface. Rule expansion lives behind +// Expand*/Apply* so that callers never build provider native commands themselves. type FilterClient interface { Name() string // ufw firewalld iptables Start() error @@ -27,7 +27,13 @@ type FilterClient interface { ListAddress() ([]client.FireInfo, error) Port(port client.FireInfo, operation string) error - RichRules(rule client.FireInfo, operation string) error + + // ExpandPortRule turns one logical rule into the ordered native operations + // this provider needs. It runs no command. + ExpandPortRule(rule client.FireInfo) []client.PortUnit + ApplyPortUnit(unit client.PortUnit, operation string) error + ExpandAddressRule(rule client.FireInfo) []client.AddressUnit + ApplyAddressUnit(unit client.AddressUnit, operation string) error } func NewFirewallClient() (FilterClient, error) { diff --git a/agent/utils/firewall/client/firewalld.go b/agent/utils/firewall/client/firewalld.go index b9cd4fa8c71d..4f1c1d46afea 100644 --- a/agent/utils/firewall/client/firewalld.go +++ b/agent/utils/firewall/client/firewalld.go @@ -131,7 +131,7 @@ func (f *Firewall) Port(port FireInfo, operation string) error { return buserr.New("ErrCmdIllegal") } - if err := cmd.NewCommandMgr().Run("firewall-cmd", "--zone=public", "--"+operation+"-port="+port.Port+"/"+port.Protocol, "--permanent"); err != nil { + if err := cmd.NewCommandMgr().Run("firewall-cmd", buildFirewalldPortArgs(port, operation)...); err != nil { return fmt.Errorf("%s (port: %s/%s strategy: %s) failed, %v", operation, port.Port, port.Protocol, port.Strategy, err) } return nil @@ -141,6 +141,38 @@ func (f *Firewall) RichRules(rule FireInfo, operation string) error { if cmd.CheckIllegal(operation, rule.Address, rule.Protocol, rule.Port, rule.Strategy) { return buserr.New("ErrCmdIllegal") } + for _, ruleStr := range buildFirewalldRichRuleStrings(rule) { + if err := cmd.NewCommandMgr().Run("firewall-cmd", buildFirewalldRichRuleArgs(ruleStr, operation)...); err != nil { + return fmt.Errorf("%s rich rules (%s) failed, %v", operation, ruleStr, err) + } + } + return nil +} + +func (f *Firewall) ExpandPortRule(rule FireInfo) []PortUnit { + return expandPortRule(rule, rule.Chain) +} + +func (f *Firewall) ApplyPortUnit(unit PortUnit, operation string) error { + if needsRichRule(unit.Apply) { + return f.RichRules(unit.Apply, operation) + } + return f.Port(unit.Apply, operation) +} + +func (f *Firewall) ExpandAddressRule(rule FireInfo) []AddressUnit { + return expandAddressRule(rule, "") +} + +func (f *Firewall) ApplyAddressUnit(unit AddressUnit, operation string) error { + return f.RichRules(unit.Apply, operation) +} + +func buildFirewalldPortArgs(port FireInfo, operation string) []string { + return []string{"--zone=public", "--" + operation + "-port=" + port.Port + "/" + port.Protocol, "--permanent"} +} + +func buildFirewalldRichRuleString(rule FireInfo) string { ruleStr := "rule family=ipv4 " if strings.Contains(rule.Address, ":") { ruleStr = "rule family=ipv6 " @@ -154,17 +186,20 @@ func (f *Firewall) RichRules(rule FireInfo, operation string) error { if len(rule.Protocol) != 0 { ruleStr += fmt.Sprintf("protocol=%s ", rule.Protocol) } - ruleStr += rule.Strategy - if err := cmd.NewCommandMgr().Run("firewall-cmd", "--zone=public", "--"+operation+"-rich-rule", ruleStr, "--permanent"); err != nil { - return fmt.Errorf("%s rich rules (%s) failed, %v", operation, ruleStr, err) - } + return ruleStr + rule.Strategy +} + +func buildFirewalldRichRuleStrings(rule FireInfo) []string { + ruleStr := buildFirewalldRichRuleString(rule) + rules := []string{ruleStr} if len(rule.Address) == 0 { - ipv6Rule := strings.ReplaceAll(ruleStr, "family=ipv4 ", "family=ipv6 ") - if err := cmd.NewCommandMgr().Run("firewall-cmd", "--zone=public", "--"+operation+"-rich-rule", ipv6Rule, "--permanent"); err != nil { - return fmt.Errorf("%s rich rules (%s) failed, %v", operation, ipv6Rule, err) - } + rules = append(rules, strings.ReplaceAll(ruleStr, "family=ipv4 ", "family=ipv6 ")) } - return nil + return rules +} + +func buildFirewalldRichRuleArgs(ruleStr, operation string) []string { + return []string{"--zone=public", "--" + operation + "-rich-rule", ruleStr, "--permanent"} } func (f *Firewall) loadInfo(line string) FireInfo { diff --git a/agent/utils/firewall/client/iptables.go b/agent/utils/firewall/client/iptables.go index 40c46424aa92..b8a8cdfbf262 100644 --- a/agent/utils/firewall/client/iptables.go +++ b/agent/utils/firewall/client/iptables.go @@ -118,22 +118,10 @@ func (i *Iptables) Port(port FireInfo, operation string) error { port.Chain = iptables.Chain1PanelBasic } - portSpec, err := normalizePortSpec(port.Port) + ruleArgs, err := buildIptablesPortRuleArgs(port) if err != nil { return err } - - protocol := port.Protocol - if protocol == "" { - protocol = "tcp" - } - - action := "ACCEPT" - if port.Strategy == "drop" { - action = "DROP" - } - - ruleArgs := []string{"-p", protocol, "--dport", portSpec, "-j", action} if operation == "add" { if err := iptables.AddRule(iptables.FilterTab, port.Chain, ruleArgs...); err != nil { return err @@ -162,60 +150,105 @@ func (i *Iptables) RichRules(rule FireInfo, operation string) error { rule.Chain = iptables.Chain1PanelBasic } + ruleArgs, err := buildIptablesRichRuleArgs(rule) + if err != nil { + return err + } + if operation == "add" { + if err := iptables.AddRule(iptables.FilterTab, rule.Chain, ruleArgs...); err != nil { + return err + } + } else { + if err := iptables.DeleteRule(iptables.FilterTab, rule.Chain, ruleArgs...); err != nil { + return err + } + } + + name := iptables.BasicFileName + if rule.Chain == iptables.Chain1PanelBasicBefore { + name = iptables.BasicBeforeFileName + } + if err := iptables.SaveRulesToFile(iptables.FilterTab, rule.Chain, name); err != nil { + global.LOG.Errorf("persistence for %s failed, err: %v", iptables.Chain1PanelBasic, err) + } + return nil +} + +func (i *Iptables) ExpandPortRule(rule FireInfo) []PortUnit { + chain := rule.Chain + if len(chain) == 0 { + chain = iptables.Chain1PanelBasic + } + return expandPortRule(rule, chain) +} + +func (i *Iptables) ApplyPortUnit(unit PortUnit, operation string) error { + apply := unit.Apply + apply.Chain = unit.Chain + if needsRichRule(apply) { + return i.RichRules(apply, operation) + } + return i.Port(apply, operation) +} + +func (i *Iptables) ExpandAddressRule(rule FireInfo) []AddressUnit { + return expandAddressRule(rule, iptables.Chain1PanelBasic) +} + +func (i *Iptables) ApplyAddressUnit(unit AddressUnit, operation string) error { + apply := unit.Apply + apply.Chain = unit.Chain + return i.RichRules(apply, operation) +} + +func buildIptablesPortRuleArgs(port FireInfo) ([]string, error) { + portSpec, err := normalizePortSpec(port.Port) + if err != nil { + return nil, err + } + protocol := port.Protocol + if protocol == "" { + protocol = "tcp" + } + action := "ACCEPT" + if port.Strategy == "drop" { + action = "DROP" + } + return []string{"-p", protocol, "--dport", portSpec, "-j", action}, nil +} + +func buildIptablesRichRuleArgs(rule FireInfo) ([]string, error) { address := strings.TrimSpace(rule.Address) if strings.EqualFold(address, "Anywhere") { address = "" } - action := "ACCEPT" if rule.Strategy == "drop" { action = "DROP" } - var ruleArgs []string if address != "" { ruleArgs = append(ruleArgs, "-s", address) } - protocol := strings.TrimSpace(rule.Protocol) if rule.Port != "" && protocol == "" { protocol = "tcp" } - if protocol != "" { ruleArgs = append(ruleArgs, "-p", protocol) } - if rule.Port != "" { portSegment, err := normalizePortSpec(rule.Port) if err != nil { - return err + return nil, err } if protocol == "" { - return fmt.Errorf("protocol is required when specifying a port") + return nil, fmt.Errorf("protocol is required when specifying a port") } ruleArgs = append(ruleArgs, "--dport", portSegment) } - ruleArgs = append(ruleArgs, "-j", action) - if operation == "add" { - if err := iptables.AddRule(iptables.FilterTab, rule.Chain, ruleArgs...); err != nil { - return err - } - } else { - if err := iptables.DeleteRule(iptables.FilterTab, rule.Chain, ruleArgs...); err != nil { - return err - } - } - - name := iptables.BasicFileName - if rule.Chain == iptables.Chain1PanelBasicBefore { - name = iptables.BasicBeforeFileName - } - if err := iptables.SaveRulesToFile(iptables.FilterTab, rule.Chain, name); err != nil { - global.LOG.Errorf("persistence for %s failed, err: %v", iptables.Chain1PanelBasic, err) - } - return nil + return ruleArgs, nil } func parsePort(portStr string) (int, error) { diff --git a/agent/utils/firewall/client/rule.go b/agent/utils/firewall/client/rule.go new file mode 100644 index 000000000000..6625d35752a2 --- /dev/null +++ b/agent/utils/firewall/client/rule.go @@ -0,0 +1,131 @@ +package client + +import "strings" + +// PortUnit is one native operation a provider runs for a single logical port rule. +type PortUnit struct { + Apply FireInfo // rule handed to the provider command + Record FireInfo // rule persisted in the 1Panel record, differs from Apply on ufw + Chain string // owning 1PANEL chain, empty on providers without managed chains +} + +// AddressUnit is the address rule counterpart of PortUnit. Applied and recorded +// shapes never differ for address rules, so there is no Record here. +type AddressUnit struct { + Apply FireInfo + Chain string +} + +// splitRuleAddresses always yields at least one entry so a rule without a source +// still expands into a single "any source" unit. +func splitRuleAddresses(address string) []string { + parts := strings.Split(strings.TrimSuffix(address, ","), ",") + addresses := make([]string, 0, len(parts)) + for _, part := range parts { + addresses = append(addresses, normalizeRuleAddress(part)) + } + if len(addresses) == 0 { + return []string{""} + } + return addresses +} + +// normalizeRuleAddress maps the ufw "Anywhere" display value back to an empty source. +func normalizeRuleAddress(address string) string { + address = strings.TrimSpace(address) + if strings.EqualFold(address, "Anywhere") { + return "" + } + return address +} + +// expandPortRule is shared by firewalld and iptables: protocols x (port range or +// comma list) x addresses. A range is passed through untouched, everything else +// is split on commas. +func expandPortRule(rule FireInfo, chain string) []PortUnit { + addresses := splitRuleAddresses(rule.Address) + var units []PortUnit + for _, protocol := range strings.Split(rule.Protocol, "/") { + ports := []string{rule.Port} + if !strings.Contains(rule.Port, "-") { + ports = strings.Split(rule.Port, ",") + } + for _, port := range ports { + if len(port) == 0 { + continue + } + for _, address := range addresses { + item := rule + item.Port = port + item.Protocol = protocol + item.Address = address + units = append(units, PortUnit{Apply: item, Record: item, Chain: chain}) + } + } + } + return units +} + +// expandUfwPortRule keeps the two ufw specifics: a port list or range is applied +// with ":" but recorded with "-", and a single port drops the protocol so that +// tcp/udp becomes one rule instead of two. +func expandUfwPortRule(rule FireInfo) []PortUnit { + addresses := splitRuleAddresses(rule.Address) + var units []PortUnit + if strings.Contains(rule.Port, ",") || strings.Contains(rule.Port, "-") { + for _, protocol := range strings.Split(rule.Protocol, "/") { + for _, address := range addresses { + apply, record := rule, rule + apply.Protocol, record.Protocol = protocol, protocol + apply.Port = strings.ReplaceAll(rule.Port, "-", ":") + record.Port = strings.ReplaceAll(rule.Port, ":", "-") + apply.Address, record.Address = address, ufwRecordAddress(address) + units = append(units, PortUnit{Apply: apply, Record: record, Chain: rule.Chain}) + } + } + return units + } + for _, address := range addresses { + apply, record := rule, rule + if rule.Protocol == "tcp/udp" { + apply.Protocol = "" + } + apply.Address, record.Address = address, ufwRecordAddress(address) + units = append(units, PortUnit{Apply: apply, Record: record, Chain: rule.Chain}) + } + return units +} + +func ufwRecordAddress(address string) string { + if len(address) == 0 { + return "Anywhere" + } + return address +} + +// expandAddressRule is shared by every provider: one native rule per source, +// empty entries are dropped instead of becoming an "any source" rule. +func expandAddressRule(rule FireInfo, chain string) []AddressUnit { + var units []AddressUnit + for _, address := range strings.Split(rule.Address, ",") { + if len(address) == 0 { + continue + } + item := rule + item.Address = address + units = append(units, AddressUnit{Apply: item, Chain: chain}) + } + return units +} + +// needsRichRule is the firewalld/iptables choice between the port shortcut and a +// full rule; ufw has its own rule in ufwNeedsRichRule. +func needsRichRule(rule FireInfo) bool { + return len(rule.Address) != 0 || rule.Strategy == "drop" +} + +// ufwNeedsRichRule is the ufw variant: ufw denies a port through the port +// shortcut as well, only a source forces the longer form. +func ufwNeedsRichRule(rule FireInfo) bool { + return len(rule.Address) != 0 && !strings.EqualFold(rule.Address, "Anywhere") +} diff --git a/agent/utils/firewall/client/ufw.go b/agent/utils/firewall/client/ufw.go index 9779b51c4841..f2be261dd895 100644 --- a/agent/utils/firewall/client/ufw.go +++ b/agent/utils/firewall/client/ufw.go @@ -129,25 +129,14 @@ func (f *Ufw) ListAddress() ([]FireInfo, error) { } func (f *Ufw) Port(port FireInfo, operation string) error { - switch port.Strategy { - case "accept": - port.Strategy = "allow" - case "drop": - port.Strategy = "deny" - default: - return fmt.Errorf("unsupported strategy %s", port.Strategy) + args, err := buildUfwPortArgs(port, operation) + if err != nil { + return err } if cmd.CheckIllegal(port.Protocol, port.Port) { return buserr.New("ErrCmdIllegal") } - args := []string{port.Strategy, port.Port} - if operation == "remove" { - args = []string{"delete", port.Strategy, port.Port} - } - if len(port.Protocol) != 0 { - args[len(args)-1] += "/" + port.Protocol - } if err := f.run(args...); err != nil { return fmt.Errorf("%s (%s) failed, %v", operation, strings.Join(args, " "), err) } @@ -155,36 +144,18 @@ func (f *Ufw) Port(port FireInfo, operation string) error { } func (f *Ufw) RichRules(rule FireInfo, operation string) error { - switch rule.Strategy { - case "accept": - rule.Strategy = "allow" - case "drop": - rule.Strategy = "deny" - default: - return fmt.Errorf("unsupported strategy %s", rule.Strategy) + strategy, err := normalizeUfwStrategy(rule.Strategy) + if err != nil { + return err } + rule.Strategy = strategy if cmd.CheckIllegal(operation, rule.Protocol, rule.Address, rule.Port) { return buserr.New("ErrCmdIllegal") } insertNum := f.loadInsertNum(rule, operation) - args := []string{"insert", strconv.Itoa(insertNum), rule.Strategy} - if operation == "remove" { - args = []string{"delete", rule.Strategy} - } - if len(rule.Protocol) != 0 { - args = append(args, "proto", rule.Protocol) - } - if strings.Contains(rule.Address, "-") { - parts := strings.Split(rule.Address, "-") - args = append(args, "from", parts[0], "to", parts[1]) - } else { - args = append(args, "from", rule.Address) - } - if len(rule.Port) != 0 { - args = append(args, "to", "any", "port", rule.Port) - } + args := buildUfwRichRuleArgs(rule, operation, insertNum) stdout, err := f.runWithStdout(args...) if err != nil { @@ -203,6 +174,71 @@ func (f *Ufw) RichRules(rule FireInfo, operation string) error { return nil } +func (f *Ufw) ExpandPortRule(rule FireInfo) []PortUnit { + return expandUfwPortRule(rule) +} + +func (f *Ufw) ApplyPortUnit(unit PortUnit, operation string) error { + if ufwNeedsRichRule(unit.Apply) { + return f.RichRules(unit.Apply, operation) + } + return f.Port(unit.Apply, operation) +} + +func (f *Ufw) ExpandAddressRule(rule FireInfo) []AddressUnit { + return expandAddressRule(rule, "") +} + +func (f *Ufw) ApplyAddressUnit(unit AddressUnit, operation string) error { + return f.RichRules(unit.Apply, operation) +} + +func normalizeUfwStrategy(strategy string) (string, error) { + switch strategy { + case "accept": + return "allow", nil + case "drop": + return "deny", nil + default: + return "", fmt.Errorf("unsupported strategy %s", strategy) + } +} + +func buildUfwPortArgs(port FireInfo, operation string) ([]string, error) { + strategy, err := normalizeUfwStrategy(port.Strategy) + if err != nil { + return nil, err + } + args := []string{strategy, port.Port} + if operation == "remove" { + args = []string{"delete", strategy, port.Port} + } + if len(port.Protocol) != 0 { + args[len(args)-1] += "/" + port.Protocol + } + return args, nil +} + +func buildUfwRichRuleArgs(rule FireInfo, operation string, insertNum int) []string { + args := []string{"insert", strconv.Itoa(insertNum), rule.Strategy} + if operation == "remove" { + args = []string{"delete", rule.Strategy} + } + if len(rule.Protocol) != 0 { + args = append(args, "proto", rule.Protocol) + } + if strings.Contains(rule.Address, "-") { + parts := strings.Split(rule.Address, "-") + args = append(args, "from", parts[0], "to", parts[1]) + } else { + args = append(args, "from", rule.Address) + } + if len(rule.Port) != 0 { + args = append(args, "to", "any", "port", rule.Port) + } + return args +} + func (f *Ufw) loadInfo(line string, fireType string) FireInfo { fields := strings.Fields(line) var itemInfo FireInfo From d636597e412f11457e571ae25d9287ecf8bd43a9 Mon Sep 17 00:00:00 2001 From: HynoR <20227709+HynoR@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:53:39 +0800 Subject: [PATCH 3/6] refactor(firewall): push port whitelist sync into clients The port whitelist was the last provider switch left in the service. What each provider does with it is genuinely different: ufw and firewalld only act while the service is running and add or remove ports through the same native port command as user rules, while iptables acts only once its chains are initialized and writes required ports into 1PANEL_BASIC_BEFORE and configured ports into 1PANEL_BASIC, then persists both chains. FilterClient gains AddPortWhiteList, which re-adds everything after the provider was started, and SyncPortWhiteList, which applies the difference against the previously configured list. The service resolves the whitelist state - configured entries, the required 1Panel and SSH ports, and the previous configured entries - and hands that struct down; deciding what to do with it belongs to the provider. The iptables chain writing moves next to the iptables client, and the advanced init path calls it directly with persistence turned off because it persists the chains itself once initialization finishes. The whitelist state is now resolved before the provider decides whether it can act, so on a stopped ufw/firewalld or an uninitialized iptables an unreadable 1Panel service port surfaces as an error instead of silently doing nothing. --- agent/app/service/firewall.go | 17 +- agent/app/service/firewall_setting.go | 136 ++++----------- agent/app/service/iptables.go | 115 +------------ agent/utils/firewall/client.go | 5 + agent/utils/firewall/client/firewalld.go | 8 + agent/utils/firewall/client/iptables.go | 15 ++ agent/utils/firewall/client/ufw.go | 8 + agent/utils/firewall/client/whitelist.go | 209 +++++++++++++++++++++++ 8 files changed, 284 insertions(+), 229 deletions(-) create mode 100644 agent/utils/firewall/client/whitelist.go diff --git a/agent/app/service/firewall.go b/agent/app/service/firewall.go index 16b3ff267d05..0c87ad27d002 100644 --- a/agent/app/service/firewall.go +++ b/agent/app/service/firewall.go @@ -420,24 +420,11 @@ func (u *FirewallService) cleanUnUsedData(client firewall.FilterClient) { } func (u *FirewallService) addPortsBeforeStart(client firewall.FilterClient) error { - if client.Name() == "iptables" { - isInit, _ := iptables.LoadInitStatus("iptables", "base") - if !isInit { - return nil - } - return syncIptablesFirewallPortWhiteList(true) - } - portWhiteList, err := loadFirewallPortWhiteList() + list, err := loadFirewallPortWhiteList("") if err != nil { return err } - for _, item := range portWhiteList { - if err := client.Port(fireClient.FireInfo{Port: item.Port, Protocol: item.Protocol, Strategy: "accept"}, "add"); err != nil { - return err - } - } - - return client.Reload() + return client.AddPortWhiteList(list) } func (u *FirewallService) addPortRecord(req dto.PortRuleOperate) error { diff --git a/agent/app/service/firewall_setting.go b/agent/app/service/firewall_setting.go index d37c17cde44f..9dd14a40ec8e 100644 --- a/agent/app/service/firewall_setting.go +++ b/agent/app/service/firewall_setting.go @@ -8,15 +8,9 @@ import ( "github.com/1Panel-dev/1Panel/agent/constant" "github.com/1Panel-dev/1Panel/agent/utils/firewall" fireClient "github.com/1Panel-dev/1Panel/agent/utils/firewall/client" - "github.com/1Panel-dev/1Panel/agent/utils/firewall/client/iptables" ) -type firewallPortWhitelist struct { - Port string - Protocol string -} - -func loadConfiguredFirewallPortWhiteList() ([]firewallPortWhitelist, error) { +func loadConfiguredFirewallPortWhiteList() ([]fireClient.PortWhiteListEntry, error) { value, err := settingRepo.GetValueByKey(constant.FirewallPortWhiteList) if err != nil { value = constant.FirewallPortWhiteListValue @@ -27,34 +21,42 @@ func loadConfiguredFirewallPortWhiteList() ([]firewallPortWhitelist, error) { return parseFirewallPortWhiteList(value) } -func loadFirewallPortWhiteList() ([]firewallPortWhitelist, error) { - portWhiteList, err := loadConfiguredFirewallPortWhiteList() - if err != nil { - return nil, err - } - requiredPorts, err := loadRequiredFirewallPortWhiteList() - if err != nil { - return nil, err - } - return normalizeFirewallPortWhiteList(append(portWhiteList, requiredPorts...)), nil -} - -func loadRequiredFirewallPortWhiteList() ([]firewallPortWhitelist, error) { +func loadRequiredFirewallPortWhiteList() ([]fireClient.PortWhiteListEntry, error) { panelPort := LoadPanelPort() if panelPort == "" { return nil, fmt.Errorf("find 1panel service port failed") } - return normalizeFirewallPortWhiteList([]firewallPortWhitelist{ + return []fireClient.PortWhiteListEntry{ {Port: panelPort, Protocol: "tcp"}, {Port: loadSSHPort(), Protocol: "tcp"}, - }), nil + }, nil +} + +// loadFirewallPortWhiteList resolves the whitelist state for a provider. oldValue +// is the raw setting value replaced by this change, empty when nothing is replaced. +func loadFirewallPortWhiteList(oldValue string) (fireClient.PortWhiteList, error) { + var list fireClient.PortWhiteList + configured, err := loadConfiguredFirewallPortWhiteList() + if err != nil { + return list, err + } + required, err := loadRequiredFirewallPortWhiteList() + if err != nil { + return list, err + } + previous, err := parseFirewallPortWhiteList(oldValue) + if err != nil { + return list, err + } + list.Configured, list.Required, list.Previous = configured, required, previous + return list, nil } -func parseFirewallPortWhiteList(value string) ([]firewallPortWhitelist, error) { +func parseFirewallPortWhiteList(value string) ([]fireClient.PortWhiteListEntry, error) { items := strings.FieldsFunc(value, func(r rune) bool { return r == ',' || r == '\n' || r == ';' || r == ' ' }) - ports := make([]firewallPortWhitelist, 0, len(items)) + ports := make([]fireClient.PortWhiteListEntry, 0, len(items)) exists := make(map[string]struct{}) for _, item := range items { item = strings.TrimSpace(item) @@ -79,97 +81,23 @@ func parseFirewallPortWhiteList(value string) ([]firewallPortWhitelist, error) { continue } exists[key] = struct{}{} - ports = append(ports, firewallPortWhitelist{Port: strconv.Itoa(portNum), Protocol: protocol}) + ports = append(ports, fireClient.PortWhiteListEntry{Port: strconv.Itoa(portNum), Protocol: protocol}) } return ports, nil } -func normalizeFirewallPortWhiteList(portWhiteList []firewallPortWhitelist) []firewallPortWhitelist { - ports := make([]firewallPortWhitelist, 0, len(portWhiteList)) - exists := make(map[string]struct{}) - for _, item := range portWhiteList { - if item.Port == "" { - continue - } - key := fmt.Sprintf("%s/%s", item.Port, item.Protocol) - if _, ok := exists[key]; ok { - continue - } - exists[key] = struct{}{} - ports = append(ports, item) - } - return ports -} - func syncFirewallPortWhiteListAfterUpdate(oldValue string) error { client, err := firewall.NewFirewallClient() if err != nil { return err } - if client.Name() == "iptables" { - isInit, _ := iptables.LoadInitStatus("iptables", "base") - if !isInit { - return nil - } - oldPortWhiteList, err := parseFirewallPortWhiteList(oldValue) - if err != nil { - return err - } - return syncIptablesFirewallPortWhiteList(true, oldPortWhiteList) - } + return syncFirewallPortWhiteListAfterUpdateWithClient(client, oldValue) +} - isActive, _ := client.Status() - if !isActive { - return nil - } - portWhiteList, err := loadFirewallPortWhiteList() +func syncFirewallPortWhiteListAfterUpdateWithClient(client firewall.FilterClient, oldValue string) error { + list, err := loadFirewallPortWhiteList(oldValue) if err != nil { return err } - oldPortWhiteList, err := parseFirewallPortWhiteList(oldValue) - if err != nil { - return err - } - requiredPorts, err := loadRequiredFirewallPortWhiteList() - if err != nil { - return err - } - oldPortWhiteList = normalizeFirewallPortWhiteList(append(oldPortWhiteList, requiredPorts...)) - return syncFirewallClientPortWhiteList(client, oldPortWhiteList, portWhiteList) -} - -func syncFirewallClientPortWhiteList(client firewall.FilterClient, oldPortWhiteList, portWhiteList []firewallPortWhitelist) error { - oldPorts := firewallPortWhiteListMap(oldPortWhiteList) - newPorts := firewallPortWhiteListMap(portWhiteList) - for _, item := range oldPortWhiteList { - key := firewallPortWhiteListKey(item) - if _, ok := newPorts[key]; ok { - continue - } - if err := client.Port(fireClient.FireInfo{Port: item.Port, Protocol: item.Protocol, Strategy: "accept"}, "remove"); err != nil { - return err - } - } - for _, item := range portWhiteList { - key := firewallPortWhiteListKey(item) - if _, ok := oldPorts[key]; ok { - continue - } - if err := client.Port(fireClient.FireInfo{Port: item.Port, Protocol: item.Protocol, Strategy: "accept"}, "add"); err != nil { - return err - } - } - return client.Reload() -} - -func firewallPortWhiteListMap(portWhiteList []firewallPortWhitelist) map[string]struct{} { - ports := make(map[string]struct{}) - for _, item := range portWhiteList { - ports[firewallPortWhiteListKey(item)] = struct{}{} - } - return ports -} - -func firewallPortWhiteListKey(item firewallPortWhitelist) string { - return item.Port + "/" + item.Protocol + return client.SyncPortWhiteList(list) } diff --git a/agent/app/service/iptables.go b/agent/app/service/iptables.go index 174adb5dba67..bca81cb98b1d 100644 --- a/agent/app/service/iptables.go +++ b/agent/app/service/iptables.go @@ -11,6 +11,7 @@ import ( "github.com/1Panel-dev/1Panel/agent/constant" "github.com/1Panel-dev/1Panel/agent/global" "github.com/1Panel-dev/1Panel/agent/utils/cmd" + fireClient "github.com/1Panel-dev/1Panel/agent/utils/firewall/client" "github.com/1Panel-dev/1Panel/agent/utils/firewall/client/iptables" ) @@ -370,125 +371,19 @@ func initPreRules() error { if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasicBefore, "-m", "conntrack", "--ctstate", "RELATED,ESTABLISHED", "-j", "ACCEPT", "-m", "comment", "--comment", "ESTABLISHED Whitelist"); err != nil { return err } - if err := syncIptablesFirewallPortWhiteList(false); err != nil { - return err - } - if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasicAfter, "-p", "tcp", "-j", "DROP"); err != nil { - return err - } - if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasicAfter, "-p", "udp", "-j", "DROP"); err != nil { - return err - } - return nil -} - -func syncIptablesFirewallPortWhiteList(withSave bool, oldConfiguredPortWhiteList ...[]firewallPortWhitelist) error { - requiredPorts, err := loadRequiredFirewallPortWhiteList() - if err != nil { - return err - } - if err := applyRequiredFirewallPortWhiteListRules(requiredPorts, withSave); err != nil { - return err - } - portWhiteList, err := loadConfiguredFirewallPortWhiteList() + list, err := loadFirewallPortWhiteList("") if err != nil { return err } - return applyFirewallPortWhiteListRules(portWhiteList, withSave, oldConfiguredPortWhiteList...) -} - -func applyRequiredFirewallPortWhiteListRules(portWhiteList []firewallPortWhitelist, withSave bool) error { - if err := syncRequiredFirewallPortWhiteListRules(portWhiteList); err != nil { - return err - } - for _, item := range portWhiteList { - if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasicBefore, "-p", item.Protocol, "-m", item.Protocol, "--dport", item.Port, "-j", "ACCEPT"); err != nil { - return err - } - } - if !withSave { - return nil - } - if err := iptables.SaveRulesToFile(iptables.FilterTab, iptables.Chain1PanelBasicBefore, iptables.BasicBeforeFileName); err != nil { - return err - } - return iptables.SaveRulesToFile(iptables.FilterTab, iptables.Chain1PanelBasicAfter, iptables.BasicAfterFileName) -} - -func applyFirewallPortWhiteListRules(portWhiteList []firewallPortWhitelist, withSave bool, oldConfiguredPortWhiteList ...[]firewallPortWhitelist) error { - if err := syncFirewallPortWhiteListRules(portWhiteList, oldConfiguredPortWhiteList...); err != nil { - return err - } - for _, item := range portWhiteList { - if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasic, "-p", item.Protocol, "-m", item.Protocol, "--dport", item.Port, "-j", "ACCEPT"); err != nil { - return err - } - } - if !withSave { - return nil - } - return iptables.SaveRulesToFile(iptables.FilterTab, iptables.Chain1PanelBasic, iptables.BasicFileName) -} - -func syncRequiredFirewallPortWhiteListRules(portWhiteList []firewallPortWhitelist) error { - tcpWhitelist := make(map[string]struct{}) - udpWhitelist := make(map[string]struct{}) - for _, item := range portWhiteList { - if item.Protocol == "udp" { - udpWhitelist[item.Port] = struct{}{} - continue - } - tcpWhitelist[item.Port] = struct{}{} - } - - if err := cleanExtraFirewallPortRules(iptables.Chain1PanelBasicBefore, "tcp", tcpWhitelist); err != nil { + if err := fireClient.SyncIptablesPortWhiteList(list, false); err != nil { return err } - if err := cleanExtraFirewallPortRules(iptables.Chain1PanelBasicBefore, "udp", udpWhitelist); err != nil { + if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasicAfter, "-p", "tcp", "-j", "DROP"); err != nil { return err } - return cleanExtraFirewallPortRules(iptables.Chain1PanelBasicAfter, "udp", map[string]struct{}{}) -} - -func syncFirewallPortWhiteListRules(portWhiteList []firewallPortWhitelist, oldConfiguredPortWhiteList ...[]firewallPortWhitelist) error { - portWhitelist := firewallPortWhiteListMap(portWhiteList) - if len(oldConfiguredPortWhiteList) == 0 { - return nil - } - for _, item := range oldConfiguredPortWhiteList[0] { - if _, ok := portWhitelist[firewallPortWhiteListKey(item)]; ok { - continue - } - if !iptables.CheckRuleExist(iptables.FilterTab, iptables.Chain1PanelBasic, "-p", item.Protocol, "--dport", item.Port, "-j", "ACCEPT") { - continue - } - if err := iptables.DeleteRule(iptables.FilterTab, iptables.Chain1PanelBasic, "-p", item.Protocol, "--dport", item.Port, "-j", "ACCEPT"); err != nil { - return err - } - } - return nil -} - -func cleanExtraFirewallPortRules(chain, protocol string, whitelist map[string]struct{}) error { - rules, err := iptables.ReadFilterRulesByChain(chain) - if err != nil { + if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasicAfter, "-p", "udp", "-j", "DROP"); err != nil { return err } - kept := make(map[string]struct{}) - for _, rule := range rules { - if rule.Strategy != "accept" || rule.Protocol != protocol || rule.DstPort == "" || rule.SrcIP != "" || rule.DstIP != "" || rule.SrcPort != "" { - continue - } - if _, ok := whitelist[rule.DstPort]; ok { - if _, seen := kept[rule.DstPort]; !seen { - kept[rule.DstPort] = struct{}{} - continue - } - } - if err := iptables.DeleteRule(iptables.FilterTab, chain, "-p", protocol, "-m", protocol, "--dport", rule.DstPort, "-j", "ACCEPT"); err != nil { - return err - } - } return nil } diff --git a/agent/utils/firewall/client.go b/agent/utils/firewall/client.go index bf6b5aff0914..b141da4cc55b 100644 --- a/agent/utils/firewall/client.go +++ b/agent/utils/firewall/client.go @@ -34,6 +34,11 @@ type FilterClient interface { ApplyPortUnit(unit client.PortUnit, operation string) error ExpandAddressRule(rule client.FireInfo) []client.AddressUnit ApplyAddressUnit(unit client.AddressUnit, operation string) error + + // AddPortWhiteList re-adds the whole whitelist after the provider has been + // started, SyncPortWhiteList applies the difference against PortWhiteList.Previous. + AddPortWhiteList(list client.PortWhiteList) error + SyncPortWhiteList(list client.PortWhiteList) error } func NewFirewallClient() (FilterClient, error) { diff --git a/agent/utils/firewall/client/firewalld.go b/agent/utils/firewall/client/firewalld.go index 4f1c1d46afea..0a4ddcaaf475 100644 --- a/agent/utils/firewall/client/firewalld.go +++ b/agent/utils/firewall/client/firewalld.go @@ -168,6 +168,14 @@ func (f *Firewall) ApplyAddressUnit(unit AddressUnit, operation string) error { return f.RichRules(unit.Apply, operation) } +func (f *Firewall) AddPortWhiteList(list PortWhiteList) error { + return addNativePortWhiteList(f, list) +} + +func (f *Firewall) SyncPortWhiteList(list PortWhiteList) error { + return syncNativePortWhiteList(f, list) +} + func buildFirewalldPortArgs(port FireInfo, operation string) []string { return []string{"--zone=public", "--" + operation + "-port=" + port.Port + "/" + port.Protocol, "--permanent"} } diff --git a/agent/utils/firewall/client/iptables.go b/agent/utils/firewall/client/iptables.go index b8a8cdfbf262..8679bd92dbb9 100644 --- a/agent/utils/firewall/client/iptables.go +++ b/agent/utils/firewall/client/iptables.go @@ -201,6 +201,21 @@ func (i *Iptables) ApplyAddressUnit(unit AddressUnit, operation string) error { return i.RichRules(apply, operation) } +func (i *Iptables) AddPortWhiteList(list PortWhiteList) error { + if isInit, _ := iptables.LoadInitStatus("iptables", "base"); !isInit { + return nil + } + list.Previous = nil + return SyncIptablesPortWhiteList(list, true) +} + +func (i *Iptables) SyncPortWhiteList(list PortWhiteList) error { + if isInit, _ := iptables.LoadInitStatus("iptables", "base"); !isInit { + return nil + } + return SyncIptablesPortWhiteList(list, true) +} + func buildIptablesPortRuleArgs(port FireInfo) ([]string, error) { portSpec, err := normalizePortSpec(port.Port) if err != nil { diff --git a/agent/utils/firewall/client/ufw.go b/agent/utils/firewall/client/ufw.go index f2be261dd895..d04107a5f0d0 100644 --- a/agent/utils/firewall/client/ufw.go +++ b/agent/utils/firewall/client/ufw.go @@ -193,6 +193,14 @@ func (f *Ufw) ApplyAddressUnit(unit AddressUnit, operation string) error { return f.RichRules(unit.Apply, operation) } +func (f *Ufw) AddPortWhiteList(list PortWhiteList) error { + return addNativePortWhiteList(f, list) +} + +func (f *Ufw) SyncPortWhiteList(list PortWhiteList) error { + return syncNativePortWhiteList(f, list) +} + func normalizeUfwStrategy(strategy string) (string, error) { switch strategy { case "accept": diff --git a/agent/utils/firewall/client/whitelist.go b/agent/utils/firewall/client/whitelist.go new file mode 100644 index 000000000000..b42d0e143d52 --- /dev/null +++ b/agent/utils/firewall/client/whitelist.go @@ -0,0 +1,209 @@ +package client + +import ( + "github.com/1Panel-dev/1Panel/agent/utils/firewall/client/iptables" +) + +// PortWhiteListEntry is one always open port of the 1Panel port whitelist. +type PortWhiteListEntry struct { + Port string + Protocol string +} + +// PortWhiteList is the whitelist state a provider needs in order to sync itself. +// Required entries (1Panel and SSH ports) are never removed, Previous is the +// configured list before the change and stays empty when the whole list is +// re-applied instead of diffed. +type PortWhiteList struct { + Configured []PortWhiteListEntry + Required []PortWhiteListEntry + Previous []PortWhiteListEntry +} + +func (e PortWhiteListEntry) key() string { + return e.Port + "/" + e.Protocol +} + +func (e PortWhiteListEntry) rule() FireInfo { + return FireInfo{Port: e.Port, Protocol: e.Protocol, Strategy: "accept"} +} + +func (l PortWhiteList) desired() []PortWhiteListEntry { + return normalizePortWhiteList(append(append([]PortWhiteListEntry{}, l.Configured...), l.Required...)) +} + +func (l PortWhiteList) previous() []PortWhiteListEntry { + return normalizePortWhiteList(append(append([]PortWhiteListEntry{}, l.Previous...), l.Required...)) +} + +func normalizePortWhiteList(entries []PortWhiteListEntry) []PortWhiteListEntry { + ports := make([]PortWhiteListEntry, 0, len(entries)) + exists := make(map[string]struct{}) + for _, item := range entries { + if item.Port == "" { + continue + } + if _, ok := exists[item.key()]; ok { + continue + } + exists[item.key()] = struct{}{} + ports = append(ports, item) + } + return ports +} + +func portWhiteListKeys(entries []PortWhiteListEntry) map[string]struct{} { + keys := make(map[string]struct{}) + for _, item := range entries { + keys[item.key()] = struct{}{} + } + return keys +} + +// nativePortWriter is the surface the ufw/firewalld whitelist needs; both write +// the whitelist through the same native port command they use for user rules. +type nativePortWriter interface { + Status() (bool, error) + Reload() error + Port(port FireInfo, operation string) error +} + +func addNativePortWhiteList(client nativePortWriter, list PortWhiteList) error { + for _, item := range list.desired() { + if err := client.Port(item.rule(), "add"); err != nil { + return err + } + } + return client.Reload() +} + +func syncNativePortWhiteList(client nativePortWriter, list PortWhiteList) error { + isActive, _ := client.Status() + if !isActive { + return nil + } + desired, previous := list.desired(), list.previous() + desiredKeys, previousKeys := portWhiteListKeys(desired), portWhiteListKeys(previous) + for _, item := range previous { + if _, ok := desiredKeys[item.key()]; ok { + continue + } + if err := client.Port(item.rule(), "remove"); err != nil { + return err + } + } + for _, item := range desired { + if _, ok := previousKeys[item.key()]; ok { + continue + } + if err := client.Port(item.rule(), "add"); err != nil { + return err + } + } + return client.Reload() +} + +// SyncIptablesPortWhiteList writes the whitelist into the managed chains: required +// ports into 1PANEL_BASIC_BEFORE, configured ports into 1PANEL_BASIC. withSave is +// false while the chains are still being built and the caller persists them itself. +func SyncIptablesPortWhiteList(list PortWhiteList, withSave bool) error { + if err := applyRequiredIptablesPortWhiteList(normalizePortWhiteList(list.Required), withSave); err != nil { + return err + } + return applyIptablesPortWhiteList(list.Configured, list.Previous, withSave) +} + +func applyRequiredIptablesPortWhiteList(entries []PortWhiteListEntry, withSave bool) error { + if err := cleanRequiredIptablesPortWhiteList(entries); err != nil { + return err + } + for _, item := range entries { + if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasicBefore, "-p", item.Protocol, "-m", item.Protocol, "--dport", item.Port, "-j", "ACCEPT"); err != nil { + return err + } + } + if !withSave { + return nil + } + if err := iptables.SaveRulesToFile(iptables.FilterTab, iptables.Chain1PanelBasicBefore, iptables.BasicBeforeFileName); err != nil { + return err + } + return iptables.SaveRulesToFile(iptables.FilterTab, iptables.Chain1PanelBasicAfter, iptables.BasicAfterFileName) +} + +func applyIptablesPortWhiteList(entries, previous []PortWhiteListEntry, withSave bool) error { + if err := cleanIptablesPortWhiteList(entries, previous); err != nil { + return err + } + for _, item := range entries { + if err := iptables.AddRule(iptables.FilterTab, iptables.Chain1PanelBasic, "-p", item.Protocol, "-m", item.Protocol, "--dport", item.Port, "-j", "ACCEPT"); err != nil { + return err + } + } + if !withSave { + return nil + } + return iptables.SaveRulesToFile(iptables.FilterTab, iptables.Chain1PanelBasic, iptables.BasicFileName) +} + +func cleanRequiredIptablesPortWhiteList(entries []PortWhiteListEntry) error { + tcpWhitelist := make(map[string]struct{}) + udpWhitelist := make(map[string]struct{}) + for _, item := range entries { + if item.Protocol == "udp" { + udpWhitelist[item.Port] = struct{}{} + continue + } + tcpWhitelist[item.Port] = struct{}{} + } + + if err := cleanExtraIptablesPortRules(iptables.Chain1PanelBasicBefore, "tcp", tcpWhitelist); err != nil { + return err + } + if err := cleanExtraIptablesPortRules(iptables.Chain1PanelBasicBefore, "udp", udpWhitelist); err != nil { + return err + } + return cleanExtraIptablesPortRules(iptables.Chain1PanelBasicAfter, "udp", map[string]struct{}{}) +} + +func cleanIptablesPortWhiteList(entries, previous []PortWhiteListEntry) error { + if len(previous) == 0 { + return nil + } + keys := portWhiteListKeys(entries) + for _, item := range previous { + if _, ok := keys[item.key()]; ok { + continue + } + if !iptables.CheckRuleExist(iptables.FilterTab, iptables.Chain1PanelBasic, "-p", item.Protocol, "--dport", item.Port, "-j", "ACCEPT") { + continue + } + if err := iptables.DeleteRule(iptables.FilterTab, iptables.Chain1PanelBasic, "-p", item.Protocol, "--dport", item.Port, "-j", "ACCEPT"); err != nil { + return err + } + } + return nil +} + +func cleanExtraIptablesPortRules(chain, protocol string, whitelist map[string]struct{}) error { + rules, err := iptables.ReadFilterRulesByChain(chain) + if err != nil { + return err + } + kept := make(map[string]struct{}) + for _, rule := range rules { + if rule.Strategy != "accept" || rule.Protocol != protocol || rule.DstPort == "" || rule.SrcIP != "" || rule.DstIP != "" || rule.SrcPort != "" { + continue + } + if _, ok := whitelist[rule.DstPort]; ok { + if _, seen := kept[rule.DstPort]; !seen { + kept[rule.DstPort] = struct{}{} + continue + } + } + if err := iptables.DeleteRule(iptables.FilterTab, chain, "-p", protocol, "-m", protocol, "--dport", rule.DstPort, "-j", "ACCEPT"); err != nil { + return err + } + } + return nil +} From 535f56d124dc68868bc2b5fe57d5213d275916ab Mon Sep 17 00:00:00 2001 From: HynoR <20227709+HynoR@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:54:08 +0800 Subject: [PATCH 4/6] test(firewall): pin provider command sequences Two layers, so that a future change to one provider cannot quietly change another. Expansion layer: a table of rules - single port, range, colon range, comma list, tcp/udp, empty/single/CIDR/multiple/trailing-comma/Anywhere sources, drop, explicit chain - run through all three real ExpandPortRule and ExpandAddressRule implementations. Expansion runs no command, so the real clients are used directly. The expected output records what is applied, what is recorded, which chain owns it and whether the unit takes the rich rule path, which is the whole per-provider decision in one place. Flow layer: the service loop driven by the real expansion with a recorded apply step and a recorded firewall record repository, so the order of native operations and the interleaving of record writes are both pinned. Alongside these, the command builders extracted in the previous commits get argv assertions, including that no ufw or firewalld command ever mentions a 1PANEL chain. One recorded difference against dev-v2: ufw returned before Reload for port rules while the shared flow always reloads. Ufw.Reload is a no-op, so the trailing reload changes nothing on a real host; the flow test comment says so. --- agent/app/service/firewall_contract_test.go | 123 +++++++ agent/app/service/firewall_rule_flow_test.go | 313 ++++++++++++++++++ .../client/external_cmd_contract_test.go | 157 +++++++++ .../firewall/client/iptables_contract_test.go | 132 ++++++++ .../utils/firewall/client/rule_expand_test.go | 296 +++++++++++++++++ 5 files changed, 1021 insertions(+) create mode 100644 agent/app/service/firewall_contract_test.go create mode 100644 agent/app/service/firewall_rule_flow_test.go create mode 100644 agent/utils/firewall/client/external_cmd_contract_test.go create mode 100644 agent/utils/firewall/client/iptables_contract_test.go create mode 100644 agent/utils/firewall/client/rule_expand_test.go diff --git a/agent/app/service/firewall_contract_test.go b/agent/app/service/firewall_contract_test.go new file mode 100644 index 000000000000..66c565e8f911 --- /dev/null +++ b/agent/app/service/firewall_contract_test.go @@ -0,0 +1,123 @@ +package service + +import ( + "reflect" + "testing" + + "github.com/1Panel-dev/1Panel/agent/app/dto" + "github.com/1Panel-dev/1Panel/agent/constant" + fireClient "github.com/1Panel-dev/1Panel/agent/utils/firewall/client" +) + +func TestParseFirewallPortWhiteListContract(t *testing.T) { + tests := []struct { + name string + value string + want []fireClient.PortWhiteListEntry + wantErr bool + }{ + { + name: "default value", + value: constant.FirewallPortWhiteListValue, + want: []fireClient.PortWhiteListEntry{ + {Port: "80", Protocol: "tcp"}, + {Port: "443", Protocol: "tcp"}, + {Port: "443", Protocol: "udp"}, + }, + }, + { + name: "dedup", + value: "80/tcp,80/tcp,443/tcp", + want: []fireClient.PortWhiteListEntry{ + {Port: "80", Protocol: "tcp"}, + {Port: "443", Protocol: "tcp"}, + }, + }, + { + name: "default protocol tcp", + value: "8080", + want: []fireClient.PortWhiteListEntry{ + {Port: "8080", Protocol: "tcp"}, + }, + }, + { + name: "invalid protocol", + value: "80/icmp", + wantErr: true, + }, + { + name: "invalid port", + value: "99999/tcp", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseFirewallPortWhiteList(tt.value) + if tt.wantErr { + if err == nil { + t.Fatal("expected error") + } + return + } + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("got %#v want %#v", got, tt.want) + } + }) + } +} + +func TestCheckPortUsedContract(t *testing.T) { + apps := []portOfApp{ + {AppName: "wordpress", HttpPort: "8080", HttpsPort: "8443"}, + {AppName: "1panel", HttpPort: "9999"}, + } + if got := checkPortUsed("8080", "tcp", apps); got != "wordpress" { + t.Fatalf("got %q want wordpress", got) + } + if got := checkPortUsed("9999", "tcp", apps); got != "1panel" { + t.Fatalf("got %q want 1panel", got) + } +} + +func TestFirewallAPIDtoSnapshot(t *testing.T) { + assertStructFields(t, dto.PortRuleOperate{}, []string{ + "ID", "Operation", "Chain", "Address", "Port", "Protocol", "Strategy", "Description", + }) + assertStructFields(t, dto.AddrRuleOperate{}, []string{ + "ID", "Operation", "Address", "Strategy", "Description", + }) + assertStructFields(t, dto.ForwardRuleOperate{}, []string{ + "ForceDelete", "Rules", + }) + assertStructFields(t, dto.FirewallBaseInfo{}, []string{ + "Name", "IsExist", "IsActive", "IsInit", "IsBind", "Version", "PingStatus", + }) + assertStructFields(t, dto.FirewallOperation{}, []string{ + "Operation", "WithDockerRestart", + }) + assertStructFields(t, dto.BatchRuleOperate{}, []string{ + "Type", "Rules", + }) + assertStructFields(t, dto.IptablesRuleOp{}, []string{ + "Operation", "ID", "Chain", "Protocol", "SrcIP", "SrcPort", "DstIP", "DstPort", "Strategy", "Description", + }) +} + +func assertStructFields(t *testing.T, sample any, want []string) { + t.Helper() + typ := reflect.TypeOf(sample) + if typ.Kind() != reflect.Struct { + t.Fatalf("expected struct, got %s", typ.Kind()) + } + got := make([]string, 0, typ.NumField()) + for i := 0; i < typ.NumField(); i++ { + got = append(got, typ.Field(i).Name) + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("%s fields changed\ngot %#v\nwant %#v", typ.Name(), got, want) + } +} diff --git a/agent/app/service/firewall_rule_flow_test.go b/agent/app/service/firewall_rule_flow_test.go new file mode 100644 index 000000000000..b65a166fe92b --- /dev/null +++ b/agent/app/service/firewall_rule_flow_test.go @@ -0,0 +1,313 @@ +package service + +import ( + "errors" + "fmt" + "reflect" + "testing" + + "github.com/1Panel-dev/1Panel/agent/app/dto" + "github.com/1Panel-dev/1Panel/agent/app/model" + "github.com/1Panel-dev/1Panel/agent/app/repo" + "github.com/1Panel-dev/1Panel/agent/utils/firewall" + fireClient "github.com/1Panel-dev/1Panel/agent/utils/firewall/client" + "github.com/1Panel-dev/1Panel/agent/utils/firewall/client/iptables" +) + +// The rule flow is provider agnostic, so these sequences pair the real client +// expansion with a recorded apply step and show how record writes interleave. +// One intentional difference against dev-v2: ufw returned before Reload for port +// rules, the shared flow always reloads. Ufw.Reload is a no-op, so the recorded +// trailing "reload" has no effect on a real host. + +type recordingFilterClient struct { + firewall.FilterClient + events *[]string + err error +} + +func (r *recordingFilterClient) ApplyPortUnit(unit fireClient.PortUnit, operation string) error { + *r.events = append(*r.events, fmt.Sprintf("apply-port %s apply(port=%s proto=%s addr=%s) record(port=%s proto=%s addr=%s) chain=%s", + operation, + unit.Apply.Port, unit.Apply.Protocol, unit.Apply.Address, + unit.Record.Port, unit.Record.Protocol, unit.Record.Address, + unit.Chain)) + return r.err +} + +func (r *recordingFilterClient) ApplyAddressUnit(unit fireClient.AddressUnit, operation string) error { + *r.events = append(*r.events, fmt.Sprintf("apply-address %s addr=%s strategy=%s chain=%s", + operation, unit.Apply.Address, unit.Apply.Strategy, unit.Chain)) + return r.err +} + +func (r *recordingFilterClient) Port(port fireClient.FireInfo, operation string) error { + *r.events = append(*r.events, fmt.Sprintf("port %s port=%s proto=%s strategy=%s", operation, port.Port, port.Protocol, port.Strategy)) + return r.err +} + +func (r *recordingFilterClient) Reload() error { + *r.events = append(*r.events, "reload") + return r.err +} + +type recordingHostRepo struct { + repo.IHostRepo + events *[]string +} + +func (r *recordingHostRepo) SaveFirewallRecord(record *model.Firewall) error { + *r.events = append(*r.events, fmt.Sprintf("save-record %s chain=%s port=%s proto=%s addr=%s strategy=%s description=%s", + record.Type, record.Chain, record.DstPort, record.Protocol, record.SrcIP, record.Strategy, record.Description)) + return nil +} + +func (r *recordingHostRepo) DeleteFirewallRecordByID(id uint) error { + *r.events = append(*r.events, fmt.Sprintf("delete-record id=%d", id)) + return nil +} + +func newRecordingFlow(t *testing.T, name string) (*recordingFilterClient, *[]string) { + t.Helper() + var client firewall.FilterClient + var err error + switch name { + case "ufw": + client, err = fireClient.NewUfw() + case "firewalld": + client, err = fireClient.NewFirewalld() + case "iptables": + client, err = fireClient.NewIptables() + default: + t.Fatalf("unknown provider %s", name) + } + if err != nil { + t.Fatal(err) + } + events := make([]string, 0) + original := hostRepo + hostRepo = &recordingHostRepo{events: &events} + t.Cleanup(func() { hostRepo = original }) + return &recordingFilterClient{FilterClient: client, events: &events}, &events +} + +func TestOperatePortRuleFlowGoldenSequence(t *testing.T) { + tests := []struct { + name string + provider string + req dto.PortRuleOperate + want []string + }{ + { + name: "ufw range applies colon syntax and records dash syntax", + provider: "ufw", + req: dto.PortRuleOperate{Operation: "add", Port: "8000-8010", Protocol: "tcp", Strategy: "accept", Description: "web"}, + want: []string{ + "apply-port add apply(port=8000:8010 proto=tcp addr=) record(port=8000-8010 proto=tcp addr=Anywhere) chain=", + "save-record port chain= port=8000-8010 proto=tcp addr=Anywhere strategy=accept description=web", + "reload", + }, + }, + { + name: "ufw dual protocol single port stays one rule", + provider: "ufw", + req: dto.PortRuleOperate{Operation: "add", Port: "53", Protocol: "tcp/udp", Strategy: "accept", Description: "dns"}, + want: []string{ + "apply-port add apply(port=53 proto= addr=) record(port=53 proto=tcp/udp addr=Anywhere) chain=", + "save-record port chain= port=53 proto=tcp/udp addr=Anywhere strategy=accept description=dns", + "reload", + }, + }, + { + name: "firewalld expands protocols then ports then sources", + provider: "firewalld", + req: dto.PortRuleOperate{Operation: "add", Port: "80,443", Protocol: "tcp/udp", Address: "1.1.1.1,2.2.2.2", Strategy: "accept", Description: "web"}, + want: []string{ + "apply-port add apply(port=80 proto=tcp addr=1.1.1.1) record(port=80 proto=tcp addr=1.1.1.1) chain=", + "save-record port chain= port=80 proto=tcp addr=1.1.1.1 strategy=accept description=web", + "apply-port add apply(port=80 proto=tcp addr=2.2.2.2) record(port=80 proto=tcp addr=2.2.2.2) chain=", + "save-record port chain= port=80 proto=tcp addr=2.2.2.2 strategy=accept description=web", + "apply-port add apply(port=443 proto=tcp addr=1.1.1.1) record(port=443 proto=tcp addr=1.1.1.1) chain=", + "save-record port chain= port=443 proto=tcp addr=1.1.1.1 strategy=accept description=web", + "apply-port add apply(port=443 proto=tcp addr=2.2.2.2) record(port=443 proto=tcp addr=2.2.2.2) chain=", + "save-record port chain= port=443 proto=tcp addr=2.2.2.2 strategy=accept description=web", + "apply-port add apply(port=80 proto=udp addr=1.1.1.1) record(port=80 proto=udp addr=1.1.1.1) chain=", + "save-record port chain= port=80 proto=udp addr=1.1.1.1 strategy=accept description=web", + "apply-port add apply(port=80 proto=udp addr=2.2.2.2) record(port=80 proto=udp addr=2.2.2.2) chain=", + "save-record port chain= port=80 proto=udp addr=2.2.2.2 strategy=accept description=web", + "apply-port add apply(port=443 proto=udp addr=1.1.1.1) record(port=443 proto=udp addr=1.1.1.1) chain=", + "save-record port chain= port=443 proto=udp addr=1.1.1.1 strategy=accept description=web", + "apply-port add apply(port=443 proto=udp addr=2.2.2.2) record(port=443 proto=udp addr=2.2.2.2) chain=", + "save-record port chain= port=443 proto=udp addr=2.2.2.2 strategy=accept description=web", + "reload", + }, + }, + { + name: "iptables records the owning chain", + provider: "iptables", + req: dto.PortRuleOperate{Operation: "add", Port: "8000-8010", Protocol: "tcp", Strategy: "accept", Description: "web"}, + want: []string{ + "apply-port add apply(port=8000-8010 proto=tcp addr=) record(port=8000-8010 proto=tcp addr=) chain=1PANEL_BASIC", + "save-record port chain=1PANEL_BASIC port=8000-8010 proto=tcp addr= strategy=accept description=web", + "reload", + }, + }, + { + name: "iptables keeps an explicit chain", + provider: "iptables", + req: dto.PortRuleOperate{Operation: "add", Chain: iptables.Chain1PanelBasicBefore, Port: "80", Protocol: "tcp", Strategy: "accept", Description: "panel"}, + want: []string{ + "apply-port add apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC_BEFORE", + "save-record port chain=1PANEL_BASIC_BEFORE port=80 proto=tcp addr= strategy=accept description=panel", + "reload", + }, + }, + { + name: "a rule without description writes no record", + provider: "iptables", + req: dto.PortRuleOperate{Operation: "add", Port: "80", Protocol: "tcp", Strategy: "accept"}, + want: []string{ + "apply-port add apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC", + "reload", + }, + }, + { + name: "removing a rule deletes its record once per unit", + provider: "firewalld", + req: dto.PortRuleOperate{ID: 7, Operation: "remove", Port: "80,443", Protocol: "tcp", Strategy: "accept"}, + want: []string{ + "apply-port remove apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=", + "delete-record id=7", + "apply-port remove apply(port=443 proto=tcp addr=) record(port=443 proto=tcp addr=) chain=", + "delete-record id=7", + "reload", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, events := newRecordingFlow(t, tt.provider) + if err := (&FirewallService{}).operatePortRuleWithClient(client, tt.req, true); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(*events, tt.want) { + t.Fatalf("flow changed\ngot %#v\nwant %#v", *events, tt.want) + } + }) + } +} + +func TestOperateAddressRuleFlowGoldenSequence(t *testing.T) { + tests := []struct { + name string + provider string + req dto.AddrRuleOperate + want []string + }{ + { + name: "external providers own no chain", + provider: "ufw", + req: dto.AddrRuleOperate{Operation: "add", Address: "1.1.1.1,2.2.2.2", Strategy: "drop", Description: "block"}, + want: []string{ + "apply-address add addr=1.1.1.1 strategy=drop chain=", + "save-record address chain= port= proto= addr=1.1.1.1 strategy=drop description=block", + "apply-address add addr=2.2.2.2 strategy=drop chain=", + "save-record address chain= port= proto= addr=2.2.2.2 strategy=drop description=block", + "reload", + }, + }, + { + name: "iptables records the basic chain", + provider: "iptables", + req: dto.AddrRuleOperate{Operation: "add", Address: "10.0.0.1", Strategy: "drop", Description: "block"}, + want: []string{ + "apply-address add addr=10.0.0.1 strategy=drop chain=1PANEL_BASIC", + "save-record address chain=1PANEL_BASIC port= proto= addr=10.0.0.1 strategy=drop description=block", + "reload", + }, + }, + { + name: "empty entries are dropped", + provider: "firewalld", + req: dto.AddrRuleOperate{ID: 3, Operation: "remove", Address: "1.1.1.1,,2.2.2.2", Strategy: "drop"}, + want: []string{ + "apply-address remove addr=1.1.1.1 strategy=drop chain=", + "delete-record id=3", + "apply-address remove addr=2.2.2.2 strategy=drop chain=", + "delete-record id=3", + "reload", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, events := newRecordingFlow(t, tt.provider) + if err := (&FirewallService{}).operateAddressRuleWithClient(client, tt.req, true); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(*events, tt.want) { + t.Fatalf("flow changed\ngot %#v\nwant %#v", *events, tt.want) + } + }) + } +} + +func TestOperateFirewallPortsGoldenSequence(t *testing.T) { + want := []string{ + "port add port=2222 proto=tcp strategy=accept", + "port remove port=22 proto=tcp strategy=accept", + "reload", + } + for _, name := range []string{"ufw", "firewalld", "iptables"} { + t.Run(name, func(t *testing.T) { + client, events := newRecordingFlow(t, name) + if err := operateFirewallPorts(client, []int{22}, []int{2222}); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(*events, want) { + t.Fatalf("got %#v want %#v", *events, want) + } + }) + } +} + +func TestRuleFlowReturnsClientErrorWithoutFallback(t *testing.T) { + nativeErr := errors.New("native firewall failure") + client, events := newRecordingFlow(t, "ufw") + client.err = nativeErr + err := (&FirewallService{}).operatePortRuleWithClient(client, dto.PortRuleOperate{ + Operation: "add", + Port: "80", + Protocol: "tcp", + Strategy: "accept", + Description: "web", + }, true) + if !errors.Is(err, nativeErr) { + t.Fatalf("flow must return the client error, got %v", err) + } + want := []string{"apply-port add apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=Anywhere) chain="} + if !reflect.DeepEqual(*events, want) { + t.Fatalf("a failed apply must not write a record: %#v", *events) + } +} + +func TestLoadInitStatusExternalParity(t *testing.T) { + for _, tt := range []struct { + name string + tab string + }{ + {name: "ufw", tab: "base"}, + {name: "ufw", tab: "port"}, + {name: "firewalld", tab: "base"}, + {name: "firewalld", tab: "advance"}, + } { + t.Run(tt.name+"/"+tt.tab, func(t *testing.T) { + isInit, isBind := iptables.LoadInitStatus(tt.name, tt.tab) + if !isInit || !isBind { + t.Fatalf("expected dev-v2 true,true, got %v,%v", isInit, isBind) + } + }) + } +} diff --git a/agent/utils/firewall/client/external_cmd_contract_test.go b/agent/utils/firewall/client/external_cmd_contract_test.go new file mode 100644 index 000000000000..c5f4655df3b8 --- /dev/null +++ b/agent/utils/firewall/client/external_cmd_contract_test.go @@ -0,0 +1,157 @@ +package client + +import ( + "strings" + "testing" +) + +// UFW/firewalld filter operations remain thin native proxies and never emit managed filter chains. + +func TestUfwPortCommandArgsContract(t *testing.T) { + tests := []struct { + name string + port FireInfo + operation string + want []string + }{ + { + name: "add accept tcp", + port: FireInfo{Port: "80", Protocol: "tcp", Strategy: "accept"}, + operation: "add", + want: []string{"allow", "80/tcp"}, + }, + { + name: "remove drop udp", + port: FireInfo{Port: "53", Protocol: "udp", Strategy: "drop"}, + operation: "remove", + want: []string{"delete", "deny", "53/udp"}, + }, + { + name: "add without protocol", + port: FireInfo{Port: "443", Strategy: "accept"}, + operation: "add", + want: []string{"allow", "443"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := buildUfwPortArgs(tt.port, tt.operation) + if err != nil { + t.Fatal(err) + } + assertNoManagedChainToken(t, got) + assertStringSliceEqual(t, got, tt.want) + }) + } +} + +func TestUfwRichRuleCommandArgsContract(t *testing.T) { + rule := FireInfo{ + Address: "10.0.0.1", + Port: "22", + Protocol: "tcp", + Strategy: "accept", + } + var err error + rule.Strategy, err = normalizeUfwStrategy(rule.Strategy) + if err != nil { + t.Fatal(err) + } + got := buildUfwRichRuleArgs(rule, "add", 1) + want := []string{"insert", "1", "allow", "proto", "tcp", "from", "10.0.0.1", "to", "any", "port", "22"} + assertNoManagedChainToken(t, got) + assertStringSliceEqual(t, got, want) + + removeRule := FireInfo{ + Address: "10.0.0.1", + Protocol: "tcp", + Strategy: "drop", + } + removeRule.Strategy, err = normalizeUfwStrategy(removeRule.Strategy) + if err != nil { + t.Fatal(err) + } + removeArgs := buildUfwRichRuleArgs(removeRule, "remove", 1) + wantRemove := []string{"delete", "deny", "proto", "tcp", "from", "10.0.0.1"} + assertNoManagedChainToken(t, removeArgs) + assertStringSliceEqual(t, removeArgs, wantRemove) +} + +func TestFirewalldPortCommandArgsContract(t *testing.T) { + got := buildFirewalldPortArgs(FireInfo{Port: "80", Protocol: "tcp"}, "add") + want := []string{"--zone=public", "--add-port=80/tcp", "--permanent"} + assertNoManagedChainToken(t, got) + assertStringSliceEqual(t, got, want) +} + +func TestFirewalldRichRuleCommandContract(t *testing.T) { + ruleStr := buildFirewalldRichRuleString(FireInfo{ + Address: "1.2.3.4", + Port: "443", + Protocol: "tcp", + Strategy: "accept", + }) + if strings.Contains(ruleStr, "1PANEL_") { + t.Fatalf("firewalld rich rule must not contain managed chain: %s", ruleStr) + } + want := "rule family=ipv4 source address=1.2.3.4 port port=443 protocol=tcp accept" + if ruleStr != want { + t.Fatalf("got %q want %q", ruleStr, want) + } + + args := buildFirewalldRichRuleArgs(ruleStr, "add") + assertNoManagedChainToken(t, args) + assertStringSliceEqual(t, args, []string{"--zone=public", "--add-rich-rule", ruleStr, "--permanent"}) + + dualStack := buildFirewalldRichRuleStrings(FireInfo{Port: "53", Protocol: "udp", Strategy: "drop"}) + if len(dualStack) != 2 || + !strings.Contains(dualStack[0], "family=ipv4") || + !strings.Contains(dualStack[1], "family=ipv6") { + t.Fatalf("empty-source rich rule must preserve dev-v2 dual-stack commands: %#v", dualStack) + } +} + +func TestExternalFilterCommandsNeverEmitManagedChains(t *testing.T) { + samples := [][]string{ + mustUfwPortArgs(t, FireInfo{Port: "80", Protocol: "tcp", Strategy: "accept"}, "add"), + mustUfwPortArgs(t, FireInfo{Port: "443", Protocol: "tcp", Strategy: "drop"}, "remove"), + mustUfwRichRuleArgs(t, FireInfo{Address: "8.8.8.8", Port: "53", Protocol: "udp", Strategy: "accept"}, "add", 2), + buildFirewalldPortArgs(FireInfo{Port: "22", Protocol: "tcp"}, "remove"), + buildFirewalldRichRuleArgs(buildFirewalldRichRuleString(FireInfo{Address: "2001:db8::1", Strategy: "drop"}), "add"), + } + for i, args := range samples { + assertNoManagedChainToken(t, args) + joined := strings.Join(args, " ") + if strings.Contains(joined, "1PANEL_") { + t.Fatalf("sample %d contains managed chain token: %s", i, joined) + } + } +} + +func mustUfwPortArgs(t *testing.T, port FireInfo, operation string) []string { + t.Helper() + args, err := buildUfwPortArgs(port, operation) + if err != nil { + t.Fatal(err) + } + return args +} + +func mustUfwRichRuleArgs(t *testing.T, rule FireInfo, operation string, insertNum int) []string { + t.Helper() + var err error + rule.Strategy, err = normalizeUfwStrategy(rule.Strategy) + if err != nil { + t.Fatal(err) + } + return buildUfwRichRuleArgs(rule, operation, insertNum) +} + +func assertNoManagedChainToken(t *testing.T, args []string) { + t.Helper() + for _, arg := range args { + if strings.Contains(arg, "1PANEL_") { + t.Fatalf("external command args must not contain managed chain token: %#v", args) + } + } +} diff --git a/agent/utils/firewall/client/iptables_contract_test.go b/agent/utils/firewall/client/iptables_contract_test.go new file mode 100644 index 000000000000..138d48c22229 --- /dev/null +++ b/agent/utils/firewall/client/iptables_contract_test.go @@ -0,0 +1,132 @@ +package client + +import ( + "strings" + "testing" + + "github.com/1Panel-dev/1Panel/agent/utils/firewall/client/iptables" +) + +func TestNormalizePortSpec(t *testing.T) { + tests := []struct { + name string + input string + want string + wantErr bool + }{ + {name: "single", input: "80", want: "80"}, + {name: "range dash", input: "8000-8010", want: "8000:8010"}, + {name: "range colon", input: "8000:8010", want: "8000:8010"}, + {name: "trim", input: " 443 ", want: "443"}, + {name: "empty", input: "", wantErr: true}, + {name: "invalid", input: "abc", wantErr: true}, + {name: "bad range order", input: "90-80", wantErr: true}, + {name: "out of range", input: "70000", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := normalizePortSpec(tt.input) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error for %q", tt.input) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Fatalf("normalizePortSpec(%q)=%q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestParsePort(t *testing.T) { + if _, err := parsePort("0"); err == nil { + t.Fatal("expected error for port 0") + } + got, err := parsePort("22") + if err != nil { + t.Fatal(err) + } + if got != 22 { + t.Fatalf("got %d", got) + } +} + +func TestIptablesPortRuleArgsContract(t *testing.T) { + tests := []struct { + name string + port FireInfo + want []string + }{ + { + name: "accept tcp default chain", + port: FireInfo{Port: "80", Protocol: "tcp", Strategy: "accept"}, + want: []string{"-p", "tcp", "--dport", "80", "-j", "ACCEPT"}, + }, + { + name: "drop udp", + port: FireInfo{Port: "53", Protocol: "udp", Strategy: "drop"}, + want: []string{"-p", "udp", "--dport", "53", "-j", "DROP"}, + }, + { + name: "range", + port: FireInfo{Port: "8000-8010", Protocol: "tcp", Strategy: "accept"}, + want: []string{"-p", "tcp", "--dport", "8000:8010", "-j", "ACCEPT"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := buildIptablesPortRuleArgs(tt.port) + if err != nil { + t.Fatal(err) + } + assertStringSliceEqual(t, got, tt.want) + }) + } +} + +func TestIptablesRichRuleArgsContract(t *testing.T) { + got, err := buildIptablesRichRuleArgs(FireInfo{ + Address: "1.2.3.4", + Port: "443", + Protocol: "tcp", + Strategy: "drop", + }) + if err != nil { + t.Fatal(err) + } + want := []string{"-s", "1.2.3.4", "-p", "tcp", "--dport", "443", "-j", "DROP"} + assertStringSliceEqual(t, got, want) +} + +func TestIptablesDefaultChainIsPanelOwned(t *testing.T) { + if iptables.Chain1PanelBasic != "1PANEL_BASIC" { + t.Fatalf("unexpected basic chain: %s", iptables.Chain1PanelBasic) + } + for _, chain := range []string{ + iptables.Chain1PanelBasicBefore, + iptables.Chain1PanelBasic, + iptables.Chain1PanelBasicAfter, + iptables.Chain1PanelInput, + iptables.Chain1PanelOutput, + } { + if !strings.HasPrefix(chain, "1PANEL_") { + t.Fatalf("legacy chain %q must be 1PANEL_ owned", chain) + } + } +} + +func assertStringSliceEqual(t *testing.T, got, want []string) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("len=%d want %d\ngot %#v\nwant %#v", len(got), len(want), got, want) + } + for i := range got { + if got[i] != want[i] { + t.Fatalf("index %d: got %q want %q\nfull got %#v\nfull want %#v", i, got[i], want[i], got, want) + } + } +} diff --git a/agent/utils/firewall/client/rule_expand_test.go b/agent/utils/firewall/client/rule_expand_test.go new file mode 100644 index 000000000000..ef306753eab9 --- /dev/null +++ b/agent/utils/firewall/client/rule_expand_test.go @@ -0,0 +1,296 @@ +package client + +import ( + "fmt" + "reflect" + "testing" + + "github.com/1Panel-dev/1Panel/agent/utils/firewall/client/iptables" +) + +// Expansion is the only place provider specific rule shapes are decided, so the +// expected sequences below are the dev-v2 command order written down literally. + +type portExpander struct { + name string + expand func(FireInfo) []PortUnit + rich func(FireInfo) bool +} + +func portExpanders(t *testing.T) []portExpander { + t.Helper() + ufw, err := NewUfw() + if err != nil { + t.Fatal(err) + } + firewalld, err := NewFirewalld() + if err != nil { + t.Fatal(err) + } + iptablesClient, err := NewIptables() + if err != nil { + t.Fatal(err) + } + return []portExpander{ + {name: "ufw", expand: ufw.ExpandPortRule, rich: ufwNeedsRichRule}, + {name: "firewalld", expand: firewalld.ExpandPortRule, rich: needsRichRule}, + {name: "iptables", expand: iptablesClient.ExpandPortRule, rich: needsRichRule}, + } +} + +func formatPortUnit(unit PortUnit, rich bool) string { + return fmt.Sprintf("apply(port=%s proto=%s addr=%s) record(port=%s proto=%s addr=%s) chain=%s rich=%v", + unit.Apply.Port, unit.Apply.Protocol, unit.Apply.Address, + unit.Record.Port, unit.Record.Protocol, unit.Record.Address, + unit.Chain, rich) +} + +func TestExpandPortRuleGoldenSequence(t *testing.T) { + tests := []struct { + name string + rule FireInfo + want map[string][]string + }{ + { + name: "single port", + rule: FireInfo{Port: "80", Protocol: "tcp", Strategy: "accept"}, + want: map[string][]string{ + "ufw": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=Anywhere) chain= rich=false"}, + "firewalld": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain= rich=false"}, + "iptables": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC rich=false"}, + }, + }, + { + name: "port range", + rule: FireInfo{Port: "8000-8010", Protocol: "tcp", Strategy: "accept"}, + want: map[string][]string{ + "ufw": {"apply(port=8000:8010 proto=tcp addr=) record(port=8000-8010 proto=tcp addr=Anywhere) chain= rich=false"}, + "firewalld": {"apply(port=8000-8010 proto=tcp addr=) record(port=8000-8010 proto=tcp addr=) chain= rich=false"}, + "iptables": {"apply(port=8000-8010 proto=tcp addr=) record(port=8000-8010 proto=tcp addr=) chain=1PANEL_BASIC rich=false"}, + }, + }, + { + name: "port range already colon separated", + rule: FireInfo{Port: "8000:8010", Protocol: "tcp", Strategy: "accept"}, + want: map[string][]string{ + "ufw": {"apply(port=8000:8010 proto=tcp addr=) record(port=8000:8010 proto=tcp addr=Anywhere) chain= rich=false"}, + "firewalld": {"apply(port=8000:8010 proto=tcp addr=) record(port=8000:8010 proto=tcp addr=) chain= rich=false"}, + "iptables": {"apply(port=8000:8010 proto=tcp addr=) record(port=8000:8010 proto=tcp addr=) chain=1PANEL_BASIC rich=false"}, + }, + }, + { + name: "comma separated ports", + rule: FireInfo{Port: "80,443", Protocol: "tcp", Strategy: "accept"}, + want: map[string][]string{ + "ufw": {"apply(port=80,443 proto=tcp addr=) record(port=80,443 proto=tcp addr=Anywhere) chain= rich=false"}, + "firewalld": { + "apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain= rich=false", + "apply(port=443 proto=tcp addr=) record(port=443 proto=tcp addr=) chain= rich=false", + }, + "iptables": { + "apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC rich=false", + "apply(port=443 proto=tcp addr=) record(port=443 proto=tcp addr=) chain=1PANEL_BASIC rich=false", + }, + }, + }, + { + name: "dual protocol single port", + rule: FireInfo{Port: "53", Protocol: "tcp/udp", Strategy: "accept"}, + want: map[string][]string{ + "ufw": {"apply(port=53 proto= addr=) record(port=53 proto=tcp/udp addr=Anywhere) chain= rich=false"}, + "firewalld": { + "apply(port=53 proto=tcp addr=) record(port=53 proto=tcp addr=) chain= rich=false", + "apply(port=53 proto=udp addr=) record(port=53 proto=udp addr=) chain= rich=false", + }, + "iptables": { + "apply(port=53 proto=tcp addr=) record(port=53 proto=tcp addr=) chain=1PANEL_BASIC rich=false", + "apply(port=53 proto=udp addr=) record(port=53 proto=udp addr=) chain=1PANEL_BASIC rich=false", + }, + }, + }, + { + name: "dual protocol comma separated ports", + rule: FireInfo{Port: "80,443", Protocol: "tcp/udp", Strategy: "accept"}, + want: map[string][]string{ + "ufw": { + "apply(port=80,443 proto=tcp addr=) record(port=80,443 proto=tcp addr=Anywhere) chain= rich=false", + "apply(port=80,443 proto=udp addr=) record(port=80,443 proto=udp addr=Anywhere) chain= rich=false", + }, + "firewalld": { + "apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain= rich=false", + "apply(port=443 proto=tcp addr=) record(port=443 proto=tcp addr=) chain= rich=false", + "apply(port=80 proto=udp addr=) record(port=80 proto=udp addr=) chain= rich=false", + "apply(port=443 proto=udp addr=) record(port=443 proto=udp addr=) chain= rich=false", + }, + "iptables": { + "apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC rich=false", + "apply(port=443 proto=tcp addr=) record(port=443 proto=tcp addr=) chain=1PANEL_BASIC rich=false", + "apply(port=80 proto=udp addr=) record(port=80 proto=udp addr=) chain=1PANEL_BASIC rich=false", + "apply(port=443 proto=udp addr=) record(port=443 proto=udp addr=) chain=1PANEL_BASIC rich=false", + }, + }, + }, + { + name: "single source", + rule: FireInfo{Port: "22", Protocol: "tcp", Strategy: "accept", Address: "10.0.0.1"}, + want: map[string][]string{ + "ufw": {"apply(port=22 proto=tcp addr=10.0.0.1) record(port=22 proto=tcp addr=10.0.0.1) chain= rich=true"}, + "firewalld": {"apply(port=22 proto=tcp addr=10.0.0.1) record(port=22 proto=tcp addr=10.0.0.1) chain= rich=true"}, + "iptables": {"apply(port=22 proto=tcp addr=10.0.0.1) record(port=22 proto=tcp addr=10.0.0.1) chain=1PANEL_BASIC rich=true"}, + }, + }, + { + name: "cidr source", + rule: FireInfo{Port: "22", Protocol: "tcp", Strategy: "accept", Address: "10.0.0.0/24"}, + want: map[string][]string{ + "ufw": {"apply(port=22 proto=tcp addr=10.0.0.0/24) record(port=22 proto=tcp addr=10.0.0.0/24) chain= rich=true"}, + "firewalld": {"apply(port=22 proto=tcp addr=10.0.0.0/24) record(port=22 proto=tcp addr=10.0.0.0/24) chain= rich=true"}, + "iptables": {"apply(port=22 proto=tcp addr=10.0.0.0/24) record(port=22 proto=tcp addr=10.0.0.0/24) chain=1PANEL_BASIC rich=true"}, + }, + }, + { + name: "multiple sources", + rule: FireInfo{Port: "22", Protocol: "tcp", Strategy: "accept", Address: "1.1.1.1,2.2.2.2"}, + want: map[string][]string{ + "ufw": { + "apply(port=22 proto=tcp addr=1.1.1.1) record(port=22 proto=tcp addr=1.1.1.1) chain= rich=true", + "apply(port=22 proto=tcp addr=2.2.2.2) record(port=22 proto=tcp addr=2.2.2.2) chain= rich=true", + }, + "firewalld": { + "apply(port=22 proto=tcp addr=1.1.1.1) record(port=22 proto=tcp addr=1.1.1.1) chain= rich=true", + "apply(port=22 proto=tcp addr=2.2.2.2) record(port=22 proto=tcp addr=2.2.2.2) chain= rich=true", + }, + "iptables": { + "apply(port=22 proto=tcp addr=1.1.1.1) record(port=22 proto=tcp addr=1.1.1.1) chain=1PANEL_BASIC rich=true", + "apply(port=22 proto=tcp addr=2.2.2.2) record(port=22 proto=tcp addr=2.2.2.2) chain=1PANEL_BASIC rich=true", + }, + }, + }, + { + name: "trailing comma source", + rule: FireInfo{Port: "22", Protocol: "tcp", Strategy: "accept", Address: "1.1.1.1,"}, + want: map[string][]string{ + "ufw": {"apply(port=22 proto=tcp addr=1.1.1.1) record(port=22 proto=tcp addr=1.1.1.1) chain= rich=true"}, + "firewalld": {"apply(port=22 proto=tcp addr=1.1.1.1) record(port=22 proto=tcp addr=1.1.1.1) chain= rich=true"}, + "iptables": {"apply(port=22 proto=tcp addr=1.1.1.1) record(port=22 proto=tcp addr=1.1.1.1) chain=1PANEL_BASIC rich=true"}, + }, + }, + { + name: "anywhere source", + rule: FireInfo{Port: "80", Protocol: "tcp", Strategy: "accept", Address: "Anywhere"}, + want: map[string][]string{ + "ufw": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=Anywhere) chain= rich=false"}, + "firewalld": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain= rich=false"}, + "iptables": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC rich=false"}, + }, + }, + { + name: "drop without source", + rule: FireInfo{Port: "3306", Protocol: "tcp", Strategy: "drop"}, + want: map[string][]string{ + "ufw": {"apply(port=3306 proto=tcp addr=) record(port=3306 proto=tcp addr=Anywhere) chain= rich=false"}, + "firewalld": {"apply(port=3306 proto=tcp addr=) record(port=3306 proto=tcp addr=) chain= rich=true"}, + "iptables": {"apply(port=3306 proto=tcp addr=) record(port=3306 proto=tcp addr=) chain=1PANEL_BASIC rich=true"}, + }, + }, + { + name: "explicit chain is preserved", + rule: FireInfo{Port: "80", Protocol: "tcp", Strategy: "accept", Chain: iptables.Chain1PanelBasicBefore}, + want: map[string][]string{ + "ufw": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=Anywhere) chain=1PANEL_BASIC_BEFORE rich=false"}, + "firewalld": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC_BEFORE rich=false"}, + "iptables": {"apply(port=80 proto=tcp addr=) record(port=80 proto=tcp addr=) chain=1PANEL_BASIC_BEFORE rich=false"}, + }, + }, + } + + for _, tt := range tests { + for _, expander := range portExpanders(t) { + t.Run(tt.name+"/"+expander.name, func(t *testing.T) { + units := expander.expand(tt.rule) + got := make([]string, 0, len(units)) + for _, unit := range units { + got = append(got, formatPortUnit(unit, expander.rich(unit.Apply))) + } + want := tt.want[expander.name] + if !reflect.DeepEqual(got, want) { + t.Fatalf("expansion changed\ngot %#v\nwant %#v", got, want) + } + }) + } + } +} + +func TestExpandAddressRuleGoldenSequence(t *testing.T) { + ufw, err := NewUfw() + if err != nil { + t.Fatal(err) + } + firewalld, err := NewFirewalld() + if err != nil { + t.Fatal(err) + } + iptablesClient, err := NewIptables() + if err != nil { + t.Fatal(err) + } + expanders := map[string]func(FireInfo) []AddressUnit{ + "ufw": ufw.ExpandAddressRule, + "firewalld": firewalld.ExpandAddressRule, + "iptables": iptablesClient.ExpandAddressRule, + } + chains := map[string]string{"ufw": "", "firewalld": "", "iptables": iptables.Chain1PanelBasic} + + tests := []struct { + name string + rule FireInfo + addresses []string + }{ + {name: "single source", rule: FireInfo{Address: "10.0.0.1", Strategy: "drop"}, addresses: []string{"10.0.0.1"}}, + {name: "cidr source", rule: FireInfo{Address: "10.0.0.0/24", Strategy: "drop"}, addresses: []string{"10.0.0.0/24"}}, + {name: "multiple sources", rule: FireInfo{Address: "1.1.1.1,2.2.2.2", Strategy: "accept"}, addresses: []string{"1.1.1.1", "2.2.2.2"}}, + {name: "empty entries dropped", rule: FireInfo{Address: "1.1.1.1,,2.2.2.2", Strategy: "accept"}, addresses: []string{"1.1.1.1", "2.2.2.2"}}, + {name: "trailing comma", rule: FireInfo{Address: "1.1.1.1,", Strategy: "accept"}, addresses: []string{"1.1.1.1"}}, + {name: "empty source expands to nothing", rule: FireInfo{Address: "", Strategy: "accept"}}, + // address rules are not normalized, unlike port rules + {name: "anywhere is kept verbatim", rule: FireInfo{Address: "Anywhere", Strategy: "drop"}, addresses: []string{"Anywhere"}}, + } + + for _, tt := range tests { + for name, expand := range expanders { + t.Run(tt.name+"/"+name, func(t *testing.T) { + units := expand(tt.rule) + if len(units) != len(tt.addresses) { + t.Fatalf("got %d units want %d: %#v", len(units), len(tt.addresses), units) + } + for i, unit := range units { + if unit.Apply.Address != tt.addresses[i] { + t.Fatalf("unit %d address %q want %q", i, unit.Apply.Address, tt.addresses[i]) + } + if unit.Apply.Strategy != tt.rule.Strategy { + t.Fatalf("unit %d strategy %q want %q", i, unit.Apply.Strategy, tt.rule.Strategy) + } + if unit.Chain != chains[name] { + t.Fatalf("unit %d chain %q want %q", i, unit.Chain, chains[name]) + } + } + }) + } + } +} + +func TestNormalizePortWhiteListContract(t *testing.T) { + got := normalizePortWhiteList([]PortWhiteListEntry{ + {Port: "22", Protocol: "tcp"}, + {Port: "22", Protocol: "tcp"}, + {Port: "", Protocol: "tcp"}, + {Port: "80", Protocol: "tcp"}, + }) + want := []PortWhiteListEntry{ + {Port: "22", Protocol: "tcp"}, + {Port: "80", Protocol: "tcp"}, + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("got %#v want %#v", got, want) + } +} From 75892452d35343bd8ae007853ae8d62ec11ef1d7 Mon Sep 17 00:00:00 2001 From: HynoR <20227709+HynoR@users.noreply.github.com> Date: Thu, 23 Jul 2026 14:17:25 +0800 Subject: [PATCH 5/6] refactor(firewall): simplify forwarding rule search structure Removed the ForwardRuleSearch struct and replaced it with a more generic RuleSearch in the forwarding service. Updated related search functions and tests to accommodate this change, streamlining the search process for forwarding rules. Additionally, refactored the handling of protocol loading in iptables and improved the readability of the code by renaming functions for clarity. --- agent/app/api/v2/firewall.go | 7 +-- agent/app/dto/forwarding.go | 7 --- agent/app/service/forwarding.go | 15 +++---- agent/app/service/forwarding_contract_test.go | 4 +- agent/utils/firewall/client/iptables.go | 5 +-- .../utils/firewall/client/iptables/filter.go | 13 +++--- agent/utils/firewall/client/rule.go | 5 ++- agent/utils/firewall/forwarding/legacy.go | 44 ++----------------- 8 files changed, 24 insertions(+), 76 deletions(-) diff --git a/agent/app/api/v2/firewall.go b/agent/app/api/v2/firewall.go index 814366510e60..72cc0c36ce7a 100644 --- a/agent/app/api/v2/firewall.go +++ b/agent/app/api/v2/firewall.go @@ -57,12 +57,7 @@ func (b *BaseApi) SearchFirewallRule(c *gin.Context) { err error ) if req.Type == "forward" { - total, list, err = forwardingService.SearchWithPage(dto.ForwardRuleSearch{ - PageInfo: req.PageInfo, - Info: req.Info, - Status: req.Status, - Strategy: req.Strategy, - }) + total, list, err = forwardingService.SearchWithPage(req) } else { total, list, err = firewallService.SearchWithPage(req) } diff --git a/agent/app/dto/forwarding.go b/agent/app/dto/forwarding.go index 4bbe92563004..e294536fa2e9 100644 --- a/agent/app/dto/forwarding.go +++ b/agent/app/dto/forwarding.go @@ -1,12 +1,5 @@ package dto -type ForwardRuleSearch struct { - PageInfo - Info string `json:"info"` - Status string `json:"status"` - Strategy string `json:"strategy"` -} - // ForwardRule preserves the existing firewall search response shape while // keeping forwarding data separate from the filter client model. type ForwardRule struct { diff --git a/agent/app/service/forwarding.go b/agent/app/service/forwarding.go index 23da154a6eca..30d15de4517b 100644 --- a/agent/app/service/forwarding.go +++ b/agent/app/service/forwarding.go @@ -16,7 +16,7 @@ import ( type IForwardingService interface { LoadBaseInfo() (dto.FirewallBaseInfo, error) - SearchWithPage(search dto.ForwardRuleSearch) (int64, interface{}, error) + SearchWithPage(search dto.RuleSearch) (int64, interface{}, error) Operate(req dto.ForwardRuleOperate) error Enable() error Replay() error @@ -73,7 +73,10 @@ func (s *ForwardingService) LoadBaseInfo() (dto.FirewallBaseInfo, error) { return baseInfo, nil } -func (s *ForwardingService) SearchWithPage(req dto.ForwardRuleSearch) (int64, interface{}, error) { +func (s *ForwardingService) SearchWithPage(req dto.RuleSearch) (int64, interface{}, error) { + if req.Strategy != "" { + return 0, make([]dto.ForwardRule, 0), nil + } adapter, err := s.adapterFactory() if err != nil { return 0, nil, err @@ -82,9 +85,6 @@ func (s *ForwardingService) SearchWithPage(req dto.ForwardRuleSearch) (int64, in if err != nil { return 0, nil, err } - if req.Strategy != "" { - return 0, nil, nil - } var filtered []forwardClient.Rule for _, rule := range rules { @@ -103,10 +103,7 @@ func (s *ForwardingService) SearchWithPage(req dto.ForwardRuleSearch) (int64, in end = total } pageRules := filtered[start:end] - var items []dto.ForwardRule - if pageRules != nil { - items = make([]dto.ForwardRule, 0, len(pageRules)) - } + items := make([]dto.ForwardRule, 0, len(pageRules)) for _, rule := range pageRules { items = append(items, dto.ForwardRule{ Num: rule.Num, diff --git a/agent/app/service/forwarding_contract_test.go b/agent/app/service/forwarding_contract_test.go index c9b76149589c..2516dab58c61 100644 --- a/agent/app/service/forwarding_contract_test.go +++ b/agent/app/service/forwarding_contract_test.go @@ -79,7 +79,7 @@ func TestForwardingSearchPreservesAPIShapeAndPagination(t *testing.T) { {Num: "2", Protocol: "udp", Port: "5353", TargetIP: "127.0.0.1", TargetPort: "53"}, }} service := forwardingServiceWithAdapter(adapter) - total, value, err := service.SearchWithPage(dto.ForwardRuleSearch{PageInfo: dto.PageInfo{Page: 1, PageSize: 10}, Info: "10.0.0.2"}) + total, value, err := service.SearchWithPage(dto.RuleSearch{PageInfo: dto.PageInfo{Page: 1, PageSize: 10}, Info: "10.0.0.2"}) if err != nil { t.Fatal(err) } @@ -145,7 +145,7 @@ func TestForwardingOperatePreservesDuplicateAndOrderingContracts(t *testing.T) { func TestForwardingSearchReturnsAdapterError(t *testing.T) { wantErr := errors.New("list failed") service := forwardingServiceWithAdapter(&fakeForwardingAdapter{name: "firewalld", listErr: wantErr}) - _, _, err := service.SearchWithPage(dto.ForwardRuleSearch{PageInfo: dto.PageInfo{Page: 1, PageSize: 20}}) + _, _, err := service.SearchWithPage(dto.RuleSearch{PageInfo: dto.PageInfo{Page: 1, PageSize: 20}}) if !errors.Is(err, wantErr) { t.Fatalf("got %v want %v", err, wantErr) } diff --git a/agent/utils/firewall/client/iptables.go b/agent/utils/firewall/client/iptables.go index 8679bd92dbb9..174c20be7390 100644 --- a/agent/utils/firewall/client/iptables.go +++ b/agent/utils/firewall/client/iptables.go @@ -202,11 +202,8 @@ func (i *Iptables) ApplyAddressUnit(unit AddressUnit, operation string) error { } func (i *Iptables) AddPortWhiteList(list PortWhiteList) error { - if isInit, _ := iptables.LoadInitStatus("iptables", "base"); !isInit { - return nil - } list.Previous = nil - return SyncIptablesPortWhiteList(list, true) + return i.SyncPortWhiteList(list) } func (i *Iptables) SyncPortWhiteList(list PortWhiteList) error { diff --git a/agent/utils/firewall/client/iptables/filter.go b/agent/utils/firewall/client/iptables/filter.go index d41bbc5fb5d1..72161e9b6f1f 100644 --- a/agent/utils/firewall/client/iptables/filter.go +++ b/agent/utils/firewall/client/iptables/filter.go @@ -89,7 +89,7 @@ func ReadFilterRulesByChain(chain string) ([]FilterRules, error) { } itemRule := FilterRules{ Chain: chain, - Protocol: loadProtocol(fields[1]), + Protocol: LoadProtocol(fields[1]), SrcPort: loadPort("src", fields), DstPort: loadPort("dst", fields), SrcIP: loadIP(fields[3]), @@ -150,7 +150,7 @@ func LoadInitStatus(clientName, tab string) (bool, bool) { fmt.Sprintf("-A %s -j %s", ChainInput, Chain1PanelBasic), fmt.Sprintf("-A %s -j %s", ChainInput, Chain1PanelBasicAfter), } - return checkWithInitAndBind(initRules, bindRules, lines) + return CheckWithInitAndBind(initRules, bindRules, lines) case "advance": filterRules, err := RunWithStd(FilterTab, "-S") if err != nil { @@ -165,13 +165,15 @@ func LoadInitStatus(clientName, tab string) (bool, bool) { fmt.Sprintf("-A %s -j %s", ChainInput, Chain1PanelInput), fmt.Sprintf("-A %s -j %s", ChainOutput, Chain1PanelOutput), } - return checkWithInitAndBind(initRules, bindRules, lines) + return CheckWithInitAndBind(initRules, bindRules, lines) default: return false, false } } -func checkWithInitAndBind(initRules, bindRules []string, lines []string) (bool, bool) { +// CheckWithInitAndBind reports whether every init rule and every bind rule is +// present in the `iptables -S` output lines. +func CheckWithInitAndBind(initRules, bindRules []string, lines []string) (bool, bool) { for _, rule := range initRules { found := false for _, line := range lines { @@ -230,7 +232,8 @@ func loadIP(ipStr string) string { return ipStr } -func loadProtocol(protocol string) string { +// LoadProtocol maps an iptables numeric protocol to its name. +func LoadProtocol(protocol string) string { switch protocol { case "0": return "all" diff --git a/agent/utils/firewall/client/rule.go b/agent/utils/firewall/client/rule.go index 6625d35752a2..4fdeaeda2b1a 100644 --- a/agent/utils/firewall/client/rule.go +++ b/agent/utils/firewall/client/rule.go @@ -125,7 +125,8 @@ func needsRichRule(rule FireInfo) bool { } // ufwNeedsRichRule is the ufw variant: ufw denies a port through the port -// shortcut as well, only a source forces the longer form. +// shortcut as well, only a source forces the longer form. Expansion has already +// normalized "Anywhere" to an empty address. func ufwNeedsRichRule(rule FireInfo) bool { - return len(rule.Address) != 0 && !strings.EqualFold(rule.Address, "Anywhere") + return len(rule.Address) != 0 } diff --git a/agent/utils/firewall/forwarding/legacy.go b/agent/utils/firewall/forwarding/legacy.go index ca97fdd0d6dc..d5b67111fdd1 100644 --- a/agent/utils/firewall/forwarding/legacy.go +++ b/agent/utils/firewall/forwarding/legacy.go @@ -209,7 +209,7 @@ func (l *legacyNATAdapter) InitStatus() (bool, bool) { if err != nil { return false, false } - natInit, natBind := checkInitAndBind( + natInit, natBind := iptables.CheckWithInitAndBind( []string{"-N " + ChainPreRouting, "-N " + ChainPostRouting}, []string{"-A PREROUTING -j " + ChainPreRouting, "-A POSTROUTING -j " + ChainPostRouting}, strings.Split(natRules, "\n"), @@ -221,7 +221,7 @@ func (l *legacyNATAdapter) InitStatus() (bool, bool) { if err != nil { return false, false } - filterInit, filterBind := checkInitAndBind( + filterInit, filterBind := iptables.CheckWithInitAndBind( []string{"-N " + ChainForward}, []string{"-A FORWARD -j " + ChainForward}, strings.Split(filterRules, "\n"), @@ -229,29 +229,6 @@ func (l *legacyNATAdapter) InitStatus() (bool, bool) { return natInit && filterInit, natBind && filterBind } -func checkInitAndBind(initRules, bindRules, lines []string) (bool, bool) { - for _, rule := range initRules { - if !containsExactRule(lines, rule) { - return false, false - } - } - for _, rule := range bindRules { - if !containsExactRule(lines, rule) { - return true, false - } - } - return true, true -} - -func containsExactRule(lines []string, rule string) bool { - for _, line := range lines { - if strings.TrimSpace(line) == strings.TrimSpace(rule) { - return true - } - } - return false -} - func (l *legacyNATAdapter) Replay() error { for _, item := range []struct { table string @@ -278,7 +255,7 @@ func parseLegacyRules(stdout string) []Rule { } rule := Rule{ Num: fields[0], - Protocol: loadProtocol(fields[4]), + Protocol: iptables.LoadProtocol(fields[4]), Interface: fields[6], Port: loadSourcePort(fields[11]), } @@ -301,21 +278,6 @@ func parseLegacyRules(stdout string) []Rule { return rules } -func loadProtocol(protocol string) string { - switch protocol { - case "0": - return "all" - case "1": - return "icmp" - case "6": - return "tcp" - case "17": - return "udp" - default: - return protocol - } -} - func loadSourcePort(value string) string { port := "" if strings.Contains(value, "dpt:") { From 6b99db559e19defbc27e8ace807e9abff2fd066d Mon Sep 17 00:00:00 2001 From: HynoR <20227709+HynoR@users.noreply.github.com> Date: Thu, 30 Jul 2026 14:55:00 +0800 Subject: [PATCH 6/6] test(firewall): cover whitelist synchronization flows --- agent/app/service/firewall_contract_test.go | 48 ++++++++ agent/utils/firewall/client/whitelist_test.go | 106 ++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 agent/utils/firewall/client/whitelist_test.go diff --git a/agent/app/service/firewall_contract_test.go b/agent/app/service/firewall_contract_test.go index 66c565e8f911..240f2dbe39df 100644 --- a/agent/app/service/firewall_contract_test.go +++ b/agent/app/service/firewall_contract_test.go @@ -5,10 +5,32 @@ import ( "testing" "github.com/1Panel-dev/1Panel/agent/app/dto" + "github.com/1Panel-dev/1Panel/agent/app/repo" "github.com/1Panel-dev/1Panel/agent/constant" + "github.com/1Panel-dev/1Panel/agent/global" + "github.com/1Panel-dev/1Panel/agent/utils/firewall" fireClient "github.com/1Panel-dev/1Panel/agent/utils/firewall/client" ) +type firewallSettingRepoStub struct { + repo.ISettingRepo + value string +} + +func (r *firewallSettingRepoStub) GetValueByKey(string) (string, error) { + return r.value, nil +} + +type whitelistFilterClient struct { + firewall.FilterClient + list fireClient.PortWhiteList +} + +func (c *whitelistFilterClient) SyncPortWhiteList(list fireClient.PortWhiteList) error { + c.list = list + return nil +} + func TestParseFirewallPortWhiteListContract(t *testing.T) { tests := []struct { name string @@ -70,6 +92,32 @@ func TestParseFirewallPortWhiteListContract(t *testing.T) { } } +func TestSyncFirewallPortWhiteListBuildsProviderState(t *testing.T) { + originalRepo, originalConf, originalMaster := settingRepo, global.CONF, global.IsMaster + settingRepo = &firewallSettingRepoStub{value: "443/tcp"} + global.IsMaster = false + global.CONF.Base.Port = "9999" + t.Cleanup(func() { + settingRepo = originalRepo + global.CONF = originalConf + global.IsMaster = originalMaster + }) + + client := &whitelistFilterClient{} + if err := syncFirewallPortWhiteListAfterUpdateWithClient(client, "80/tcp"); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(client.list.Configured, []fireClient.PortWhiteListEntry{{Port: "443", Protocol: "tcp"}}) { + t.Fatalf("unexpected configured list: %#v", client.list.Configured) + } + if !reflect.DeepEqual(client.list.Previous, []fireClient.PortWhiteListEntry{{Port: "80", Protocol: "tcp"}}) { + t.Fatalf("unexpected previous list: %#v", client.list.Previous) + } + if len(client.list.Required) != 2 || client.list.Required[0] != (fireClient.PortWhiteListEntry{Port: "9999", Protocol: "tcp"}) { + t.Fatalf("unexpected required list: %#v", client.list.Required) + } +} + func TestCheckPortUsedContract(t *testing.T) { apps := []portOfApp{ {AppName: "wordpress", HttpPort: "8080", HttpsPort: "8443"}, diff --git a/agent/utils/firewall/client/whitelist_test.go b/agent/utils/firewall/client/whitelist_test.go new file mode 100644 index 000000000000..c841806a3d70 --- /dev/null +++ b/agent/utils/firewall/client/whitelist_test.go @@ -0,0 +1,106 @@ +package client + +import ( + "errors" + "reflect" + "testing" +) + +type recordingNativePortWriter struct { + active bool + failOn string + events []string +} + +func (r *recordingNativePortWriter) Status() (bool, error) { + r.events = append(r.events, "status") + return r.active, nil +} + +func (r *recordingNativePortWriter) Reload() error { + r.events = append(r.events, "reload") + if r.failOn == "reload" { + return errors.New("reload failed") + } + return nil +} + +func (r *recordingNativePortWriter) Port(port FireInfo, operation string) error { + event := operation + " " + port.Port + "/" + port.Protocol + r.events = append(r.events, event) + if r.failOn == event { + return errors.New("port operation failed") + } + return nil +} + +func TestAddNativePortWhiteListSequence(t *testing.T) { + writer := &recordingNativePortWriter{active: true} + list := PortWhiteList{ + Configured: []PortWhiteListEntry{ + {Port: "80", Protocol: "tcp"}, + {Port: "80", Protocol: "tcp"}, + }, + Required: []PortWhiteListEntry{ + {Port: "22", Protocol: "tcp"}, + {Port: "80", Protocol: "tcp"}, + }, + } + + if err := addNativePortWhiteList(writer, list); err != nil { + t.Fatal(err) + } + want := []string{"add 80/tcp", "add 22/tcp", "reload"} + if !reflect.DeepEqual(writer.events, want) { + t.Fatalf("got %#v want %#v", writer.events, want) + } +} + +func TestSyncNativePortWhiteListSequence(t *testing.T) { + writer := &recordingNativePortWriter{active: true} + list := PortWhiteList{ + Configured: []PortWhiteListEntry{{Port: "443", Protocol: "tcp"}}, + Required: []PortWhiteListEntry{{Port: "22", Protocol: "tcp"}}, + Previous: []PortWhiteListEntry{{Port: "80", Protocol: "tcp"}}, + } + + if err := syncNativePortWhiteList(writer, list); err != nil { + t.Fatal(err) + } + want := []string{"status", "remove 80/tcp", "add 443/tcp", "reload"} + if !reflect.DeepEqual(writer.events, want) { + t.Fatalf("got %#v want %#v", writer.events, want) + } +} + +func TestSyncNativePortWhiteListInactiveIsNoop(t *testing.T) { + writer := &recordingNativePortWriter{} + list := PortWhiteList{ + Configured: []PortWhiteListEntry{{Port: "443", Protocol: "tcp"}}, + Previous: []PortWhiteListEntry{{Port: "80", Protocol: "tcp"}}, + } + + if err := syncNativePortWhiteList(writer, list); err != nil { + t.Fatal(err) + } + want := []string{"status"} + if !reflect.DeepEqual(writer.events, want) { + t.Fatalf("got %#v want %#v", writer.events, want) + } +} + +func TestSyncNativePortWhiteListStopsOnError(t *testing.T) { + writer := &recordingNativePortWriter{active: true, failOn: "remove 80/tcp"} + list := PortWhiteList{ + Configured: []PortWhiteListEntry{{Port: "443", Protocol: "tcp"}}, + Previous: []PortWhiteListEntry{{Port: "80", Protocol: "tcp"}}, + } + + if err := syncNativePortWhiteList(writer, list); err == nil { + t.Fatal("expected remove error") + } + want := []string{"status", "remove 80/tcp"} + if !reflect.DeepEqual(writer.events, want) { + t.Fatalf("got %#v want %#v", writer.events, want) + } +}