From 5a207846e63842fed0bd4f7c419951799ea5f4f7 Mon Sep 17 00:00:00 2001 From: Nicholas Ambur Date: Thu, 19 Mar 2020 21:01:31 -0700 Subject: [PATCH 1/2] add KEYPHRASE_ENROLLMENT_APPLICATION permission Split the model management permission controll into two separate permissions. KEYPHRASE_ENROLLMENT_APPLICATION is reserved for enrollment applications detected by KeyphraseEnrollmentInfo class while MANAGE_VOICE_KEYPHRASES is reserved for VoiceInteractionService implementations. Keyphrase enrollment applications are allowed to enroll at anytime, and VoiceInteractionService implementations are only allowed to enroll when they are the active service (see VoiceInteractionManagerService). Bug: 151405284 Test: Enroll voice model from both enrollment application and active voiceinteraction service. Change-Id: I12eeb09e308af9aa934174dd059f47f789a9ce39 Change-Id: Iffdfeab1d09ec6c8cf4b30a9768ef6efc9bdfa01 --- core/res/AndroidManifest.xml | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index f7400bd79a929..d5bccdb437785 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -3449,12 +3449,25 @@ - + + + + From 64fb254c632001a3135582fcb8284a43b826ecd9 Mon Sep 17 00:00:00 2001 From: Nicholas Ambur Date: Thu, 19 Mar 2020 21:16:10 -0700 Subject: [PATCH 2/2] fix enrollment application permission check Remove check associated with enrollment application UID, and allow enrollment applications holding the KEYPHRASE_ENROLLMENT_APPLICATION privledged permission to enroll. Bug: 151405284 Test: Confirm enrollment from both enrollment application and active voiceinteraction service. Confirm enrollment can still occurr after system user switch. Change-Id: I8686f705fe5405523004eef5de834282a7382464 --- .../soundtrigger/KeyphraseEnrollmentInfo.java | 21 -------- .../VoiceInteractionManagerService.java | 54 +++++++------------ 2 files changed, 18 insertions(+), 57 deletions(-) diff --git a/core/java/android/hardware/soundtrigger/KeyphraseEnrollmentInfo.java b/core/java/android/hardware/soundtrigger/KeyphraseEnrollmentInfo.java index 1aeb76a396b44..05935452e8abd 100644 --- a/core/java/android/hardware/soundtrigger/KeyphraseEnrollmentInfo.java +++ b/core/java/android/hardware/soundtrigger/KeyphraseEnrollmentInfo.java @@ -43,7 +43,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Set; /** * Enrollment information about the different available keyphrases. @@ -119,11 +118,6 @@ public class KeyphraseEnrollmentInfo { */ private final KeyphraseMetadata[] mKeyphrases; - /** - * Set of UIDs associated with the detected enrollment applications. - */ - private final Set mEnrollmentApplicationUids; - /** * Map between KeyphraseMetadata and the package name of the enrollment app that provides it. */ @@ -142,13 +136,11 @@ public class KeyphraseEnrollmentInfo { mParseError = "No enrollment applications found"; mKeyphrasePackageMap = Collections.emptyMap(); mKeyphrases = null; - mEnrollmentApplicationUids = Collections.emptySet(); return; } List parseErrors = new LinkedList(); mKeyphrasePackageMap = new HashMap(); - mEnrollmentApplicationUids = new ArraySet<>(); for (ResolveInfo ri : ris) { try { ApplicationInfo ai = pm.getApplicationInfo( @@ -170,7 +162,6 @@ public class KeyphraseEnrollmentInfo { getKeyphraseMetadataFromApplicationInfo(pm, ai, parseErrors); if (metadata != null) { mKeyphrasePackageMap.put(metadata, ai.packageName); - mEnrollmentApplicationUids.add(ai.uid); } } catch (PackageManager.NameNotFoundException e) { String error = "error parsing voice enrollment meta-data for " @@ -373,21 +364,9 @@ public class KeyphraseEnrollmentInfo { return null; } - /** - * Tests if the input UID matches a supported enrollment application. - * - * @param uid UID of the caller to test against. - * @return Returns true if input uid matches the uid of a supported enrollment application. - * False if not. - */ - public boolean isUidSupportedEnrollmentApplication(int uid) { - return mEnrollmentApplicationUids.contains(uid); - } - @Override public String toString() { return "KeyphraseEnrollmentInfo [KeyphrasePackageMap=" + mKeyphrasePackageMap.toString() - + ", enrollmentApplicationUids=" + mEnrollmentApplicationUids.toString() + ", ParseError=" + mParseError + "]"; } } diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index 0eba07b118b54..18d581964f2ad 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -41,7 +41,6 @@ import android.content.pm.UserInfo; import android.content.res.Resources; import android.database.ContentObserver; import android.hardware.soundtrigger.IRecognitionStatusCallback; -import android.hardware.soundtrigger.KeyphraseEnrollmentInfo; import android.hardware.soundtrigger.KeyphraseMetadata; import android.hardware.soundtrigger.ModelParams; import android.hardware.soundtrigger.SoundTrigger; @@ -100,7 +99,7 @@ import java.util.concurrent.Executor; * SystemService that publishes an IVoiceInteractionManagerService. */ public class VoiceInteractionManagerService extends SystemService { - static final String TAG = "VoiceInteractionManagerService"; + static final String TAG = "VoiceInteractionManager"; static final boolean DEBUG = false; final Context mContext; @@ -172,17 +171,17 @@ public class VoiceInteractionManagerService extends SystemService { } @Override - public void onStartUser(@NonNull UserInfo userInfo) { - if (DEBUG_USER) Slog.d(TAG, "onStartUser(" + userInfo + ")"); + public void onUserStarting(@NonNull TargetUser user) { + if (DEBUG_USER) Slog.d(TAG, "onUserStarting(" + user + ")"); - mServiceStub.initForUser(userInfo.id); + mServiceStub.initForUser(user.getUserIdentifier()); } @Override - public void onUnlockUser(@NonNull UserInfo userInfo) { - if (DEBUG_USER) Slog.d(TAG, "onUnlockUser(" + userInfo + ")"); + public void onUserUnlocking(@NonNull TargetUser user) { + if (DEBUG_USER) Slog.d(TAG, "onUserUnlocking(" + user + ")"); - mServiceStub.initForUser(userInfo.id); + mServiceStub.initForUser(user.getUserIdentifier()); mServiceStub.switchImplementationIfNeeded(false); } @@ -224,7 +223,6 @@ public class VoiceInteractionManagerService extends SystemService { class VoiceInteractionManagerServiceStub extends IVoiceInteractionManagerService.Stub { VoiceInteractionManagerServiceImpl mImpl; - KeyphraseEnrollmentInfo mEnrollmentApplicationInfo; private boolean mSafeMode; private int mCurUser; @@ -449,15 +447,6 @@ public class VoiceInteractionManagerService extends SystemService { } } - private void getOrCreateEnrollmentApplicationInfo() { - synchronized (this) { - if (mEnrollmentApplicationInfo == null) { - mEnrollmentApplicationInfo = new KeyphraseEnrollmentInfo( - mContext.getPackageManager()); - } - } - } - private void setCurrentUserLocked(@UserIdInt int userHandle) { mCurUser = userHandle; final UserInfo userInfo = mUserManagerInternal.getUserInfo(mCurUser); @@ -1391,11 +1380,6 @@ public class VoiceInteractionManagerService extends SystemService { pw.println(" mCurUserUnlocked: " + mCurUserUnlocked); pw.println(" mCurUserSupported: " + mCurUserSupported); dumpSupportedUsers(pw, " "); - if (mEnrollmentApplicationInfo == null) { - pw.println(" (No enrollment application info)"); - } else { - pw.println(" " + mEnrollmentApplicationInfo.toString()); - } mDbHelper.dump(pw); if (mImpl == null) { pw.println(" (No active implementation)"); @@ -1425,9 +1409,13 @@ public class VoiceInteractionManagerService extends SystemService { } } + private boolean isCallerHoldingPermission(String permission) { + return mContext.checkCallingOrSelfPermission(permission) + == PackageManager.PERMISSION_GRANTED; + } + private void enforceCallingPermission(String permission) { - if (mContext.checkCallingOrSelfPermission(permission) - != PackageManager.PERMISSION_GRANTED) { + if (!isCallerHoldingPermission(permission)) { throw new SecurityException("Caller does not hold the permission " + permission); } } @@ -1440,12 +1428,12 @@ public class VoiceInteractionManagerService extends SystemService { } private void enforceCallerAllowedToEnrollVoiceModel() { - enforceCallingPermission(Manifest.permission.MANAGE_VOICE_KEYPHRASES); - if (!isCallerCurrentVoiceInteractionService() - && !isCallerTrustedEnrollmentApplication()) { - throw new SecurityException("Caller is required to be the current voice interaction" - + " service or a system enrollment application to enroll voice models"); + if (isCallerHoldingPermission(Manifest.permission.KEYPHRASE_ENROLLMENT_APPLICATION)) { + return; } + + enforceCallingPermission(Manifest.permission.MANAGE_VOICE_KEYPHRASES); + enforceIsCurrentVoiceInteractionService(); } private boolean isCallerCurrentVoiceInteractionService() { @@ -1453,12 +1441,6 @@ public class VoiceInteractionManagerService extends SystemService { && mImpl.mInfo.getServiceInfo().applicationInfo.uid == Binder.getCallingUid(); } - private boolean isCallerTrustedEnrollmentApplication() { - getOrCreateEnrollmentApplicationInfo(); - return mEnrollmentApplicationInfo.isUidSupportedEnrollmentApplication( - Binder.getCallingUid()); - } - private void setImplLocked(VoiceInteractionManagerServiceImpl impl) { mImpl = impl; mAtmInternal.notifyActiveVoiceInteractionServiceChanged(