From 31316ca79d0245599b7dbaba7aa45b8c308ec171 Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Mon, 17 Apr 2023 14:59:12 -0700 Subject: [PATCH 1/2] Plumb trusted config through soundtrigger stack Add a new argument for attaching a session for a trusted middleman. In this case, the soundtrigger stack will not attribute data delivery ops when calling back, since the trusted middleman will handle this attribution instead. Primarily used for HotwordDetectionService, which only delivers data and attributes ops onDetected. Bug: 272147641 Fixes: 278626527 Test: atest SoundTriggerManagerTest Test: atest AlwaysOnHotwordDetectorTest Test: Manual verification hotword, now playing functionality Change-Id: If0ade4a816d0972de1647d275f23e3fdb773f279 --- .../hardware/soundtrigger/SoundTrigger.java | 2 +- .../soundtrigger/SoundTriggerModule.java | 6 ++- .../ISoundTriggerMiddlewareService.aidl | 6 ++- .../android/server/SoundTriggerInternal.java | 9 ++++- .../SoundTriggerMiddlewareImplTest.java | 38 +++++++++---------- ...ndTriggerMiddlewareLoggingLatencyTest.java | 21 ++++++---- .../soundtrigger/SoundTriggerService.java | 11 ++++-- .../ISoundTriggerMiddlewareInternal.java | 5 ++- .../SoundTriggerMiddlewareImpl.java | 3 +- .../SoundTriggerMiddlewareLogging.java | 8 ++-- .../SoundTriggerMiddlewarePermission.java | 17 ++++++--- .../SoundTriggerMiddlewareService.java | 6 +-- .../SoundTriggerMiddlewareValidation.java | 4 +- .../VoiceInteractionManagerService.java | 4 +- 14 files changed, 86 insertions(+), 54 deletions(-) diff --git a/core/java/android/hardware/soundtrigger/SoundTrigger.java b/core/java/android/hardware/soundtrigger/SoundTrigger.java index 6d43ddf7fe948..44fe6671e0da9 100644 --- a/core/java/android/hardware/soundtrigger/SoundTrigger.java +++ b/core/java/android/hardware/soundtrigger/SoundTrigger.java @@ -2269,7 +2269,7 @@ public class SoundTrigger { Looper looper = handler != null ? handler.getLooper() : Looper.getMainLooper(); try { return new SoundTriggerModule(getService(), moduleId, listener, looper, - middlemanIdentity, originatorIdentity); + middlemanIdentity, originatorIdentity, false); } catch (Exception e) { Log.e(TAG, "", e); return null; diff --git a/core/java/android/hardware/soundtrigger/SoundTriggerModule.java b/core/java/android/hardware/soundtrigger/SoundTriggerModule.java index 37c5213e0359a..c836150733944 100644 --- a/core/java/android/hardware/soundtrigger/SoundTriggerModule.java +++ b/core/java/android/hardware/soundtrigger/SoundTriggerModule.java @@ -83,7 +83,8 @@ public class SoundTriggerModule { */ public SoundTriggerModule(@NonNull ISoundTriggerMiddlewareService service, int moduleId, @NonNull SoundTrigger.StatusListener listener, @NonNull Looper looper, - @NonNull Identity middlemanIdentity, @NonNull Identity originatorIdentity) { + @NonNull Identity middlemanIdentity, @NonNull Identity originatorIdentity, + boolean isTrusted) { mId = moduleId; mEventHandlerDelegate = new EventHandlerDelegate(listener, looper); @@ -91,7 +92,8 @@ public class SoundTriggerModule { try (SafeCloseable ignored = ClearCallingIdentityContext.create()) { mService = service.attachAsMiddleman(moduleId, middlemanIdentity, originatorIdentity, - mEventHandlerDelegate); + mEventHandlerDelegate, + isTrusted); } mService.asBinder().linkToDeath(mEventHandlerDelegate, 0); } catch (RemoteException e) { diff --git a/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerMiddlewareService.aidl b/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerMiddlewareService.aidl index 531b3ae0c2305..bc6a2591f386c 100644 --- a/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerMiddlewareService.aidl +++ b/media/aidl/android/media/soundtrigger_middleware/ISoundTriggerMiddlewareService.aidl @@ -80,14 +80,16 @@ interface ISoundTriggerMiddlewareService { * This implies that the caller must clear its caller identity to protect from the case where * it resides in the same process as the callee. * - The identity of the entity on behalf of which module operations are to be performed. - * + * @param isTrusted - {@code true} if the middleware should not audit data delivery, since the + * callback is being delivered to another trusted component which will audit access. * listModules() must be called prior to calling this method and the provided handle must be * one of the handles from the returned list. */ ISoundTriggerModule attachAsMiddleman(int handle, in Identity middlemanIdentity, in Identity originatorIdentity, - ISoundTriggerCallback callback); + ISoundTriggerCallback callback, + boolean isTrusted); /** * Attach an injection interface interface to the ST mock HAL. diff --git a/services/core/java/com/android/server/SoundTriggerInternal.java b/services/core/java/com/android/server/SoundTriggerInternal.java index f184574c9a361..f8830ea705c65 100644 --- a/services/core/java/com/android/server/SoundTriggerInternal.java +++ b/services/core/java/com/android/server/SoundTriggerInternal.java @@ -47,7 +47,14 @@ public interface SoundTriggerInternal { int STATUS_OK = SoundTrigger.STATUS_OK; // Attach to a specific underlying STModule - Session attach(@NonNull IBinder client, ModuleProperties underlyingModule); + /** + * Attach to a specific underlying STModule. + * @param client - Binder token representing the app client for death notifications + * @param underlyingModule - Properties of the underlying STModule to attach to + * @param isTrusted - {@code true} if callbacks will be appropriately AppOps attributed by + * a trusted component prior to delivery to the ultimate client. + */ + Session attach(@NonNull IBinder client, ModuleProperties underlyingModule, boolean isTrusted); // Enumerate possible STModules to attach to List listModuleProperties(Identity originatorIdentity); diff --git a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java index 3bebc94fe0edb..fe3e37b6d02dd 100644 --- a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java +++ b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImplTest.java @@ -163,7 +163,7 @@ public class SoundTriggerMiddlewareImplTest { public void testAttachDetach() throws Exception { // Normal attachment / detachment. ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); assertNotNull(module); module.detach(); } @@ -171,7 +171,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testLoadUnloadModel() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); final int hwHandle = 7; int handle = loadGenericModel(module, hwHandle).first; @@ -182,7 +182,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testLoadPreemptModel() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); final int hwHandle = 7; Pair loadResult = loadGenericModel(module, hwHandle); @@ -201,7 +201,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testLoadUnloadPhraseModel() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); final int hwHandle = 73; int handle = loadPhraseModel(module, hwHandle).first; @@ -212,7 +212,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testStartStopRecognition() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 7; @@ -237,7 +237,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testStartRecognitionBusy() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 7; @@ -261,7 +261,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testStartStopPhraseRecognition() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 67; @@ -286,7 +286,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testRecognition() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 7; @@ -331,7 +331,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testPhraseRecognition() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 7; @@ -361,7 +361,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testForceRecognition() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 17; @@ -398,7 +398,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testForceRecognitionNotSupported() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 17; @@ -429,7 +429,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testForcePhraseRecognition() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 17; @@ -466,7 +466,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testForcePhraseRecognitionNotSupported() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 17; @@ -498,7 +498,7 @@ public class SoundTriggerMiddlewareImplTest { public void testAbortRecognition() throws Exception { // Make sure the HAL doesn't support concurrent capture. ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 11; @@ -528,7 +528,7 @@ public class SoundTriggerMiddlewareImplTest { public void testAbortPhraseRecognition() throws Exception { // Make sure the HAL doesn't support concurrent capture. ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); // Load the model. final int hwHandle = 11; @@ -557,7 +557,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testParameterSupported() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); final int hwHandle = 12; int modelHandle = loadGenericModel(module, hwHandle).first; @@ -579,7 +579,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testParameterNotSupported() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); final int hwHandle = 13; int modelHandle = loadGenericModel(module, hwHandle).first; @@ -597,7 +597,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testGetParameter() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); final int hwHandle = 14; int modelHandle = loadGenericModel(module, hwHandle).first; @@ -614,7 +614,7 @@ public class SoundTriggerMiddlewareImplTest { @Test public void testSetParameter() throws Exception { ISoundTriggerCallback callback = createCallbackMock(); - ISoundTriggerModule module = mService.attach(0, callback); + ISoundTriggerModule module = mService.attach(0, callback, false); final int hwHandle = 17; int modelHandle = loadGenericModel(module, hwHandle).first; diff --git a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLoggingLatencyTest.java b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLoggingLatencyTest.java index f8a068c7f1218..444d5eed95f7a 100644 --- a/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLoggingLatencyTest.java +++ b/services/tests/voiceinteractiontests/src/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLoggingLatencyTest.java @@ -20,6 +20,7 @@ import static com.android.internal.util.LatencyTracker.ACTION_SHOW_VOICE_INTERAC import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.verify; @@ -105,8 +106,9 @@ public class SoundTriggerMiddlewareLoggingLatencyTest { throws RemoteException { ArgumentCaptor soundTriggerCallbackCaptor = ArgumentCaptor.forClass( ISoundTriggerCallback.class); - mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback); - verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture()); + mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback, false); + verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture(), + anyBoolean()); triggerPhraseRecognitionEvent(soundTriggerCallbackCaptor.getValue(), RecognitionStatus.SUCCESS, Optional.of(100) /* keyphraseId */); @@ -120,8 +122,9 @@ public class SoundTriggerMiddlewareLoggingLatencyTest { public void testOnPhraseRecognitionRestartsActiveSession() throws RemoteException { ArgumentCaptor soundTriggerCallbackCaptor = ArgumentCaptor.forClass( ISoundTriggerCallback.class); - mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback); - verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture()); + mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback, false); + verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture(), + anyBoolean()); triggerPhraseRecognitionEvent(soundTriggerCallbackCaptor.getValue(), RecognitionStatus.SUCCESS, Optional.of(100) /* keyphraseId */); @@ -141,8 +144,9 @@ public class SoundTriggerMiddlewareLoggingLatencyTest { throws RemoteException { ArgumentCaptor soundTriggerCallbackCaptor = ArgumentCaptor.forClass( ISoundTriggerCallback.class); - mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback); - verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture()); + mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback, false); + verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture(), + anyBoolean()); triggerPhraseRecognitionEvent(soundTriggerCallbackCaptor.getValue(), RecognitionStatus.ABORTED, Optional.of(100) /* keyphraseId */); @@ -158,8 +162,9 @@ public class SoundTriggerMiddlewareLoggingLatencyTest { throws RemoteException { ArgumentCaptor soundTriggerCallbackCaptor = ArgumentCaptor.forClass( ISoundTriggerCallback.class); - mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback); - verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture()); + mSoundTriggerMiddlewareLogging.attach(0, mISoundTriggerCallback, false); + verify(mDelegateMiddleware).attach(anyInt(), soundTriggerCallbackCaptor.capture(), + anyBoolean()); triggerPhraseRecognitionEvent(soundTriggerCallbackCaptor.getValue(), RecognitionStatus.SUCCESS, Optional.empty() /* keyphraseId */); diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java index 913535e06a214..30ea127eca6b7 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java @@ -266,6 +266,10 @@ public class SoundTriggerService extends SystemService { private SoundTriggerHelper newSoundTriggerHelper( ModuleProperties moduleProperties, EventLogger eventLogger) { + return newSoundTriggerHelper(moduleProperties, eventLogger, false); + } + private SoundTriggerHelper newSoundTriggerHelper( + ModuleProperties moduleProperties, EventLogger eventLogger, boolean isTrusted) { Identity middlemanIdentity = new Identity(); middlemanIdentity.packageName = ActivityThread.currentOpPackageName(); @@ -288,7 +292,7 @@ public class SoundTriggerService extends SystemService { eventLogger, (SoundTrigger.StatusListener statusListener) -> new SoundTriggerModule( mMiddlewareService, moduleId, statusListener, - Looper.getMainLooper(), middlemanIdentity, originatorIdentity), + Looper.getMainLooper(), middlemanIdentity, originatorIdentity, isTrusted), moduleId, () -> listUnderlyingModuleProperties(originatorIdentity) ); @@ -1667,7 +1671,8 @@ public class SoundTriggerService extends SystemService { } @Override - public Session attach(@NonNull IBinder client, ModuleProperties underlyingModule) { + public Session attach(@NonNull IBinder client, ModuleProperties underlyingModule, + boolean isTrusted) { var identity = IdentityContext.getNonNull(); int sessionId = mSessionIdCounter.getAndIncrement(); mServiceEventLogger.enqueue(new ServiceEvent( @@ -1676,7 +1681,7 @@ public class SoundTriggerService extends SystemService { "LocalSoundTriggerEventLogger for package: " + identity.packageName + "#" + sessionId); - return new SessionImpl(newSoundTriggerHelper(underlyingModule, eventLogger), + return new SessionImpl(newSoundTriggerHelper(underlyingModule, eventLogger, isTrusted), client, eventLogger, identity); } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/ISoundTriggerMiddlewareInternal.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/ISoundTriggerMiddlewareInternal.java index 60f89da8e4d3d..ca35b51885155 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/ISoundTriggerMiddlewareInternal.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/ISoundTriggerMiddlewareInternal.java @@ -38,7 +38,10 @@ public interface ISoundTriggerMiddlewareInternal { * * listModules() must be called prior to calling this method and the provided handle must be * one of the handles from the returned list. + * @param isTrusted - {@code true} if this service should not note AppOps for recognitions, + * and should delegate these checks to the **trusted** client. */ public ISoundTriggerModule attach(int handle, - ISoundTriggerCallback callback); + ISoundTriggerCallback callback, + boolean isTrusted); } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImpl.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImpl.java index c8c0f3d00cbf4..3b800de2f30b6 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImpl.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareImpl.java @@ -116,7 +116,8 @@ public class SoundTriggerMiddlewareImpl implements ISoundTriggerMiddlewareIntern @Override public @NonNull - ISoundTriggerModule attach(int handle, @NonNull ISoundTriggerCallback callback) { + ISoundTriggerModule attach(int handle, @NonNull ISoundTriggerCallback callback, + boolean isTrusted) { return mModules[handle].attach(callback); } } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java index 0e796d1afbd6b..c6a8c470bd950 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareLogging.java @@ -137,22 +137,22 @@ public class SoundTriggerMiddlewareLogging implements ISoundTriggerMiddlewareInt @Override public @NonNull - ISoundTriggerModule attach(int handle, ISoundTriggerCallback callback) { + ISoundTriggerModule attach(int handle, ISoundTriggerCallback callback, boolean isTrusted) { try { var originatorIdentity = IdentityContext.getNonNull(); String packageIdentification = originatorIdentity.packageName - + mSessionCount.getAndIncrement(); + + mSessionCount.getAndIncrement() + (isTrusted ? "trusted" : ""); ModuleLogging result = new ModuleLogging(); var eventLogger = new EventLogger(SESSION_MAX_EVENT_SIZE, "Session logger for: " + packageIdentification); var callbackWrapper = new CallbackLogging(callback, eventLogger, originatorIdentity); - result.attach(mDelegate.attach(handle, callbackWrapper), eventLogger); + result.attach(mDelegate.attach(handle, callbackWrapper, isTrusted), eventLogger); mServiceEventLogger.enqueue(ServiceEvent.createForReturn( ServiceEvent.Type.ATTACH, - packageIdentification, result, handle, callback) + packageIdentification, result, handle, callback, isTrusted) .printLog(ALOGI, TAG)); mSessionEventLoggers.add(eventLogger); diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java index 00cedd77414e6..2247d67ef84f9 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewarePermission.java @@ -85,11 +85,11 @@ public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddleware @Override public @NonNull ISoundTriggerModule attach(int handle, - @NonNull ISoundTriggerCallback callback) { + @NonNull ISoundTriggerCallback callback, boolean isTrusted) { Identity identity = getIdentity(); enforcePermissionsForPreflight(identity); - ModuleWrapper wrapper = new ModuleWrapper(identity, callback); - return wrapper.attach(mDelegate.attach(handle, wrapper.getCallbackWrapper())); + ModuleWrapper wrapper = new ModuleWrapper(identity, callback, isTrusted); + return wrapper.attach(mDelegate.attach(handle, wrapper.getCallbackWrapper(), isTrusted)); } // Override toString() in order to have the delegate's ID in it. @@ -204,11 +204,14 @@ public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddleware private ISoundTriggerModule mDelegate; private final @NonNull Identity mOriginatorIdentity; private final @NonNull CallbackWrapper mCallbackWrapper; + private final boolean mIsTrusted; ModuleWrapper(@NonNull Identity originatorIdentity, - @NonNull ISoundTriggerCallback callback) { + @NonNull ISoundTriggerCallback callback, + boolean isTrusted) { mOriginatorIdentity = originatorIdentity; mCallbackWrapper = new CallbackWrapper(callback); + mIsTrusted = isTrusted; } ModuleWrapper attach(@NonNull ISoundTriggerModule delegate) { @@ -348,7 +351,11 @@ public class SoundTriggerMiddlewarePermission implements ISoundTriggerMiddleware } private void enforcePermissions(String reason) { - enforcePermissionsForDataDelivery(mOriginatorIdentity, reason); + if (mIsTrusted) { + enforcePermissionsForPreflight(mOriginatorIdentity); + } else { + enforcePermissionsForDataDelivery(mOriginatorIdentity, reason); + } } } } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java index 91e5466969714..902f10f66a481 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareService.java @@ -104,17 +104,17 @@ public class SoundTriggerMiddlewareService extends ISoundTriggerMiddlewareServic public ISoundTriggerModule attachAsOriginator(int handle, Identity identity, ISoundTriggerCallback callback) { try (SafeCloseable ignored = establishIdentityDirect(Objects.requireNonNull(identity))) { - return new ModuleService(mDelegate.attach(handle, callback)); + return new ModuleService(mDelegate.attach(handle, callback, /* isTrusted= */ false)); } } @Override public ISoundTriggerModule attachAsMiddleman(int handle, Identity middlemanIdentity, - Identity originatorIdentity, ISoundTriggerCallback callback) { + Identity originatorIdentity, ISoundTriggerCallback callback, boolean isTrusted) { try (SafeCloseable ignored = establishIdentityIndirect( Objects.requireNonNull(middlemanIdentity), Objects.requireNonNull(originatorIdentity))) { - return new ModuleService(mDelegate.attach(handle, callback)); + return new ModuleService(mDelegate.attach(handle, callback, isTrusted)); } } diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java index 15c9ba923d3a3..c2f2939f0b967 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java @@ -191,7 +191,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware @Override public @NonNull ISoundTriggerModule attach(int handle, - @NonNull ISoundTriggerCallback callback) { + @NonNull ISoundTriggerCallback callback, boolean isTrusted) { // Input validation. Objects.requireNonNull(callback); Objects.requireNonNull(callback.asBinder()); @@ -209,7 +209,7 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware // From here on, every exception isn't client's fault. try { Session session = new Session(handle, callback); - session.attach(mDelegate.attach(handle, session.getCallbackWrapper())); + session.attach(mDelegate.attach(handle, session.getCallbackWrapper(), isTrusted)); return session; } catch (Exception e) { throw handleException(e); diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index 27f3fb3898ee0..f2824683837a1 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -408,7 +408,7 @@ public class VoiceInteractionManagerService extends SystemService { try (SafeCloseable ignored = PermissionUtil.establishIdentityDirect( originatorIdentity)) { session = new SoundTriggerSession(mSoundTriggerInternal.attach(client, - moduleProperties)); + moduleProperties, false)); } } return new SoundTriggerSessionBinderProxy(session); @@ -423,7 +423,7 @@ public class VoiceInteractionManagerService extends SystemService { return Binder.withCleanCallingIdentity(() -> { try (SafeCloseable ignored = IdentityContext.create(identity)) { return new SoundTriggerSession( - mSoundTriggerInternal.attach(client, moduleProperties)); + mSoundTriggerInternal.attach(client, moduleProperties, false)); } }); } From 62b2bbe5ffd8d62fa0456f4b36ecf24d472c7fd4 Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Sun, 16 Apr 2023 17:11:48 -0700 Subject: [PATCH 2/2] Remove voiceinteraction identity fabrication VoiceInteractionManagerService uses its own identity instead of its clients when attaching to the SoundTrigger stack to prevent appop attribution if an event is actually delivered. Now that ST has a notion of a trusted client, remove this special handling, and pass the real client identity. Attach as trusted middleman. Bug: 272147641 Fixes: 278626726 Test: AlwaysOnHotwordDetectorTest Test: Manual verification of hotword, now playing Change-Id: I4cfc57af0ead5a21baadc3f328551c0011b3c38b --- .../VoiceInteractionManagerService.java | 60 ++++--------------- 1 file changed, 10 insertions(+), 50 deletions(-) diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index f2824683837a1..423a81ac05239 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -381,51 +381,21 @@ public class VoiceInteractionManagerService extends SystemService { @NonNull Identity originatorIdentity, IBinder client, ModuleProperties moduleProperties) { Objects.requireNonNull(originatorIdentity); - boolean forHotwordDetectionService; + boolean forHotwordDetectionService = false; synchronized (VoiceInteractionManagerServiceStub.this) { enforceIsCurrentVoiceInteractionService(); forHotwordDetectionService = mImpl != null && mImpl.mHotwordDetectionConnection != null; } - IVoiceInteractionSoundTriggerSession session; - if (forHotwordDetectionService) { - // Use our own identity and handle the permission checks ourselves. This allows - // properly checking/noting against the voice interactor or hotword detection - // service as needed. - if (HotwordDetectionConnection.DEBUG) { - Slog.d(TAG, "Creating a SoundTriggerSession for a HotwordDetectionService"); - } - originatorIdentity.uid = Binder.getCallingUid(); - originatorIdentity.pid = Binder.getCallingPid(); - session = new SoundTriggerSessionPermissionsDecorator( - createSoundTriggerSessionForSelfIdentity(client, moduleProperties), - mContext, - originatorIdentity); - } else { - if (HotwordDetectionConnection.DEBUG) { - Slog.d(TAG, "Creating a SoundTriggerSession"); - } - try (SafeCloseable ignored = PermissionUtil.establishIdentityDirect( - originatorIdentity)) { - session = new SoundTriggerSession(mSoundTriggerInternal.attach(client, - moduleProperties, false)); - } + if (HotwordDetectionConnection.DEBUG) { + Slog.d(TAG, "Creating a SoundTriggerSession, for HDS: " + + forHotwordDetectionService); + } + try (SafeCloseable ignored = PermissionUtil.establishIdentityDirect( + originatorIdentity)) { + return new SoundTriggerSession(mSoundTriggerInternal.attach(client, + moduleProperties, forHotwordDetectionService)); } - return new SoundTriggerSessionBinderProxy(session); - } - - private IVoiceInteractionSoundTriggerSession createSoundTriggerSessionForSelfIdentity( - IBinder client, ModuleProperties moduleProperties) { - Identity identity = new Identity(); - identity.uid = Process.myUid(); - identity.pid = Process.myPid(); - identity.packageName = ActivityThread.currentOpPackageName(); - return Binder.withCleanCallingIdentity(() -> { - try (SafeCloseable ignored = IdentityContext.create(identity)) { - return new SoundTriggerSession( - mSoundTriggerInternal.attach(client, moduleProperties, false)); - } - }); } @Override @@ -1700,11 +1670,7 @@ public class VoiceInteractionManagerService extends SystemService { return null; } - /** - * Implementation of SoundTriggerSession. Does not implement {@link #asBinder()} as it's - * intended to be wrapped by an {@link IVoiceInteractionSoundTriggerSession.Stub} object. - */ - private class SoundTriggerSession implements IVoiceInteractionSoundTriggerSession { + private class SoundTriggerSession extends IVoiceInteractionSoundTriggerSession.Stub { final SoundTriggerInternal.Session mSession; private IHotwordRecognitionStatusCallback mSessionExternalCallback; private IRecognitionStatusCallback mSessionInternalCallback; @@ -1850,12 +1816,6 @@ public class VoiceInteractionManagerService extends SystemService { } } - @Override - public IBinder asBinder() { - throw new UnsupportedOperationException( - "This object isn't intended to be used as a Binder."); - } - @Override public void detach() { mSession.detach();