External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 05:35:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

network: validate network NAT range

Browse Source 
This patch modifies virSocketAddrGetRange() to function properly when
the containing network/prefix of the address range isn't known, for
example in the case of the NAT range of a virtual network (since it is
a range of addresses on the *host*, not within the network itself). We
then take advantage of this new functionality to validate the NAT
range of a virtual network.

Extra test cases are also added to verify that virSocketAddrGetRange()
works properly in both positive and negative cases when the network
pointer is NULL.

This is the *real* fix for:

https://bugzilla.redhat.com/show_bug.cgi?id=985653

Commits 1e334a and 48e8b9 had earlier been pushed as fixes for that
bug, but I had neglected to read the report carefully, so instead of
fixing validation for the NAT range, I had fixed validation for the
DHCP range. sigh.
This commit is contained in:
Laine Stump 2015-08-08 17:46:41 -04:00
parent cf0404455c
commit a6f9af8292
3 changed files with 144 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/conf/network_conf.c
View File

@ -1731,6 +1731,10 @@ virNetworkForwardNatDefParseXML(const char *networkName,
goto cleanup;
}
/* verify that start <= end */
if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0)
goto cleanup;
/* ports for SNAT and MASQUERADE */
nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes);
if (nNatPorts < 0) {

184
src/util/virsocketaddr.c
View File

@ -628,126 +628,136 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end,
virSocketAddr netmask;
char *startStr = NULL, *endStr = NULL, *netStr = NULL;
if (start == NULL || end == NULL || network == NULL) {
if (start == NULL || end == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("NULL argument - %p %p %p"), start, end, network);
_("NULL argument - %p %p"), start, end);
goto error;
}
startStr = virSocketAddrFormat(start);
endStr = virSocketAddrFormat(end);
netStr = virSocketAddrFormat(network);
if (!(startStr && endStr && netStr))
if (!startStr || !endStr)
goto error; /*error already reported */
if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(end) ||
VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network)) {
if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(end)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("mismatch of address family in "
"range %s - %s for network %s"),
startStr, endStr, netStr);
_("mismatch of address family in range %s - %s"),
startStr, endStr);
goto error;
}
if (prefix < 0 ||
virSocketAddrPrefixToNetmask(prefix, &netmask,
VIR_SOCKET_ADDR_FAMILY(network)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("bad prefix %d for network %s when "
" checking range %s - %s"),
prefix, netStr, startStr, endStr);
goto error;
}
if (network) {
/* some checks can only be done if we have details of the
* network the range should be within
*/
if (!(netStr = virSocketAddrFormat(network)))
goto error;
/* 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) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("range %s - %s is not entirely within "
"network %s/%d"),
startStr, endStr, netStr, prefix);
goto error;
if (VIR_SOCKET_ADDR_FAMILY(start) != VIR_SOCKET_ADDR_FAMILY(network)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("mismatch of address family in "
"range %s - %s for network %s"),
startStr, endStr, netStr);
goto error;
}
if (prefix < 0 ||
virSocketAddrPrefixToNetmask(prefix, &netmask,
VIR_SOCKET_ADDR_FAMILY(network)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("bad prefix %d for network %s when "
" checking range %s - %s"),
prefix, netStr, startStr, endStr);
goto error;
}
/* both start and end of range need to be within network */
if (virSocketAddrCheckNetmask(start, network, &netmask) <= 0 ||
virSocketAddrCheckNetmask(end, network, &netmask) <= 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("range %s - %s is not entirely within "
"network %s/%d"),
startStr, endStr, netStr, prefix);
goto error;
}
if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) {
virSocketAddr netaddr, broadcast;
if (virSocketAddrBroadcast(network, &netmask, &broadcast) < 0 ||
virSocketAddrMask(network, &netmask, &netaddr) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to construct broadcast or network "
"address for network %s/%d"),
netStr, prefix);
goto error;
}
/* 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)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("start of range %s - %s in network %s/%d "
"is the network address"),
startStr, endStr, netStr, prefix);
goto error;
}
if (virSocketAddrEqual(end, &broadcast)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("end of range %s - %s in network %s/%d "
"is the broadcast address"),
startStr, endStr, netStr, prefix);
goto error;
}
}
}
if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET)) {
virSocketAddrIPv4 t1, t2;
virSocketAddr netaddr, broadcast;
if (virSocketAddrBroadcast(network, &netmask, &broadcast) < 0 ||
virSocketAddrMask(network, &netmask, &netaddr) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to construct broadcast or network "
"address for network %s/%d"),
netStr, prefix);
goto error;
}
/* 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)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("start of range %s - %s in network %s/%d "
"is the network address"),
startStr, endStr, netStr, prefix);
goto error;
}
if (virSocketAddrEqual(end, &broadcast)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("end of range %s - %s in network %s/%d "
"is the broadcast address"),
startStr, endStr, netStr, prefix);
goto error;
}
if ((virSocketAddrGetIPv4Addr(start, &t1) < 0) ||
(virSocketAddrGetIPv4Addr(end, &t2) < 0)) {
if (virSocketAddrGetIPv4Addr(start, &t1) < 0 ||
virSocketAddrGetIPv4Addr(end, &t2) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to get IPv4 address "
"for start or end of range %s - %s "
"in network %s/%d"),
startStr, endStr, netStr, prefix);
"for start or end of range %s - %s"),
startStr, endStr);
goto error;
}
/* legacy check that everything except the last two bytes are
* the same
/* legacy check that everything except the last two bytes
* are the same
*/
for (i = 0; i < 2; i++) {
if (t1[i] != t2[i]) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("range %s - %s is too large (> 65535) "
"in network %s/%d"),
startStr, endStr, netStr, prefix);
_("range %s - %s is too large (> 65535)"),
startStr, endStr);
goto error;
}
}
ret = (t2[2] - t1[2]) * 256 + (t2[3] - t1[3]);
if (ret < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("range %s - %s is reversed "
"in network %s/%d"),
startStr, endStr, netStr, prefix);
_("range %s - %s is reversed "),
startStr, endStr);
goto error;
}
ret++;
} else if (VIR_SOCKET_ADDR_IS_FAMILY(start, AF_INET6)) {
virSocketAddrIPv6 t1, t2;
if ((virSocketAddrGetIPv6Addr(start, &t1) < 0) ||
(virSocketAddrGetIPv6Addr(end, &t2) < 0)) {
if (virSocketAddrGetIPv6Addr(start, &t1) < 0 ||
virSocketAddrGetIPv6Addr(end, &t2) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("failed to get IPv6 address "
"for start or end of range %s - %s "
"in network %s/%d"),
startStr, endStr, netStr, prefix);
"for start or end of range %s - %s"),
startStr, endStr);
goto error;
}
@ -757,29 +767,27 @@ virSocketAddrGetRange(virSocketAddrPtr start, virSocketAddrPtr end,
for (i = 0; i < 7; i++) {
if (t1[i] != t2[i]) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("range %s - %s is too large (> 65535) "
"in network %s/%d"),
startStr, endStr, netStr, prefix);
_("range %s - %s is too large (> 65535)"),
startStr, endStr);
goto error;
}
}
ret = t2[7] - t1[7];
if (ret < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("range %s - %s start larger than end "
"in network %s/%d"),
startStr, endStr, netStr, prefix);
_("range %s - %s start larger than end"),
startStr, endStr);
goto error;
}
ret++;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unsupported address family "
"for range %s - %s "
"in network %s/%d, must be ipv4 or ipv6"),
startStr, endStr, netStr, prefix);
"for range %s - %s, must be ipv4 or ipv6"),
startStr, endStr);
goto error;
}
cleanup:
VIR_FREE(startStr);
VIR_FREE(endStr);

