Skip to content

Commit b052827

Browse files
jpopelkajpopelka
committed
React to firewalld's reload/restart
When firewalld (or iptables service) restarts/reloads, all previously added docker firewall rules are flushed. With firewalld we can react to its Reloaded() [1] D-Bus signal and recreate the firewall rules. Also when firewalld gets restarted (stopped & started) we can catch the NameOwnerChanged signal [2]. To specify which signals we want to react to we use AddMatch [3]. Libvirt has been doing this for quite a long time now. Docker changes firewall rules on basically 3 places. 1) daemon/networkdriver/portmapper/mapper.go - port mappings Portmapper fortunatelly keeps list of mapped ports, so we can easily recreate firewall rules on firewalld restart/reload New ReMapAll() function does that 2) daemon/networkdriver/bridge/driver.go When setting a bridge, basic firewall rules are created. This is done at once during start, it's parametrized and nowhere tracked so how can one know what and how to set it again when there's been firewalld restart/reload ? The only solution that came to my mind is using of closures [4], i.e. I keep list of references to closures (anonymous functions together with a referencing environment) and when there's firewalld restart/reload I re-call them in the same order. 3) links/links.go - linking containers Link is added in Enable() and removed in Disable(). In Enable() we add a callback function, which creates the link, that's OK so far. It'd be ideal if we could remove the same function from the list in Disable(). Unfortunatelly that's not possible AFAICT, because we don't know the reference to that function at that moment, so we can only add a reference to function, which removes the link. That means that after creating and removing a link there are 2 functions in the list, one adding and one removing the link and after firewalld restart/reload both are called. It works, but it's far from ideal. [1] https://jpopelka.fedorapeople.org/firewalld/doc/firewalld.dbus.html#FirewallD1.Signals.Reloaded [2] http://dbus.freedesktop.org/doc/dbus-specification.html#bus-messages-name-owner-changed [3] http://dbus.freedesktop.org/doc/dbus-specification.html#message-bus-routing-match-rules [4] https://en.wikipedia.org/wiki/Closure_%28computer_programming%29 Signed-off-by: Jiri Popelka <[email protected]>
1 parent 8301dcc commit b052827

4 files changed

Lines changed: 99 additions & 3 deletions

File tree

‎daemon/networkdriver/bridge/driver.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ func InitDriver(config *Config) error {
232232
logrus.Errorf("Error configuing iptables: %s", err)
233233
return err
234234
}
235-
235+
// call this on Firewalld reload
236+
iptables.OnReloaded(func() { setupIPTables(addrv4, config.InterContainerCommunication, config.EnableIpMasq) })
236237
}
237238

