Merge "Make NtpTrustedTime safer / expand docs"

am: 49c903a0d2

Change-Id: Ic7f5229626383d2f9d25aead72966c617684f346

6 files changed, 249 insertions, 128 deletions

diff --git a/core/java/android/util/NtpTrustedTime.java b/core/java/android/util/NtpTrustedTime.java
index fa994ba76fd7..0892c94d5bec 100644
--- a/core/java/android/util/NtpTrustedTime.java
+++ b/core/java/android/util/NtpTrustedTime.java
@@ -16,6 +16,8 @@
 
 package android.util;
 
+import android.annotation.NonNull;
+import android.annotation.Nullable;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.content.ContentResolver;
 import android.content.Context;
@@ -25,172 +27,270 @@ import android.net.Network;
 import android.net.NetworkInfo;
 import android.net.SntpClient;
 import android.os.SystemClock;
-import android.os.TimestampedValue;
 import android.provider.Settings;
 import android.text.TextUtils;
 
+import com.android.internal.annotations.GuardedBy;
+
+import java.util.Objects;
+import java.util.function.Supplier;
+
 /**
- * {@link TrustedTime} that connects with a remote NTP server as its trusted
- * time source.
+ * A singleton that connects with a remote NTP server as its trusted time source. This class
+ * is thread-safe. The {@link #forceRefresh()} method is synchronous, i.e. it may occupy the
+ * current thread while performing an NTP request. All other threads calling {@link #forceRefresh()}
+ * will block during that request.
  *
  * @hide
  */
 public class NtpTrustedTime implements TrustedTime {
+
+    /**
+     * The result of a successful NTP query.
+     *
+     * @hide
+     */
+    public static class TimeResult {
+        private final long mTimeMillis;
+        private final long mElapsedRealtimeMillis;
+        private final long mCertaintyMillis;
+
+        public TimeResult(long timeMillis, long elapsedRealtimeMillis, long certaintyMillis) {
+            mTimeMillis = timeMillis;
+            mElapsedRealtimeMillis = elapsedRealtimeMillis;
+            mCertaintyMillis = certaintyMillis;
+        }
+
+        public long getTimeMillis() {
+            return mTimeMillis;
+        }
+
+        public long getElapsedRealtimeMillis() {
+            return mElapsedRealtimeMillis;
+        }
+
+        public long getCertaintyMillis() {
+            return mCertaintyMillis;
+        }
+
+        /** Calculates and returns the current time accounting for the age of this result. */
+        public long currentTimeMillis() {
+            return mTimeMillis + getAgeMillis();
+        }
+
+        /** Calculates and returns the age of this result. */
+        public long getAgeMillis() {
+            return SystemClock.elapsedRealtime() - mElapsedRealtimeMillis;
+        }
+
+        @Override
+        public String toString() {
+            return "TimeResult{"
+                    + "mTimeMillis=" + mTimeMillis
+                    + ", mElapsedRealtimeMillis=" + mElapsedRealtimeMillis
+                    + ", mCertaintyMillis=" + mCertaintyMillis
+                    + '}';
+        }
+    }
+
     private static final String TAG = "NtpTrustedTime";
     private static final boolean LOGD = false;
 
     private static NtpTrustedTime sSingleton;
-    private static Context sContext;
 
-    private final String mServer;
-    private final long mTimeout;
+    @NonNull
+    private final Context mContext;
+
+    /**
+     * A supplier that returns the ConnectivityManager. The Supplier can return null if
+     * ConnectivityService isn't running yet.
+     */
+    private final Supplier<ConnectivityManager> mConnectivityManagerSupplier =
+            new Supplier<ConnectivityManager>() {
+        private ConnectivityManager mConnectivityManager;
 
-    private ConnectivityManager mCM;
+        @Nullable
+        @Override
+        public synchronized ConnectivityManager get() {
+            // We can't do this at initialization time: ConnectivityService might not be running
+            // yet.
+            if (mConnectivityManager == null) {
+                mConnectivityManager = mContext.getSystemService(ConnectivityManager.class);
+            }
+            return mConnectivityManager;
+        }
+    };
 
-    private boolean mHasCache;
-    private long mCachedNtpTime;
-    private long mCachedNtpElapsedRealtime;
-    private long mCachedNtpCertainty;
+    // Declared volatile and accessed outside of synchronized blocks to avoid blocking reads during
+    // forceRefresh().
+    private volatile TimeResult mTimeResult;
 
-    private NtpTrustedTime(String server, long timeout) {
-        if (LOGD) Log.d(TAG, "creating NtpTrustedTime using " + server);
-        mServer = server;
-        mTimeout = timeout;
+    private NtpTrustedTime(Context context) {
+        mContext = Objects.requireNonNull(context);
     }
 
     @UnsupportedAppUsage
     public static synchronized NtpTrustedTime getInstance(Context context) {
         if (sSingleton == null) {
-            final Resources res = context.getResources();
-            final ContentResolver resolver = context.getContentResolver();
-
-            final String defaultServer = res.getString(
-                    com.android.internal.R.string.config_ntpServer);
-            final long defaultTimeout = res.getInteger(
-                    com.android.internal.R.integer.config_ntpTimeout);
-
-            final String secureServer = Settings.Global.getString(
-                    resolver, Settings.Global.NTP_SERVER);
-            final long timeout = Settings.Global.getLong(
-                    resolver, Settings.Global.NTP_TIMEOUT, defaultTimeout);
-
-            final String server = secureServer != null ? secureServer : defaultServer;
-            sSingleton = new NtpTrustedTime(server, timeout);
-            sContext = context;
+            Context appContext = context.getApplicationContext();
+            sSingleton = new NtpTrustedTime(appContext);
         }
-
         return sSingleton;
     }
 
-    @Override
     @UnsupportedAppUsage
     public boolean forceRefresh() {
-        // We can't do this at initialization time: ConnectivityService might not be running yet.
         synchronized (this) {
-            if (mCM == null) {
-                mCM = sContext.getSystemService(ConnectivityManager.class);
+            NtpConnectionInfo connectionInfo = getNtpConnectionInfo();
+            if (connectionInfo == null) {
+                // missing server config, so no trusted time available
+                if (LOGD) Log.d(TAG, "forceRefresh: invalid server config");
+                return false;
             }
-        }
 
-        final Network network = mCM == null ? null : mCM.getActiveNetwork();
-        return forceRefresh(network);
-    }
-
-    public boolean forceRefresh(Network network) {
-        if (TextUtils.isEmpty(mServer)) {
-            // missing server, so no trusted time available
-            return false;
-        }
-
-        // We can't do this at initialization time: ConnectivityService might not be running yet.
-        synchronized (this) {
-            if (mCM == null) {
-                mCM = sContext.getSystemService(ConnectivityManager.class);
+            ConnectivityManager connectivityManager = mConnectivityManagerSupplier.get();
+            if (connectivityManager == null) {
+                if (LOGD) Log.d(TAG, "forceRefresh: no ConnectivityManager");
+                return false;
+            }
+            final Network network = connectivityManager.getActiveNetwork();
+            final NetworkInfo ni = connectivityManager.getNetworkInfo(network);
+            if (ni == null || !ni.isConnected()) {
+                if (LOGD) Log.d(TAG, "forceRefresh: no connectivity");
+                return false;
             }
-        }
-
-        final NetworkInfo ni = mCM == null ? null : mCM.getNetworkInfo(network);
-        if (ni == null || !ni.isConnected()) {
-            if (LOGD) Log.d(TAG, "forceRefresh: no connectivity");
-            return false;
-        }
-
 
-        if (LOGD) Log.d(TAG, "forceRefresh() from cache miss");
-        final SntpClient client = new SntpClient();
-        if (client.requestTime(mServer, (int) mTimeout, network)) {
-            mHasCache = true;
-            mCachedNtpTime = client.getNtpTime();
-            mCachedNtpElapsedRealtime = client.getNtpTimeReference();
-            mCachedNtpCertainty = client.getRoundTripTime() / 2;
-            return true;
-        } else {
-            return false;
+            if (LOGD) Log.d(TAG, "forceRefresh() from cache miss");
+            final SntpClient client = new SntpClient();
+            final String serverName = connectionInfo.getServer();
+            final int timeoutMillis = connectionInfo.getTimeoutMillis();
+            if (client.requestTime(serverName, timeoutMillis, network)) {
+                long ntpCertainty = client.getRoundTripTime() / 2;
+                mTimeResult = new TimeResult(
+                        client.getNtpTime(), client.getNtpTimeReference(), ntpCertainty);
+                return true;
+            } else {
+                return false;
+            }
         }
     }
 
-    @Override
+    /**
+     * Only kept for UnsupportedAppUsage.
+     *
+     * @deprecated Use {@link #getCachedTimeResult()} to obtain a {@link TimeResult} atomically.
+     */
+    @Deprecated
     @UnsupportedAppUsage
     public boolean hasCache() {
-        return mHasCache;
+        return mTimeResult != null;
     }
 
+    /**
+     * Only kept for UnsupportedAppUsage.
+     *
+     * @deprecated Use {@link #getCachedTimeResult()} to obtain a {@link TimeResult} atomically.
+     */
+    @Deprecated
     @Override
     public long getCacheAge() {
-        if (mHasCache) {
-            return SystemClock.elapsedRealtime() - mCachedNtpElapsedRealtime;
+        TimeResult timeResult = mTimeResult;
+        if (timeResult != null) {
+            return SystemClock.elapsedRealtime() - timeResult.getElapsedRealtimeMillis();
         } else {
             return Long.MAX_VALUE;
         }
     }
 
-    @Override
-    public long getCacheCertainty() {
-        if (mHasCache) {
-            return mCachedNtpCertainty;
-        } else {
-            return Long.MAX_VALUE;
-        }
-    }
-
-    @Override
+    /**
+     * Only kept for UnsupportedAppUsage.
+     *
+     * @deprecated Use {@link #getCachedTimeResult()} to obtain a {@link TimeResult} atomically.
+     */
+    @Deprecated
     @UnsupportedAppUsage
     public long currentTimeMillis() {
-        if (!mHasCache) {
+        TimeResult timeResult = mTimeResult;
+        if (timeResult == null) {
             throw new IllegalStateException("Missing authoritative time source");
         }
         if (LOGD) Log.d(TAG, "currentTimeMillis() cache hit");
 
         // current time is age after the last ntp cache; callers who
-        // want fresh values will hit makeAuthoritative() first.
-        return mCachedNtpTime + getCacheAge();
+        // want fresh values will hit forceRefresh() first.
+        return timeResult.currentTimeMillis();
     }
 
+    /**
+     * Only kept for UnsupportedAppUsage.
+     *
+     * @deprecated Use {@link #getCachedTimeResult()} to obtain a {@link TimeResult} atomically.
+     */
+    @Deprecated
     @UnsupportedAppUsage
     public long getCachedNtpTime() {
         if (LOGD) Log.d(TAG, "getCachedNtpTime() cache hit");
-        return mCachedNtpTime;
+        TimeResult timeResult = mTimeResult;
+        return timeResult == null ? 0 : timeResult.getTimeMillis();
     }
 
+    /**
+     * Only kept for UnsupportedAppUsage.
+     *
+     * @deprecated Use {@link #getCachedTimeResult()} to obtain a {@link TimeResult} atomically.
+     */
+    @Deprecated
     @UnsupportedAppUsage
     public long getCachedNtpTimeReference() {
-        return mCachedNtpElapsedRealtime;
+        TimeResult timeResult = mTimeResult;
+        return timeResult == null ? 0 : timeResult.getElapsedRealtimeMillis();
     }
 
     /**
-     * Returns the combination of {@link #getCachedNtpTime()} and {@link
-     * #getCachedNtpTimeReference()} as a {@link TimestampedValue}. This method is useful when
-     * passing the time to another component that will adjust for elapsed time.
-     *
-     * @throws IllegalStateException if there is no cached value
+     * Returns an object containing the latest NTP information available. Can return {@code null} if
+     * no information is available.
      */
-    public TimestampedValue<Long> getCachedNtpTimeSignal() {
-        if (!mHasCache) {
-            throw new IllegalStateException("Missing authoritative time source");
+    @Nullable
+    public TimeResult getCachedTimeResult() {
+        return mTimeResult;
+    }
+
+    private static class NtpConnectionInfo {
+
+        @NonNull private final String mServer;
+        private final int mTimeoutMillis;
+
+        NtpConnectionInfo(@NonNull String server, int timeoutMillis) {
+            mServer = Objects.requireNonNull(server);
+            mTimeoutMillis = timeoutMillis;
         }
-        if (LOGD) Log.d(TAG, "getCachedNtpTimeSignal() cache hit");
 
-        return new TimestampedValue<>(mCachedNtpElapsedRealtime, mCachedNtpTime);
+        @NonNull
+        public String getServer() {
+            return mServer;
+        }
+
+        int getTimeoutMillis() {
+            return mTimeoutMillis;
+        }
     }
 
+    @GuardedBy("this")
+    private NtpConnectionInfo getNtpConnectionInfo() {
+        final ContentResolver resolver = mContext.getContentResolver();
+
+        final Resources res = mContext.getResources();
+        final String defaultServer = res.getString(
+                com.android.internal.R.string.config_ntpServer);
+        final int defaultTimeoutMillis = res.getInteger(
+                com.android.internal.R.integer.config_ntpTimeout);
+
+        final String secureServer = Settings.Global.getString(
+                resolver, Settings.Global.NTP_SERVER);
+        final int timeoutMillis = Settings.Global.getInt(
+                resolver, Settings.Global.NTP_TIMEOUT, defaultTimeoutMillis);
+
+        final String server = secureServer != null ? secureServer : defaultServer;
+        return TextUtils.isEmpty(server) ? null : new NtpConnectionInfo(server, timeoutMillis);
+    }
 }
diff --git a/core/java/android/util/TrustedTime.java b/core/java/android/util/TrustedTime.java
index 1360f874507f..f41fe85fa8bb 100644
--- a/core/java/android/util/TrustedTime.java
+++ b/core/java/android/util/TrustedTime.java
@@ -20,42 +20,48 @@ import android.compat.annotation.UnsupportedAppUsage;
 
 /**
  * Interface that provides trusted time information, possibly coming from an NTP
- * server. Implementations may cache answers until {@link #forceRefresh()}.
+ * server.
  *
  * @hide
+ * @deprecated Only kept for UnsupportedAppUsage. Do not use. See {@link NtpTrustedTime}
  */
 public interface TrustedTime {
     /**
      * Force update with an external trusted time source, returning {@code true}
      * when successful.
+     *
+     * @deprecated Only kept for UnsupportedAppUsage. Do not use. See {@link NtpTrustedTime}
      */
+    @Deprecated
     @UnsupportedAppUsage
     public boolean forceRefresh();
 
     /**
      * Check if this instance has cached a response from a trusted time source.
+     *
+     * @deprecated Only kept for UnsupportedAppUsage. Do not use. See {@link NtpTrustedTime}
      */
+    @Deprecated
     @UnsupportedAppUsage
-    public boolean hasCache();
+    boolean hasCache();
 
     /**
      * Return time since last trusted time source contact, or
      * {@link Long#MAX_VALUE} if never contacted.
+     *
+     * @deprecated Only kept for UnsupportedAppUsage. Do not use. See {@link NtpTrustedTime}
      */
+    @Deprecated
     @UnsupportedAppUsage
     public long getCacheAge();
 
     /**
-     * Return certainty of cached trusted time in milliseconds, or
-     * {@link Long#MAX_VALUE} if never contacted. Smaller values are more
-     * precise.
-     */
-    public long getCacheCertainty();
-
-    /**
      * Return current time similar to {@link System#currentTimeMillis()},
      * possibly using a cached authoritative time source.
+     *
+     * @deprecated Only kept for UnsupportedAppUsage. Do not use. See {@link NtpTrustedTime}
      */
+    @Deprecated
     @UnsupportedAppUsage
-    public long currentTimeMillis();
+    long currentTimeMillis();
 }
diff --git a/services/core/java/com/android/server/AlarmManagerService.java b/services/core/java/com/android/server/AlarmManagerService.java
index d16244167c62..bbb7c523bac9 100644
--- a/services/core/java/com/android/server/AlarmManagerService.java
+++ b/services/core/java/com/android/server/AlarmManagerService.java
@@ -2167,8 +2167,9 @@ class AlarmManagerService extends SystemService {
         @Override
         public long currentNetworkTimeMillis() {
             final NtpTrustedTime time = NtpTrustedTime.getInstance(getContext());
-            if (time.hasCache()) {
-                return time.currentTimeMillis();
+            NtpTrustedTime.TimeResult ntpResult = time.getCachedTimeResult();
+            if (ntpResult != null) {
+                return ntpResult.currentTimeMillis();
             } else {
                 throw new ParcelableException(new DateTimeException("Missing NTP fix"));
             }
diff --git a/services/core/java/com/android/server/NetworkTimeUpdateServiceImpl.java b/services/core/java/com/android/server/NetworkTimeUpdateServiceImpl.java
index d20936c2d217..7894788cb0b6 100644
--- a/services/core/java/com/android/server/NetworkTimeUpdateServiceImpl.java
+++ b/services/core/java/com/android/server/NetworkTimeUpdateServiceImpl.java
@@ -154,17 +154,20 @@ public class NetworkTimeUpdateServiceImpl extends Binder implements NetworkTimeU
 
     private void onPollNetworkTimeUnderWakeLock(int event) {
         // Force an NTP fix when outdated
-        if (mTime.getCacheAge() >= mPollingIntervalMs) {
+        NtpTrustedTime.TimeResult cachedNtpResult = mTime.getCachedTimeResult();
+        if (cachedNtpResult == null || cachedNtpResult.getAgeMillis() >= mPollingIntervalMs) {
             if (DBG) Log.d(TAG, "Stale NTP fix; forcing refresh");
             mTime.forceRefresh();
+            cachedNtpResult = mTime.getCachedTimeResult();
         }
 
-        if (mTime.getCacheAge() < mPollingIntervalMs) {
+        if (cachedNtpResult != null && cachedNtpResult.getAgeMillis() < mPollingIntervalMs) {
             // Obtained fresh fix; schedule next normal update
             resetAlarm(mPollingIntervalMs);
 
             // Suggest the time to the time detector. It may choose use it to set the system clock.
-            TimestampedValue<Long> timeSignal = mTime.getCachedNtpTimeSignal();
+            TimestampedValue<Long> timeSignal = new TimestampedValue<>(
+                    cachedNtpResult.getElapsedRealtimeMillis(), cachedNtpResult.getTimeMillis());
             NetworkTimeSuggestion timeSuggestion = new NetworkTimeSuggestion(timeSignal);
             timeSuggestion.addDebugInfo("Origin: NetworkTimeUpdateServiceImpl. event=" + event);
             mTimeDetector.suggestNetworkTime(timeSuggestion);
@@ -275,8 +278,11 @@ public class NetworkTimeUpdateServiceImpl extends Binder implements NetworkTimeU
         TimeUtils.formatDuration(mPollingIntervalShorterMs, pw);
         pw.println("\nTryAgainTimesMax: " + mTryAgainTimesMax);
         pw.println("\nTryAgainCounter: " + mTryAgainCounter);
-        pw.println("NTP cache age: " + mTime.getCacheAge());
-        pw.println("NTP cache certainty: " + mTime.getCacheCertainty());
+        NtpTrustedTime.TimeResult ntpResult = mTime.getCachedTimeResult();
+        pw.println("NTP cache result: " + ntpResult);
+        if (ntpResult != null) {
+            pw.println("NTP result age: " + ntpResult.getAgeMillis());
+        }
         pw.println();
     }
 }
diff --git a/services/core/java/com/android/server/location/NtpTimeHelper.java b/services/core/java/com/android/server/location/NtpTimeHelper.java
index 296b500eacee..88e9b50bb298 100644
--- a/services/core/java/com/android/server/location/NtpTimeHelper.java
+++ b/services/core/java/com/android/server/location/NtpTimeHelper.java
@@ -130,7 +130,8 @@ class NtpTimeHelper {
 
         // force refresh NTP cache when outdated
         boolean refreshSuccess = true;
-        if (mNtpTime.getCacheAge() >= NTP_INTERVAL) {
+        NtpTrustedTime.TimeResult ntpResult = mNtpTime.getCachedTimeResult();
+        if (ntpResult == null || ntpResult.getAgeMillis() >= NTP_INTERVAL) {
             // Blocking network operation.
             refreshSuccess = mNtpTime.forceRefresh();
         }
@@ -140,17 +141,17 @@ class NtpTimeHelper {
 
             // only update when NTP time is fresh
             // If refreshSuccess is false, cacheAge does not drop down.
-            if (mNtpTime.getCacheAge() < NTP_INTERVAL) {
-                long time = mNtpTime.getCachedNtpTime();
-                long timeReference = mNtpTime.getCachedNtpTimeReference();
-                long certainty = mNtpTime.getCacheCertainty();
+            ntpResult = mNtpTime.getCachedTimeResult();
+            if (ntpResult != null && ntpResult.getAgeMillis() < NTP_INTERVAL) {
+                long time = ntpResult.getTimeMillis();
+                long timeReference = ntpResult.getElapsedRealtimeMillis();
+                long certainty = ntpResult.getCertaintyMillis();
 
                 if (DEBUG) {
                     long now = System.currentTimeMillis();
                     Log.d(TAG, "NTP server returned: "
-                            + time + " (" + new Date(time)
-                            + ") reference: " + timeReference
-                            + " certainty: " + certainty
+                            + time + " (" + new Date(time) + ")"
+                            + " ntpResult: " + ntpResult
                             + " system time offset: " + (time - now));
                 }
 
diff --git a/services/robotests/src/com/android/server/location/NtpTimeHelperTest.java b/services/robotests/src/com/android/server/location/NtpTimeHelperTest.java
index a8a258fc5ff7..9c5d4ad6ceeb 100644
--- a/services/robotests/src/com/android/server/location/NtpTimeHelperTest.java
+++ b/services/robotests/src/com/android/server/location/NtpTimeHelperTest.java
@@ -3,6 +3,7 @@ package com.android.server.location;
 import static com.google.common.truth.Truth.assertThat;
 
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
 
 import android.os.Looper;
 import android.os.SystemClock;
@@ -52,8 +53,10 @@ public class NtpTimeHelperTest {
 
     @Test
     public void handleInjectNtpTime_cachedAgeLow_injectTime() throws InterruptedException {
-        doReturn(NtpTimeHelper.NTP_INTERVAL - 1).when(mMockNtpTrustedTime).getCacheAge();
-        doReturn(MOCK_NTP_TIME).when(mMockNtpTrustedTime).getCachedNtpTime();
+        NtpTrustedTime.TimeResult result = mock(NtpTrustedTime.TimeResult.class);
+        doReturn(NtpTimeHelper.NTP_INTERVAL - 1).when(result).getAgeMillis();
+        doReturn(MOCK_NTP_TIME).when(result).getTimeMillis();
+        doReturn(result).when(mMockNtpTrustedTime).getCachedTimeResult();
 
         mNtpTimeHelper.retrieveAndInjectNtpTime();
 
@@ -64,7 +67,9 @@ public class NtpTimeHelperTest {
     @Test
     public void handleInjectNtpTime_injectTimeFailed_injectTimeDelayed()
             throws InterruptedException {
-        doReturn(NtpTimeHelper.NTP_INTERVAL + 1).when(mMockNtpTrustedTime).getCacheAge();
+        NtpTrustedTime.TimeResult result1 = mock(NtpTrustedTime.TimeResult.class);
+        doReturn(NtpTimeHelper.NTP_INTERVAL + 1).when(result1).getAgeMillis();
+        doReturn(result1).when(mMockNtpTrustedTime).getCachedTimeResult();
         doReturn(false).when(mMockNtpTrustedTime).forceRefresh();
 
         mNtpTimeHelper.retrieveAndInjectNtpTime();
@@ -72,8 +77,10 @@ public class NtpTimeHelperTest {
         assertThat(mCountDownLatch.await(2, TimeUnit.SECONDS)).isFalse();
 
         doReturn(true).when(mMockNtpTrustedTime).forceRefresh();
-        doReturn(1L).when(mMockNtpTrustedTime).getCacheAge();
-        doReturn(MOCK_NTP_TIME).when(mMockNtpTrustedTime).getCachedNtpTime();
+        NtpTrustedTime.TimeResult result2 = mock(NtpTrustedTime.TimeResult.class);
+        doReturn(1L).when(result2).getAgeMillis();
+        doReturn(MOCK_NTP_TIME).when(result2).getTimeMillis();
+        doReturn(result2).when(mMockNtpTrustedTime).getCachedTimeResult();
         SystemClock.sleep(NtpTimeHelper.RETRY_INTERVAL);
 
         waitForTasksToBePostedOnHandlerAndRunThem();