From 3873a09959b3596c1dac768a68a88163eb6df771 Mon Sep 17 00:00:00 2001 From: Ahaan Ugale Date: Thu, 5 Aug 2021 19:04:25 -0700 Subject: Make Trusted Hotword session permissions follow previous behavior A prior change to the permissions flow for Trusted Hotword (I80dabaf6ae0e781028dde16ead3321fbff319542) made the system enforce the required permissions on the APIs the conventional way - by throwing a SecurityException. But the existing behavior was to silence these exceptions and instead return error results. This change brings back the old behavior which exists in the SoundTrigger layer. Also removes permissions checks for a couple of APIs to again be consistent with the old behavior (and the current behavior in the SoundTrigger layer). Fix: 193116894 Test: manual - remove permission and reboot / remove permission after boot, stop/start reco Test: atest HotwordDetectionServiceBasicTest Change-Id: I56391260fd4375a04233eb3261bacec8696bda99 --- .../service/voice/AlwaysOnHotwordDetector.java | 3 +++ .../service/voice/SoftwareHotwordDetector.java | 3 +++ .../SoundTriggerSessionPermissionsDecorator.java | 29 +++++++++++++++------- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/core/java/android/service/voice/AlwaysOnHotwordDetector.java b/core/java/android/service/voice/AlwaysOnHotwordDetector.java index 41374167cc56..face870ca1b4 100644 --- a/core/java/android/service/voice/AlwaysOnHotwordDetector.java +++ b/core/java/android/service/voice/AlwaysOnHotwordDetector.java @@ -783,6 +783,9 @@ public class AlwaysOnHotwordDetector extends AbstractHotwordDetector { * This may happen if another detector has been instantiated or the * {@link VoiceInteractionService} hosting this detector has been shut down. */ + // TODO: Remove this RequiresPermission since it isn't actually enforced. Also fix the javadoc + // about permissions enforcement (when it throws vs when it just returns false) for other + // methods in this class. @RequiresPermission(allOf = {RECORD_AUDIO, CAPTURE_AUDIO_HOTWORD}) @Override public boolean stopRecognition() { diff --git a/core/java/android/service/voice/SoftwareHotwordDetector.java b/core/java/android/service/voice/SoftwareHotwordDetector.java index 02294e5720ae..f7a3415259fd 100644 --- a/core/java/android/service/voice/SoftwareHotwordDetector.java +++ b/core/java/android/service/voice/SoftwareHotwordDetector.java @@ -82,6 +82,9 @@ class SoftwareHotwordDetector extends AbstractHotwordDetector { try { mManagerService.startListeningFromMic( mAudioFormat, new BinderCallback(mHandler, mCallback)); + } catch (SecurityException e) { + Slog.e(TAG, "startRecognition failed: " + e); + return false; } catch (RemoteException e) { e.rethrowFromSystemServer(); } diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerSessionPermissionsDecorator.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerSessionPermissionsDecorator.java index 68b2e6168b5c..c0c3e6f530db 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerSessionPermissionsDecorator.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/SoundTriggerSessionPermissionsDecorator.java @@ -60,7 +60,7 @@ final class SoundTriggerSessionPermissionsDecorator implements @Override public SoundTrigger.ModuleProperties getDspModuleProperties() throws RemoteException { - // No permission needed. + // No permission needed here (the app must have the Assistant Role to retrieve the session). return mDelegate.getDspModuleProperties(); } @@ -71,7 +71,9 @@ final class SoundTriggerSessionPermissionsDecorator implements if (DEBUG) { Slog.d(TAG, "startRecognition"); } - enforcePermissions(); + if (!isHoldingPermissions()) { + return SoundTrigger.STATUS_PERMISSION_DENIED; + } return mDelegate.startRecognition(i, s, iHotwordRecognitionStatusCallback, recognitionConfig, b); } @@ -80,25 +82,28 @@ final class SoundTriggerSessionPermissionsDecorator implements public int stopRecognition(int i, IHotwordRecognitionStatusCallback iHotwordRecognitionStatusCallback) throws RemoteException { - enforcePermissions(); + // Stopping a model does not require special permissions. Having a handle to the session is + // sufficient. return mDelegate.stopRecognition(i, iHotwordRecognitionStatusCallback); } @Override public int setParameter(int i, int i1, int i2) throws RemoteException { - enforcePermissions(); + if (!isHoldingPermissions()) { + return SoundTrigger.STATUS_PERMISSION_DENIED; + } return mDelegate.setParameter(i, i1, i2); } @Override public int getParameter(int i, int i1) throws RemoteException { - enforcePermissions(); + // No permission needed here (the app must have the Assistant Role to retrieve the session). return mDelegate.getParameter(i, i1); } @Override public SoundTrigger.ModelParamRange queryParameter(int i, int i1) throws RemoteException { - enforcePermissions(); + // No permission needed here (the app must have the Assistant Role to retrieve the session). return mDelegate.queryParameter(i, i1); } @@ -109,9 +114,15 @@ final class SoundTriggerSessionPermissionsDecorator implements } // TODO: Share this code with SoundTriggerMiddlewarePermission. - private void enforcePermissions() { - enforcePermissionForPreflight(mContext, mOriginatorIdentity, RECORD_AUDIO); - enforcePermissionForPreflight(mContext, mOriginatorIdentity, CAPTURE_AUDIO_HOTWORD); + private boolean isHoldingPermissions() { + try { + enforcePermissionForPreflight(mContext, mOriginatorIdentity, RECORD_AUDIO); + enforcePermissionForPreflight(mContext, mOriginatorIdentity, CAPTURE_AUDIO_HOTWORD); + return true; + } catch (SecurityException e) { + Slog.e(TAG, e.toString()); + return false; + } } /** -- cgit v1.2.3-59-g8ed1b