238239
if config.EnableIpForward {
@@ -262,10 +263,16 @@ func InitDriver(config *Config) error {
262263
if err != nil {
263264
return err
264265
}
266+
// call this on Firewalld reload
267+
iptables.OnReloaded(func() { iptables.NewChain("DOCKER", bridgeIface, iptables.Nat) })
268+
265269
chain, err := iptables.NewChain("DOCKER", bridgeIface, iptables.Filter)
266270
if err != nil {
267271
return err
268272
}
273+
// call this on Firewalld reload
274+
iptables.OnReloaded(func() { iptables.NewChain("DOCKER", bridgeIface, iptables.Filter) })
275+
269276
portMapper.SetIptablesChain(chain)
270277
}
271278

@@ -298,6 +305,10 @@ func InitDriver(config *Config) error {
298305
// Block BridgeIP in IP allocator
299306
ipAllocator.RequestIP(bridgeIPv4Network, bridgeIPv4Network.IP)
300307

308+
if config.EnableIptables {
309+
iptables.OnReloaded(portMapper.ReMapAll) // call this on Firewalld reload
310+
}
311+
301312
return nil
302313
}
303314

‎daemon/networkdriver/portmapper/mapper.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ func (pm *PortMapper) Map(container net.Addr, hostIP net.IP, hostPort int) (host
132132
return m.host, nil
133133
}
134134

135+
// re-apply all port mappings
136+
func (pm *PortMapper) ReMapAll() {
137+
logrus.Debugln("Re-applying all port mappings.")
138+
for _, data := range pm.currentMappings {
139+
containerIP, containerPort := getIPAndPort(data.container)
140+
hostIP, hostPort := getIPAndPort(data.host)
141+
if err := pm.forward(iptables.Append, data.proto, hostIP, hostPort, containerIP.String(), containerPort); err != nil {
142+
logrus.Errorf("Error on iptables add: %s", err)
143+
}
144+
}
145+
}
146+
135147
func (pm *PortMapper) Unmap(host net.Addr) error {
136148
pm.lock.Lock()
137149
defer pm.lock.Unlock()

‎links/links.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/docker/docker/daemon/networkdriver/bridge"
99
"github.com/docker/docker/nat"
10+
"github.com/docker/docker/pkg/iptables"
1011
)
1112

1213
type Link struct {
@@ -143,6 +144,8 @@ func (l *Link) Enable() error {
143144
if err := l.toggle("-A", false); err != nil {
144145
return err
145146
}
147+
// call this on Firewalld reload
148+
iptables.OnReloaded(func() { l.toggle("-I", false) })
146149
l.IsEnabled = true
147150
return nil
148151
}
@@ -152,7 +155,8 @@ func (l *Link) Disable() {
152155
// exist in iptables
153156
// -D == iptables delete flag
154157
l.toggle("-D", true)
155-
158+
// call this on Firewalld reload
159+
iptables.OnReloaded(func() { l.toggle("-D", true) })
156160
l.IsEnabled = false
157161
}
158162

‎pkg/iptables/firewalld.go‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package iptables
22

33
import (
4+
"fmt"
45
"github.com/Sirupsen/logrus"
56
"github.com/godbus/dbus"
7+
"strings"
68
)
79

810
type IPV string
@@ -26,7 +28,8 @@ type Conn struct {
2628

2729
var (
2830
connection *Conn
29-
firewalldRunning bool // is Firewalld service running
31+
firewalldRunning bool // is Firewalld service running
32+
onReloaded []*func() // callbacks when Firewalld has been reloaded
3033
)
3134

3235
func FirewalldInit() {
@@ -63,9 +66,75 @@ func (c *Conn) initConnection() error {
6366
// This never fails, even if the service is not running atm.
6467
c.sysobj = c.sysconn.Object(dbusInterface, dbus.ObjectPath(dbusPath))
6568

69+
rule := fmt.Sprintf("type='signal',path='%s',interface='%s',sender='%s',member='Reloaded'",
70+
dbusPath, dbusInterface, dbusInterface)
71+
c.sysconn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, rule)
72+
73+
rule = fmt.Sprintf("type='signal',interface='org.freedesktop.DBus',member='NameOwnerChanged',path='/org/freedesktop/DBus',sender='org.freedesktop.DBus',arg0='%s'",
74+
dbusInterface)
75+
c.sysconn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, rule)
76+
77+
c.signal = make(chan *dbus.Signal, 10)
78+
c.sysconn.Signal(c.signal)
79+
go signalHandler()
80+
6681
return nil
6782
}
6883

84+
func signalHandler() {
85+
if connection != nil {
86+
for signal := range connection.signal {
87+
if strings.Contains(signal.Name, "NameOwnerChanged") {
88+
firewalldRunning = checkRunning()
89+
dbusConnectionChanged(signal.Body)
90+
} else if strings.Contains(signal.Name, "Reloaded") {
91+
reloaded()
92+
}
93+
}
94+
}
95+
}
96+
97+
func dbusConnectionChanged(args []interface{}) {
98+
name := args[0].(string)
99+
old_owner := args[1].(string)
100+
new_owner := args[2].(string)
101+
102+
if name != dbusInterface {
103+
return
104+
}
105+
106+
if len(new_owner) > 0 {
107+
connectionEstablished()
108+
} else if len(old_owner) > 0 {
109+
connectionLost()
110+
}
111+
}
112+
113+
func connectionEstablished() {
114+
reloaded()
115+
}
116+
117+
func connectionLost() {
118+
// Doesn't do anything for now. Libvirt also doesn't react to this.
119+
}
120+
121+
// call all callbacks
122+
func reloaded() {
123+
for _, pf := range onReloaded {
124+
(*pf)()
125+
}
126+
}
127+
128+
// add callback
129+
func OnReloaded(callback func()) {
130+
for _, pf := range onReloaded {
131+
if pf == &callback {
132+
return
133+
}
134+
}
135+
onReloaded = append(onReloaded, &callback)
136+
}
137+
69138
// Call some remote method to see whether the service is actually running.
70139
func checkRunning() bool {
71140
var zone string

0 commit comments

Comments
 (0)