Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 08dac677 authored by Neil Fuller's avatar Neil Fuller
Browse files

Refactoring before supporting multiple NTP servers

Refactoring changes to make it easier to support NTP configuration that
contains multiple NTP servers to try. As part of this change, the NTP
server configuration is being changed from a simple hostname to a URI in
the form ntp://<host>[:port]. This is a local scheme introduced for this
commit for this purpose and not (currently) an official, registered IANA
URI scheme.

Settings parsing in NtpTrustedTime retains support for the simple
hostname-only form to avoid introducing incompatibilities when devices
upgrade and have settings values from earlier releases.  All other cases
(config.xml, commandline) are updated here to only support the new URI
form as these don't have backwards compatibility guarantees.

A later commit will add:
+ The settings / config changes to support a list of URIs in the format
  introduced in this commit, i.e. in place of the single one supported
  today.
+ The server request loop / server stickiness need to support multiple
  servers.

Switching to a URI over a simpler encoding scheme means:
1) Not having to deal with parsing awkward host + port cases, like IPv6
   addresses which contain colons, e.g. "[:::1]:1234" (these are used in
   in our tests).
2) Opens up the possibility of future protocols like NTS via a different
   URI scheme, and future parameterization of NTP server config with
   query params, paths, or similar.
3) Easy delimiting of lists later using a whitespace character like
   space, which will be encoded when part of a URI. This can be used for
   extending settings parsing to support multiple, space-delimited
   configs as settings code doesn't support multi-value settings out of
   the box (unlike config / commandline cases, which are comparatively
   easy).

This commit contains a new OWNERS file for NtpTrustedTimeTest. The file
has been populated with ownership information for TypedValueTest using
information from the OWNERS for the code under test.

Ignore-AOSP-First: The OWNERS change relates to a file landing
  internally
Bug: 223365217
Test: atest frameworks/base/core/tests/coretests/src/android/util/NtpTrustedTimeTest.java
Test: atest cts/tests/tests/os/src/android/os/cts/SystemClockSntpTest.java
Change-Id: I3afbd04659b4e4ee336619ea5ac6db7588057d6e
parent 28f3fe36
Loading
Loading
Loading
Loading
+17 −1
Original line number Diff line number Diff line
@@ -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.
         *
         * <p>The legacy form is the NTP server name as a string.
         * <p>Newer code should use the form: ntp://{server name}[:port] (the standard NTP port,
         * 123, is used if not specified).
         *
         * <p>For example, the following examples are valid:
         * <ul>
         *     <li>time.android.com</li>
         *     <li>ntp://time.android.com</li>
         *     <li>ntp://time.android.com:123</li>
         * </ul>
         *
         * @hide
         */
        @Readable
        public static final String NTP_SERVER = "ntp_server";
        /** Timeout in milliseconds to wait for NTP server. {@hide} */
+151 −77
Original line number Diff line number Diff line
@@ -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 {
    @GuardedBy("this")
    private NtpConfig getNtpConfig() {
        if (mNtpConfigForTests != null) {
            return mNtpConfigForTests;
        }

        final ContentResolver resolver = mContext.getContentResolver();
        final Resources res = mContext.getResources();

        @NonNull private final String mServer;
        private final int mPort;
        private final int mTimeoutMillis;
        // 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);

        NtpConnectionInfo(@NonNull String server, int port, int timeoutMillis) {
            mServer = Objects.requireNonNull(server);
            mPort = port;
            mTimeoutMillis = timeoutMillis;
        URI ntpServerUri;
        if (settingsServerInfo != null) {
            ntpServerUri = settingsServerInfo;
        } else {
            String configValue = res.getString(com.android.internal.R.string.config_ntpServer);
            try {
                ntpServerUri = parseNtpUriStrict(configValue);
            } catch (URISyntaxException e) {
                ntpServerUri = null;
            }

        @NonNull
        public String getServer() {
            return mServer;
        }

        @NonNull
        public int getPort() {
            return mPort;
        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);
    }

        int getTimeoutMillis() {
            return mTimeoutMillis;
    /**
     * Parses and returns an NTP server config URI, or throws an exception if the URI doesn't
     * conform to expectations.
     *
     * <p>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);
    }

        @Override
        public String toString() {
            return "NtpConnectionInfo{"
                    + "mServer='" + mServer + '\''
                    + ", mPort='" + mPort + '\''
                    + ", mTimeoutMillis=" + mTimeoutMillis
                    + '}';
        }
    /**
     * 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.
     *
     * <p>NTP server config URIs are in the form "ntp://{hostname}[:port]". This is not a registered
     * IANA URI scheme.
     *
     * <p>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;
            }

    @GuardedBy("this")
    private NtpConnectionInfo getNtpConnectionInfo() {
        final ContentResolver resolver = mContext.getContentResolver();

        final Resources res = mContext.getResources();

        final String hostname;
        if (mHostnameForTests != null) {
            hostname = mHostnameForTests;
        } 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);
            // This is the legacy settings path. Assumes that the string is just a host name and
            // creates a URI in the form ntp://<host name>
            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;
            }
        }

        final Integer port;
        if (mPortForTests != null) {
            port = mPortForTests;
        } else {
            port = SntpClient.STANDARD_NTP_PORT;
    }

        final int timeoutMillis;
        if (mTimeoutForTests != null) {
            timeoutMillis = (int) mTimeoutForTests.toMillis();
        } else {
            int defaultTimeoutMillis =
                    res.getInteger(com.android.internal.R.integer.config_ntpTimeout);
            timeoutMillis = Settings.Global.getInt(
                    resolver, Settings.Global.NTP_TIMEOUT, defaultTimeoutMillis);
    /**
     * 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 TextUtils.isEmpty(hostname) ? null :
            new NtpConnectionInfo(hostname, port, timeoutMillis);
        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()="
+4 −2
Original line number Diff line number Diff line
@@ -2308,8 +2308,10 @@
         it should be disabled in that locale's resources. -->
    <bool name="config_actionMenuItemAllCaps">true</bool>

    <!-- Remote server that can provide NTP responses. -->
    <string translatable="false" name="config_ntpServer">time.android.com</string>
    <!-- Remote server that can provide NTP responses.
         Values must be in the form: "ntp://<host>[:port]"
         This is not a registered IANA URI scheme. -->
    <string translatable="false" name="config_ntpServer">ntp://time.android.com</string>
    <!-- Normal polling frequency in milliseconds -->
    <integer name="config_ntpPollingInterval">64800000</integer>
    <!-- Try-again polling interval in milliseconds, in case the network request failed -->
+141 −0
Original line number Diff line number Diff line
/*
 * 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<String> 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<String> BAD_RELATIVE_NTP_URIS = Arrays.asList(
            "foobar",
            "/foobar",
            "foobar:456"
    );

    // Valid NTP server URIs: input value -> expected URI.toString() value.
    private static final Map<String, String> 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<String, String> 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<String, String> 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));
    }
}
+2 −0
Original line number Diff line number Diff line
per-file NtpTrustedTimeTest.java = file:/services/core/java/com/android/server/timezonedetector/OWNERS
per-file TypedValueTest.kt = file:/core/java/android/content/res/OWNERS
Loading