diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index a513d5eb961b0..a295164f711e5 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -11998,7 +11998,23 @@ public final class Settings { public static final String NITZ_NETWORK_DISCONNECT_RETENTION = "nitz_network_disconnect_retention"; - /** Preferred NTP server. {@hide} */ + /** + * The preferred NTP server. This setting overrides Android's static xml configuration when + * present and valid. + * + *

The legacy form is the NTP server name as a string. + *

Newer code should use the form: ntp://{server name}[:port] (the standard NTP port, + * 123, is used if not specified). + * + *

For example, the following examples are valid: + *

+ * + * @hide + */ @Readable public static final String NTP_SERVER = "ntp_server"; /** Timeout in milliseconds to wait for NTP server. {@hide} */ diff --git a/core/java/android/util/NtpTrustedTime.java b/core/java/android/util/NtpTrustedTime.java index 4036800129364..c544839f58896 100644 --- a/core/java/android/util/NtpTrustedTime.java +++ b/core/java/android/util/NtpTrustedTime.java @@ -32,8 +32,11 @@ import android.provider.Settings; import android.text.TextUtils; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import java.io.PrintWriter; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Duration; import java.time.Instant; import java.util.Objects; @@ -49,6 +52,56 @@ import java.util.function.Supplier; */ public class NtpTrustedTime implements TrustedTime { + private static final String URI_SCHEME_NTP = "ntp"; + + /** + * NTP server configuration. + * + * @hide + */ + public static final class NtpConfig { + + @NonNull private final URI mServerUri; + @NonNull private final Duration mTimeout; + + /** + * Creates an instance. If the arguments are invalid then an {@link + * IllegalArgumentException} will be thrown. See {@link #parseNtpUriStrict(String)} and + * {@link #parseNtpServerSetting(String)} to create valid URIs. + */ + public NtpConfig(@NonNull URI serverUri, @NonNull Duration timeout) + throws IllegalArgumentException { + try { + mServerUri = validateNtpServerUri(Objects.requireNonNull(serverUri)); + } catch (URISyntaxException e) { + throw new IllegalArgumentException("Bad URI", e); + } + + if (timeout.isNegative() || timeout.isZero()) { + throw new IllegalArgumentException("timeout < 0"); + } + mTimeout = timeout; + } + + @NonNull + public URI getServerUri() { + return mServerUri; + } + + @NonNull + public Duration getTimeout() { + return mTimeout; + } + + @Override + public String toString() { + return "NtpConnectionInfo{" + + "mServerUri=" + mServerUri + + ", mTimeout=" + mTimeout + + '}'; + } + } + /** * The result of a successful NTP query. * @@ -139,15 +192,7 @@ public class NtpTrustedTime implements TrustedTime { /** An in-memory config override for use during tests. */ @Nullable - private String mHostnameForTests; - - /** An in-memory config override for use during tests. */ - @Nullable - private Integer mPortForTests; - - /** An in-memory config override for use during tests. */ - @Nullable - private Duration mTimeoutForTests; + private NtpConfig mNtpConfigForTests; // Declared volatile and accessed outside of synchronized blocks to avoid blocking reads during // forceRefresh(). @@ -170,19 +215,16 @@ public class NtpTrustedTime implements TrustedTime { * Overrides the NTP server config for tests. Passing {@code null} to a parameter clears the * test value, i.e. so the normal value will be used next time. */ - public void setServerConfigForTests( - @Nullable String hostname, @Nullable Integer port, @Nullable Duration timeout) { + public void setServerConfigForTests(@NonNull NtpConfig ntpConfig) { synchronized (this) { - mHostnameForTests = hostname; - mPortForTests = port; - mTimeoutForTests = timeout; + mNtpConfigForTests = ntpConfig; } } @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553) public boolean forceRefresh() { synchronized (this) { - NtpConnectionInfo connectionInfo = getNtpConnectionInfo(); + NtpConfig connectionInfo = getNtpConfig(); if (connectionInfo == null) { // missing server config, so no NTP time available if (LOGD) Log.d(TAG, "forceRefresh: invalid server config"); @@ -213,9 +255,11 @@ public class NtpTrustedTime implements TrustedTime { if (LOGD) Log.d(TAG, "forceRefresh() from cache miss"); final SntpClient client = new SntpClient(); - final String serverName = connectionInfo.getServer(); - final int port = connectionInfo.getPort(); - final int timeoutMillis = connectionInfo.getTimeoutMillis(); + final URI ntpServerUri = connectionInfo.getServerUri(); + final String serverName = ntpServerUri.getHost(); + final int port = ntpServerUri.getPort() == -1 + ? SntpClient.STANDARD_NTP_PORT : ntpServerUri.getPort(); + final int timeoutMillis = saturatedCast(connectionInfo.getTimeout().toMillis()); if (client.requestTime(serverName, port, timeoutMillis, network)) { int ntpUncertaintyMillis = saturatedCast(client.getRoundTripTime() / 2); mTimeResult = new TimeResult( @@ -327,85 +371,115 @@ public class NtpTrustedTime implements TrustedTime { } } - private static class NtpConnectionInfo { - - @NonNull private final String mServer; - private final int mPort; - private final int mTimeoutMillis; - - NtpConnectionInfo(@NonNull String server, int port, int timeoutMillis) { - mServer = Objects.requireNonNull(server); - mPort = port; - mTimeoutMillis = timeoutMillis; - } - - @NonNull - public String getServer() { - return mServer; - } - - @NonNull - public int getPort() { - return mPort; - } - - int getTimeoutMillis() { - return mTimeoutMillis; - } - - @Override - public String toString() { - return "NtpConnectionInfo{" - + "mServer='" + mServer + '\'' - + ", mPort='" + mPort + '\'' - + ", mTimeoutMillis=" + mTimeoutMillis - + '}'; - } - } - @GuardedBy("this") - private NtpConnectionInfo getNtpConnectionInfo() { - final ContentResolver resolver = mContext.getContentResolver(); + private NtpConfig getNtpConfig() { + if (mNtpConfigForTests != null) { + return mNtpConfigForTests; + } + final ContentResolver resolver = mContext.getContentResolver(); final Resources res = mContext.getResources(); - final String hostname; - if (mHostnameForTests != null) { - hostname = mHostnameForTests; + // The Settings value has priority over static config. Check settings first. + final String serverGlobalSetting = + Settings.Global.getString(resolver, Settings.Global.NTP_SERVER); + final URI settingsServerInfo = parseNtpServerSetting(serverGlobalSetting); + + URI ntpServerUri; + if (settingsServerInfo != null) { + ntpServerUri = settingsServerInfo; } else { - String serverGlobalSetting = - Settings.Global.getString(resolver, Settings.Global.NTP_SERVER); - if (serverGlobalSetting != null) { - hostname = serverGlobalSetting; - } else { - hostname = res.getString(com.android.internal.R.string.config_ntpServer); + String configValue = res.getString(com.android.internal.R.string.config_ntpServer); + try { + ntpServerUri = parseNtpUriStrict(configValue); + } catch (URISyntaxException e) { + ntpServerUri = null; } } - final Integer port; - if (mPortForTests != null) { - port = mPortForTests; - } else { - port = SntpClient.STANDARD_NTP_PORT; - } + final int defaultTimeoutMillis = + res.getInteger(com.android.internal.R.integer.config_ntpTimeout); + final Duration timeout = Duration.ofMillis(Settings.Global.getInt( + resolver, Settings.Global.NTP_TIMEOUT, defaultTimeoutMillis)); + return ntpServerUri == null ? null : new NtpConfig(ntpServerUri, timeout); + } - final int timeoutMillis; - if (mTimeoutForTests != null) { - timeoutMillis = (int) mTimeoutForTests.toMillis(); + /** + * Parses and returns an NTP server config URI, or throws an exception if the URI doesn't + * conform to expectations. + * + *

NTP server config URIs are in the form "ntp://{hostname}[:port]". This is not a registered + * IANA URI scheme. + */ + public static URI parseNtpUriStrict(String ntpServerUriString) throws URISyntaxException { + // java.net.URI is used in preference to android.net.Uri, since android.net.Uri is very + // forgiving of obvious errors. URI catches issues sooner. + URI unvalidatedUri = new URI(ntpServerUriString); + return validateNtpServerUri(unvalidatedUri); + } + + /** + * Parses a setting string and returns a URI that will be accepted by {@link NtpConfig}, or + * {@code null} if the string does not produce a URI considered valid. + * + *

NTP server config URIs are in the form "ntp://{hostname}[:port]". This is not a registered + * IANA URI scheme. + * + *

Unlike {@link #parseNtpUriStrict(String)} this method will not throw an exception. It + * checks for a leading "ntp:" and will call through to {@link #parseNtpUriStrict(String)} to + * attempt to parse it, returning {@code null} if it fails. To support legacy settings values, + * it will also accept a string that only consists of a server name, which will be coerced into + * a URI in the form "ntp://{server name}". + */ + @VisibleForTesting + public static URI parseNtpServerSetting(String ntpServerSetting) { + if (TextUtils.isEmpty(ntpServerSetting)) { + return null; + } else if (ntpServerSetting.startsWith(URI_SCHEME_NTP + ":")) { + try { + return parseNtpUriStrict(ntpServerSetting); + } catch (URISyntaxException e) { + Log.w(TAG, "Rejected NTP uri setting=" + ntpServerSetting, e); + return null; + } } else { - int defaultTimeoutMillis = - res.getInteger(com.android.internal.R.integer.config_ntpTimeout); - timeoutMillis = Settings.Global.getInt( - resolver, Settings.Global.NTP_TIMEOUT, defaultTimeoutMillis); + // This is the legacy settings path. Assumes that the string is just a host name and + // creates a URI in the form ntp:// + try { + URI uri = new URI(URI_SCHEME_NTP, /*host=*/ntpServerSetting, + /*path=*/null, /*fragment=*/null); + // Paranoia: validate just in case the host name somehow results in a bad URI. + return validateNtpServerUri(uri); + } catch (URISyntaxException e) { + Log.w(TAG, "Rejected NTP legacy setting=" + ntpServerSetting, e); + return null; + } } - return TextUtils.isEmpty(hostname) ? null : - new NtpConnectionInfo(hostname, port, timeoutMillis); + } + + /** + * Checks that the supplied URI can be used to identify an NTP server. + * This method currently ignores Uri components that are not used, only checking the parts that + * must be present. Returns the supplied {@code uri} if validation is successful. + */ + private static URI validateNtpServerUri(URI uri) throws URISyntaxException { + if (!uri.isAbsolute()) { + throw new URISyntaxException(uri.toString(), "Relative URI not supported"); + } + if (!URI_SCHEME_NTP.equals(uri.getScheme())) { + throw new URISyntaxException(uri.toString(), "Unrecognized scheme"); + } + String host = uri.getHost(); + if (TextUtils.isEmpty(host)) { + throw new URISyntaxException(uri.toString(), "Missing host"); + } + return uri; } /** Prints debug information. */ public void dump(PrintWriter pw) { synchronized (this) { - pw.println("getNtpConnectionInfo()=" + getNtpConnectionInfo()); + pw.println("getNtpConfig()=" + getNtpConfig()); pw.println("mTimeResult=" + mTimeResult); if (mTimeResult != null) { pw.println("mTimeResult.getAgeMillis()=" diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index 2876161628c80..622414e3e381d 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -2308,8 +2308,10 @@ it should be disabled in that locale's resources. --> true - - time.android.com + + ntp://time.android.com 64800000 diff --git a/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java b/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java new file mode 100644 index 0000000000000..4d088b41310df --- /dev/null +++ b/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java @@ -0,0 +1,141 @@ +/* + * Copyright (C) 2022 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.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.net.URI; +import java.net.URISyntaxException; +import java.time.Duration; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +@SmallTest +@RunWith(AndroidJUnit4.class) +public class NtpTrustedTimeTest { + + // Valid absolute URIs, but not ones that will be accepted as NTP server URIs. + private static final List BAD_ABSOLUTE_NTP_URIS = Arrays.asList( + "ntp://:123/", + "ntp://:123", + "ntp://:/", + "ntp://:", + "ntp://foobar:abc/", + "ntp://foobar:abc", + "ntp://foobar:456:789/", + "ntp://foobar:456:789", + "ntp://foobar:456:abc/", + "ntp://foobar:456:abc" + ); + + // Valid relative URIs, but not ones that will be accepted as NTP server URIs. + private static final List BAD_RELATIVE_NTP_URIS = Arrays.asList( + "foobar", + "/foobar", + "foobar:456" + ); + + // Valid NTP server URIs: input value -> expected URI.toString() value. + private static final Map GOOD_NTP_URIS = Map.of( + "ntp://foobar", "ntp://foobar", + "ntp://foobar/", "ntp://foobar/", + "ntp://foobar:456/", "ntp://foobar:456/", + "ntp://foobar:456", "ntp://foobar:456" + ); + + private static final URI VALID_SERVER_URI = URI.create("ntp://foobar/"); + + @Test + public void testParseNtpServerSetting() { + assertEquals(URI.create("ntp://foobar"), NtpTrustedTime.parseNtpServerSetting("foobar")); + + // Legacy settings values which could easily be confused with relative URIs. Parsing of this + // legacy form doesn't have to be robust / treated as errors: Android has never supported + // string like these, and so they won't work properly. + assertNull(NtpTrustedTime.parseNtpServerSetting("foobar:123")); + assertNull(NtpTrustedTime.parseNtpServerSetting("/foobar")); + + // NTP URI cases that must not be accepted. + for (String badNtpUri : BAD_ABSOLUTE_NTP_URIS) { + assertNull("Input: \"" + badNtpUri + "\"", + NtpTrustedTime.parseNtpServerSetting(badNtpUri)); + } + + // Valid URIs + for (Map.Entry goodNtpUri : GOOD_NTP_URIS.entrySet()) { + URI uri = NtpTrustedTime.parseNtpServerSetting(goodNtpUri.getKey()); + assertNotNull(goodNtpUri.getKey(), uri); + assertEquals(goodNtpUri.getValue(), uri.toString()); + } + } + + @Test + public void testParseNtpUriStrict() throws Exception { + // ntp: URI cases that must not be accepted. + for (String badNtpUri : BAD_ABSOLUTE_NTP_URIS) { + assertParseNtpUriStrictThrows(badNtpUri); + } + + for (String badNtpUri : BAD_RELATIVE_NTP_URIS) { + assertParseNtpUriStrictThrows(badNtpUri); + } + + // Bad scheme. + assertParseNtpUriStrictThrows("notntp://foobar:123"); + + // Valid NTP URIs + for (Map.Entry goodNtpUri : GOOD_NTP_URIS.entrySet()) { + URI uri = NtpTrustedTime.parseNtpUriStrict(goodNtpUri.getKey()); + assertNotNull(goodNtpUri.getKey(), uri); + assertEquals(goodNtpUri.getValue(), uri.toString()); + } + } + + private void assertParseNtpUriStrictThrows(String badNtpUri) throws Exception { + assertThrows("Input: \"" + badNtpUri, URISyntaxException.class, + () -> NtpTrustedTime.parseNtpUriStrict(badNtpUri)); + } + + @Test(expected = NullPointerException.class) + public void testNtpConfig_nullConstructorServerInfo() { + new NtpTrustedTime.NtpConfig(null, Duration.ofSeconds(5)); + } + + @Test(expected = NullPointerException.class) + public void testNtpConfig_nullConstructorTimeout() { + new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, null); + } + + @Test(expected = IllegalArgumentException.class) + public void testNtpConfig_zeroTimeout() { + new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, Duration.ofMillis(0)); + } + + @Test(expected = IllegalArgumentException.class) + public void testNtpConfig_negativeTimeout() { + new NtpTrustedTime.NtpConfig(VALID_SERVER_URI, Duration.ofMillis(-1)); + } +} diff --git a/core/tests/coretests/src/android/util/OWNERS b/core/tests/coretests/src/android/util/OWNERS new file mode 100644 index 0000000000000..460e8c0737825 --- /dev/null +++ b/core/tests/coretests/src/android/util/OWNERS @@ -0,0 +1,2 @@ +per-file NtpTrustedTimeTest.java = file:/services/core/java/com/android/server/timezonedetector/OWNERS +per-file TypedValueTest.kt = file:/core/java/android/content/res/OWNERS diff --git a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java index 25f3f6190b111..48888f2e00dde 100644 --- a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java +++ b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateService.java @@ -195,15 +195,12 @@ public class NetworkTimeUpdateService extends Binder { * Overrides the NTP server config for tests. Passing {@code null} to a parameter clears the * test value, i.e. so the normal value will be used next time. */ - void setServerConfigForTests( - @Nullable String hostname, @Nullable Integer port, @Nullable Duration timeout) { + void setServerConfigForTests(@Nullable NtpTrustedTime.NtpConfig ntpConfig) { mContext.enforceCallingPermission( android.Manifest.permission.SET_TIME, "set NTP server config for tests"); - mLocalLog.log("Setting server config for tests: hostname=" + hostname - + ", port=" + port - + ", timeout=" + timeout); - mTime.setServerConfigForTests(hostname, port, timeout); + mLocalLog.log("Setting server config for tests: ntpConnectionInfo=" + ntpConfig); + mTime.setServerConfigForTests(ntpConfig); } private void onPollNetworkTime(int event) { diff --git a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java index 18dda55c42701..c8636b9943af1 100644 --- a/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java +++ b/services/core/java/com/android/server/timedetector/NetworkTimeUpdateServiceShellCommand.java @@ -18,8 +18,11 @@ package com.android.server.timedetector; import android.annotation.NonNull; import android.os.ShellCommand; +import android.util.NtpTrustedTime; import java.io.PrintWriter; +import java.net.URI; +import java.net.URISyntaxException; import java.time.Duration; import java.util.Objects; @@ -42,13 +45,18 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { private static final String SHELL_COMMAND_FORCE_REFRESH = "force_refresh"; /** - * A shell command that sets the NTP server config for tests. Config is cleared on reboot. + * A shell command that sets the NTP server config for tests. Config is cleared on reboot or + * using {@link #SHELL_COMMAND_RESET_SERVER_CONFIG}. */ - private static final String SHELL_COMMAND_SET_SERVER_CONFIG = "set_server_config"; - private static final String SET_SERVER_CONFIG_HOSTNAME_ARG = "--hostname"; - private static final String SET_SERVER_CONFIG_PORT_ARG = "--port"; + private static final String SHELL_COMMAND_SET_SERVER_CONFIG = "set_server_config_for_tests"; + private static final String SET_SERVER_CONFIG_SERVER_ARG = "--server"; private static final String SET_SERVER_CONFIG_TIMEOUT_ARG = "--timeout_millis"; + /** + * A shell command that resets the NTP server config for tests. + */ + private static final String SHELL_COMMAND_RESET_SERVER_CONFIG = "reset_server_config_for_tests"; + @NonNull private final NetworkTimeUpdateService mNetworkTimeUpdateService; @@ -69,6 +77,8 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { return runForceRefresh(); case SHELL_COMMAND_SET_SERVER_CONFIG: return runSetServerConfig(); + case SHELL_COMMAND_RESET_SERVER_CONFIG: + return runResetServerConfig(); default: { return handleDefaultCommands(cmd); } @@ -87,18 +97,17 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { } private int runSetServerConfig() { - String hostname = null; - Integer port = null; + URI serverUri = null; Duration timeout = null; String opt; while ((opt = getNextArg()) != null) { switch (opt) { - case SET_SERVER_CONFIG_HOSTNAME_ARG: { - hostname = getNextArgRequired(); - break; - } - case SET_SERVER_CONFIG_PORT_ARG: { - port = Integer.parseInt(getNextArgRequired()); + case SET_SERVER_CONFIG_SERVER_ARG: { + try { + serverUri = NtpTrustedTime.parseNtpUriStrict(getNextArgRequired()); + } catch (URISyntaxException e) { + throw new IllegalArgumentException("Bad NTP server value", e); + } break; } case SET_SERVER_CONFIG_TIMEOUT_ARG: { @@ -110,7 +119,23 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { } } } - mNetworkTimeUpdateService.setServerConfigForTests(hostname, port, timeout); + + if (serverUri == null) { + throw new IllegalArgumentException( + "Missing required option: --" + SET_SERVER_CONFIG_SERVER_ARG); + } + if (timeout == null) { + throw new IllegalArgumentException( + "Missing required option: --" + SET_SERVER_CONFIG_TIMEOUT_ARG); + } + + NtpTrustedTime.NtpConfig ntpConfig = new NtpTrustedTime.NtpConfig(serverUri, timeout); + mNetworkTimeUpdateService.setServerConfigForTests(ntpConfig); + return 0; + } + + private int runResetServerConfig() { + mNetworkTimeUpdateService.setServerConfigForTests(null); return 0; } @@ -126,11 +151,13 @@ class NetworkTimeUpdateServiceShellCommand extends ShellCommand { pw.printf(" Refreshes the latest time. Prints whether it was successful.\n"); pw.printf(" %s\n", SHELL_COMMAND_SET_SERVER_CONFIG); pw.printf(" Sets the NTP server config for tests. The config is not persisted.\n"); - pw.printf(" Options: [%s ] [%s ] [%s ]\n", - SET_SERVER_CONFIG_HOSTNAME_ARG, SET_SERVER_CONFIG_PORT_ARG, - SET_SERVER_CONFIG_TIMEOUT_ARG); - pw.printf(" Each key/value is optional and must be specified to override the\n"); - pw.printf(" normal value, not specifying a key causes it to reset to the original.\n"); + pw.printf(" Options: %s %s \n", + SET_SERVER_CONFIG_SERVER_ARG, SET_SERVER_CONFIG_TIMEOUT_ARG); + pw.printf(" The URI must be in the form \"ntp://hostname\" or" + + " \"ntp://hostname:port\""); + pw.printf(" %s\n", SHELL_COMMAND_RESET_SERVER_CONFIG); + pw.printf(" Resets/clears the NTP server config set via %s.\n", + SHELL_COMMAND_SET_SERVER_CONFIG); pw.println(); } }