Merge changes I7dcdef58,Ibba26a57 into rvc-dev

* changes:
  Make BiometricServiceBase more robust to HAL crash
  Fix BiometricServiceBase crash in handleEnumerate

2 files changed, 211 insertions, 31 deletions

diff --git a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java
index 45b93834c1e2..4431abe43136 100644
--- a/services/core/java/com/android/server/biometrics/BiometricServiceBase.java
+++ b/services/core/java/com/android/server/biometrics/BiometricServiceBase.java
@@ -17,6 +17,7 @@
 package com.android.server.biometrics;
 
 import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND_SERVICE;
+import static android.hardware.biometrics.BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE;
 
 import android.app.ActivityManager;
 import android.app.ActivityTaskManager;
@@ -43,6 +44,7 @@ import android.os.Handler;
 import android.os.IBinder;
 import android.os.IHwBinder;
 import android.os.IRemoteCallback;
+import android.os.Looper;
 import android.os.PowerManager;
 import android.os.Process;
 import android.os.RemoteException;
@@ -52,6 +54,8 @@ import android.os.UserHandle;
 import android.os.UserManager;
 import android.util.Slog;
 
+import com.android.internal.R;
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.logging.MetricsLogger;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.util.FrameworkStatsLog;
@@ -93,7 +97,22 @@ public abstract class BiometricServiceBase extends SystemService
     protected final Map<Integer, Long> mAuthenticatorIds =
             Collections.synchronizedMap(new HashMap<>());
     protected final AppOpsManager mAppOps;
-    protected final H mHandler = new H();
+
+    /**
+     * Handler which all subclasses should post events to.
+     */
+    protected final Handler mHandler = new Handler(Looper.getMainLooper()) {
+        @Override
+        public void handleMessage(android.os.Message msg) {
+            switch (msg.what) {
+                case MSG_USER_SWITCHING:
+                    handleUserSwitching(msg.arg1);
+                    break;
+                default:
+                    Slog.w(getTag(), "Unknown message:" + msg.what);
+            }
+        }
+    };
 
     private final IBinder mToken = new Binder(); // Used for internal enumeration
     private final ArrayList<UserTemplate> mUnknownHALTemplates = new ArrayList<>();
@@ -483,23 +502,6 @@ public abstract class BiometricServiceBase extends SystemService
         void resetLockout(byte[] token) throws RemoteException;
     }
 
-    /**
-     * Handler which all subclasses should post events to.
-     */
-    protected final class H extends Handler {
-        @Override
-        public void handleMessage(android.os.Message msg) {
-            switch (msg.what) {
-                case MSG_USER_SWITCHING:
-                    handleUserSwitching(msg.arg1);
-                    break;
-
-                default:
-                    Slog.w(getTag(), "Unknown message:" + msg.what);
-            }
-        }
-    }
-
     private final Runnable mOnTaskStackChangedRunnable = new Runnable() {
         @Override
         public void run() {
@@ -647,8 +649,9 @@ public abstract class BiometricServiceBase extends SystemService
         mContext = context;
         mStatusBarService = IStatusBarService.Stub.asInterface(
                 ServiceManager.getService(Context.STATUS_BAR_SERVICE));
-        mKeyguardPackage = ComponentName.unflattenFromString(context.getResources().getString(
-                com.android.internal.R.string.config_keyguardComponent)).getPackageName();
+        final ComponentName keyguardComponent = ComponentName.unflattenFromString(
+                context.getResources().getString(R.string.config_keyguardComponent));
+        mKeyguardPackage = keyguardComponent != null ? keyguardComponent.getPackageName() : null;
         mAppOps = context.getSystemService(AppOpsManager.class);
         mActivityTaskManager = ((ActivityTaskManager) context.getSystemService(
                 Context.ACTIVITY_TASK_SERVICE)).getService();
@@ -671,8 +674,8 @@ public abstract class BiometricServiceBase extends SystemService
 
         // All client lifecycle must be managed on the handler.
         mHandler.post(() -> {
-            handleError(getHalDeviceId(), BiometricConstants.BIOMETRIC_ERROR_HW_UNAVAILABLE,
-                    0 /*vendorCode */);
+            Slog.e(getTag(), "Sending BIOMETRIC_ERROR_HW_UNAVAILABLE after HAL crash");
+            handleError(getHalDeviceId(), BIOMETRIC_ERROR_HW_UNAVAILABLE, 0 /* vendorCode */);
         });
 
         FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED,
@@ -798,9 +801,10 @@ public abstract class BiometricServiceBase extends SystemService
     }
 
     protected void handleEnumerate(BiometricAuthenticator.Identifier identifier, int remaining) {
-        ClientMonitor client = getCurrentClient();
-
-        client.onEnumerationResult(identifier, remaining);
+        ClientMonitor client = mCurrentClient;
+        if (client != null) {
+            client.onEnumerationResult(identifier, remaining);
+        }
 
         // All templates in the HAL for this user were enumerated
         if (remaining == 0) {
@@ -818,7 +822,7 @@ public abstract class BiometricServiceBase extends SystemService
                 }
                 removeClient(client);
                 startCleanupUnknownHALTemplates();
-            } else {
+            } else if (client != null) {
                 removeClient(client);
             }
         }
