From ff715ac551fcb09640acdf28278384ca2d5f85c0 Mon Sep 17 00:00:00 2001 From: Tony Mak Date: Tue, 19 Apr 2016 20:44:12 +0100 Subject: [PATCH] Fix the misuse of calling user id as group id 1. The userId stored in different clients are actually calling user id. groupId should be used to do all the operations. Also, renamed the variable to avoid further confusion. 2. Fix the bug null is always returned in getFingerprintDaemon in RemovalClient. 3. Fix the misuse of calling uid as calling user id in startAuthentication. Fix: 28268635 Fix: 28264725 Change-Id: I618ac3c6d913ae5c86e7b04cb3f9ead39828f216 --- .../fingerprint/AuthenticationClient.java | 4 +-- .../server/fingerprint/ClientMonitor.java | 12 +++---- .../server/fingerprint/EnrollClient.java | 7 ++-- .../server/fingerprint/EnumerateClient.java | 2 +- .../fingerprint/FingerprintService.java | 34 +++++++++---------- .../server/fingerprint/RemovalClient.java | 2 +- 6 files changed, 32 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/fingerprint/AuthenticationClient.java b/services/core/java/com/android/server/fingerprint/AuthenticationClient.java index d2f015fad881e..10284bb18c75e 100644 --- a/services/core/java/com/android/server/fingerprint/AuthenticationClient.java +++ b/services/core/java/com/android/server/fingerprint/AuthenticationClient.java @@ -39,9 +39,9 @@ public abstract class AuthenticationClient extends ClientMonitor { public abstract void resetFailedAttempts(); public AuthenticationClient(Context context, long halDeviceId, IBinder token, - IFingerprintServiceReceiver receiver, int userId, int groupId, long opId, + IFingerprintServiceReceiver receiver, int callingUserId, int groupId, long opId, boolean restricted, String owner) { - super(context, halDeviceId, token, receiver, userId, groupId, restricted, owner); + super(context, halDeviceId, token, receiver, callingUserId, groupId, restricted, owner); mOpId = opId; } diff --git a/services/core/java/com/android/server/fingerprint/ClientMonitor.java b/services/core/java/com/android/server/fingerprint/ClientMonitor.java index 90998edd6d33f..45b6d3efc4c6b 100644 --- a/services/core/java/com/android/server/fingerprint/ClientMonitor.java +++ b/services/core/java/com/android/server/fingerprint/ClientMonitor.java @@ -38,7 +38,7 @@ public abstract class ClientMonitor implements IBinder.DeathRecipient { protected static final boolean DEBUG = FingerprintService.DEBUG; private IBinder mToken; private IFingerprintServiceReceiver mReceiver; - private int mUserId; + private int mCallingUserId; private int mGroupId; private boolean mIsRestricted; // True if client does not have MANAGE_FINGERPRINT permission private String mOwner; @@ -50,20 +50,20 @@ public abstract class ClientMonitor implements IBinder.DeathRecipient { * @param halDeviceId the HAL device ID of the associated fingerprint hardware * @param token a unique token for the client * @param receiver recipient of related events (e.g. authentication) - * @param userId userId for the fingerprint set + * @param callingUserId user id of calling user * @param groupId groupId for the fingerprint set * @param restricted whether or not client has the {@link Manifest#MANAGE_FINGERPRINT} * permission * @param owner name of the client that owns this */ public ClientMonitor(Context context, long halDeviceId, IBinder token, - IFingerprintServiceReceiver receiver, int userId, int groupId,boolean restricted, + IFingerprintServiceReceiver receiver, int callingUserId, int groupId,boolean restricted, String owner) { mContext = context; mHalDeviceId = halDeviceId; mToken = token; mReceiver = receiver; - mUserId = userId; + mCallingUserId = callingUserId; mGroupId = groupId; mIsRestricted = restricted; mOwner = owner; @@ -197,8 +197,8 @@ public abstract class ClientMonitor implements IBinder.DeathRecipient { return mIsRestricted; } - public final int getUserId() { - return mUserId; + public final int getCallingUserId() { + return mCallingUserId; } public final int getGroupId() { diff --git a/services/core/java/com/android/server/fingerprint/EnrollClient.java b/services/core/java/com/android/server/fingerprint/EnrollClient.java index ce5b89080fb48..b636ce56e01c5 100644 --- a/services/core/java/com/android/server/fingerprint/EnrollClient.java +++ b/services/core/java/com/android/server/fingerprint/EnrollClient.java @@ -46,9 +46,12 @@ public abstract class EnrollClient extends ClientMonitor { @Override public boolean onEnrollResult(int fingerId, int groupId, int remaining) { + if (groupId != getGroupId()) { + Slog.w(TAG, "groupId != getGroupId(), groupId: " + groupId + + " getGroupId():" + getGroupId()); + } if (remaining == 0) { - FingerprintUtils.getInstance().addFingerprintForUser(getContext(), fingerId, - getUserId()); + FingerprintUtils.getInstance().addFingerprintForUser(getContext(), fingerId, groupId); } return sendEnrollResult(fingerId, groupId, remaining); } diff --git a/services/core/java/com/android/server/fingerprint/EnumerateClient.java b/services/core/java/com/android/server/fingerprint/EnumerateClient.java index b2e4099451cac..e826fee66da5a 100644 --- a/services/core/java/com/android/server/fingerprint/EnumerateClient.java +++ b/services/core/java/com/android/server/fingerprint/EnumerateClient.java @@ -41,7 +41,7 @@ public abstract class EnumerateClient extends ClientMonitor { try { final int result = daemon.enumerate(); if (result != 0) { - Slog.w(TAG, "start enumerate for user " + getUserId() + Slog.w(TAG, "start enumerate for user " + getCallingUserId() + " failed, result=" + result); onError(FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE); return result; diff --git a/services/core/java/com/android/server/fingerprint/FingerprintService.java b/services/core/java/com/android/server/fingerprint/FingerprintService.java index c770620873e22..fcf7bf53d4570 100644 --- a/services/core/java/com/android/server/fingerprint/FingerprintService.java +++ b/services/core/java/com/android/server/fingerprint/FingerprintService.java @@ -356,7 +356,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe } } - void startRemove(IBinder token, int fingerId, int userId, int groupId, + void startRemove(IBinder token, int fingerId, int callingUserId, int groupId, IFingerprintServiceReceiver receiver, boolean restricted) { IFingerprintDaemon daemon = getFingerprintDaemon(); if (daemon == null) { @@ -364,7 +364,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe return; } RemovalClient client = new RemovalClient(getContext(), mHalDeviceId, token, - receiver, userId, groupId, fingerId, restricted, token.toString()) { + receiver, callingUserId, groupId, fingerId, restricted, token.toString()) { @Override public void notifyUserActivity() { FingerprintService.this.userActivity(); @@ -372,8 +372,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe @Override public IFingerprintDaemon getFingerprintDaemon() { - FingerprintService.this.getFingerprintDaemon(); - return null; + return FingerprintService.this.getFingerprintDaemon(); } }; startClient(client, true); @@ -494,7 +493,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe } } - private void startAuthentication(IBinder token, long opId, int realUserId, int groupId, + private void startAuthentication(IBinder token, long opId, int callingUserId, int groupId, IFingerprintServiceReceiver receiver, int flags, boolean restricted, String opPackageName) { updateActiveGroup(groupId, opPackageName); @@ -502,7 +501,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe if (DEBUG) Slog.v(TAG, "startAuthentication(" + opPackageName + ")"); AuthenticationClient client = new AuthenticationClient(getContext(), mHalDeviceId, token, - receiver, realUserId, groupId, opId, restricted, opPackageName) { + receiver, callingUserId, groupId, opId, restricted, opPackageName) { @Override public boolean handleFailedAttempt() { mFailedAttempts++; @@ -541,13 +540,13 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe startClient(client, true /* initiatedByClient */); } - private void startEnrollment(IBinder token, byte [] cryptoToken, int userId, int groupId, + private void startEnrollment(IBinder token, byte [] cryptoToken, int callingUserId, int groupId, IFingerprintServiceReceiver receiver, int flags, boolean restricted, String opPackageName) { updateActiveGroup(groupId, opPackageName); EnrollClient client = new EnrollClient(getContext(), mHalDeviceId, token, receiver, - userId, groupId, cryptoToken, restricted, opPackageName) { + callingUserId, groupId, cryptoToken, restricted, opPackageName) { @Override public IFingerprintDaemon getFingerprintDaemon() { @@ -687,9 +686,9 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe checkPermission(MANAGE_FINGERPRINT); final int limit = mContext.getResources().getInteger( com.android.internal.R.integer.config_fingerprintMaxTemplatesPerUser); - final int callingUid = Binder.getCallingUid(); - final int userId = UserHandle.getUserId(callingUid); - final int enrolled = FingerprintService.this.getEnrolledFingerprints(userId).size(); + final int callingUserId = UserHandle.getCallingUserId(); + final int enrolled = FingerprintService.this. + getEnrolledFingerprints(callingUserId).size(); if (enrolled >= limit) { Slog.w(TAG, "Too many fingerprints registered"); return; @@ -705,7 +704,7 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe mHandler.post(new Runnable() { @Override public void run() { - startEnrollment(token, cryptoToken, userId, groupId, receiver, flags, + startEnrollment(token, cryptoToken, callingUserId, groupId, receiver, flags, restricted, opPackageName); } }); @@ -735,7 +734,8 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe public void authenticate(final IBinder token, final long opId, final int groupId, final IFingerprintServiceReceiver receiver, final int flags, final String opPackageName) { - final int realUserId = Binder.getCallingUid(); + final int callingUid = Binder.getCallingUid(); + final int callingUserId = UserHandle.getCallingUserId(); final int pid = Binder.getCallingPid(); final boolean restricted = isRestricted(); mHandler.post(new Runnable() { @@ -743,11 +743,11 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe public void run() { MetricsLogger.histogram(mContext, "fingerprint_token", opId != 0L ? 1 : 0); if (!canUseFingerprint(opPackageName, true /* foregroundOnly */, - realUserId, pid)) { + callingUid, pid)) { if (DEBUG) Slog.v(TAG, "authenticate(): reject " + opPackageName); return; } - startAuthentication(token, opId, realUserId, groupId, receiver, + startAuthentication(token, opId, callingUserId, groupId, receiver, flags, restricted, opPackageName); } }); @@ -797,11 +797,11 @@ public class FingerprintService extends SystemService implements IBinder.DeathRe final IFingerprintServiceReceiver receiver) { checkPermission(MANAGE_FINGERPRINT); // TODO: Maybe have another permission final boolean restricted = isRestricted(); - final int realUserId = Binder.getCallingUid(); + final int callingUserId = UserHandle.getCallingUserId(); mHandler.post(new Runnable() { @Override public void run() { - startRemove(token, fingerId, realUserId, groupId, receiver, restricted); + startRemove(token, fingerId, callingUserId, groupId, receiver, restricted); } }); diff --git a/services/core/java/com/android/server/fingerprint/RemovalClient.java b/services/core/java/com/android/server/fingerprint/RemovalClient.java index 69a96e127ef72..ffa3c3f9fc83d 100644 --- a/services/core/java/com/android/server/fingerprint/RemovalClient.java +++ b/services/core/java/com/android/server/fingerprint/RemovalClient.java @@ -45,7 +45,7 @@ public abstract class RemovalClient extends ClientMonitor { IFingerprintDaemon daemon = getFingerprintDaemon(); // The fingerprint template ids will be removed when we get confirmation from the HAL try { - final int result = daemon.remove(mFingerId, getUserId()); + final int result = daemon.remove(mFingerId, getGroupId()); if (result != 0) { Slog.w(TAG, "startRemove with id = " + mFingerId + " failed, result=" + result); onError(FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE);