From 41708cb6ffb03bd8aa3ca8a7f95e01962dd2bf01 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Tue, 15 Aug 2023 17:05:23 +0200
Subject: [PATCH] libnetwork/iptables: make firewalldInit more atomic

firewalldInit was returning an error if we failed to set up the docker
zone, but did not close the D-Bus connection. Given that we consider
firewalld to "not be usable" in case of an error, let's also close
the connection;

    unable to initialize firewalld; using raw iptables instead

And return the connection on success, instead of implicitly setting the
package-level `firewalld` variable.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 libnetwork/iptables/firewalld.go      | 16 ++++++++--------
 libnetwork/iptables/firewalld_test.go |  4 +++-
 libnetwork/iptables/iptables.go       |  4 +++-
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/libnetwork/iptables/firewalld.go b/libnetwork/iptables/firewalld.go
index a03549a01b..3eaa45ec36 100644
--- a/libnetwork/iptables/firewalld.go
+++ b/libnetwork/iptables/firewalld.go
@@ -44,22 +44,22 @@ var (
 )
 
 // firewalldInit initializes firewalld management code.
-func firewalldInit() error {
-	var err error
-	firewalld, err = newConnection()
+func firewalldInit() (*firewalldConnection, error) {
+	fwd, err := newConnection()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// start handling D-Bus signals that were registered.
-	firewalld.handleSignals()
+	fwd.handleSignals()
 
-	err = firewalld.setupDockerZone()
+	err = fwd.setupDockerZone()
 	if err != nil {
-		return err
+		_ = fwd.conn.Close()
+		return nil, err
 	}
 
-	return nil
+	return fwd, nil
 }
 
 // newConnection establishes a connection to the system D-Bus and registers
diff --git a/libnetwork/iptables/firewalld_test.go b/libnetwork/iptables/firewalld_test.go
index 1ec9233940..029c874b79 100644
--- a/libnetwork/iptables/firewalld_test.go
+++ b/libnetwork/iptables/firewalld_test.go
@@ -27,9 +27,11 @@ func skipIfNoFirewalld(t *testing.T) {
 
 func TestFirewalldInit(t *testing.T) {
 	skipIfNoFirewalld(t)
-	if err := firewalldInit(); err != nil {
+	fwd, err := firewalldInit()
+	if err != nil {
 		t.Fatal(err)
 	}
+	_ = fwd.conn.Close()
 }
 
 func TestReloaded(t *testing.T) {
diff --git a/libnetwork/iptables/iptables.go b/libnetwork/iptables/iptables.go
index 0c845fc36d..3b3d14a7aa 100644
--- a/libnetwork/iptables/iptables.go
+++ b/libnetwork/iptables/iptables.go
@@ -138,7 +138,9 @@ func initFirewalld() {
 		log.G(context.TODO()).Info("skipping firewalld management for rootless mode")
 		return
 	}
-	if err := firewalldInit(); err != nil {
+	var err error
+	firewalld, err = firewalldInit()
+	if err != nil {
 		log.G(context.TODO()).WithError(err).Debugf("unable to initialize firewalld; using raw iptables instead")
 	}
 }