Improve logging of DHCP parse errors using exceptions. (496906ee) · Commits · e / os / android_frameworks_base

services/net/java/android/net/dhcp/DhcpClient.java

+9 −8

Original line number	Diff line number	Diff line
		@@ -345,21 +345,22 @@ public class DhcpClient extends BaseDhcpStateMachine {
		public void run() {
		maybeLog("Receive thread started");
		while (!stopped) {
		int length = 0; // Or compiler can't tell it's initialized if a parse error occurs.
		try {
		int length = Os.read(mPacketSock, mPacket, 0, mPacket.length);
		length = Os.read(mPacketSock, mPacket, 0, mPacket.length);
		DhcpPacket packet = null;
		packet = DhcpPacket.decodeFullPacket(mPacket, length, DhcpPacket.ENCAP_L2);
		if (packet != null) {
		maybeLog("Received packet: " + packet);
		sendMessage(CMD_RECEIVED_PACKET, packet);
		} else if (PACKET_DBG) {
		Log.d(TAG,
		"Can't parse packet" + HexDump.dumpHexString(mPacket, 0, length));
		}
		} catch (IOException\|ErrnoException e) {
		if (!stopped) {
		Log.e(TAG, "Read error", e);
		}
		} catch (DhcpPacket.ParseException e) {
		Log.e(TAG, "Can't parse packet: " + e.getMessage());
		if (PACKET_DBG) {
		Log.d(TAG, HexDump.dumpHexString(mPacket, 0, length));
		}
		}
		}
		maybeLog("Receive thread stopped");

services/net/java/android/net/dhcp/DhcpPacket.java

+40 −20

Original line number	Diff line number	Diff line
		@@ -113,6 +113,11 @@ abstract class DhcpPacket {
		*/
		protected static final int MAX_LENGTH = 1500;

		/**
		* The magic cookie that identifies this as a DHCP packet instead of BOOTP.
		*/
		private static final int DHCP_MAGIC_COOKIE = 0x63825363;

		/**
		* DHCP Optional Type: DHCP Subnet Mask
		*/
		@@ -403,7 +408,7 @@ abstract class DhcpPacket {
		(HWADDR_LEN - mClientMac.length) // pad addr to 16 bytes
		+ 64 // empty server host name (64 bytes)
		+ 128); // empty boot file name (128 bytes)
		buf.putInt(0x63825363); // magic number
		buf.putInt(DHCP_MAGIC_COOKIE); // magic number
		finishPacket(buf);

		// round up to an even number of octets
		@@ -668,6 +673,12 @@ abstract class DhcpPacket {
		return new String(bytes, 0, length, StandardCharsets.US_ASCII);
		}

		public static class ParseException extends Exception {
		public ParseException(String msg, Object... args) {
		super(String.format(msg, args));
		}
		}

		/**
		* Creates a concrete DhcpPacket from the supplied ByteBuffer. The
		* buffer may have an L2 encapsulation (which is the full EthernetII
		@@ -677,7 +688,7 @@ abstract class DhcpPacket {
		* A subset of the optional parameters are parsed and are stored
		* in object fields.
		*/
		public static DhcpPacket decodeFullPacket(ByteBuffer packet, int pktType)
		public static DhcpPacket decodeFullPacket(ByteBuffer packet, int pktType) throws ParseException
		{
		// bootp parameters
		int transactionId;
		@@ -720,7 +731,8 @@ abstract class DhcpPacket {
		// check to see if we need to parse L2, IP, and UDP encaps
		if (pktType == ENCAP_L2) {
		if (packet.remaining() < MIN_PACKET_LENGTH_L2) {
		return null;
		throw new ParseException("L2 packet too short, %d < %d",
		packet.remaining(), MIN_PACKET_LENGTH_L2);
		}

		byte[] l2dst = new byte[6];
		@@ -732,18 +744,20 @@ abstract class DhcpPacket {
		short l2type = packet.getShort();

		if (l2type != OsConstants.ETH_P_IP)
		return null;
		throw new ParseException("Unexpected L2 type 0x%04x, expected 0x%04x",
		l2type, OsConstants.ETH_P_IP);
		}

		if (pktType <= ENCAP_L3) {
		if (packet.remaining() < MIN_PACKET_LENGTH_L3) {
		return null;
		throw new ParseException("L3 packet too short, %d < %d",
		packet.remaining(), MIN_PACKET_LENGTH_L3);
		}

		byte ipTypeAndLength = packet.get();
		int ipVersion = (ipTypeAndLength & 0xf0) >> 4;
		if (ipVersion != 4) {
		return null;
		throw new ParseException("Invalid IP version %d", ipVersion);
		}

		// System.out.println("ipType is " + ipType);
		@@ -759,8 +773,9 @@ abstract class DhcpPacket {
		ipSrc = readIpAddress(packet);
		ipDst = readIpAddress(packet);

		if (ipProto != IP_TYPE_UDP) // UDP
		return null;
		if (ipProto != IP_TYPE_UDP) {
		throw new ParseException("Protocol not UDP: %d", ipProto);
		}

		// Skip options. This cannot cause us to read beyond the end of the buffer because the
		// IPv4 header cannot be more than (0x0f * 4) = 60 bytes long, and that is less than
		@@ -776,13 +791,15 @@ abstract class DhcpPacket {
		short udpLen = packet.getShort();
		short udpChkSum = packet.getShort();

		if ((udpSrcPort != DHCP_SERVER) && (udpSrcPort != DHCP_CLIENT))
		return null;
		if ((udpSrcPort != DHCP_SERVER) && (udpSrcPort != DHCP_CLIENT)) {
		throw new ParseException("Invalid source port %d", udpSrcPort);
		}
		}

		// We need to check the length even for ENCAP_L3 because the IPv4 header is variable-length.
		if (pktType > ENCAP_BOOTP \|\| packet.remaining() < MIN_PACKET_LENGTH_BOOTP) {
		return null;
		throw new ParseException("Invalid type or BOOTP packet too short, %d < %d",
		packet.remaining(), MIN_PACKET_LENGTH_BOOTP);
		}

		byte type = packet.get();
		@@ -805,7 +822,7 @@ abstract class DhcpPacket {
		packet.get(ipv4addr);
		relayIp = (Inet4Address) Inet4Address.getByAddress(ipv4addr);
		} catch (UnknownHostException ex) {
		return null;
		throw new ParseException("Invalid IPv4 address: %s", Arrays.toString(ipv4addr));
		}

		// Some DHCP servers have been known to announce invalid client hardware address values such
		@@ -828,8 +845,10 @@ abstract class DhcpPacket {

		int dhcpMagicCookie = packet.getInt();

		if (dhcpMagicCookie != 0x63825363)
		return null;
		if (dhcpMagicCookie != DHCP_MAGIC_COOKIE) {
		throw new ParseException("Bad magic cookie 0x%08x, should be 0x%08x", dhcpMagicCookie,
		DHCP_MAGIC_COOKIE);
		}

		// parse options
		boolean notFinishedOptions = true;
		@@ -937,18 +956,20 @@ abstract class DhcpPacket {
		}

		if (expectedLen != optionLen) {
		return null;
		throw new ParseException("Invalid length %d for option %d, expected %d",
		optionLen, optionType, expectedLen);
		}
		}
		} catch (BufferUnderflowException e) {
		return null;
		throw new ParseException("BufferUnderflowException");
		}
		}

		DhcpPacket newPacket;

		switch(dhcpType) {
		case -1: return null;
		case (byte) 0xFF:
		throw new ParseException("No DHCP message type option");
		case DHCP_MESSAGE_TYPE_DISCOVER:
		newPacket = new DhcpDiscoverPacket(
		transactionId, secs, clientMac, broadcast);
		@@ -981,8 +1002,7 @@ abstract class DhcpPacket {
		clientMac);
		break;
		default:
		System.out.println("Unimplemented type: " + dhcpType);
		return null;
		throw new ParseException("Unimplemented DHCP type %d", dhcpType);
		}

		newPacket.mBroadcastAddress = bcAddr;
		@@ -1009,7 +1029,7 @@ abstract class DhcpPacket {
		* Parse a packet from an array of bytes, stopping at the given length.
		*/
		public static DhcpPacket decodeFullPacket(byte[] packet, int length, int pktType)
		{
		throws ParseException {
		ByteBuffer buffer = ByteBuffer.wrap(packet, 0, length).order(ByteOrder.BIG_ENDIAN);
		return decodeFullPacket(buffer, pktType);
		}

services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java

+14 −6

Original line number	Diff line number	Diff line
		@@ -117,7 +117,7 @@ public class DhcpPacketTest extends TestCase {

		private void assertDomainAndVendorInfoParses(
		String expectedDomain, byte[] domainBytes,
		String expectedVendorInfo, byte[] vendorInfoBytes) {
		String expectedVendorInfo, byte[] vendorInfoBytes) throws Exception {
		ByteBuffer packet = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER)
		.setDomainBytes(domainBytes)
		.setVendorInfoBytes(vendorInfoBytes)
		@@ -158,17 +158,25 @@ public class DhcpPacketTest extends TestCase {
		}

		private void assertLeaseTimeParses(boolean expectValid, Integer rawLeaseTime,
		long leaseTimeMillis, byte[] leaseTimeBytes) {
		long leaseTimeMillis, byte[] leaseTimeBytes) throws Exception {
		TestDhcpPacket testPacket = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER);
		if (leaseTimeBytes != null) {
		testPacket.setLeaseTimeBytes(leaseTimeBytes);
		}
		ByteBuffer packet = testPacket.build();
		DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
		DhcpPacket offerPacket = null;

		if (!expectValid) {
		assertNull(offerPacket);
		try {
		offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
		fail("Invalid packet parsed successfully: " + offerPacket);
		} catch (ParseException expected) {
		}
		return;
		}

		offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
		assertNotNull(offerPacket);
		assertEquals(rawLeaseTime, offerPacket.mLeaseTime);
		DhcpResults dhcpResults = offerPacket.toDhcpResults(); // Just check this doesn't crash.
		assertEquals(leaseTimeMillis, offerPacket.getLeaseTimeMillis());
		@@ -200,14 +208,14 @@ public class DhcpPacketTest extends TestCase {
		}

		private void checkIpAddress(String expected, Inet4Address clientIp, Inet4Address yourIp,
		byte[] netmaskBytes) {
		byte[] netmaskBytes) throws Exception {
		checkIpAddress(expected, DHCP_MESSAGE_TYPE_OFFER, clientIp, yourIp, netmaskBytes);
		checkIpAddress(expected, DHCP_MESSAGE_TYPE_ACK, clientIp, yourIp, netmaskBytes);
		}

		private void checkIpAddress(String expected, byte type,
		Inet4Address clientIp, Inet4Address yourIp,
		byte[] netmaskBytes) {
		byte[] netmaskBytes) throws Exception {
		ByteBuffer packet = new TestDhcpPacket(type, clientIp, yourIp)
		.setNetmaskBytes(netmaskBytes)
		.build();