Fix DefaultChannelId MAC address parsing bug

Motivation:
DefaultChannelId provides a regular expression which validates if a user provided MAC address is valid. This regular expression may allow invalid MAC addresses and also not allow valid MAC addresses.

Modifications:
- Introduce a MacAddressUtil#parseMac method which can parse and validate the MAC address at the same time. The regular expression check before hand is additional overhead if we have to parse the MAC address.

Result:
Fixes https://github.com/netty/netty/issues/6132.
This commit is contained in:
Scott Mitchell 2016-12-14 12:13:16 -08:00
parent 3f82b53bae
commit 3d11334151
3 changed files with 197 additions and 76 deletions

View File

@ -16,6 +16,10 @@
package io.netty.util.internal;
import io.netty.util.NetUtil;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;
import java.net.InetAddress;
import java.net.NetworkInterface;
import java.net.SocketException;
@ -25,21 +29,14 @@ import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;
import io.netty.util.NetUtil;
import io.netty.util.internal.logging.InternalLogger;
import io.netty.util.internal.logging.InternalLoggerFactory;
import static io.netty.util.internal.EmptyArrays.EMPTY_BYTES;
public final class MacAddressUtil {
/**
* Length of a valid MAC address.
*/
public static final int MAC_ADDRESS_LENGTH = 8;
private static final byte[] NOT_FOUND = { -1 };
private static final InternalLogger logger = InternalLoggerFactory.getInstance(MacAddressUtil.class);
private static final int EUI64_MAC_ADDRESS_LENGTH = 8;
private static final int EUI48_MAC_ADDRESS_LENGTH = 6;
/**
* Obtains the best MAC address found on local network interfaces.
* Generally speaking, an active network interface used on public
@ -49,7 +46,7 @@ public final class MacAddressUtil {
*/
public static byte[] bestAvailableMac() {
// Find the best MAC address available.
byte[] bestMacAddr = NOT_FOUND;
byte[] bestMacAddr = EMPTY_BYTES;
InetAddress bestInetAddr = NetUtil.LOCALHOST4;
// Retrieve the list of available network interfaces.
@ -110,13 +107,13 @@ public final class MacAddressUtil {
}
}
if (bestMacAddr == NOT_FOUND) {
if (bestMacAddr == EMPTY_BYTES) {
return null;
}
switch (bestMacAddr.length) {
case 6: // EUI-48 - convert to EUI-64
byte[] newAddr = new byte[MAC_ADDRESS_LENGTH];
case EUI48_MAC_ADDRESS_LENGTH: // EUI-48 - convert to EUI-64
byte[] newAddr = new byte[EUI64_MAC_ADDRESS_LENGTH];
System.arraycopy(bestMacAddr, 0, newAddr, 0, 3);
newAddr[3] = (byte) 0xFF;
newAddr[4] = (byte) 0xFE;
@ -124,12 +121,73 @@ public final class MacAddressUtil {
bestMacAddr = newAddr;
break;
default: // Unknown
bestMacAddr = Arrays.copyOf(bestMacAddr, MAC_ADDRESS_LENGTH);
bestMacAddr = Arrays.copyOf(bestMacAddr, EUI64_MAC_ADDRESS_LENGTH);
}
return bestMacAddr;
}
/**
* Returns the result of {@link #bestAvailableMac()} if non-{@code null} otherwise returns a random EUI-64 MAC
* address.
*/
public static byte[] defaultMachineId() {
byte[] bestMacAddr = MacAddressUtil.bestAvailableMac();
if (bestMacAddr == null) {
bestMacAddr = new byte[EUI64_MAC_ADDRESS_LENGTH];
ThreadLocalRandom.current().nextBytes(bestMacAddr);
logger.warn(
"Failed to find a usable hardware address from the network interfaces; using random bytes: {}",
MacAddressUtil.formatAddress(bestMacAddr));
}
return bestMacAddr;
}
/**
* Parse a EUI-48, MAC-48, or EUI-64 MAC address from a {@link String} and return it as a {@code byte[]}.
* @param value The string representation of the MAC address.
* @return The byte representation of the MAC address.
*/
public static byte[] parseMAC(String value) {
final byte[] machineId;
final char separator;
switch (value.length()) {
case 17:
separator = value.charAt(2);
validateMacSeparator(separator);
machineId = new byte[EUI48_MAC_ADDRESS_LENGTH];
break;
case 23:
separator = value.charAt(2);
validateMacSeparator(separator);
machineId = new byte[EUI64_MAC_ADDRESS_LENGTH];
break;
default:
throw new IllegalArgumentException("value is not supported [MAC-48, EUI-48, EUI-64]");
}
final int end = machineId.length - 1;
int j = 0;
for (int i = 0; i < end; ++i, j += 3) {
final int sIndex = j + 2;
machineId[i] = (byte) Integer.parseInt(value.substring(j, sIndex), 16);
if (value.charAt(sIndex) != separator) {
throw new IllegalArgumentException("expected separator '" + separator + " but got '" +
value.charAt(sIndex) + "' at index: " + sIndex);
}
}
machineId[end] = (byte) Integer.parseInt(value.substring(j, value.length()), 16);
return machineId;
}
private static void validateMacSeparator(char separator) {
if (separator != ':' && separator != '-') {
throw new IllegalArgumentException("unsupported seperator: " + separator + " (expected: [:-])");
}
}
/**
* @param addr byte array of a MAC address.
* @return hex formatted MAC address.
@ -146,12 +204,7 @@ public final class MacAddressUtil {
* @return positive - current is better, 0 - cannot tell from MAC addr, negative - candidate is better.
*/
private static int compareAddresses(byte[] current, byte[] candidate) {
if (candidate == null) {
return 1;
}
// Must be EUI-48 or longer.
if (candidate.length < 6) {
if (candidate == null || candidate.length < EUI48_MAC_ADDRESS_LENGTH) {
return 1;
}
@ -174,7 +227,7 @@ public final class MacAddressUtil {
}
// Prefer globally unique address.
if ((current[0] & 2) == 0) {
if (current.length == 0 || (current[0] & 2) == 0) {
if ((candidate[0] & 2) == 0) {
// Both current and candidate are globally unique addresses.
return 0;
@ -182,15 +235,12 @@ public final class MacAddressUtil {
// Only current is globally unique.
return 1;
}
} else {
if ((candidate[0] & 2) == 0) {
// Only candidate is globally unique.
return -1;
} else {
// Both current and candidate are non-unique.
return 0;
}
} else if ((candidate[0] & 2) == 0) {
// Only candidate is globally unique.
return -1;
}
// Both current and candidate are non-unique.
return 0;
}
/**

View File

@ -0,0 +1,99 @@
/*
* Copyright 2016 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.util.internal;
import org.junit.Test;
import static io.netty.util.internal.MacAddressUtil.parseMAC;
import static org.junit.Assert.assertArrayEquals;
public class MacAddressUtilTest {
@Test
public void testParseMacEUI48() {
assertArrayEquals(new byte[]{0, (byte) 0xaa, 0x11, (byte) 0xbb, 0x22, (byte) 0xcc},
parseMAC("00-AA-11-BB-22-CC"));
assertArrayEquals(new byte[]{0, (byte) 0xaa, 0x11, (byte) 0xbb, 0x22, (byte) 0xcc},
parseMAC("00:AA:11:BB:22:CC"));
}
@Test
public void testParseMacMAC48ToEUI64() {
// MAC-48 into an EUI-64
assertArrayEquals(new byte[]{0, (byte) 0xaa, 0x11, (byte) 0xff, (byte) 0xff, (byte) 0xbb, 0x22, (byte) 0xcc},
parseMAC("00-AA-11-FF-FF-BB-22-CC"));
assertArrayEquals(new byte[]{0, (byte) 0xaa, 0x11, (byte) 0xff, (byte) 0xff, (byte) 0xbb, 0x22, (byte) 0xcc},
parseMAC("00:AA:11:FF:FF:BB:22:CC"));
}
@Test
public void testParseMacEUI48ToEUI64() {
// EUI-48 into an EUI-64
assertArrayEquals(new byte[]{0, (byte) 0xaa, 0x11, (byte) 0xff, (byte) 0xfe, (byte) 0xbb, 0x22, (byte) 0xcc},
parseMAC("00-AA-11-FF-FE-BB-22-CC"));
assertArrayEquals(new byte[]{0, (byte) 0xaa, 0x11, (byte) 0xff, (byte) 0xfe, (byte) 0xbb, 0x22, (byte) 0xcc},
parseMAC("00:AA:11:FF:FE:BB:22:CC"));
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalid7HexGroupsA() {
parseMAC("00-AA-11-BB-22-CC-FF");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalid7HexGroupsB() {
parseMAC("00:AA:11:BB:22:CC:FF");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI48MixedSeparatorA() {
parseMAC("00-AA:11-BB-22-CC");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI48MixedSeparatorB() {
parseMAC("00:AA-11:BB:22:CC");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI64MixedSeparatorA() {
parseMAC("00-AA-11-FF-FE-BB-22:CC");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI64MixedSeparatorB() {
parseMAC("00:AA:11:FF:FE:BB:22-CC");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI48TrailingSeparatorA() {
parseMAC("00-AA-11-BB-22-CC-");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI48TrailingSeparatorB() {
parseMAC("00:AA:11:BB:22:CC:");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI64TrailingSeparatorA() {
parseMAC("00-AA-11-FF-FE-BB-22-CC-");
}
@Test(expected = IllegalArgumentException.class)
public void testParseMacInvalidEUI64TrailingSeparatorB() {
parseMAC("00:AA:11:FF:FE:BB:22:CC:");
}
}

View File

@ -28,7 +28,9 @@ import io.netty.util.internal.logging.InternalLoggerFactory;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Pattern;
import static io.netty.util.internal.MacAddressUtil.defaultMachineId;
import static io.netty.util.internal.MacAddressUtil.parseMAC;
/**
* The default {@link ChannelId} implementation.
@ -38,9 +40,6 @@ public final class DefaultChannelId implements ChannelId {
private static final long serialVersionUID = 3884076183504074063L;
private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultChannelId.class);
private static final Pattern MACHINE_ID_PATTERN = Pattern.compile("^(?:[0-9a-fA-F][:-]?){6,8}$");
private static final int MACHINE_ID_LEN = MacAddressUtil.MAC_ADDRESS_LENGTH;
private static final byte[] MACHINE_ID;
private static final int PROCESS_ID_LEN = 4;
private static final int PROCESS_ID;
@ -54,9 +53,7 @@ public final class DefaultChannelId implements ChannelId {
* Returns a new {@link DefaultChannelId} instance.
*/
public static DefaultChannelId newInstance() {
DefaultChannelId id = new DefaultChannelId();
id.init();
return id;
return new DefaultChannelId();
}
static {
@ -89,11 +86,13 @@ public final class DefaultChannelId implements ChannelId {
byte[] machineId = null;
String customMachineId = SystemPropertyUtil.get("io.netty.machineId");
if (customMachineId != null) {
if (MACHINE_ID_PATTERN.matcher(customMachineId).matches()) {
machineId = parseMachineId(customMachineId);
try {
machineId = parseMAC(customMachineId);
} catch (Exception e) {
logger.warn("-Dio.netty.machineId: {} (malformed)", customMachineId, e);
}
if (machineId != null) {
logger.debug("-Dio.netty.machineId: {} (user-set)", customMachineId);
} else {
logger.warn("-Dio.netty.machineId: {} (malformed)", customMachineId);
}
}
@ -107,31 +106,6 @@ public final class DefaultChannelId implements ChannelId {
MACHINE_ID = machineId;
}
@SuppressWarnings("DynamicRegexReplaceableByCompiledPattern")
private static byte[] parseMachineId(String value) {
// Strip separators.
value = value.replaceAll("[:-]", "");
byte[] machineId = new byte[MACHINE_ID_LEN];
for (int i = 0; i < value.length(); i += 2) {
machineId[i] = (byte) Integer.parseInt(value.substring(i, i + 2), 16);
}
return machineId;
}
private static byte[] defaultMachineId() {
byte[] bestMacAddr = MacAddressUtil.bestAvailableMac();
if (bestMacAddr == null) {
bestMacAddr = new byte[MacAddressUtil.MAC_ADDRESS_LENGTH];
ThreadLocalRandom.current().nextBytes(bestMacAddr);
logger.warn(
"Failed to find a usable hardware address from the network interfaces; using random bytes: {}",
MacAddressUtil.formatAddress(bestMacAddr));
}
return bestMacAddr;
}
private static int defaultProcessId() {
final ClassLoader loader = PlatformDependent.getClassLoader(DefaultChannelId.class);
String value;
@ -178,20 +152,19 @@ public final class DefaultChannelId implements ChannelId {
return pid;
}
private final byte[] data = new byte[MACHINE_ID_LEN + PROCESS_ID_LEN + SEQUENCE_LEN + TIMESTAMP_LEN + RANDOM_LEN];
private final byte[] data;
private int hashCode;
private transient String shortValue;
private transient String longValue;
private DefaultChannelId() { }
private void init() {
private DefaultChannelId() {
data = new byte[MACHINE_ID.length + PROCESS_ID_LEN + SEQUENCE_LEN + TIMESTAMP_LEN + RANDOM_LEN];
int i = 0;
// machineId
System.arraycopy(MACHINE_ID, 0, data, i, MACHINE_ID_LEN);
i += MACHINE_ID_LEN;
System.arraycopy(MACHINE_ID, 0, data, i, MACHINE_ID.length);
i += MACHINE_ID.length;
// processId
i = writeInt(i, PROCESS_ID);
@ -234,8 +207,7 @@ public final class DefaultChannelId implements ChannelId {
public String asShortText() {
String shortValue = this.shortValue;
if (shortValue == null) {
this.shortValue = shortValue = ByteBufUtil.hexDump(
data, MACHINE_ID_LEN + PROCESS_ID_LEN + SEQUENCE_LEN + TIMESTAMP_LEN, RANDOM_LEN);
this.shortValue = shortValue = ByteBufUtil.hexDump(data, data.length - RANDOM_LEN, RANDOM_LEN);
}
return shortValue;
}
@ -252,7 +224,7 @@ public final class DefaultChannelId implements ChannelId {
private String newLongValue() {
StringBuilder buf = new StringBuilder(2 * data.length + 5);
int i = 0;
i = appendHexDumpField(buf, i, MACHINE_ID_LEN);
i = appendHexDumpField(buf, i, MACHINE_ID.length);
i = appendHexDumpField(buf, i, PROCESS_ID_LEN);
i = appendHexDumpField(buf, i, SEQUENCE_LEN);
i = appendHexDumpField(buf, i, TIMESTAMP_LEN);