Fix bug in Killswitch implementation

The current implementation does not delete the CompatibilityVersion
directory if the killswitch is activated while the device is powered
off.

This CL fixes the bug and centralizes the handling of the killswitch
inside the CertificateTransparencyJob class, instead of spreading it
among multiple classes. Hopefully this change makes the killswitch
logic easier to follow and less-prone to mistakes.

Bug: 384688420
Test: adb shell device_config put network_security CertificateTransparencyLogList__service_enabled true/false
Flag: com.android.net.ct.flags.certificate_transparency_service
Change-Id: Ic1d0d78879753abc42829be18ce15f32a86532c5

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

@@ -26,7 +26,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;
@@ -56,8 +55,6 @@ class CertificateTransparencyDownloader extends BroadcastReceiver {
 
     private final List<CompatibilityVersion> mCompatVersions = new ArrayList<>();
 
-    private boolean started = false;
-
     CertificateTransparencyDownloader(
             Context context,
             DataStore dataStore,
@@ -75,33 +72,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