Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add docker interfaces to firewalld docker zone #2548

Merged
merged 1 commit into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 146 additions & 10 deletions iptables/firewalld.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,46 @@ const (
// Ebtables point to bridge table
Ebtables IPV = "eb"
)

const (
dbusInterface = "org.fedoraproject.FirewallD1"
dbusPath = "/org/fedoraproject/FirewallD1"
dbusInterface = "org.fedoraproject.FirewallD1"
dbusPath = "/org/fedoraproject/FirewallD1"
dbusConfigPath = "/org/fedoraproject/FirewallD1/config"
dockerZone = "docker"
)

// Conn is a connection to firewalld dbus endpoint.
type Conn struct {
sysconn *dbus.Conn
sysobj dbus.BusObject
signal chan *dbus.Signal
sysconn *dbus.Conn
sysObj dbus.BusObject
sysConfObj dbus.BusObject
signal chan *dbus.Signal
}

// ZoneSettings holds the firewalld zone settings, documented in
// https://firewalld.org/documentation/man-pages/firewalld.dbus.html
type ZoneSettings struct {
version string
name string
description string
unused bool
target string
services []string
ports [][]interface{}
icmpBlocks []string
masquerade bool
forwardPorts [][]interface{}
interfaces []string
sourceAddresses []string
richRules []string
protocols []string
sourcePorts [][]interface{}
icmpBlockInversion bool
}

var (
connection *Conn
connection *Conn

firewalldRunning bool // is Firewalld service running
onReloaded []*func() // callbacks when Firewalld has been reloaded
)
Expand All @@ -51,6 +77,9 @@ func FirewalldInit() error {
}
if connection != nil {
go signalHandler()
if err := setupDockerZone(); err != nil {
return err
}
}

return nil
Expand All @@ -76,8 +105,8 @@ func (c *Conn) initConnection() error {
}

// This never fails, even if the service is not running atm.
c.sysobj = c.sysconn.Object(dbusInterface, dbus.ObjectPath(dbusPath))

