Handle v6 rules for the mgmt bridge with nftables/iptables (#2397)

* added v6 rule for mgmt bridge with nftables * delete nftables v6 rules * added iptables based add/delete for v6 * use nft cli instead of ip6tables * time to dive deeper * try ubuntu 24.04 * install v6 tables with nftables only if v6 tables are available * skip checks if ip6 tables not found * bring back tmate * delete v6 rules only if supported
srl-labs · Jan 16, 2025 · 6257f4f · 6257f4f
1 parent 663511e
commit 6257f4f
Show file tree

Hide file tree

Showing 6 changed files with 171 additions and 35 deletions.
diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml
@@ -9,7 +9,7 @@ name: smoke-tests
 
 jobs:
   smoke-tests:
-    runs-on: ubuntu-22.04
+    runs-on: ubuntu-24.04
     timeout-minutes: 5
     strategy:
       fail-fast: false
@@ -70,6 +70,9 @@ jobs:
       - name: Sanitize test-suite name
         run: echo "TEST_SUITE=$(echo ${{ matrix.test-suite }} | tr -d '*')" >> $GITHUB_ENV
 
+      # - name: setup tmate session
+      #   uses: mxschmitt/action-tmate@v3
+
       - name: Run smoke tests
         run: |
           bash ./tests/rf-run.sh ${{ matrix.runtime }} ./tests/01-smoke/${{ matrix.test-suite }}

diff --git a/docs/manual/network.md b/docs/manual/network.md
@@ -247,17 +247,22 @@ With this approach, users can prevent IP address overlap with nodes deployed on
 
 #### external access
 
-Containerlab will attempt to enable external access to the nodes by default. This means that external systems/hosts will be able to communicate with the nodes of your topology without requiring any manual iptables/nftables rules to be installed.
+Containerlab will attempt to enable external management access to the nodes by default. This means that external systems/hosts will be able to communicate with the nodes of your topology without requiring any manual iptables/nftables rules to be installed.
 
-To allow external communications containerlab installs a rule in the `DOCKER-USER` chain, allowing all packets targeting containerlab's management network. The rule looks like follows:
+To allow external communications containerlab installs a rule in the `DOCKER-USER` chain for v4 and v6, allowing all packets targeting containerlab's management network. The rule looks like follows:
 
 ```shell
-❯ sudo iptables -vnL DOCKER-USER
+sudo iptables -vnL DOCKER-USER
+```
+
+<div class="embed-result">
+```{.no-copy .no-select}
 Chain DOCKER-USER (1 references)
- pkts bytes target     prot opt in     out     source               destination         
+ pkts bytes target     prot opt in     out     source               destination
     0     0 ACCEPT     all  --  *      br-a8b9fc8b33a2  0.0.0.0/0            0.0.0.0/0            /* set by containerlab */
-12719   79M RETURN     all  --  *      *       0.0.0.0/0            0.0.0.0/0      
+12719   79M RETURN     all  --  *      *       0.0.0.0/0            0.0.0.0/0
 ```
+</div>
 
 1. The `br-a8b9fc8b33a2` bridge interface is the interface that backs up the containerlab's management network (`clab` docker network).
 

diff --git a/runtime/docker/firewall.go b/runtime/docker/firewall.go
@@ -20,7 +20,7 @@ func (d *DockerRuntime) deleteFwdRule() (err error) {
 }
 
 // installFwdRule installs the `allow` rule for traffic destined to the nodes
-// on the clab management network.
+// on the clab management network for v4 and v6.
 // This rule is required for external access to the nodes.
 func (d *DockerRuntime) installFwdRule() (err error) {
 	if !*d.mgmt.ExternalAccess {

diff --git a/runtime/docker/firewall/iptables/client.go b/runtime/docker/firewall/iptables/client.go
@@ -13,32 +13,41 @@ import (
 )
 
 const (
-	iptCheckCmd = "-vL DOCKER-USER"
-	iptAllowCmd = "-I DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\""
-	iptDelCmd   = "-D DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\""
-	ipTables    = "ip_tables"
+	iptCheckArgs = "-vL DOCKER-USER"
+	iptAllowArgs = "-I DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\""
+	iptDelArgs   = "-D DOCKER-USER -o %s -j ACCEPT -m comment --comment \"" + definitions.IPTablesRuleComment + "\""
+	ipTables     = "ip_tables"
+
+	v4AF         = "v4"
+	ip4tablesCmd = "iptables"
+	v6AF         = "v6"
+	ip6tablesCmd = "ip6tables"
 )
 
 // IpTablesClient is a client for iptables.
 type IpTablesClient struct {
 	bridgeName string
+	ip6_tables bool
 }
 
 // NewIpTablesClient returns a new IpTablesClient.
 func NewIpTablesClient(bridgeName string) (*IpTablesClient, error) {
-	loaded, err := utils.IsKernelModuleLoaded("ip_tables")
+	v4ModLoaded, err := utils.IsKernelModuleLoaded("ip_tables")
 	if err != nil {
 		return nil, err
 	}
 
-	if !loaded {
+	v6ModLoaded, _ := utils.IsKernelModuleLoaded("ip6_tables")
+
+	if !v4ModLoaded {
 		log.Debug("ip_tables kernel module not available")
 		// module is not loaded
 		return nil, definitions.ErrNotAvailable
 	}
 
 	return &IpTablesClient{
 		bridgeName: bridgeName,
+		ip6_tables: v6ModLoaded,
 	}, nil
 }
 
@@ -47,28 +56,40 @@ func (*IpTablesClient) Name() string {
 	return ipTables
 }
 
-// InstallForwardingRules installs the forwarding rules.
+// InstallForwardingRules installs the forwarding rules for v4 and v6 address families.
 func (c *IpTablesClient) InstallForwardingRules() error {
-	// first check if a rule already exists to not create duplicates
-	res, err := exec.Command("iptables", strings.Split(iptCheckCmd, " ")...).Output()
-	if bytes.Contains(res, []byte(c.bridgeName)) {
-		log.Debugf("found iptables forwarding rule targeting the bridge %q. Skipping creation of the forwarding rule.", c.bridgeName)
+	err := c.InstallForwardingRulesForAF(v4AF)
+	if err != nil {
 		return err
 	}
-	if err != nil {
-		// non nil error typically means that DOCKER-USER chain doesn't exist
-		// this happens with old docker installations (centos7 hello) from default repos
-		return fmt.Errorf("missing DOCKER-USER iptables chain. See http://containerlab.dev/manual/network/#external-access")
+
+	if c.ip6_tables {
+		err = c.InstallForwardingRulesForAF(v6AF)
 	}
 
-	cmd, err := shlex.Split(fmt.Sprintf(iptAllowCmd, c.bridgeName))
+	return err
+}
+
+// InstallForwardingRulesForAF installs the forwarding rules for the specified address family.
+func (c *IpTablesClient) InstallForwardingRulesForAF(af string) error {
+	iptCmd := ip4tablesCmd
+	if af == v6AF {
+		iptCmd = ip6tablesCmd
+	}
+
+	// first check if a rule already exists to not create duplicates
+	if c.allowRuleForMgmtBrExists(af) {
+		return nil
+	}
+
+	cmd, err := shlex.Split(fmt.Sprintf(iptAllowArgs, c.bridgeName))
 	if err != nil {
 		return err
 	}
 
-	log.Debugf("Installing iptables rules for bridge %q", c.bridgeName)
+	log.Debugf("Installing iptables (%s) rules for bridge %q", af, c.bridgeName)
 
-	stdOutErr, err := exec.Command("iptables", cmd...).CombinedOutput()
+	stdOutErr, err := exec.Command(iptCmd, cmd...).CombinedOutput()
 	if err != nil {
 		log.Warnf("Iptables install stdout/stderr result is: %s", stdOutErr)
 		return fmt.Errorf("unable to install iptables rule using '%s' command: %w", cmd, err)
@@ -77,10 +98,29 @@ func (c *IpTablesClient) InstallForwardingRules() error {
 	return nil
 }
 
-// DeleteForwardingRules deletes the forwarding rules.
+// DeleteForwardingRules deletes the forwarding rules for v4 and v6 address families.
 func (c *IpTablesClient) DeleteForwardingRules() error {
+	err := c.DeleteForwardingRulesForAF(v4AF)
+	if err != nil {
+		return err
+	}
+
+	if c.ip6_tables {
+		err = c.InstallForwardingRulesForAF(v6AF)
+	}
+
+	return err
+}
+
+// DeleteForwardingRulesForAF deletes the forwarding rules for a specified AF.
+func (c *IpTablesClient) DeleteForwardingRulesForAF(af string) error {
+	iptCmd := ip4tablesCmd
+	if af == v6AF {
+		iptCmd = ip6tablesCmd
+	}
+
 	// first check if a rule exists before trying to delete it
-	res, err := exec.Command("iptables", strings.Split(iptCheckCmd, " ")...).Output()
+	res, err := exec.Command(iptCmd, strings.Split(iptCheckArgs, " ")...).Output()
 	if err != nil {
 		// non nil error typically means that DOCKER-USER chain doesn't exist
 		// this happens with old docker installations (centos7 hello) from default repos
@@ -101,19 +141,43 @@ func (c *IpTablesClient) DeleteForwardingRules() error {
 		return nil
 	}
 
-	cmd, err := shlex.Split(fmt.Sprintf(iptDelCmd, c.bridgeName))
+	cmd, err := shlex.Split(fmt.Sprintf(iptDelArgs, c.bridgeName))
 	if err != nil {
 		return err
 	}
 
 	log.Debugf("removing clab iptables rules for bridge %q", c.bridgeName)
 	log.Debugf("trying to delete the forwarding rule with cmd: iptables %s", cmd)
 
-	stdOutErr, err := exec.Command("iptables", cmd...).CombinedOutput()
+	stdOutErr, err := exec.Command(iptCmd, cmd...).CombinedOutput()
 	if err != nil {
 		log.Warnf("Iptables delete stdout/stderr result is: %s", stdOutErr)
 		return fmt.Errorf("unable to delete iptables rules: %w", err)
 	}
 
 	return nil
 }
+
+// allowRuleForMgmtBrExists checks if an allow rule for the provided bridge name exists.
+// The actual check doesn't verify that `allow` is set, it just checks if the rule
+// has the provided bridge name in the output interface.
+func (c *IpTablesClient) allowRuleForMgmtBrExists(af string) bool {
+	iptCmd := ip4tablesCmd
+	if af == v6AF {
+		iptCmd = ip6tablesCmd
+	}
+
+	res, err := exec.Command(iptCmd, strings.Split(iptCheckArgs, " ")...).CombinedOutput()
+	if err != nil {
+		log.Warnf("iptables check error: %s. Output: %s", err, string(res))
+		// if we errored on check we don't want to try setting up the rule
+		return true
+	}
+	if bytes.Contains(res, []byte(c.bridgeName)) {
+		log.Debugf("found iptables forwarding rule targeting the bridge %q. Skipping creation of the forwarding rule.", c.bridgeName)
+
+		return true
+	}
+
+	return false
+}
diff --git a/runtime/docker/firewall/nftables/client.go b/runtime/docker/firewall/nftables/client.go
@@ -19,6 +19,8 @@ const nfTables = "nf_tables"
 type NftablesClient struct {
 	nftConn    *nftables.Conn
 	bridgeName string
+	// is ip6_tables supported
+	ip6_tables bool
 }
 
 // NewNftablesClient returns a new NftablesClient.
@@ -43,6 +45,12 @@ func NewNftablesClient(bridgeName string) (*NftablesClient, error) {
 		return nil, definitions.ErrNotAvailable
 	}
 
+	// check if ip6_tables is available
+	v6Tables, err := nftC.nftConn.ListTablesOfFamily(nftables.TableFamilyIPv6)
+	if err != nil || len(v6Tables) == 0 {
+		nftC.ip6_tables = false
+	}
+
 	return nftC, nil
 }
 
@@ -58,15 +66,24 @@ func (c *NftablesClient) DeleteForwardingRules() error {
 		return nil
 	}
 
-	// first check if a rule already exists to not create duplicates
 	defer c.close()
 
-	rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4)
+	allRules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4)
 	if err != nil {
 		return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err)
 	}
 
-	mgmtBrRules := c.getRulesForMgmtBr(c.bridgeName, rules)
+	var v6rules []*nftables.Rule
+	if c.ip6_tables {
+		v6rules, err = c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv6)
+		if err != nil {
+			return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err)
+		}
+	}
+
+	allRules = append(allRules, v6rules...)
+
+	mgmtBrRules := c.getRulesForMgmtBr(c.bridgeName, allRules)
 	if len(mgmtBrRules) == 0 {
 		log.Debug("external access iptables rule doesn't exist. Skipping deletion")
 		return nil
@@ -90,11 +107,26 @@ func (c *NftablesClient) DeleteForwardingRules() error {
 	return nil
 }
 
-// InstallForwardingRules installs the forwarding rules.
+// InstallForwardingRules installs the forwarding rules for v4 and v6 address families.
 func (c *NftablesClient) InstallForwardingRules() error {
 	defer c.close()
 
-	rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4)
+	err := c.InstallForwardingRulesForAF(nftables.TableFamilyIPv4)
+	if err != nil {
+		return err
+	}
+
+	if c.ip6_tables {
+		err = c.InstallForwardingRulesForAF(nftables.TableFamilyIPv6)
+	}
+
+	return err
+}
+
+// InstallForwardingRulesForAF installs the forwarding rules for the specified address family.
+func (c *NftablesClient) InstallForwardingRulesForAF(af nftables.TableFamily) error {
+
+	rules, err := c.getRules(definitions.DockerFWUserChain, definitions.DockerFWTable, af)
 	if err != nil {
 		return fmt.Errorf("%w. See http://containerlab.dev/manual/network/#external-access", err)
 	}
@@ -107,7 +139,7 @@ func (c *NftablesClient) InstallForwardingRules() error {
 	log.Debugf("Installing iptables rules for bridge %q", c.bridgeName)
 
 	// create a new rule
-	rule, err := c.newClabNftablesRule(definitions.DockerFWUserChain, definitions.DockerFWTable, nftables.TableFamilyIPv4, 0)
+	rule, err := c.newClabNftablesRule(definitions.DockerFWUserChain, definitions.DockerFWTable, af, 0)
 	if err != nil {
 		return err
 	}

diff --git a/tests/01-smoke/01-basic-flow.robot b/tests/01-smoke/01-basic-flow.robot
@@ -317,6 +317,23 @@ Verify iptables allow rule is set
     ...    ignore_case=True
     ...    collapse_spaces=True
 
+Verify ip6tables allow rule is set
+    [Documentation]    Checking if ip6tables allow rule is set so that external traffic can reach containerlab management network
+    Skip If    '${runtime}' != 'docker'
+
+    # Add check for ip6tables availability
+    ${rc}    ${output} =    Run And Return Rc And Output    which nft
+    Skip If    ${rc} != 0    nft command not found
+
+    ${rc}    ${output} =    Run And Return Rc And Output    sudo nft list tables
+    Skip If    'ip6 filter' not in '''${output}'''    ip6 filter chain not found
+
+    ${ipt} =    Run
+    ...    sudo nft list chain ip6 filter DOCKER-USER
+    Log    ${ipt}
+    Should Match Regexp    ${ipt}    oifname.*${MgmtBr}.*accept
+
+
 Verify DNS-Server Config
     [Documentation]    Check if the DNS config did take effect
     Skip If    '${runtime}' != 'docker'
@@ -403,6 +420,21 @@ Verify iptables allow rule are gone
     Log    ${ipt}
     Should Not Contain    ${ipt}    ${MgmtBr}
 
+Verify ip6tables allow rule are gone
+    [Documentation]    Checking if ip6tables allow rule is removed once the lab is destroyed
+    Skip If    '${runtime}' != 'docker'
+
+    # Add check for ip6tables availability
+    ${rc}    ${output} =    Run And Return Rc And Output    which nft
+    Skip If    ${rc} != 0    nft command not found
+
+    ${rc}    ${output} =    Run And Return Rc And Output    sudo nft list tables
+    Skip If    'ip6 filter' not in '''${output}'''    ip6 filter chain not found
+
+    ${ipt} =    Run
+    ...    sudo nft list chain ip6 filter DOCKER-USER
+    Log    ${ipt}
+    Should Not Contain    ${ipt}    ${MgmtBr}
 
 *** Keywords ***
 Match IPv6 Address