[libvirt] [PATCH] Fix formatting of network address in iptables helpers

The network address was being set to 192.168.122.0 instead of 192.168.122.0/24. Fix this by removing the unneccessary 'network' field from virNetworkDef and just pass the network address and netmask into the iptables APIs directly. * src/conf/network_conf.h, src/conf/network_conf.c: Remove the 'network' field from virNEtworkDef. * src/network/bridge_driver.c: Update for iptables API changes * src/util/iptables.c, src/util/iptables.h: Require the network address + netmask pair to be passed in --- src/conf/network_conf.c | 4 - src/conf/network_conf.h | 1 - src/network/bridge_driver.c | 62 ++++++++++++------ src/util/iptables.c | 149 +++++++++++++++++++++++++------------------ src/util/iptables.h | 24 +++++-- 5 files changed, 142 insertions(+), 98 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 0bc5a54..3f2bf1f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -438,10 +438,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; } - def->network = def->ipAddress; - def->network.data.inet4.sin_addr.s_addr &= - def->netmask.data.inet4.sin_addr.s_addr; - if ((ip = virXPathNode("./ip[1]", ctxt)) && virNetworkIPParseXML(def, ip) < 0) goto error; diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 5a01bbf..7d31693 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -72,7 +72,6 @@ struct _virNetworkDef { virSocketAddr ipAddress; /* Bridge IP address */ virSocketAddr netmask; - virSocketAddr network; unsigned int nranges; /* Zero or more dhcp ranges */ virNetworkDHCPRangeDefPtr ranges; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5ee47eb..8721747 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -671,7 +671,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, int err; /* allow forwarding packets from the bridge interface */ if ((err = iptablesAddForwardAllowOut(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev))) { virReportSystemError(err, @@ -682,9 +683,10 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* allow forwarding packets to the bridge interface if they are part of an existing connection */ if ((err = iptablesAddForwardAllowRelatedIn(driver->iptables, - &network->def->network, - network->def->bridge, - network->def->forwardDev))) { + &network->def->ipAddress, + &network->def->netmask, + network->def->bridge, + network->def->forwardDev))) { virReportSystemError(err, _("failed to add iptables rule to allow forwarding to '%s'"), network->def->bridge); @@ -716,7 +718,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* First the generic masquerade rule for other protocols */ if ((err = iptablesAddForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, NULL))) { virReportSystemError(err, @@ -727,7 +730,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* UDP with a source port restriction */ if ((err = iptablesAddForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, "udp"))) { virReportSystemError(err, @@ -738,7 +742,8 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, /* TCP with a source port restriction */ if ((err = iptablesAddForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, "tcp"))) { virReportSystemError(err, @@ -751,22 +756,26 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver, masqerr5: iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, "udp"); masqerr4: iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, NULL); masqerr3: iptablesRemoveForwardAllowRelatedIn(driver->iptables, - &network->def->network, - network->def->bridge, - network->def->forwardDev); + &network->def->ipAddress, + &network->def->netmask, + network->def->bridge, + network->def->forwardDev); masqerr2: iptablesRemoveForwardAllowOut(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev); masqerr1: @@ -779,7 +788,8 @@ networkAddRoutingIptablesRules(struct network_driver *driver, int err; /* allow routing packets from the bridge interface */ if ((err = iptablesAddForwardAllowOut(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev))) { virReportSystemError(err, @@ -790,7 +800,8 @@ networkAddRoutingIptablesRules(struct network_driver *driver, /* allow routing packets to the bridge interface */ if ((err = iptablesAddForwardAllowIn(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev))) { virReportSystemError(err, @@ -804,7 +815,8 @@ networkAddRoutingIptablesRules(struct network_driver *driver, routeerr2: iptablesRemoveForwardAllowOut(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev); routeerr1: @@ -943,29 +955,35 @@ networkRemoveIptablesRules(struct network_driver *driver, if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE) { if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, "tcp"); iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, "udp"); iptablesRemoveForwardMasquerade(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->forwardDev, NULL); iptablesRemoveForwardAllowRelatedIn(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev); } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) iptablesRemoveForwardAllowIn(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev); iptablesRemoveForwardAllowOut(driver->iptables, - &network->def->network, + &network->def->ipAddress, + &network->def->netmask, network->def->bridge, network->def->forwardDev); } diff --git a/src/util/iptables.c b/src/util/iptables.c index 64efd45..fc656b9 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -44,8 +44,9 @@ #include "virterror_internal.h" #include "logging.h" +#define VIR_FROM_THIS VIR_FROM_NONE #define iptablesError(code, ...) \ - virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__, \ + virReportErrorHelper(NULL, VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) enum { @@ -323,26 +324,55 @@ iptablesRemoveUdpInput(iptablesContext *ctx, } +static char *iptablesFormatNetwork(virSocketAddr *netaddr, + virSocketAddr *netmask) +{ + virSocketAddr network; + int prefix; + char *netstr; + char *ret; + + if (!VIR_SOCKET_IS_FAMILY(netaddr, AF_INET) || + !VIR_SOCKET_IS_FAMILY(netmask, AF_INET)) { + iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Only IPv4 addresses can be used with iptables")); + return NULL; + } + + network = *netaddr; + network.data.inet4.sin_addr.s_addr &= + netmask->data.inet4.sin_addr.s_addr; + + prefix = virSocketGetNumNetmaskBits(netmask); + + netstr = virSocketFormatAddr(&network); + + if (!netstr) + return NULL; + + if (virAsprintf(&ret, "%s/%d", netstr, prefix) < 0) + virReportOOMError(); + + VIR_FREE(netstr); + return ret; +} + + /* Allow all traffic coming from the bridge, with a valid network address * to proceed to WAN */ static int iptablesForwardAllowOut(iptablesContext *ctx, - virSocketAddr *network, - const char *iface, - const char *physdev, - int action) + virSocketAddr *netaddr, + virSocketAddr *netmask, + const char *iface, + const char *physdev, + int action) { int ret; char *networkstr; - if (!VIR_SOCKET_IS_FAMILY(network, AF_INET)) { - iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Only IPv4 addresses can be used with iptables")); - return -1; - } - - if (!(networkstr = virSocketFormatAddr(network))) + if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) return -1; if (physdev && physdev[0]) { @@ -380,11 +410,12 @@ iptablesForwardAllowOut(iptablesContext *ctx, */ int iptablesAddForwardAllowOut(iptablesContext *ctx, - virSocketAddr *network, - const char *iface, - const char *physdev) + virSocketAddr *netaddr, + virSocketAddr *netmask, + const char *iface, + const char *physdev) { - return iptablesForwardAllowOut(ctx, network, iface, physdev, ADD); + return iptablesForwardAllowOut(ctx, netaddr, netmask, iface, physdev, ADD); } /** @@ -402,11 +433,12 @@ iptablesAddForwardAllowOut(iptablesContext *ctx, */ int iptablesRemoveForwardAllowOut(iptablesContext *ctx, - virSocketAddr *network, - const char *iface, - const char *physdev) + virSocketAddr *netaddr, + virSocketAddr *netmask, + const char *iface, + const char *physdev) { - return iptablesForwardAllowOut(ctx, network, iface, physdev, REMOVE); + return iptablesForwardAllowOut(ctx, netaddr, netmask, iface, physdev, REMOVE); } @@ -415,21 +447,16 @@ iptablesRemoveForwardAllowOut(iptablesContext *ctx, */ static int iptablesForwardAllowRelatedIn(iptablesContext *ctx, - virSocketAddr *network, - const char *iface, - const char *physdev, - int action) + virSocketAddr *netaddr, + virSocketAddr *netmask, + const char *iface, + const char *physdev, + int action) { int ret; char *networkstr; - if (!VIR_SOCKET_IS_FAMILY(network, AF_INET)) { - iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Only IPv4 addresses can be used with iptables")); - return -1; - } - - if (!(networkstr = virSocketFormatAddr(network))) + if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) return -1; if (physdev && physdev[0]) { @@ -471,11 +498,12 @@ iptablesForwardAllowRelatedIn(iptablesContext *ctx, */ int iptablesAddForwardAllowRelatedIn(iptablesContext *ctx, - virSocketAddr *network, - const char *iface, - const char *physdev) + virSocketAddr *netaddr, + virSocketAddr *netmask, + const char *iface, + const char *physdev) { - return iptablesForwardAllowRelatedIn(ctx, network, iface, physdev, ADD); + return iptablesForwardAllowRelatedIn(ctx, netaddr, netmask, iface, physdev, ADD); } /** @@ -493,18 +521,20 @@ iptablesAddForwardAllowRelatedIn(iptablesContext *ctx, */ int iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx, - virSocketAddr *network, - const char *iface, - const char *physdev) + virSocketAddr *netaddr, + virSocketAddr *netmask, + const char *iface, + const char *physdev) { - return iptablesForwardAllowRelatedIn(ctx, network, iface, physdev, REMOVE); + return iptablesForwardAllowRelatedIn(ctx, netaddr, netmask, iface, physdev, REMOVE); } /* Allow all traffic destined to the bridge, with a valid network address */ static int iptablesForwardAllowIn(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev, int action) @@ -512,13 +542,7 @@ iptablesForwardAllowIn(iptablesContext *ctx, int ret; char *networkstr; - if (!VIR_SOCKET_IS_FAMILY(network, AF_INET)) { - iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Only IPv4 addresses can be used with iptables")); - return -1; - } - - if (!(networkstr = virSocketFormatAddr(network))) + if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) return -1; if (physdev && physdev[0]) { @@ -556,11 +580,12 @@ iptablesForwardAllowIn(iptablesContext *ctx, */ int iptablesAddForwardAllowIn(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev) { - return iptablesForwardAllowIn(ctx, network, iface, physdev, ADD); + return iptablesForwardAllowIn(ctx, netaddr, netmask, iface, physdev, ADD); } /** @@ -578,11 +603,12 @@ iptablesAddForwardAllowIn(iptablesContext *ctx, */ int iptablesRemoveForwardAllowIn(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev) { - return iptablesForwardAllowIn(ctx, network, iface, physdev, REMOVE); + return iptablesForwardAllowIn(ctx, netaddr, netmask, iface, physdev, REMOVE); } @@ -744,7 +770,8 @@ iptablesRemoveForwardRejectIn(iptablesContext *ctx, */ static int iptablesForwardMasquerade(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *physdev, const char *protocol, int action) @@ -752,13 +779,7 @@ iptablesForwardMasquerade(iptablesContext *ctx, int ret; char *networkstr; - if (!VIR_SOCKET_IS_FAMILY(network, AF_INET)) { - iptablesError(VIR_ERR_CONFIG_UNSUPPORTED, - _("Only IPv4 addresses can be used with iptables")); - return -1; - } - - if (!(networkstr = virSocketFormatAddr(network))) + if (!(networkstr = iptablesFormatNetwork(netaddr, netmask))) return -1; if (protocol && protocol[0]) { @@ -819,11 +840,12 @@ iptablesForwardMasquerade(iptablesContext *ctx, */ int iptablesAddForwardMasquerade(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *physdev, const char *protocol) { - return iptablesForwardMasquerade(ctx, network, physdev, protocol, ADD); + return iptablesForwardMasquerade(ctx, netaddr, netmask, physdev, protocol, ADD); } /** @@ -841,11 +863,12 @@ iptablesAddForwardMasquerade(iptablesContext *ctx, */ int iptablesRemoveForwardMasquerade(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *physdev, const char *protocol) { - return iptablesForwardMasquerade(ctx, network, physdev, protocol, REMOVE); + return iptablesForwardMasquerade(ctx, netaddr, netmask, physdev, protocol, REMOVE); } diff --git a/src/util/iptables.h b/src/util/iptables.h index fd49685..ed843ec 100644 --- a/src/util/iptables.h +++ b/src/util/iptables.h @@ -44,29 +44,35 @@ int iptablesRemoveUdpInput (iptablesContext *ctx, int port); int iptablesAddForwardAllowOut (iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev); int iptablesRemoveForwardAllowOut (iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev); int iptablesAddForwardAllowRelatedIn(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev); int iptablesRemoveForwardAllowRelatedIn(iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev); int iptablesAddForwardAllowIn (iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev); int iptablesRemoveForwardAllowIn (iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *iface, const char *physdev); @@ -86,11 +92,13 @@ int iptablesRemoveForwardRejectIn (iptablesContext *ctx, const char *iface); int iptablesAddForwardMasquerade (iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *physdev, const char *protocol); int iptablesRemoveForwardMasquerade (iptablesContext *ctx, - virSocketAddr *network, + virSocketAddr *netaddr, + virSocketAddr *netmask, const char *physdev, const char *protocol); int iptablesAddOutputFixUdpChecksum (iptablesContext *ctx, -- 1.7.2.3