c.sysObj = c.sysconn.Object(dbusInterface, dbus.ObjectPath(dbusPath))
c.sysConfObj = c.sysconn.Object(dbusInterface, dbus.ObjectPath(dbusConfigPath))
rule := fmt.Sprintf("type='signal',path='%s',interface='%s',sender='%s',member='Reloaded'",
dbusPath, dbusInterface, dbusInterface)
c.sysconn.BusObject().Call("org.freedesktop.DBus.AddMatch", 0, rule)
Expand Down Expand Up @@ -150,7 +179,7 @@ func checkRunning() bool {
var err error

if connection != nil {
err = connection.sysobj.Call(dbusInterface+".getDefaultZone", 0).Store(&zone)
err = connection.sysObj.Call(dbusInterface+".getDefaultZone", 0).Store(&zone)
return err == nil
}
return false
Expand All @@ -160,8 +189,115 @@ func checkRunning() bool {
func Passthrough(ipv IPV, args ...string) ([]byte, error) {
var output string
logrus.Debugf("Firewalld passthrough: %s, %s", ipv, args)
if err := connection.sysobj.Call(dbusInterface+".direct.passthrough", 0, ipv, args).Store(&output); err != nil {
if err := connection.sysObj.Call(dbusInterface+".direct.passthrough", 0, ipv, args).Store(&output); err != nil {
return nil, err
}
return []byte(output), nil
}

// getDockerZoneSettings converts the ZoneSettings struct into a interface slice
func getDockerZoneSettings() []interface{} {
settings := ZoneSettings{
version: "1.0",
name: dockerZone,
description: "zone for docker bridge network interfaces",
target: "ACCEPT",
}
slice := []interface{}{
settings.version,
settings.name,
settings.description,
settings.unused,
settings.target,
settings.services,
settings.ports,
settings.icmpBlocks,
settings.masquerade,
settings.forwardPorts,
settings.interfaces,
settings.sourceAddresses,
settings.richRules,
settings.protocols,
settings.sourcePorts,
settings.icmpBlockInversion,
}
return slice

}

// setupDockerZone creates a zone called docker in firewalld which includes docker interfaces to allow
// container networking
func setupDockerZone() error {
var zones []string
// Check if zone exists
if err := connection.sysObj.Call(dbusInterface+".zone.getZones", 0).Store(&zones); err != nil {
return err
}
if contains(zones, dockerZone) {
logrus.Infof("Firewalld: %s zone already exists, returning", dockerZone)
return nil
}
logrus.Debugf("Firewalld: creating %s zone", dockerZone)

settings := getDockerZoneSettings()
// Permanent
if err := connection.sysConfObj.Call(dbusInterface+".config.addZone", 0, dockerZone, settings).Err; err != nil {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note that the addZone() method has been deprecated. The next firewalld feature release introduces addZone2() which is dictionary based and doesn't require all fields be filled. That being said, docker has to support older versions of firewalld so this is the correct one to call. In the future support for addZone2() should be considered.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, libvirt ships a static zone definition as part of the package. Therefore they avoid the addZone() and reload(). They also allow other things; dhcp, dns, icmp, ipv6-icmp. But they're also filtering (denying) other packets destined to the host.

return err
}
// Reload for change to take effect
if err := connection.sysObj.Call(dbusInterface+".reload", 0).Err; err != nil {
return err
}

return nil
}

// AddInterfaceFirewalld adds the interface to the trusted zone
func AddInterfaceFirewalld(intf string) error {
var intfs []string
// Check if interface is already added to the zone
if err := connection.sysObj.Call(dbusInterface+".zone.getInterfaces", 0, dockerZone).Store(&intfs); err != nil {
return err
}
// Return if interface is already part of the zone
if contains(intfs, intf) {
logrus.Infof("Firewalld: interface %s already part of %s zone, returning", intf, dockerZone)
return nil
}

logrus.Debugf("Firewalld: adding %s interface to %s zone", intf, dockerZone)
// Runtime
if err := connection.sysObj.Call(dbusInterface+".zone.addInterface", 0, dockerZone, intf).Err; err != nil {
return err
}
return nil
Comment on lines +258 to +273
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're probably aware, but this is racey. It's possible the interface gets deleted/added by another entity/user between the getInterfaces() and addInterface() calls. It may be simpler to always call addInterface() and handle the error codes appropriately. e.g. ALREADY_ENABLED.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep it this way, since AFAIK I would need to compare the error string to handle the above case, which might change in the future

}

// DelInterfaceFirewalld removes the interface from the trusted zone
func DelInterfaceFirewalld(intf string) error {
var intfs []string
// Check if interface is part of the zone
if err := connection.sysObj.Call(dbusInterface+".zone.getInterfaces", 0, dockerZone).Store(&intfs); err != nil {
return err
}
// Remove interface if it exists
if !contains(intfs, intf) {
return fmt.Errorf("Firewalld: unable to find interface %s in %s zone", intf, dockerZone)
}

logrus.Debugf("Firewalld: removing %s interface from %s zone", intf, dockerZone)
// Runtime
if err := connection.sysObj.Call(dbusInterface+".zone.removeInterface", 0, dockerZone, intf).Err; err != nil {
return err
}
Comment on lines +280 to +292
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment as above. Maybe always call removeInterface() and handle the errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment as above

return nil
}

func contains(list []string, val string) bool {
for _, v := range list {
if v == val {
return true
}
}
return false
}
13 changes: 13 additions & 0 deletions iptables/iptables.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,19 @@ func ProgramChain(c *ChainInfo, bridgeName string, hairpinMode, enable bool) err
return errors.New("Could not program chain, missing chain name")
}

// Either add or remove the interface from the firewalld zone
if firewalldRunning {
if enable {
if err := AddInterfaceFirewalld(bridgeName); err != nil {
return err
}
} else {
if err := DelInterfaceFirewalld(bridgeName); err != nil {
return err
}
}
}

switch c.Table {
case Nat:
preroute := []string{
Expand Down