Expressly forbid IP string literals as Private DNS hostnames
For obvious bootstrapping reasons, DNS settings have always used IP address string literals in input fields. However, since we can use the network-assigned nameservers to bootstrap our way to multiple IP addresses of multiple families (!), hostnames provide a clear simplicity and future-proofing advantage. Permitting IP address literals means not only making sure that we can validate X.509v3 certificates for IP addresses, but coping with the inevitable broken configurations where users may have configured IPv4 addresses but no IPv6 addresses. This will unnecessarily complicate life on IPv6-only networks. =) Test: as follows - built - flashed - booted - tried to enter IP string literals - make -j50 RunSettingsRoboTests ROBOTEST_FILTER=PrivateDnsModeDialogPreferenceTest Bug: 34953048 Bug: 64133961 Bug: 73641539 Change-Id: I7a58e86ed640ff5600906fb3d8cb9a2c75598831
This commit is contained in:
@@ -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() {
|
||||
|
@@ -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.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.mEditText.setText(invalid);
|
||||
|
||||
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();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@@ -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;
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user