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, 73 insertions(+), 73 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/77/20177/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..b29230d 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'); - } + List<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..962cf01 --- /dev/null +++ b/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/MacAddressRangeUtils.java @@ -0,0 +1,51 @@ +package org.ovirt.engine.core.utils; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.apache.commons.lang.StringUtils; + +public class MacAddressRangeUtils { + + private static final long MAC_ADDRESS_MULTICAST_BIT = 0x010000000000L; + private static final int HEX_RADIX = 16; + + public static List<String> initRange(String start, String end) { + return innerInitRange(start, end, false); + } + + private static List<String> innerInitRange(String start, String end, boolean stopOnFirst) { + String parsedRangeStart = StringUtils.remove(start, ':'); + String parsedRangeEnd = StringUtils.remove(end, ':'); + if (parsedRangeEnd == null || parsedRangeStart == null) { + return Collections.emptyList(); + } + + long startNum = Long.parseLong(parsedRangeStart, HEX_RADIX); + long endNum = Long.parseLong(parsedRangeEnd, HEX_RADIX); + if (startNum > endNum) { + return Collections.emptyList(); + } + + List<String> macAddresses = new ArrayList<String>(); + for (long i = startNum; i <= endNum; i++) { + if ((MAC_ADDRESS_MULTICAST_BIT & i) != 0) { + continue; + } + + String value = String.format("%012x", i); + macAddresses.add(StringUtils.join(value.split("(?<=\\G..)"), ':')); + + if (stopOnFirst) { + return macAddresses; + } + } + + return macAddresses; + } + + public static boolean isRangeValid(String start, String end) { + return !innerInitRange(start, end, true).isEmpty(); + } +} 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..e772c94 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 @@ -4,6 +4,7 @@ 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 +50,12 @@ rangeEnd, rangeStart)); } + + if (!MacAddressRangeUtils.isRangeValid(rangeStart, rangeEnd)) { + 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..dacd590 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 @@ -30,19 +30,20 @@ @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 }, + { "00:00:00:00:00:00-00:00:00:00:00:FF", 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/20177 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I24113379e8fbbc15a63bdb1be4e6c719d7cb764f Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3 Gerrit-Owner: Moti Asayag <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
