Merge "Deal with null characters in string options." into m-wireless-dev

3 files changed, 135 insertions, 6 deletions

diff --git a/services/net/java/android/net/dhcp/DhcpPacket.java b/services/net/java/android/net/dhcp/DhcpPacket.java
index a232a6e..d41629d 100644
--- a/services/net/java/android/net/dhcp/DhcpPacket.java
+++ b/services/net/java/android/net/dhcp/DhcpPacket.java
@@ -611,10 +611,22 @@ abstract class DhcpPacket {
     /**
      * Reads a string of specified length from the buffer.
      */
-    private static String readAsciiString(ByteBuffer buf, int byteCount) {
+    private static String readAsciiString(ByteBuffer buf, int byteCount, boolean nullOk) {
         byte[] bytes = new byte[byteCount];
         buf.get(bytes);
-        return new String(bytes, 0, bytes.length, StandardCharsets.US_ASCII);
+        int length = bytes.length;
+        if (!nullOk) {
+            // Stop at the first null byte. This is because some DHCP options (e.g., the domain
+            // name) are passed to netd via FrameworkListener, which refuses arguments containing
+            // null bytes. We don't do this by default because vendorInfo is an opaque string which
+            // could in theory contain null bytes.
+            for (length = 0; length < bytes.length; length++) {
+                if (bytes[length] == 0) {
+                    break;
+                }
+            }
+        }
+        return new String(bytes, 0, length, StandardCharsets.US_ASCII);
     }
 
     /**
@@ -797,7 +809,7 @@ abstract class DhcpPacket {
                             break;
                         case DHCP_HOST_NAME:
                             expectedLen = optionLen;
-                            hostName = readAsciiString(packet, optionLen);
+                            hostName = readAsciiString(packet, optionLen, false);
                             break;
                         case DHCP_MTU:
                             expectedLen = 2;
@@ -805,7 +817,7 @@ abstract class DhcpPacket {
                             break;
                         case DHCP_DOMAIN_NAME:
                             expectedLen = optionLen;
-                            domainName = readAsciiString(packet, optionLen);
+                            domainName = readAsciiString(packet, optionLen, false);
                             break;
                         case DHCP_BROADCAST_ADDRESS:
                             bcAddr = readIpAddress(packet);
@@ -834,7 +846,7 @@ abstract class DhcpPacket {
                             break;
                         case DHCP_MESSAGE:
                             expectedLen = optionLen;
-                            message = readAsciiString(packet, optionLen);
+                            message = readAsciiString(packet, optionLen, false);
                             break;
                         case DHCP_MAX_MESSAGE_SIZE:
                             expectedLen = 2;
@@ -850,7 +862,7 @@ abstract class DhcpPacket {
                             break;
                         case DHCP_VENDOR_CLASS_ID:
                             expectedLen = optionLen;
-                            vendorId = readAsciiString(packet, optionLen);
+                            vendorId = readAsciiString(packet, optionLen, true);
                             break;
                         case DHCP_CLIENT_IDENTIFIER: { // Client identifier
                             byte[] id = new byte[optionLen];
diff --git a/services/tests/servicestests/Android.mk b/services/tests/servicestests/Android.mk
index f25fc62..33979b1 100644
--- a/services/tests/servicestests/Android.mk
+++ b/services/tests/servicestests/Android.mk
@@ -10,6 +10,7 @@ LOCAL_SRC_FILES := $(call all-java-files-under, src)
 LOCAL_STATIC_JAVA_LIBRARIES := \
     services.core \
     services.devicepolicy \
+    services.net \
     easymocklib \
     guava \
     mockito-target
diff --git a/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
new file mode 100644
index 0000000..2658937
--- /dev/null
+++ b/services/tests/servicestests/src/android/net/dhcp/DhcpPacketTest.java
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed 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 android.net.dhcp;
+
+import android.net.NetworkUtils;
+import android.system.OsConstants;
+import android.test.suitebuilder.annotation.SmallTest;
+import junit.framework.TestCase;
+
+import java.net.Inet4Address;
+import java.nio.ByteBuffer;
+
+import static android.net.dhcp.DhcpPacket.*;
+
+
+public class DhcpPacketTest extends TestCase {
+
+    private static Inet4Address SERVER_ADDR =
+            (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.1");
+    private static Inet4Address CLIENT_ADDR =
+            (Inet4Address) NetworkUtils.numericToInetAddress("192.0.2.234");
+    private static byte[] CLIENT_MAC = new byte[] { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05 };
+
+    class TestDhcpPacket extends DhcpPacket {
+        private byte mType;
+        // TODO: Make this a map of option numbers to bytes instead.
+        private byte[] mDomainBytes, mVendorInfoBytes;
+
+        public TestDhcpPacket(byte type, byte[] domainBytes, byte[] vendorInfoBytes) {
+            super(0xdeadbeef, INADDR_ANY, CLIENT_ADDR, INADDR_ANY, INADDR_ANY, CLIENT_MAC, true);
+            mType = type;
+            mDomainBytes = domainBytes;
+            mVendorInfoBytes = vendorInfoBytes;
+        }
+
+        public ByteBuffer buildPacket(int encap, short unusedDestUdp, short unusedSrcUdp) {
+            ByteBuffer result = ByteBuffer.allocate(MAX_LENGTH);
+            fillInPacket(encap, CLIENT_ADDR, SERVER_ADDR,
+                         DHCP_CLIENT, DHCP_SERVER, result, DHCP_BOOTREPLY, false);
+            return result;
+        }
+
+        public void finishPacket(ByteBuffer buffer) {
+            addTlv(buffer, DHCP_MESSAGE_TYPE, mType);
+            if (mDomainBytes != null) {
+                addTlv(buffer, DHCP_DOMAIN_NAME, mDomainBytes);
+            }
+            if (mVendorInfoBytes != null) {
+                addTlv(buffer, DHCP_VENDOR_CLASS_ID, mVendorInfoBytes);
+            }
+            addTlvEnd(buffer);
+        }
+
+        // Convenience method.
+        public ByteBuffer build() {
+            // ENCAP_BOOTP packets don't contain ports, so just pass in 0.
+            ByteBuffer pkt = buildPacket(ENCAP_BOOTP, (short) 0, (short) 0);
+            pkt.flip();
+            return pkt;
+        }
+    }
+
+    private void assertDomainAndVendorInfoParses(
+            String expectedDomain, byte[] domainBytes,
+            String expectedVendorInfo, byte[] vendorInfoBytes) {
+        ByteBuffer packet = new TestDhcpPacket(DHCP_MESSAGE_TYPE_OFFER,
+                domainBytes, vendorInfoBytes).build();
+        DhcpPacket offerPacket = DhcpPacket.decodeFullPacket(packet, ENCAP_BOOTP);
+        assertEquals(expectedDomain, offerPacket.mDomainName);
+        assertEquals(expectedVendorInfo, offerPacket.mVendorId);
+    }
+
+    @SmallTest
+    public void testDomainName() throws Exception {
+        byte[] nullByte = new byte[] { 0x00 };
+        byte[] twoNullBytes = new byte[] { 0x00, 0x00 };
+        byte[] nonNullDomain = new byte[] {
+            (byte) 'g', (byte) 'o', (byte) 'o', (byte) '.', (byte) 'g', (byte) 'l'
+        };
+        byte[] trailingNullDomain = new byte[] {
+            (byte) 'g', (byte) 'o', (byte) 'o', (byte) '.', (byte) 'g', (byte) 'l', 0x00
+        };
+        byte[] embeddedNullsDomain = new byte[] {
+            (byte) 'g', (byte) 'o', (byte) 'o', 0x00, 0x00, (byte) 'g', (byte) 'l'
+        };
+        byte[] metered = "ANDROID_METERED".getBytes("US-ASCII");
+
+        byte[] meteredEmbeddedNull = metered.clone();
+        meteredEmbeddedNull[7] = (char) 0;
+
+        byte[] meteredTrailingNull = metered.clone();
+        meteredTrailingNull[meteredTrailingNull.length - 1] = (char) 0;
+
+        assertDomainAndVendorInfoParses("", nullByte, "\u0000", nullByte);
+        assertDomainAndVendorInfoParses("", twoNullBytes, "\u0000\u0000", twoNullBytes);
+        assertDomainAndVendorInfoParses("goo.gl", nonNullDomain, "ANDROID_METERED", metered);
+        assertDomainAndVendorInfoParses("goo", embeddedNullsDomain,
+                                        "ANDROID\u0000METERED", meteredEmbeddedNull);
+        assertDomainAndVendorInfoParses("goo.gl", trailingNullDomain,
+                                        "ANDROID_METERE\u0000", meteredTrailingNull);
+    }
+}