Merge "Fix bug in Killswitch implementation" into main

networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyDownloader.java

@@ -21,7 +21,6 @@ import android.app.DownloadManager;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
-import android.content.IntentFilter;
 import android.net.Uri;
 import android.os.Build;
 import android.util.Log;
@@ -51,8 +50,6 @@ class CertificateTransparencyDownloader extends BroadcastReceiver {
 
     private final List<CompatibilityVersion> mCompatVersions = new ArrayList<>();
 
-    private boolean started = false;
-
     CertificateTransparencyDownloader(
             Context context,
             DataStore dataStore,
@@ -70,33 +67,8 @@ class CertificateTransparencyDownloader extends BroadcastReceiver {
         mCompatVersions.add(compatVersion);
     }
 
-    void start() {
-        if (started) {
-            return;
-        }
-        mContext.registerReceiver(
-                this,
-                new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE),
-                Context.RECEIVER_EXPORTED);
-        mDataStore.load();
-        started = true;
-
-        if (Config.DEBUG) {
-            Log.d(TAG, "CertificateTransparencyDownloader started.");
-        }
-    }
-
-    void stop() {
-        if (!started) {
-            return;
-        }
-        mContext.unregisterReceiver(this);
-        mDataStore.delete();
-        started = false;
-
-        if (Config.DEBUG) {
-            Log.d(TAG, "CertificateTransparencyDownloader stopped.");
-        }
+    void clearCompatibilityVersions() {
+        mCompatVersions.clear();
     }
 
     long startPublicKeyDownload() {

networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyJob.java

@@ -17,12 +17,12 @@ package com.android.server.net.ct;
 
 import android.annotation.RequiresApi;
 import android.app.AlarmManager;
+import android.app.DownloadManager;
 import android.app.PendingIntent;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
-import android.content.pm.PackageManager;
 import android.os.Build;
 import android.os.ConfigUpdate;
 import android.os.SystemClock;
@@ -33,28 +33,28 @@ import android.util.Log;
 public class CertificateTransparencyJob extends BroadcastReceiver {
 
     private static final String TAG = "CertificateTransparencyJob";
-    private static final String UPDATE_CONFIG_PERMISSION = "android.permission.UPDATE_CONFIG";
 
     private final Context mContext;
-    private final CompatibilityVersion mCompatVersion;
+    private final DataStore mDataStore;
     private final CertificateTransparencyDownloader mCertificateTransparencyDownloader;
+    private final CompatibilityVersion mCompatVersion;
     private final AlarmManager mAlarmManager;
     private final PendingIntent mPendingIntent;
 
+    private boolean mScheduled = false;
     private boolean mDependenciesReady = false;
 
     /** Creates a new {@link CertificateTransparencyJob} object. */
     public CertificateTransparencyJob(
-            Context context, CertificateTransparencyDownloader certificateTransparencyDownloader) {
+            Context context,
+            DataStore dataStore,
+            CertificateTransparencyDownloader certificateTransparencyDownloader,
+            CompatibilityVersion compatVersion) {
         mContext = context;
-        mCompatVersion =
-                new CompatibilityVersion(
-                        Config.COMPATIBILITY_VERSION,
-                        Config.URL_SIGNATURE,
-                        Config.URL_LOG_LIST,
-                        Config.CT_ROOT_DIRECTORY_PATH);
+        mDataStore = dataStore;
         mCertificateTransparencyDownloader = certificateTransparencyDownloader;
-        mCertificateTransparencyDownloader.addCompatibilityVersion(mCompatVersion);
+        mCompatVersion = compatVersion;
+
         mAlarmManager = context.getSystemService(AlarmManager.class);
         mPendingIntent =
                 PendingIntent.getBroadcast(
@@ -65,15 +65,19 @@ public class CertificateTransparencyJob extends BroadcastReceiver {
     }
 
     void schedule() {
-        mContext.registerReceiver(
-                this,
-                new IntentFilter(ConfigUpdate.ACTION_UPDATE_CT_LOGS),
-                Context.RECEIVER_EXPORTED);
-        mAlarmManager.setInexactRepeating(
-                AlarmManager.ELAPSED_REALTIME,
-                SystemClock.elapsedRealtime(), // schedule first job at earliest convenient time.
-                AlarmManager.INTERVAL_DAY,
-                mPendingIntent);
+        if (!mScheduled) {
+            mContext.registerReceiver(
+                    this,
+                    new IntentFilter(ConfigUpdate.ACTION_UPDATE_CT_LOGS),
+                    Context.RECEIVER_EXPORTED);
+            mAlarmManager.setInexactRepeating(
+                    AlarmManager.ELAPSED_REALTIME,
+                    SystemClock
+                            .elapsedRealtime(), // schedule first job at earliest convenient time.
+                    AlarmManager.INTERVAL_DAY,
+                    mPendingIntent);
+        }
+        mScheduled = true;
 
         if (Config.DEBUG) {
             Log.d(TAG, "CertificateTransparencyJob scheduled.");
@@ -81,12 +85,19 @@ public class CertificateTransparencyJob extends BroadcastReceiver {
     }
 
     void cancel() {
-        mContext.unregisterReceiver(this);
-        mAlarmManager.cancel(mPendingIntent);
-        mCertificateTransparencyDownloader.stop();
-        mCompatVersion.delete();
+        if (mScheduled) {
+            mContext.unregisterReceiver(this);
+            mAlarmManager.cancel(mPendingIntent);
+        }
+        mScheduled = false;
+
+        if (mDependenciesReady) {
+            stopDependencies();
+        }
         mDependenciesReady = false;
 
+        mCompatVersion.delete();
+
         if (Config.DEBUG) {
             Log.d(TAG, "CertificateTransparencyJob canceled.");
         }
@@ -98,16 +109,11 @@ public class CertificateTransparencyJob extends BroadcastReceiver {
             Log.w(TAG, "Received unexpected broadcast with action " + intent);
             return;
         }
-        if (context.checkCallingOrSelfPermission(UPDATE_CONFIG_PERMISSION)
-                != PackageManager.PERMISSION_GRANTED) {
-            Log.e(TAG, "Caller does not have UPDATE_CONFIG permission.");
-            return;
-        }
         if (Config.DEBUG) {
             Log.d(TAG, "Starting CT daily job.");
         }
         if (!mDependenciesReady) {
-            mCertificateTransparencyDownloader.start();
+            startDependencies();
             mDependenciesReady = true;
         }
 
@@ -117,4 +123,27 @@ public class CertificateTransparencyJob extends BroadcastReceiver {
             Log.d(TAG, "Public key download started successfully.");
         }
     }
+
+    private void startDependencies() {
+        mDataStore.load();
+        mCertificateTransparencyDownloader.addCompatibilityVersion(mCompatVersion);
+        mContext.registerReceiver(
+                mCertificateTransparencyDownloader,
+                new IntentFilter(DownloadManager.ACTION_DOWNLOAD_COMPLETE),
+                Context.RECEIVER_EXPORTED);
+
+        if (Config.DEBUG) {
+            Log.d(TAG, "CertificateTransparencyJob dependencies ready.");
+        }
+    }
+
+    private void stopDependencies() {
+        mContext.unregisterReceiver(mCertificateTransparencyDownloader);
+        mCertificateTransparencyDownloader.clearCompatibilityVersions();
+        mDataStore.delete();
+
+        if (Config.DEBUG) {
+            Log.d(TAG, "CertificateTransparencyJob dependencies stopped.");
+        }
+    }
 }

networksecurity/service/src/com/android/server/net/ct/CertificateTransparencyService.java

@@ -41,8 +41,6 @@ public class CertificateTransparencyService extends ICertificateTransparencyMana
 
     private final CertificateTransparencyJob mCertificateTransparencyJob;
 
-    private boolean started = false;
-
     /**
      * @return true if the CertificateTransparency service is enabled.
      */
@@ -53,16 +51,22 @@ public class CertificateTransparencyService extends ICertificateTransparencyMana
     /** Creates a new {@link CertificateTransparencyService} object. */
     public CertificateTransparencyService(Context context) {
         DataStore dataStore = new DataStore(Config.PREFERENCES_FILE);
-        DownloadHelper downloadHelper = new DownloadHelper(context);
-        SignatureVerifier signatureVerifier = new SignatureVerifier(context);
-        CertificateTransparencyDownloader downloader =
-                new CertificateTransparencyDownloader(
+
+        mCertificateTransparencyJob =
+                new CertificateTransparencyJob(
                         context,
                         dataStore,
-                        downloadHelper,
-                        signatureVerifier,
-                        new CertificateTransparencyLoggerImpl());
-        mCertificateTransparencyJob = new CertificateTransparencyJob(context, downloader);
+                        new CertificateTransparencyDownloader(
+                                context,
+                                dataStore,
+                                new DownloadHelper(context),
+                                new SignatureVerifier(context),
+                                new CertificateTransparencyLoggerImpl()),
+                        new CompatibilityVersion(
+                                Config.COMPATIBILITY_VERSION,
+                                Config.URL_SIGNATURE,
+                                Config.URL_LOG_LIST,
+                                Config.CT_ROOT_DIRECTORY_PATH));
     }
 
     /**
@@ -104,19 +108,13 @@ public class CertificateTransparencyService extends ICertificateTransparencyMana
         if (Config.DEBUG) {
             Log.d(TAG, "CertificateTransparencyService start");
         }
-        if (!started) {
-            mCertificateTransparencyJob.schedule();
-            started = true;
-        }
+        mCertificateTransparencyJob.schedule();
     }
 
     private void stopService() {
         if (Config.DEBUG) {
             Log.d(TAG, "CertificateTransparencyService stop");
         }
-        if (started) {
-            mCertificateTransparencyJob.cancel();
-            started = false;
-        }
+        mCertificateTransparencyJob.cancel();
     }
 }

networksecurity/tests/unit/src/com/android/server/net/ct/CertificateTransparencyDownloaderTest.java

@@ -111,15 +111,15 @@ public class CertificateTransparencyDownloaderTest {
                         mContext.getFilesDir());
 
         prepareDownloadManager();
+        mDataStore.load();
         mCertificateTransparencyDownloader.addCompatibilityVersion(mCompatVersion);
-        mCertificateTransparencyDownloader.start();
     }
 
     @After
     public void tearDown() {
         mSignatureVerifier.resetPublicKey();
-        mCertificateTransparencyDownloader.stop();
         mCompatVersion.delete();
+        mDataStore.delete();
     }
 
     @Test