network: validate DHCP ranges are completely within defined network

virSocketAddrGetRange() has been updated to take the network address
and prefix, and now checks that both the start and end of the range
are within that network, thus validating that the entire range of
addresses is in the network. For IPv4, it also checks that ranges to
not start with the "network address" of the subnet, nor end with the
broadcast address of the subnet (this check doesn't apply to IPv6,
since IPv6 doesn't have a broadcast or network address)

Negative tests have been added to the network update and socket tests
to verify that bad ranges properly generate an error.

This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=985653
This commit is contained in:
Laine Stump 2015-05-22 17:32:02 -04:00
parent 48e8b95d8e
commit 1e334a0a00
10 changed files with 112 additions and 40 deletions

View File

@ -832,6 +832,7 @@ int virNetworkIpDefNetmask(const virNetworkIpDef *def,
static int static int
virSocketAddrRangeParseXML(const char *networkName, virSocketAddrRangeParseXML(const char *networkName,
virNetworkIpDefPtr ipdef,
xmlNodePtr node, xmlNodePtr node,
virSocketAddrRangePtr range) virSocketAddrRangePtr range)
{ {
@ -859,7 +860,8 @@ virSocketAddrRangeParseXML(const char *networkName,
goto cleanup; goto cleanup;
/* do a sanity check of the range */ /* do a sanity check of the range */
if (virSocketAddrGetRange(&range->start, &range->end) < 0) { if (virSocketAddrGetRange(&range->start, &range->end, &ipdef->address,
virNetworkIpDefPrefix(ipdef)) < 0) {
virReportError(VIR_ERR_XML_ERROR, virReportError(VIR_ERR_XML_ERROR,
_("Invalid dhcp range '%s' to '%s' in network '%s'"), _("Invalid dhcp range '%s' to '%s' in network '%s'"),
start, end, networkName); start, end, networkName);
@ -1009,7 +1011,7 @@ virNetworkDHCPDefParseXML(const char *networkName,
if (VIR_REALLOC_N(def->ranges, def->nranges + 1) < 0) if (VIR_REALLOC_N(def->ranges, def->nranges + 1) < 0)
return -1; return -1;
if (virSocketAddrRangeParseXML(networkName, cur, if (virSocketAddrRangeParseXML(networkName, def, cur,
&def->ranges[def->nranges]) < 0) { &def->ranges[def->nranges]) < 0) {
return -1; return -1;
} }
@ -3608,7 +3610,7 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def,
goto cleanup; goto cleanup;
} }
if (virSocketAddrRangeParseXML(def->name, ctxt->node, &range) < 0) if (virSocketAddrRangeParseXML(def->name, ipdef, ctxt->node, &range) < 0)
goto cleanup; goto cleanup;
/* check if an entry with same name/address/ip already exists */ /* check if an entry with same name/address/ip already exists */

View File

@ -1193,7 +1193,9 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
VIR_FREE(saddr); VIR_FREE(saddr);
VIR_FREE(eaddr); VIR_FREE(eaddr);
nbleases += virSocketAddrGetRange(&ipdef->ranges[r].start, nbleases += virSocketAddrGetRange(&ipdef->ranges[r].start,
&ipdef->ranges[r].end); &ipdef->ranges[r].end,
&ipdef->address,
virNetworkIpDefPrefix(ipdef));
} }
/* /*

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2009-2014 Red Hat, Inc. * Copyright (C) 2009-2015 Red Hat, Inc.
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public * modify it under the terms of the GNU Lesser General Public
@ -605,31 +605,69 @@ int virSocketAddrCheckNetmask(virSocketAddrPtr addr1, virSocketAddrPtr addr2,
* virSocketGetRange: * virSocketGetRange:
* @start: start of an IP range * @start: start of an IP range
* @end: end of an IP range * @end: end of an IP range
* @network: IP address of network that should completely contain this range
* @prefix: prefix of the network
* *
* Check the order of the 2 addresses and compute the range, this * Check the order of the 2 addresses and compute the range, this will
* will return 1 for identical addresses. Errors can come from incompatible * return 1 for identical addresses. Errors can come from incompatible
* addresses type, excessive range (>= 2^^16) where the two addresses are * addresses type, excessive range (>= 2^^16) where the two addresses
* unrelated or inverted start and end. * are unrelated, inverted start and end, or a range that is not
* within network/prefix.
* *
* Returns the size of the range or -1 in case of failure * Returns the size of the range or -1 in case of failure
*/ */
int virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end) int
virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end,
virSocketAddrPtr network, int prefix)
{ {
int ret = 0; int ret = 0;
size_t i; size_t i;
virSocketAddr netmask;
if ((start == NULL) || (end == NULL)) if (start == NULL || end == NULL || network == NULL)
return -1;
if (start->data.stor.ss_family != end->data.stor.ss_family)
return -1; return -1;
if (start->data.stor.ss_family == AF_INET) { if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(end) ||
VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network))
return -1;
if (prefix < 0 ||
virSocketAddrPrefixToNetmask(prefix, &netmask, VIR_SOCKET_ADDR_FAMILY(network)) < 0)
return -1;
/* both start and end of range need to be in the same network as
* "network"
*/
if (virSocketAddrCheckNetmask(start, network, &netmask) <= 0 ||
virSocketAddrCheckNetmask(end, network, &netmask) <= 0)
return -1;
if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) {
virSocketAddrIPv4 t1, t2; virSocketAddrIPv4 t1, t2;
virSocketAddr netaddr, broadcast;
if (virSocketAddrBroadcast(network, &netmask, &broadcast) < 0 ||
virSocketAddrMask(network, &netmask, &netaddr) < 0)
return -1;
/* Don't allow the start of the range to be the network
* address (usually "...0") or the end of the range to be the
* broadcast address (usually "...255"). (the opposite also
* isn't allowed, but checking for that is implicit in all the
* other combined checks) (IPv6 doesn't have broadcast and
* network addresses, so this check is only done for IPv4)
*/
if (virSocketAddrEqual(start, &netaddr) ||
virSocketAddrEqual(end, &broadcast))
return -1;
if ((virSocketAddrGetIPv4Addr(start, &t1) < 0) || if ((virSocketAddrGetIPv4Addr(start, &t1) < 0) ||
(virSocketAddrGetIPv4Addr(end, &t2) < 0)) (virSocketAddrGetIPv4Addr(end, &t2) < 0))
return -1; return -1;
/* legacy check that everything except the last two bytes are
* the same
*/
for (i = 0; i < 2; i++) { for (i = 0; i < 2; i++) {
if (t1[i] != t2[i]) if (t1[i] != t2[i])
return -1; return -1;
@ -638,13 +676,16 @@ int virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end)
if (ret < 0) if (ret < 0)
return -1; return -1;
ret++; ret++;
} else if (start->data.stor.ss_family == AF_INET6) { } else if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET6)) {
virSocketAddrIPv6 t1, t2; virSocketAddrIPv6 t1, t2;
if ((virSocketAddrGetIPv6Addr(start, &t1) < 0) || if ((virSocketAddrGetIPv6Addr(start, &t1) < 0) ||
(virSocketAddrGetIPv6Addr(end, &t2) < 0)) (virSocketAddrGetIPv6Addr(end, &t2) < 0))
return -1; return -1;
/* legacy check that everything except the last two bytes are
* the same
*/
for (i = 0; i < 7; i++) { for (i = 0; i < 7; i++) {
if (t1[i] != t2[i]) if (t1[i] != t2[i])
return -1; return -1;

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2009-2013 Red Hat, Inc. * Copyright (C) 2009-2013, 2015 Red Hat, Inc.
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public * modify it under the terms of the GNU Lesser General Public
@ -96,7 +96,9 @@ int virSocketAddrSetPort(virSocketAddrPtr addr, int port);
int virSocketAddrGetPort(virSocketAddrPtr addr); int virSocketAddrGetPort(virSocketAddrPtr addr);
int virSocketAddrGetRange(virSocketAddrPtr start, int virSocketAddrGetRange(virSocketAddrPtr start,
virSocketAddrPtr end); virSocketAddrPtr end,
virSocketAddrPtr network,
int prefix);
int virSocketAddrIsNetmask(virSocketAddrPtr netmask); int virSocketAddrIsNetmask(virSocketAddrPtr netmask);

View File

@ -0,0 +1 @@
<range start='10.0.0.10' end='10.0.0.100'/>

View File

@ -1 +1 @@
<range start='10.0.0.10' end='10.0.0.100'/> <range start='192.168.122.10' end='192.168.122.100'/>

View File

@ -8,7 +8,7 @@
<mac address='12:34:56:78:9a:bc'/> <mac address='12:34:56:78:9a:bc'/>
<ip address='192.168.122.1' netmask='255.255.255.0'> <ip address='192.168.122.1' netmask='255.255.255.0'>
<dhcp> <dhcp>
<range start='10.0.0.10' end='10.0.0.100'/> <range start='192.168.122.10' end='192.168.122.100'/>
<host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/> <host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/>
<host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/> <host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/>
</dhcp> </dhcp>

View File

@ -8,7 +8,7 @@
<mac address='12:34:56:78:9a:bc'/> <mac address='12:34:56:78:9a:bc'/>
<ip address='192.168.122.1' netmask='255.255.255.0'> <ip address='192.168.122.1' netmask='255.255.255.0'>
<dhcp> <dhcp>
<range start='10.0.0.10' end='10.0.0.100'/> <range start='192.168.122.10' end='192.168.122.100'/>
<host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/> <host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/>
<host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/> <host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/>
</dhcp> </dhcp>

View File

@ -202,6 +202,11 @@ mymain(void)
"dhcp6host-routed-network-range", "dhcp6host-routed-network-range",
VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST, VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST,
0); 0);
DO_TEST_INDEX_FAIL("add-dhcp-range-outside-net",
"dhcp-range-10",
"dhcp6host-routed-network",
VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST,
0);
DO_TEST_INDEX("append-dhcp-range", DO_TEST_INDEX("append-dhcp-range",
"dhcp-range", "dhcp-range",
"dhcp6host-routed-network", "dhcp6host-routed-network",

View File

@ -1,7 +1,7 @@
/* /*
* sockettest.c: Testing for src/util/network.c APIs * sockettest.c: Testing for src/util/network.c APIs
* *
* Copyright (C) 2010-2011, 2014 Red Hat, Inc. * Copyright (C) 2010-2011, 2014, 2015 Red Hat, Inc.
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public * modify it under the terms of the GNU Lesser General Public
@ -86,17 +86,22 @@ static int testFormatHelper(const void *opaque)
} }
static int testRange(const char *saddrstr, const char *eaddrstr, int size, bool pass) static int
testRange(const char *saddrstr, const char *eaddrstr,
const char *netstr, int prefix, int size, bool pass)
{ {
virSocketAddr saddr; virSocketAddr saddr;
virSocketAddr eaddr; virSocketAddr eaddr;
virSocketAddr netaddr;
if (virSocketAddrParse(&saddr, saddrstr, AF_UNSPEC) < 0) if (virSocketAddrParse(&saddr, saddrstr, AF_UNSPEC) < 0)
return -1; return -1;
if (virSocketAddrParse(&eaddr, eaddrstr, AF_UNSPEC) < 0) if (virSocketAddrParse(&eaddr, eaddrstr, AF_UNSPEC) < 0)
return -1; return -1;
if (virSocketAddrParse(&netaddr, netstr, AF_UNSPEC) < 0)
return -1;
int gotsize = virSocketAddrGetRange(&saddr, &eaddr); int gotsize = virSocketAddrGetRange(&saddr, &eaddr, &netaddr, prefix);
VIR_DEBUG("Size want %d vs got %d", size, gotsize); VIR_DEBUG("Size want %d vs got %d", size, gotsize);
if (pass) { if (pass) {
/* fail if virSocketAddrGetRange returns failure, or unexpected size */ /* fail if virSocketAddrGetRange returns failure, or unexpected size */
@ -107,16 +112,23 @@ static int testRange(const char *saddrstr, const char *eaddrstr, int size, bool
} }
} }
struct testRangeData { struct testRangeData {
const char *saddr; const char *saddr;
const char *eaddr; const char *eaddr;
const char *netaddr;
int prefix;
int size; int size;
bool pass; bool pass;
}; };
static int testRangeHelper(const void *opaque) static int testRangeHelper(const void *opaque)
{ {
const struct testRangeData *data = opaque; const struct testRangeData *data = opaque;
return testRange(data->saddr, data->eaddr, data->size, data->pass); return testRange(data->saddr, data->eaddr,
data->netaddr, data->prefix,
data->size, data->pass);
} }
@ -289,10 +301,12 @@ mymain(void)
ret = -1; \ ret = -1; \
} while (0) } while (0)
#define DO_TEST_RANGE(saddr, eaddr, size, pass) \ #define DO_TEST_RANGE(saddr, eaddr, netaddr, prefix, size, pass) \
do { \ do { \
struct testRangeData data = { saddr, eaddr, size, pass }; \ struct testRangeData data \
if (virtTestRun("Test range " saddr " -> " eaddr " size " #size, \ = { saddr, eaddr, netaddr, prefix, size, pass }; \
if (virtTestRun("Test range " saddr " -> " eaddr "(" netaddr \
"/" #prefix") size " #size, \
testRangeHelper, &data) < 0) \ testRangeHelper, &data) < 0) \
ret = -1; \ ret = -1; \
} while (0) } while (0)
@ -359,17 +373,22 @@ mymain(void)
DO_TEST_PARSE_AND_FORMAT("::1", AF_UNIX, false); DO_TEST_PARSE_AND_FORMAT("::1", AF_UNIX, false);
DO_TEST_PARSE_AND_FORMAT("::ffff", AF_UNSPEC, true); DO_TEST_PARSE_AND_FORMAT("::ffff", AF_UNSPEC, true);
DO_TEST_RANGE("192.168.122.1", "192.168.122.1", 1, true); DO_TEST_RANGE("192.168.122.1", "192.168.122.1", "192.168.122.1", 24, 1, true);
DO_TEST_RANGE("192.168.122.1", "192.168.122.20", 20, true); DO_TEST_RANGE("192.168.122.1", "192.168.122.20", "192.168.122.22", 24, 20, true);
DO_TEST_RANGE("192.168.122.0", "192.168.122.255", 256, true); DO_TEST_RANGE("192.168.122.0", "192.168.122.254", "192.168.122.1", 24, -1, false);
DO_TEST_RANGE("192.168.122.20", "192.168.122.1", -1, false); DO_TEST_RANGE("192.168.122.1", "192.168.122.255", "192.168.122.1", 24, -1, false);
DO_TEST_RANGE("10.0.0.1", "192.168.122.20", -1, false); DO_TEST_RANGE("192.168.122.0", "192.168.122.255", "192.168.122.1", 16, 256, true);
DO_TEST_RANGE("192.168.122.20", "10.0.0.1", -1, false); DO_TEST_RANGE("192.168.122.20", "192.168.122.1", "192.168.122.1", 24, -1, false);
DO_TEST_RANGE("10.0.0.1", "192.168.122.20", "192.168.122.1", 24, -1, false);
DO_TEST_RANGE("192.168.122.20", "10.0.0.1", "192.168.122.1", 24, -1, false);
DO_TEST_RANGE("172.16.0.50", "172.16.0.254", "1.2.3.4", 8, -1, false);
DO_TEST_RANGE("192.168.122.1", "192.168.123.20", "192.168.122.22", 24, -1, false);
DO_TEST_RANGE("192.168.122.1", "192.168.123.20", "192.168.122.22", 23, 276, true);
DO_TEST_RANGE("2000::1", "2000::1", 1, true); DO_TEST_RANGE("2000::1", "2000::1", "2000::1", 64, 1, true);
DO_TEST_RANGE("2000::1", "2000::2", 2, true); DO_TEST_RANGE("2000::1", "2000::2", "2000::1", 64, 2, true);
DO_TEST_RANGE("2000::2", "2000::1", -1, false); DO_TEST_RANGE("2000::2", "2000::1", "2000::1", 64, -1, false);
DO_TEST_RANGE("2000::1", "9001::1", -1, false); DO_TEST_RANGE("2000::1", "9001::1", "2000::1", 64, -1, false);
DO_TEST_NETMASK("192.168.122.1", "192.168.122.2", DO_TEST_NETMASK("192.168.122.1", "192.168.122.2",
"255.255.255.0", true); "255.255.255.0", true);