fix: handle nil chain type in FromChainToRulesArray

Author: riccardotornesello

pkg/firewall/chain.go

@@ -15,6 +15,8 @@
 package firewall
 
 import (
+	"fmt"
+
 	"github.com/google/nftables"
 	"k8s.io/klog/v2"
 
@@ -214,14 +216,22 @@ func isChainModified(nftChain *nftables.Chain, chain *firewallapi.Chain) bool {
 }
 
 // FromChainToRulesArray converts a chain to an array of rules.
-func FromChainToRulesArray(chain *firewallapi.Chain) (rules []firewallutils.Rule) {
+func FromChainToRulesArray(chain *firewallapi.Chain) (rules []firewallutils.Rule, err error) {
+	if chain == nil {
+		return nil, fmt.Errorf("chain is nil")
+	}
+
+	if chain.Type == nil {
+		return nil, fmt.Errorf("chain type is required")
+	}
+
 	switch *chain.Type {
 	case firewallapi.ChainTypeFilter:
 		rules = make([]firewallutils.Rule, len(chain.Rules.FilterRules))
 		for i := range chain.Rules.FilterRules {
 			rules[i] = &firewallutils.FilterRuleWrapper{FilterRule: &chain.Rules.FilterRules[i]}
 		}
-		return rules
+		return rules, nil
 	case firewallapi.ChainTypeNAT:
 		rules = make([]firewallutils.Rule, len(chain.Rules.NatRules))
 		for i := range chain.Rules.NatRules {
@@ -237,7 +247,7 @@ func FromChainToRulesArray(chain *firewallapi.Chain) (rules []firewallutils.Rule
 		rules = []firewallutils.Rule{}
 	}
 	// It is not necessary, but linter complains
-	return rules
+	return rules, nil
 }
 
 // cleanChain removes all the rules that are not present in the firewall configuration or that have been modified.
@@ -246,7 +256,11 @@ func cleanChain(nftconn *nftables.Conn, chain *firewallapi.Chain, nftChain *nfta
 	if err != nil {
 		return err
 	}
-	rules := FromChainToRulesArray(chain)
+	rules, err := FromChainToRulesArray(chain)
+	if err != nil {
+		return err
+	}
+
 	for i := range nftRules {
 		// If the rule is outdated, delete it.
 		outdated, ruleName := isRuleOutdated(nftRules[i], rules)

pkg/firewall/rule.go

@@ -23,7 +23,11 @@ import (
 )
 
 func addRules(nftconn *nftables.Conn, chain *firewallapi.Chain, nftchain *nftables.Chain) error {
-	apirules := FromChainToRulesArray(chain)
+	apirules, err := FromChainToRulesArray(chain)
+	if err != nil {
+		return err
+	}
+
 	nftrules, err := nftconn.GetRules(nftchain.Table, nftchain)
 	if err != nil {
 		return err

pkg/webhooks/firewallconfiguration/firewallconfiguration.go

@@ -90,7 +90,11 @@ func (w *webhookMutate) Handle(_ context.Context, req admission.Request) admissi
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 
-	generateRuleNames(firewallConfiguration.Spec.Table.Chains)
+	err = generateRuleNames(firewallConfiguration.Spec.Table.Chains)
+	if err != nil {
+		klog.Errorf("Failed generating rule names: %v", err)
+		return admission.Errored(http.StatusBadRequest, err)
+	}
 
 	return w.CreatePatchResponse(&req, firewallConfiguration)
 }

pkg/webhooks/firewallconfiguration/rule.go

@@ -25,7 +25,11 @@ import (
 )
 
 func checkRulesInChain(chain *firewallapi.Chain) error {
-	rules := firewall.FromChainToRulesArray(chain)
+	rules, err := firewall.FromChainToRulesArray(chain)
+	if err != nil {
+		return forgeChainError(chain, err)
+	}
+
 	if err := checkVoidRuleName(rules); err != nil {
 		return forgeChainError(chain, err)
 	}
@@ -60,13 +64,19 @@ func checkUniqueRuleNames(rules []firewallutils.Rule) error {
 	return nil
 }
 
-func generateRuleNames(chains []firewallapi.Chain) {
+func generateRuleNames(chains []firewallapi.Chain) error {
 	for i := range chains {
-		rules := firewall.FromChainToRulesArray(&chains[i])
+		rules, err := firewall.FromChainToRulesArray(&chains[i])
+		if err != nil {
+			return err
+		}
+
 		for j := range rules {
 			if rules[j].GetName() == nil || *rules[j].GetName() == "" {
 				rules[j].SetName(uuid.NewString())
 			}
 		}
 	}
+
+	return nil
 }