diff --git a/src/com/android/settings/network/PrivateDnsModeDialogPreference.java b/src/com/android/settings/network/PrivateDnsModeDialogPreference.java index 3b09cc2e560..71a3e0d70a0 100644 --- a/src/com/android/settings/network/PrivateDnsModeDialogPreference.java +++ b/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: - // [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 8a60cf3a033..c5521be2d4e 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 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() { - // 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 00000000000..d1ac84c60a2 --- /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; + } +}