am b0b3d0bc: Fix two parsing bugs in new DHCP client. (2893051a) · Commits · e / os / android_frameworks_base

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

+28 −2

Original line number	Original line	Diff line number	Diff line
	@@ -154,6 +154,12 @@ abstract class DhcpPacket {
	protected static final byte DHCP_BROADCAST_ADDRESS = 28;		protected static final byte DHCP_BROADCAST_ADDRESS = 28;
	protected Inet4Address mBroadcastAddress;		protected Inet4Address mBroadcastAddress;

			/**
			* DHCP Optional Type: Vendor specific information
			*/
			protected static final byte DHCP_VENDOR_INFO = 43;
			protected String mVendorInfo;

	/**		/**
	* DHCP Optional Type: DHCP Requested IP Address		* DHCP Optional Type: DHCP Requested IP Address
	*/		*/
	@@ -226,6 +232,16 @@ abstract class DhcpPacket {
	*/		*/
	protected static final byte DHCP_CLIENT_IDENTIFIER = 61;		protected static final byte DHCP_CLIENT_IDENTIFIER = 61;

			/**
			* DHCP zero-length option code: pad
			*/
			protected static final byte DHCP_OPTION_PAD = 0x00;

			/**
			* DHCP zero-length option code: end of options
			*/
			protected static final byte DHCP_OPTION_END = (byte) 0xff;

	/**		/**
	* The transaction identifier used in this particular DHCP negotiation		* The transaction identifier used in this particular DHCP negotiation
	*/		*/
	@@ -676,6 +692,7 @@ abstract class DhcpPacket {
	Inet4Address netMask = null;		Inet4Address netMask = null;
	String message = null;		String message = null;
	String vendorId = null;		String vendorId = null;
			String vendorInfo = null;
	byte[] expectedParams = null;		byte[] expectedParams = null;
	String hostName = null;		String hostName = null;
	String domainName = null;		String domainName = null;
	@@ -810,8 +827,10 @@ abstract class DhcpPacket {
	try {		try {
	byte optionType = packet.get();		byte optionType = packet.get();

	if (optionType == (byte) 0xFF) {		if (optionType == DHCP_OPTION_END) {
	notFinishedOptions = false;		notFinishedOptions = false;
			} else if (optionType == DHCP_OPTION_PAD) {
			// The pad option doesn't have a length field. Nothing to do.
	} else {		} else {
	int optionLen = packet.get() & 0xFF;		int optionLen = packet.get() & 0xFF;
	int expectedLen = 0;		int expectedLen = 0;
	@@ -885,6 +904,7 @@ abstract class DhcpPacket {
	break;		break;
	case DHCP_VENDOR_CLASS_ID:		case DHCP_VENDOR_CLASS_ID:
	expectedLen = optionLen;		expectedLen = optionLen;
			// Embedded nulls are safe as this does not get passed to netd.
	vendorId = readAsciiString(packet, optionLen, true);		vendorId = readAsciiString(packet, optionLen, true);
	break;		break;
	case DHCP_CLIENT_IDENTIFIER: { // Client identifier		case DHCP_CLIENT_IDENTIFIER: { // Client identifier
	@@ -892,6 +912,11 @@ abstract class DhcpPacket {
	packet.get(id);		packet.get(id);
	expectedLen = optionLen;		expectedLen = optionLen;
	} break;		} break;
			case DHCP_VENDOR_INFO:
			expectedLen = optionLen;
			// Embedded nulls are safe as this does not get passed to netd.
			vendorInfo = readAsciiString(packet, optionLen, true);
			break;
	default:		default:
	// ignore any other parameters		// ignore any other parameters
	for (int i = 0; i < optionLen; i++) {		for (int i = 0; i < optionLen; i++) {
	@@ -965,6 +990,7 @@ abstract class DhcpPacket {
	newPacket.mT1 = T1;		newPacket.mT1 = T1;
	newPacket.mT2 = T2;		newPacket.mT2 = T2;
	newPacket.mVendorId = vendorId;		newPacket.mVendorId = vendorId;
			newPacket.mVendorInfo = vendorInfo;
	return newPacket;		return newPacket;
	}		}

	@@ -1011,7 +1037,7 @@ abstract class DhcpPacket {
	results.dnsServers.addAll(mDnsServers);		results.dnsServers.addAll(mDnsServers);
	results.domains = mDomainName;		results.domains = mDomainName;
	results.serverAddress = mServerIdentifier;		results.serverAddress = mServerIdentifier;
	results.vendorInfo = mVendorId;		results.vendorInfo = mVendorInfo;
	results.leaseDuration = (mLeaseTime != null) ? mLeaseTime : INFINITE_LEASE;		results.leaseDuration = (mLeaseTime != null) ? mLeaseTime : INFINITE_LEASE;
	return results;		return results;
	}		}

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

+141 −9

Original line number	Original line	Diff line number	Diff line
	@@ -25,22 +25,27 @@ import junit.framework.TestCase;

	import java.net.Inet4Address;		import java.net.Inet4Address;
	import java.nio.ByteBuffer;		import java.nio.ByteBuffer;
			import java.util.ArrayList;

			import libcore.util.HexEncoding;

	import static android.net.dhcp.DhcpPacket.*;		import static android.net.dhcp.DhcpPacket.*;


	public class DhcpPacketTest extends TestCase {		public class DhcpPacketTest extends TestCase {

	private static Inet4Address SERVER_ADDR =		private static Inet4Address SERVER_ADDR = v4Address("192.0.2.1");
	(Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.1");		private static Inet4Address CLIENT_ADDR = v4Address("192.0.2.234");
	private static Inet4Address CLIENT_ADDR =
	(Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.234");
	// Use our own empty address instead of Inet4Address.ANY or INADDR_ANY to ensure that the code		// Use our own empty address instead of Inet4Address.ANY or INADDR_ANY to ensure that the code
	// doesn't use == instead of equals when comparing addresses.		// doesn't use == instead of equals when comparing addresses.
	private static Inet4Address ANY = (Inet4Address) NetworkUtils.numericToInetAddress("0.0.0.0");		private static Inet4Address ANY = (Inet4Address) v4Address("0.0.0.0");

	private static byte[] CLIENT_MAC = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };		private static byte[] CLIENT_MAC = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };

			private static final Inet4Address v4Address(String addrString) throws IllegalArgumentException {
			return (Inet4Address) NetworkUtils.numericToInetAddress(addrString);
			}

	class TestDhcpPacket extends DhcpPacket {		class TestDhcpPacket extends DhcpPacket {
	private byte mType;		private byte mType;
	// TODO: Make this a map of option numbers to bytes instead.		// TODO: Make this a map of option numbers to bytes instead.
	@@ -89,7 +94,7 @@ public class DhcpPacketTest extends TestCase {
	addTlv(buffer, DHCP_DOMAIN_NAME, mDomainBytes);		addTlv(buffer, DHCP_DOMAIN_NAME, mDomainBytes);
	}		}
	if (mVendorInfoBytes != null) {		if (mVendorInfoBytes != null) {
	addTlv(buffer, DHCP_VENDOR_CLASS_ID, mVendorInfoBytes);		addTlv(buffer, DHCP_VENDOR_INFO, mVendorInfoBytes);
	}		}
	if (mLeaseTimeBytes != null) {		if (mLeaseTimeBytes != null) {
	addTlv(buffer, DHCP_LEASE_TIME, mLeaseTimeBytes);		addTlv(buffer, DHCP_LEASE_TIME, mLeaseTimeBytes);
	@@ -118,7 +123,7 @@ public class DhcpPacketTest extends TestCase {
	.build();		.build();
	DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);		DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
	assertEquals(expectedDomain, offerPacket.mDomainName);		assertEquals(expectedDomain, offerPacket.mDomainName);
	assertEquals(expectedVendorInfo, offerPacket.mVendorId);		assertEquals(expectedVendorInfo, offerPacket.mVendorInfo);
	}		}

	@SmallTest		@SmallTest
	@@ -221,8 +226,8 @@ public class DhcpPacketTest extends TestCase {
	byte[] slash11Netmask = new byte[] { (byte) 0xff, (byte) 0xe0, 0x00, 0x00 };		byte[] slash11Netmask = new byte[] { (byte) 0xff, (byte) 0xe0, 0x00, 0x00 };
	byte[] slash24Netmask = new byte[] { (byte) 0xff, (byte) 0xff, (byte) 0xff, 0x00 };		byte[] slash24Netmask = new byte[] { (byte) 0xff, (byte) 0xff, (byte) 0xff, 0x00 };
	byte[] invalidNetmask = new byte[] { (byte) 0xff, (byte) 0xfb, (byte) 0xff, 0x00 };		byte[] invalidNetmask = new byte[] { (byte) 0xff, (byte) 0xfb, (byte) 0xff, 0x00 };
	Inet4Address example1 = (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.1");		Inet4Address example1 = v4Address("192.0.2.1");
	Inet4Address example2 = (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.43");		Inet4Address example2 = v4Address("192.0.2.43");

	// A packet without any addresses is not valid.		// A packet without any addresses is not valid.
	checkIpAddress(null, ANY, ANY, slash24Netmask);		checkIpAddress(null, ANY, ANY, slash24Netmask);
	@@ -238,4 +243,131 @@ public class DhcpPacketTest extends TestCase {
	// If there is no netmask, implicit netmasks are used.		// If there is no netmask, implicit netmasks are used.
	checkIpAddress("192.0.2.43/24", ANY, example2, null);		checkIpAddress("192.0.2.43/24", ANY, example2, null);
	}		}

			private void assertDhcpResults(String ipAddress, String gateway, String dnsServersString,
			String domains, String serverAddress, String vendorInfo, int leaseDuration,
			boolean hasMeteredHint, DhcpResults dhcpResults) throws Exception {
			assertEquals(new LinkAddress(ipAddress), dhcpResults.ipAddress);
			assertEquals(v4Address(gateway), dhcpResults.gateway);

			String[] dnsServerStrings = dnsServersString.split(",");
			ArrayList dnsServers = new ArrayList();
			for (String dnsServerString : dnsServerStrings) {
			dnsServers.add(v4Address(dnsServerString));
			}
			assertEquals(dnsServers, dhcpResults.dnsServers);

			assertEquals(domains, dhcpResults.domains);
			assertEquals(v4Address(serverAddress), dhcpResults.serverAddress);
			assertEquals(vendorInfo, dhcpResults.vendorInfo);
			assertEquals(leaseDuration, dhcpResults.leaseDuration);
			assertEquals(hasMeteredHint, dhcpResults.hasMeteredHint());
			}

			@SmallTest
			public void testOffer1() throws Exception {
			// TODO: Turn all of these into golden files. This will probably require modifying
			// Android.mk appropriately, making this into an AndroidTestCase, and adding code to read
			// the golden files from the test APK's assets via mContext.getAssets().
			final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
			// IP header.
			"451001480000000080118849c0a89003c0a89ff7" +
			// UDP header.
			"004300440134dcfa" +
			// BOOTP header.
			"02010600c997a63b0000000000000000c0a89ff70000000000000000" +
			// MAC address.
			"30766ff2a90c00000000000000000000" +
			// Server name.
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			// File.
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			// Options
			"638253633501023604c0a89003330400001c200104fffff0000304c0a89ffe06080808080808080404" +
			"3a0400000e103b040000189cff00000000000000000000"
			).toCharArray(), false));

			DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
			assertTrue(offerPacket instanceof DhcpOfferPacket); // Implicitly checks it's non-null.
			DhcpResults dhcpResults = offerPacket.toDhcpResults();
			assertDhcpResults("192.168.159.247/20", "192.168.159.254", "8.8.8.8,8.8.4.4",
			null, "192.168.144.3", null, 7200, false, dhcpResults);
			}

			@SmallTest
			public void testOffer2() throws Exception {
			final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
			// IP header.
			"450001518d0600004011144dc0a82b01c0a82bf7" +
			// UDP header.
			"00430044013d9ac7" +
			// BOOTP header.
			"02010600dfc23d1f0002000000000000c0a82bf7c0a82b0100000000" +
			// MAC address.
			"30766ff2a90c00000000000000000000" +
			// Server name.
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			// File.
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			// Options
			"638253633501023604c0a82b01330400000e103a04000007083b0400000c4e0104ffffff00" +
			"1c04c0a82bff0304c0a82b010604c0a82b012b0f414e44524f49445f4d455445524544ff"
			).toCharArray(), false));

			assertEquals(337, packet.limit());
			DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L3);
			assertTrue(offerPacket instanceof DhcpOfferPacket); // Implicitly checks it's non-null.
			DhcpResults dhcpResults = offerPacket.toDhcpResults();
			assertDhcpResults("192.168.43.247/24", "192.168.43.1", "192.168.43.1",
			null, "192.168.43.1", "ANDROID_METERED", 3600, true, dhcpResults);
			assertTrue(dhcpResults.hasMeteredHint());
			}

			@SmallTest
			public void testPadAndOverloadedOptionsOffer() throws Exception {
			// A packet observed in the real world that is interesting for two reasons:
			//
			// 1. It uses pad bytes, which we previously didn't support correctly.
			// 2. It uses DHCP option overloading, which we don't currently support (but it doesn't
			// store any information in the overloaded fields).
			//
			// For now, we just check that it parses correctly.
			final ByteBuffer packet = ByteBuffer.wrap(HexEncoding.decode((
			// Ethernet header.
			"b4cef6000000e80462236e300800" +
			// IP header.
			"4500014c00000000ff11741701010101ac119876" +
			// UDP header. TODO: fix invalid checksum (due to MAC address obfuscation).
			"004300440138ae5a" +
			// BOOTP header.
			"020106000fa0059f0000000000000000ac1198760000000000000000" +
			// MAC address.
			"b4cef600000000000000000000000000" +
			// Server name.
			"ff00000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			// File.
			"ff00000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			"0000000000000000000000000000000000000000000000000000000000000000" +
			// Options
			"638253633501023604010101010104ffff000033040000a8c03401030304ac1101010604ac110101" +
			"0000000000000000000000000000000000000000000000ff000000"
			).toCharArray(), false));

			DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_L2);
			assertTrue(offerPacket instanceof DhcpOfferPacket);
			DhcpResults dhcpResults = offerPacket.toDhcpResults();
			assertDhcpResults("172.17.152.118/16", "172.17.1.1", "172.17.1.1",
			null, "1.1.1.1", null, 43200, false, dhcpResults);
			}
	}		}