Merge "Expressly forbid IP string literals as Private DNS hostnames" into pi-dev

src/com/android/settings/network/PrivateDnsModeDialogPreference.java

@@ -18,6 +18,8 @@ package com.android.settings.network;
 import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OFF;
 import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC;
 import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_PROVIDER_HOSTNAME;
+import static android.system.OsConstants.AF_INET;
+import static android.system.OsConstants.AF_INET6;
 
 import android.app.AlertDialog;
 import android.content.ActivityNotFoundException;
@@ -27,6 +29,7 @@ import android.content.DialogInterface;
 import android.content.Intent;
 import android.provider.Settings;
 import android.support.annotation.VisibleForTesting;
+import android.system.Os;
 import android.text.Editable;
 import android.text.TextWatcher;
 import android.text.method.LinkMovementMethod;
@@ -45,6 +48,7 @@ import com.android.settings.utils.AnnotationSpan;
 import com.android.settingslib.CustomDialogPreference;
 import com.android.settingslib.HelpUtils;
 
+import java.net.InetAddress;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -67,6 +71,8 @@ public class PrivateDnsModeDialogPreference extends CustomDialogPreference imple
         PRIVATE_DNS_MAP.put(PRIVATE_DNS_MODE_PROVIDER_HOSTNAME, R.id.private_dns_mode_provider);
     }
 
+    private static final int[] ADDRESS_FAMILIES = new int[]{AF_INET, AF_INET6};
+
     @VisibleForTesting
     static final String MODE_KEY = Settings.Global.PRIVATE_DNS_MODE;
     @VisibleForTesting
@@ -180,12 +186,20 @@ public class PrivateDnsModeDialogPreference extends CustomDialogPreference imple
     }
 
     private boolean isWeaklyValidatedHostname(String hostname) {
-        // TODO(b/34953048): Find and use a better validation method.  Specifically:
+        // TODO(b/34953048): Use a validation method that permits more accurate,
-        //     [1] this should reject IP string literals, and
+        // but still inexpensive, checking of likely valid DNS hostnames.
-        //     [2] do the best, simplest, future-proof verification that
-        //         the input approximates a DNS hostname.
         final String WEAK_HOSTNAME_REGEX = "^[a-zA-Z0-9_.-]+$";
-        return hostname.matches(WEAK_HOSTNAME_REGEX);
+        if (!hostname.matches(WEAK_HOSTNAME_REGEX)) {
+            return false;
+        }
+
+        for (int address_family : ADDRESS_FAMILIES) {
+            if (Os.inet_pton(address_family, hostname) != null) {
+                return false;
+            }
+        }
+
+        return true;
     }
 
     private Button getSaveButton() {

tests/robotests/src/com/android/settings/network/PrivateDnsModeDialogPreferenceTest.java

@@ -37,16 +37,19 @@ import android.widget.LinearLayout;
 
 import com.android.settings.R;
 import com.android.settingslib.CustomDialogPreference.CustomPreferenceDialogFragment;
+import com.android.settings.testutils.shadow.ShadowOs;
 import com.android.settings.testutils.SettingsRobolectricTestRunner;
 
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.util.ReflectionHelpers;
 
 @RunWith(SettingsRobolectricTestRunner.class)
+@Config(shadows = ShadowOs.class)
 public class PrivateDnsModeDialogPreferenceTest {
 
     private static final String HOST_NAME = "dns.example.com";
@@ -61,6 +64,9 @@ public class PrivateDnsModeDialogPreferenceTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
 
+        ReflectionHelpers.setStaticField(android.system.OsConstants.class, "AF_INET", 2);
+        ReflectionHelpers.setStaticField(android.system.OsConstants.class, "AF_INET6", 10);
+
         mContext = RuntimeEnvironment.application;
         mSaveButton = new Button(mContext);
 
@@ -122,16 +128,24 @@ public class PrivateDnsModeDialogPreferenceTest {
 
     @Test
     public void testOnCheckedChanged_switchMode_saveButtonHasCorrectState() {
+        final String[] INVALID_HOST_NAMES = new String[] {
+            INVALID_HOST_NAME,
+            "2001:db8::53",  // IPv6 string literal
+            "192.168.1.1",   // IPv4 string literal
+        };
+
+        for (String invalid : INVALID_HOST_NAMES) {
             // Set invalid hostname
-        mPreference.mEditText.setText(INVALID_HOST_NAME);
+            mPreference.mEditText.setText(invalid);
-
-        mPreference.onCheckedChanged(null, R.id.private_dns_mode_opportunistic);
-        assertThat(mSaveButton.isEnabled()).isTrue();
-
-        mPreference.onCheckedChanged(null, R.id.private_dns_mode_provider);
-        assertThat(mSaveButton.isEnabled()).isFalse();
 
             mPreference.onCheckedChanged(null, R.id.private_dns_mode_off);
-        assertThat(mSaveButton.isEnabled()).isTrue();
+            assertThat(mSaveButton.isEnabled()).named("off: " + invalid).isTrue();
+
+            mPreference.onCheckedChanged(null, R.id.private_dns_mode_opportunistic);
+            assertThat(mSaveButton.isEnabled()).named("opportunistic: " + invalid).isTrue();
+
+            mPreference.onCheckedChanged(null, R.id.private_dns_mode_provider);
+            assertThat(mSaveButton.isEnabled()).named("provider: " + invalid).isFalse();
+        }
     }
 }

tests/robotests/src/com/android/settings/testutils/shadow/ShadowOs.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2018 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 com.android.settings.testutils.shadow;
+
+import static android.system.OsConstants.AF_INET;
+import static android.system.OsConstants.AF_INET6;
+
+import android.system.Os;
+
+import org.robolectric.annotation.Implementation;
+import org.robolectric.annotation.Implements;
+
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+import java.util.regex.Pattern;
+
+@Implements(Os.class)
+public class ShadowOs {
+    // These are not actually correct, but good enough for the test
+    private static final Pattern IPV4_PATTERN =
+        Pattern.compile("^\\d{1,3}(\\.\\d{1,3}){3}$");
+    private static final Pattern IPV6_PATTERN =
+        Pattern.compile("^[0-9a-f]{1,4}(:[0-9a-f]{0,4}){0,6}$");
+
+    private static final byte[] IPV4_BYTES = new byte[4];
+    private static final byte[] IPV6_BYTES = new byte[16];
+
+    @Implementation
+    public static InetAddress inet_pton(int family, String address) {
+        if ((AF_INET  == family && IPV4_PATTERN.matcher(address).find()) ||
+            (AF_INET6 == family && IPV6_PATTERN.matcher(address).find())) {
+            try {
+                return InetAddress.getByAddress((AF_INET == family) ? IPV4_BYTES : IPV6_BYTES);
+            } catch (UnknownHostException uhe) {
+                // Shouldn't be reached.
+            }
+        }
+        return null;
+    }
+}