@@ -898,12 +902,16 @@ public abstract class BiometricServiceBase extends SystemService
 
     protected void cancelAuthenticationInternal(final IBinder token, final String opPackageName,
             int callingUid, int callingPid, int callingUserId, boolean fromClient) {
+
+        if (DEBUG) Slog.v(getTag(), "cancelAuthentication(" + opPackageName + ")");
         if (fromClient) {
             // Only check this if cancel was called from the client (app). If cancel was called
             // from BiometricService, it means the dialog was dismissed due to user interaction.
             if (!canUseBiometric(opPackageName, true /* foregroundOnly */, callingUid, callingPid,
                     callingUserId)) {
-                if (DEBUG) Slog.v(getTag(), "cancelAuthentication(): reject " + opPackageName);
+                if (DEBUG) {
+                    Slog.v(getTag(), "cancelAuthentication(): reject " + opPackageName);
+                }
                 return;
             }
         }
@@ -1059,7 +1067,8 @@ public abstract class BiometricServiceBase extends SystemService
      * @param newClient the new client that wants to connect
      * @param initiatedByClient true for authenticate, remove and enroll
      */
-    private void startClient(ClientMonitor newClient, boolean initiatedByClient) {
+    @VisibleForTesting
+    void startClient(ClientMonitor newClient, boolean initiatedByClient) {
         ClientMonitor currentClient = mCurrentClient;
         if (currentClient != null) {
             if (DEBUG) Slog.v(getTag(), "request stop current client " +
@@ -1122,18 +1131,27 @@ public abstract class BiometricServiceBase extends SystemService
             Slog.e(getTag(), "Trying to start null client!");
             return;
         }
+
         if (DEBUG) Slog.v(getTag(), "starting client "
                 + mCurrentClient.getClass().getSuperclass().getSimpleName()
                 + "(" + mCurrentClient.getOwnerString() + ")"
                 + " targetUserId: " + mCurrentClient.getTargetUserId()
                 + " currentUserId: " + mCurrentUserId
                 + " cookie: " + cookie + "/" + mCurrentClient.getCookie());
+
         if (cookie != mCurrentClient.getCookie()) {
             Slog.e(getTag(), "Mismatched cookie");
             return;
         }
-        notifyClientActiveCallbacks(true);
-        mCurrentClient.start();
+
+        int status = mCurrentClient.start();
+        if (status == 0) {
+            notifyClientActiveCallbacks(true);
+        } else {
+            mCurrentClient.onError(getHalDeviceId(), BIOMETRIC_ERROR_HW_UNAVAILABLE,
+                    0 /* vendorCode */);
+            removeClient(mCurrentClient);
+        }
     }
 
     protected void removeClient(ClientMonitor client) {
@@ -1145,7 +1163,7 @@ public abstract class BiometricServiceBase extends SystemService
             }
         }
         if (mCurrentClient != null) {
-            if (DEBUG) Slog.v(getTag(), "Done with client: " + client.getOwnerString());
+            if (DEBUG) Slog.v(getTag(), "Done with client: " + mCurrentClient.getOwnerString());
             mCurrentClient = null;
         }
         if (mPendingClient == null) {
diff --git a/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceBaseTest.java b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceBaseTest.java
new file mode 100644
index 000000000000..4fe94583c8b3
--- /dev/null
+++ b/services/tests/servicestests/src/com/android/server/biometrics/BiometricServiceBaseTest.java
@@ -0,0 +1,162 @@
+/*
+ * Copyright (C) 2020 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 com.android.server.biometrics;
+
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import android.content.Context;
+import android.content.res.Resources;
+import android.hardware.biometrics.BiometricAuthenticator;
+
+import androidx.test.filters.SmallTest;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+
+import java.util.List;
+
+@SmallTest
+public class BiometricServiceBaseTest {
+    private static class TestableBiometricServiceBase extends BiometricServiceBase {
+        TestableBiometricServiceBase(Context context) {
+            super(context);
+        }
+
+        @Override
+        protected String getTag() {
+            return null;
+        }
+
+        @Override
+        protected DaemonWrapper getDaemonWrapper() {
+            return null;
+        }
+
+        @Override
+        protected BiometricUtils getBiometricUtils() {
+            return null;
+        }
+
+        @Override
+        protected Constants getConstants() {
+            return null;
+        }
+
+        @Override
+        protected boolean hasReachedEnrollmentLimit(int userId) {
+            return false;
+        }
+
+        @Override
+        protected void updateActiveGroup(int userId, String clientPackage) {
+        }
+
+        @Override
+        protected String getLockoutResetIntent() {
+            return null;
+        }
+
+        @Override
+        protected String getLockoutBroadcastPermission() {
+            return null;
+        }
+
+        @Override
+        protected long getHalDeviceId() {
+            return 0;
+        }
+
+        @Override
+        protected boolean hasEnrolledBiometrics(int userId) {
+            return false;
+        }
+
+        @Override
+        protected String getManageBiometricPermission() {
+            return null;
+        }
+
+        @Override
+        protected void checkUseBiometricPermission() {
+        }
+
+        @Override
+        protected boolean checkAppOps(int uid, String opPackageName) {
+            return false;
+        }
+
+        @Override
+        protected List<? extends BiometricAuthenticator.Identifier> getEnrolledTemplates(
+                int userId) {
+            return null;
+        }
+
+        @Override
+        protected int statsModality() {
+            return 0;
+        }
+
+        @Override
+        protected int getLockoutMode() {
+            return 0;
+        }
+    }
+
+    private static final int CLIENT_COOKIE = 0xc00c1e;
+
+    private BiometricServiceBase mBiometricServiceBase;
+
+    @Mock
+    private Context mContext;
+    @Mock
+    private Resources mResources;
+    @Mock
+    private BiometricAuthenticator.Identifier mIdentifier;
+    @Mock
+    private ClientMonitor mClient;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        when(mContext.getResources()).thenReturn(mResources);
+        when(mResources.getString(anyInt())).thenReturn("");
+        when(mClient.getCookie()).thenReturn(CLIENT_COOKIE);
+
+        mBiometricServiceBase = new TestableBiometricServiceBase(mContext);
+    }
+
+    @Test
+    public void testHandleEnumerate_doesNotCrash_withNullClient() {
+        mBiometricServiceBase.handleEnumerate(mIdentifier, 0 /* remaining */);
+    }
+
+    @Test
+    public void testStartClient_sendsErrorAndRemovesClient_onNonzeroErrorCode() {
+        when(mClient.start()).thenReturn(1);
+
+        mBiometricServiceBase.startClient(mClient, false /* initiatedByClient */);
+
+        verify(mClient).onError(anyLong(), anyInt(), anyInt());
+        verify(mClient).destroy();
+    }
+}