Moti Asayag has uploaded a new change for review. Change subject: engine: Prevent empty MAC Address range ......................................................................
engine: Prevent empty MAC Address range The patch prevents a user from entering a mac address range which might result in an empty pool, for example a range which contains multicast mac addresses only. Change-Id: I24113379e8fbbc15a63bdb1be4e6c719d7cb764f Bug-Url: https://bugzilla.redhat.com/1011912 Signed-off-by: Moti Asayag <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java A backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java M backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java M backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java 4 files changed, 109 insertions(+), 73 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/55/20155/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java index 0fe6229..9d80be1 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java @@ -9,7 +9,6 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.commons.collections.map.CaseInsensitiveMap; -import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.businessentities.network.VmNic; import org.ovirt.engine.core.common.config.Config; @@ -19,13 +18,12 @@ import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; +import org.ovirt.engine.core.utils.MacAddressRangeUtils; import org.ovirt.engine.core.utils.log.Log; import org.ovirt.engine.core.utils.log.LogFactory; public class MacPoolManager { - private static final String MAC_ADDRESS_MULTICAST_LSB = "13579bBdDfF"; - private static final int HEX_RADIX = 16; private static final String INIT_ERROR_MSG = "{0}: Error in initializing MAC Addresses pool manager."; private static final MacPoolManager INSTANCE = new MacPoolManager(); @@ -119,72 +117,15 @@ } } - private String parseRangePart(String start) { - StringBuilder builder = new StringBuilder(); - for (String part : start.split("[:]", -1)) { - String tempPart = part.trim(); - if (tempPart.length() == 1) { - builder.append('0'); - } else if (tempPart.length() > 2) { - return null; - } - builder.append(tempPart); - } - return builder.toString(); - } - private boolean initRange(String start, String end) { - String parsedRangeStart = parseRangePart(start); - String parsedRangeEnd = parseRangePart(end); - if (parsedRangeEnd == null || parsedRangeStart == null) { - return false; - } - long startNum = Long.parseLong(parseRangePart(start), HEX_RADIX); - long endNum = Long.parseLong(parseRangePart(end), HEX_RADIX); - if (startNum > endNum) { - return false; - } - for (long i = startNum; i <= endNum; i++) { - String value = String.format("%x", i); - if (value.length() > 12) { - return false; - } else if (value.length() < 12) { - value = StringUtils.leftPad(value, 12, '0'); - } + Set<String> macAddresses = MacAddressRangeUtils.initRange(start, end); - value = createMacAddress(value); - if (value == null) { - break; - } - - if (!availableMacs.contains(value)) { - availableMacs.add(value); - } - if (availableMacs.size() > Config.<Integer> GetValue(ConfigValues.MaxMacsCountInPool)) { - throw new MacPoolExceededMaxException(); - } + if (macAddresses.size() + availableMacs.size() > Config.<Integer> GetValue(ConfigValues.MaxMacsCountInPool)) { + throw new MacPoolExceededMaxException(); } + + availableMacs.addAll(macAddresses); return true; - } - - private String createMacAddress(String value) { - StringBuilder builder = new StringBuilder(); - - for (int j = 0; j < value.length(); j += 2) { - String group = value.substring(j, j + 2); - - // skip multi-cast MAC Addresses - if (j == 0 && StringUtils.contains(MAC_ADDRESS_MULTICAST_LSB, group.charAt(1))) { - return null; - } - - builder.append(group); - if (j + 2 < value.length()) { - builder.append(":"); - } - } - - return builder.toString(); } public String allocateNewMac() { diff --git a/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java new file mode 100644 index 0000000..585bcd1 --- /dev/null +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java @@ -0,0 +1,85 @@ +package org.ovirt.engine.core.utils; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import org.apache.commons.lang.StringUtils; + +public class MacAddressRangeUtils { + + private static final String MAC_ADDRESS_MULTICAST_LSB = "13579bBdDfF"; + private static final int HEX_RADIX = 16; + + public static Set<String> initRange(String start, String end) { + + String parsedRangeStart = parseRangePart(start); + String parsedRangeEnd = parseRangePart(end); + if (parsedRangeEnd == null || parsedRangeStart == null) { + return Collections.emptySet(); + } + + long startNum = Long.parseLong(parseRangePart(start), HEX_RADIX); + long endNum = Long.parseLong(parseRangePart(end), HEX_RADIX); + if (startNum > endNum) { + return Collections.emptySet(); + } + + Set<String> availableMacs = new HashSet<String>(); + + for (long i = startNum; i <= endNum; i++) { + String value = String.format("%x", i); + if (value.length() > 12) { + return availableMacs; + } else if (value.length() < 12) { + value = StringUtils.leftPad(value, 12, '0'); + } + + value = createMacAddress(value); + if (value == null) { + break; + } + + availableMacs.add(value); + } + + return availableMacs; + } + + private static String parseRangePart(String start) { + StringBuilder builder = new StringBuilder(); + for (String part : start.split("[:]", -1)) { + String tempPart = part.trim(); + if (tempPart.length() == 1) { + builder.append('0'); + } else if (tempPart.length() > 2) { + return null; + } + + builder.append(tempPart); + } + + return builder.toString(); + } + + private static String createMacAddress(String value) { + StringBuilder builder = new StringBuilder(); + + for (int j = 0; j < value.length(); j += 2) { + String group = value.substring(j, j + 2); + + // skip multi-cast MAC Addresses + if (j == 0 && StringUtils.contains(MAC_ADDRESS_MULTICAST_LSB, group.charAt(1))) { + return null; + } + + builder.append(group); + if (j + 2 < value.length()) { + builder.append(":"); + } + } + + return builder.toString(); + } + +} diff --git a/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java b/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java index 077dfd4..4b21262 100644 --- a/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java +++ b/backend/manager/tools/src/main/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelper.java @@ -1,9 +1,11 @@ package org.ovirt.engine.core.config.entity.helper; +import java.util.Set; import java.util.regex.Pattern; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.config.entity.ConfigKey; +import org.ovirt.engine.core.utils.MacAddressRangeUtils; /** * The class verifies the provided MAC address ranges to set the values of MAC addresses pool is defined properly. The @@ -49,6 +51,13 @@ rangeEnd, rangeStart)); } + + Set<String> macAddresses = MacAddressRangeUtils.initRange(rangeStart, rangeEnd); + if (macAddresses.isEmpty()) { + return new ValidationResult(false, + String.format("The entered range is invalid. %s contains no valid MAC addresses.", range)); + } + } else { return new ValidationResult(false, "The entered value is in imporper format. " + value + " should be in a format of AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB,..."); diff --git a/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java b/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java index 8d50dbb..4bd85aa 100644 --- a/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java +++ b/backend/manager/tools/src/test/java/org/ovirt/engine/core/config/entity/helper/MacAddressPoolRangesValueHelperTest.java @@ -24,25 +24,26 @@ @Test public void validateRanges() { + System.out.println(ranges); assertEquals(expectedResult, validator.validate(null, ranges).isOk()); } @Parameterized.Parameters public static Collection<Object[]> ipAddressParams() { return Arrays.asList(new Object[][] { - { "AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB", true }, - { "AA:AA:AA:AA:AA:AA-bb:bb:bb:bb:bb:bb", true }, - { "aa:aa:aa:aa:aa:aa-BB:BB:BB:BB:BB:BB", true }, - { "AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB,AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB", true }, - { "AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB,CC:CC:CC:CC:CC:CC-DD:DD:DD:DD:DD:DD", true }, - { "CC:CC:CC:CC:CC:CC-DD:DD:DD:DD:DD:DD,AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB", true }, + { "AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB", true }, + { "AA:AA:AA:AA:AA:AA-aa:aa:aa:aa:aa:ab", true }, + { "aa:aa:aa:aa:aa:aa-AA:AA:AA:AA:AA:AB", true }, + { "AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB,AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB", true }, + { "AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB,CC:CC:CC:CC:CC:CC-CC:CC:CC:CC:CC:CD", true }, + { "CC:CC:CC:CC:CC:CC-CC:CC:CC:CC:CC:CD,AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB", true }, { "BB:BB:BB:BB:BB:BB-AA:AA:AA:AA:AA:AA", false }, { "BB:BB:BB:BB:BB:BB-aa:aa:aa:aa:aa:aa", false }, { "bb:bb:bb:bb:bb:bb-AA:AA:AA:AA:AA:AA", false }, - { "AA:AA:AA:AA:AA:AA,BB:BB:BB:BB:BB:BB", false }, + { "BB:BB:BB:BB:BB:BA,BB:BB:BB:BB:BB:BB", false }, { "AA:AA:AA:AA:AA,BB:BB:BB:BB:BB:BB", false }, { "AA-AA-AA-AA-AA-AA-BB-BB-BB-BB-BB-BB", false }, - { "AA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB,XA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB", false }, + { "AA:AA:AA:AA:AA:AA-AA:AA:AA:AA:AA:AB,XA:AA:AA:AA:AA:AA-BB:BB:BB:BB:BB:BB", false }, { null, false }, { "", false }, { " ", false }, -- To view, visit http://gerrit.ovirt.org/20155 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I24113379e8fbbc15a63bdb1be4e6c719d7cb764f Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
