Merge "Expressly forbid IP string literals as Private DNS hostnames" into pi-dev
diff --git a/src/com/android/settings/network/PrivateDnsModeDialogPreference.java b/src/com/android/settings/network/PrivateDnsModeDialogPreference.java
index 3b09cc2..71a3e0d 100644
--- a/src/com/android/settings/network/PrivateDnsModeDialogPreference.java
+++ b/src/com/android/settings/network/PrivateDnsModeDialogPreference.java
@@ -18,6 +18,8 @@
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.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.settingslib.CustomDialogPreference;
import com.android.settingslib.HelpUtils;
+import java.net.InetAddress;
import java.util.HashMap;
import java.util.Map;
@@ -67,6 +71,8 @@
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 @@
}
private boolean isWeaklyValidatedHostname(String hostname) {
- // TODO(b/34953048): Find and use a better validation method. Specifically:
- // [1] this should reject IP string literals, and
- // [2] do the best, simplest, future-proof verification that
- // the input approximates a DNS hostname.
+ // TODO(b/34953048): Use a validation method that permits more accurate,
+ // but still inexpensive, checking of likely valid DNS hostnames.
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() {
diff --git a/tests/robotests/src/com/android/settings/network/PrivateDnsModeDialogPreferenceTest.java b/tests/robotests/src/com/android/settings/network/PrivateDnsModeDialogPreferenceTest.java
index 8a60cf3..c5521be 100644
--- a/tests/robotests/src/com/android/settings/network/PrivateDnsModeDialogPreferenceTest.java
+++ b/tests/robotests/src/com/android/settings/network/PrivateDnsModeDialogPreferenceTest.java
@@ -37,16 +37,19 @@
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 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 @@
@Test
public void testOnCheckedChanged_switchMode_saveButtonHasCorrectState() {
- // Set invalid hostname
- mPreference.mEditText.setText(INVALID_HOST_NAME);
+ final String[] INVALID_HOST_NAMES = new String[] {
+ INVALID_HOST_NAME,
+ "2001:db8::53", // IPv6 string literal
+ "192.168.1.1", // IPv4 string literal
+ };
- mPreference.onCheckedChanged(null, R.id.private_dns_mode_opportunistic);
- assertThat(mSaveButton.isEnabled()).isTrue();
+ for (String invalid : INVALID_HOST_NAMES) {
+ // Set invalid hostname
+ mPreference.mEditText.setText(invalid);
- 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()).named("off: " + invalid).isTrue();
- mPreference.onCheckedChanged(null, R.id.private_dns_mode_off);
- assertThat(mSaveButton.isEnabled()).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();
+ }
}
}
diff --git a/tests/robotests/src/com/android/settings/testutils/shadow/ShadowOs.java b/tests/robotests/src/com/android/settings/testutils/shadow/ShadowOs.java
new file mode 100644
index 0000000..d1ac84c
--- /dev/null
+++ b/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;
+ }
+}