46
tests/sockettest.c
View File

@ -98,10 +98,11 @@ testRange(const char *saddrstr, const char *eaddrstr,
return -1;
if (virSocketAddrParse(&eaddr, eaddrstr, AF_UNSPEC) < 0)
return -1;
if (virSocketAddrParse(&netaddr, netstr, AF_UNSPEC) < 0)
if (netstr && virSocketAddrParse(&netaddr, netstr, AF_UNSPEC) < 0)
return -1;
int gotsize = virSocketAddrGetRange(&saddr, &eaddr, &netaddr, prefix);
int gotsize = virSocketAddrGetRange(&saddr, &eaddr,
netstr ? &netaddr : NULL, prefix);
VIR_DEBUG("Size want %d vs got %d", size, gotsize);
if (pass) {
/* fail if virSocketAddrGetRange returns failure, or unexpected size */
@ -311,6 +312,15 @@ mymain(void)
ret = -1; \
} while (0)
#define DO_TEST_RANGE_SIMPLE(saddr, eaddr, size, pass) \
do { \
struct testRangeData data \
= { saddr, eaddr, NULL, 0, size, pass }; \
if (virtTestRun("Test range " saddr " -> " eaddr "size " #size, \
testRangeHelper, &data) < 0) \
ret = -1; \
} while (0)
#define DO_TEST_NETMASK(addr1, addr2, netmask, pass) \
do { \
struct testNetmaskData data = { addr1, addr2, netmask, pass }; \
@ -373,23 +383,55 @@ mymain(void)
DO_TEST_PARSE_AND_FORMAT("::1", AF_UNIX, false);
DO_TEST_PARSE_AND_FORMAT("::ffff", AF_UNSPEC, true);
/* tests that specify a network that should contain the range */
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", "192.168.122.22", 24, 20, true);
/* start of range is "network address" */
DO_TEST_RANGE("192.168.122.0", "192.168.122.254", "192.168.122.1", 24, -1, false);
/* end of range is "broadcast address" */
DO_TEST_RANGE("192.168.122.1", "192.168.122.255", "192.168.122.1", 24, -1, false);
DO_TEST_RANGE("192.168.122.0", "192.168.122.255", "192.168.122.1", 16, 256, true);
/* range is reversed */
DO_TEST_RANGE("192.168.122.20", "192.168.122.1", "192.168.122.1", 24, -1, false);
/* start address outside network */
DO_TEST_RANGE("10.0.0.1", "192.168.122.20", "192.168.122.1", 24, -1, false);
/* end address outside network and range reversed */
DO_TEST_RANGE("192.168.122.20", "10.0.0.1", "192.168.122.1", 24, -1, false);
/* entire range outside network */
DO_TEST_RANGE("172.16.0.50", "172.16.0.254", "1.2.3.4", 8, -1, false);
/* end address outside network */
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", "2000::1", 64, 1, true);
DO_TEST_RANGE("2000::1", "2000::2", "2000::1", 64, 2, true);
/* range reversed */
DO_TEST_RANGE("2000::2", "2000::1", "2000::1", 64, -1, false);
/* range too large (> 65536) */
DO_TEST_RANGE("2000::1", "9001::1", "2000::1", 64, -1, false);
/* tests that *don't* specify a containing network
* (so fewer things can be checked)
*/
DO_TEST_RANGE_SIMPLE("192.168.122.1", "192.168.122.1", 1, true);
DO_TEST_RANGE_SIMPLE("192.168.122.1", "192.168.122.20", 20, true);
DO_TEST_RANGE_SIMPLE("192.168.122.0", "192.168.122.255", 256, true);
/* range is reversed */
DO_TEST_RANGE_SIMPLE("192.168.122.20", "192.168.122.1", -1, false);
/* range too large (> 65536) */
DO_TEST_RANGE_SIMPLE("10.0.0.1", "192.168.122.20", -1, false);
/* range reversed */
DO_TEST_RANGE_SIMPLE("192.168.122.20", "10.0.0.1", -1, false);
DO_TEST_RANGE_SIMPLE("172.16.0.50", "172.16.0.254", 205, true);
DO_TEST_RANGE_SIMPLE("192.168.122.1", "192.168.123.20", 276, true);
DO_TEST_RANGE_SIMPLE("2000::1", "2000::1", 1, true);
DO_TEST_RANGE_SIMPLE("2000::1", "2000::2", 2, true);
/* range reversed */
DO_TEST_RANGE_SIMPLE("2000::2", "2000::1", -1, false);
/* range too large (> 65536) */
DO_TEST_RANGE_SIMPLE("2000::1", "9001::1", -1, false);
DO_TEST_NETMASK("192.168.122.1", "192.168.122.2",
"255.255.255.0", true);
DO_TEST_NETMASK("192.168.122.1", "192.168.122.4",