diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/DetectorSession.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/DetectorSession.java index 0a1f3c78e0e61929af0e9b566fdc0395bd3f7f30..ad7b9e69282ed46a9cadf7126c7b30cd19e38e5c 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/DetectorSession.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/DetectorSession.java @@ -411,7 +411,8 @@ abstract class DetectorSession { audioFormat, options, callback, - /* shouldCloseAudioStreamWithDelayOnDetect= */ true); + /* shouldCloseAudioStreamWithDelayOnDetect= */ true, + /* shouldCheckPermissionsAndAppOpsOnDetected= */ true); } void startListeningFromWearableLocked( @@ -481,12 +482,29 @@ abstract class DetectorSession { return null; } }; + /* + * By setting shouldCheckPermissionsAndAppOpsOnDetected to false, when the audio + * stream is sent from the sandboxed HotwordDetectionService to the non-sandboxed + * VoiceInteractionService as a result of second-stage hotword detection, audio-related + * permissions will not be checked against the VoiceInteractionService and the AppOpsManager + * will not be notified of the data flow to the VoiceInteractionService. These checks are + * not performed because the audio stream here originates from a remotely connected wearable + * device. It does not originate from the microphone of the device where this code runs on, + * or a microphone directly controlled by this system. Permission checks are expected to + * happen on the remote wearable device. From the perspective of this system, the audio + * stream is data received from an external source. + * + * Not notifying AppOpsManager allows this device's microphone indicator to remain off when + * this data flow happens. It avoids confusion since the audio does not originate from + * this device. The wearable is expected to turn on its own microphone indicator. + */ handleExternalSourceHotwordDetectionLocked( audioStream, audioFormat, options, voiceInteractionCallback, - /* shouldCloseAudioStreamWithDelayOnDetect= */ false); + /* shouldCloseAudioStreamWithDelayOnDetect= */ false, + /* shouldCheckPermissionsAndAppOpsOnDetected= */ false); } @SuppressWarnings("GuardedBy") @@ -495,7 +513,8 @@ abstract class DetectorSession { AudioFormat audioFormat, @Nullable PersistableBundle options, IMicrophoneHotwordDetectionVoiceInteractionCallback callback, - boolean shouldCloseAudioStreamWithDelayOnDetect) { + boolean shouldCloseAudioStreamWithDelayOnDetect, + boolean shouldCheckPermissionsAndAppOpsOnDetected) { if (DEBUG) { Slog.d(TAG, "#handleExternalSourceHotwordDetectionLocked"); } @@ -631,36 +650,39 @@ abstract class DetectorSession { EXTERNAL_HOTWORD_CLEANUP_MILLIS, TimeUnit.MILLISECONDS); } - try { - enforcePermissionsForDataDelivery(); - } catch (SecurityException e) { - Slog.w( - TAG, - "Ignoring #onDetected due to a " - + "SecurityException", - e); - HotwordMetricsLogger.writeDetectorEvent( - getDetectorType(), - EXTERNAL_SOURCE_DETECT_SECURITY_EXCEPTION, - mVoiceInteractionServiceUid); + if (shouldCheckPermissionsAndAppOpsOnDetected) { try { - callback.onHotwordDetectionServiceFailure( + enforcePermissionsForDataDelivery(); + } catch (SecurityException e) { + Slog.w( + TAG, + "Ignoring #onDetected due to a " + + "SecurityException", + e); + HotwordMetricsLogger.writeDetectorEvent( + getDetectorType(), + EXTERNAL_SOURCE_DETECT_SECURITY_EXCEPTION, + mVoiceInteractionServiceUid); + try { + callback.onHotwordDetectionServiceFailure( new HotwordDetectionServiceFailure( ONDETECTED_GOT_SECURITY_EXCEPTION, "Security exception occurs in " + "#onDetected method")); - } catch (RemoteException e1) { - notifyOnDetectorRemoteException(); - throw e1; + } catch (RemoteException e1) { + notifyOnDetectorRemoteException(); + throw e1; + } + return; } - return; } HotwordDetectedResult newResult; try { newResult = - mHotwordAudioStreamCopier - .startCopyingAudioStreams( - triggerResult); + mHotwordAudioStreamCopier + .startCopyingAudioStreams( + triggerResult, + shouldCheckPermissionsAndAppOpsOnDetected); } catch (IOException e) { Slog.w( TAG, diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordAudioStreamCopier.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordAudioStreamCopier.java index 65c95d1261f3d96df365d042931126ad3ab9d9a8..6f00dc82f42bfb5ce37b709911623e713f449455 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordAudioStreamCopier.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordAudioStreamCopier.java @@ -86,19 +86,32 @@ final class HotwordAudioStreamCopier { mVoiceInteractorAttributionTag = voiceInteractorAttributionTag; } + /** + * Calls {@link #startCopyingAudioStreams(HotwordDetectedResult, boolean)} and notifies + * AppOpsManager of the {@link AppOpsManager#OPSTR_RECORD_AUDIO_HOTWORD} operation. + */ + @NonNull + public HotwordDetectedResult startCopyingAudioStreams(@NonNull HotwordDetectedResult result) + throws IOException { + return startCopyingAudioStreams(result, /* shouldNotifyAppOpsManager= */ true); + } + /** * Starts copying the audio streams in the given {@link HotwordDetectedResult}. - * <p> - * The returned {@link HotwordDetectedResult} is identical the one that was passed in, except + * + * <p>The returned {@link HotwordDetectedResult} is identical the one that was passed in, except * that the {@link ParcelFileDescriptor}s within {@link HotwordDetectedResult#getAudioStreams()} * are replaced with descriptors from pipes managed by {@link HotwordAudioStreamCopier}. The * returned value should be passed on to the client (i.e., the voice interactor). - * </p> * + * @param result The {@link HotwordDetectedResult} to copy from. + * @param shouldNotifyAppOpsManager Whether the {@link AppOpsManager} should be notified of the + * {@link AppOpsManager#OPSTR_RECORD_AUDIO_HOTWORD} operation during the copy. * @throws IOException If there was an error creating the managed pipe. */ @NonNull - public HotwordDetectedResult startCopyingAudioStreams(@NonNull HotwordDetectedResult result) + public HotwordDetectedResult startCopyingAudioStreams( + @NonNull HotwordDetectedResult result, boolean shouldNotifyAppOpsManager) throws IOException { List<HotwordAudioStream> audioStreams = result.getAudioStreams(); if (audioStreams.isEmpty()) { @@ -154,8 +167,12 @@ final class HotwordAudioStreamCopier { String resultTaskId = TASK_ID_PREFIX + System.identityHashCode(result); mExecutorService.execute( - new HotwordDetectedResultCopyTask(resultTaskId, copyTaskInfos, - totalMetadataBundleSizeBytes, totalInitialAudioSizeBytes)); + new HotwordDetectedResultCopyTask( + resultTaskId, + copyTaskInfos, + totalMetadataBundleSizeBytes, + totalInitialAudioSizeBytes, + shouldNotifyAppOpsManager)); return result.buildUpon().setAudioStreams(newAudioStreams).build(); } @@ -178,13 +195,19 @@ final class HotwordAudioStreamCopier { private final int mTotalMetadataSizeBytes; private final int mTotalInitialAudioSizeBytes; private final ExecutorService mExecutorService = Executors.newCachedThreadPool(); - - HotwordDetectedResultCopyTask(String resultTaskId, List<CopyTaskInfo> copyTaskInfos, - int totalMetadataSizeBytes, int totalInitialAudioSizeBytes) { + private final boolean mShouldNotifyAppOpsManager; + + HotwordDetectedResultCopyTask( + String resultTaskId, + List<CopyTaskInfo> copyTaskInfos, + int totalMetadataSizeBytes, + int totalInitialAudioSizeBytes, + boolean shouldNotifyAppOpsManager) { mResultTaskId = resultTaskId; mCopyTaskInfos = copyTaskInfos; mTotalMetadataSizeBytes = totalMetadataSizeBytes; mTotalInitialAudioSizeBytes = totalInitialAudioSizeBytes; + mShouldNotifyAppOpsManager = shouldNotifyAppOpsManager; } @Override @@ -200,55 +223,14 @@ final class HotwordAudioStreamCopier { mVoiceInteractorUid)); } - if (mAppOpsManager.startOpNoThrow(AppOpsManager.OPSTR_RECORD_AUDIO_HOTWORD, - mVoiceInteractorUid, mVoiceInteractorPackageName, - mVoiceInteractorAttributionTag, OP_MESSAGE) == MODE_ALLOWED) { - try { - HotwordMetricsLogger.writeAudioEgressEvent(mDetectorType, - HOTWORD_AUDIO_EGRESS_EVENT_REPORTED__EVENT__STARTED, - mVoiceInteractorUid, mTotalInitialAudioSizeBytes, - mTotalMetadataSizeBytes, size); - // TODO(b/244599891): Set timeout, close after inactivity - mExecutorService.invokeAll(tasks); - - // We are including the non-streamed initial audio - // (HotwordAudioStream.getInitialAudio()) bytes in the "stream" size metrics. - int totalStreamSizeBytes = mTotalInitialAudioSizeBytes; - for (SingleAudioStreamCopyTask task : tasks) { - totalStreamSizeBytes += task.mTotalCopiedBytes; - } - - Slog.i(TAG, mResultTaskId + ": Task was completed. Total bytes egressed: " - + totalStreamSizeBytes + " (including " + mTotalInitialAudioSizeBytes - + " bytes NOT streamed), total metadata bundle size bytes: " - + mTotalMetadataSizeBytes); - HotwordMetricsLogger.writeAudioEgressEvent(mDetectorType, - HOTWORD_AUDIO_EGRESS_EVENT_REPORTED__EVENT__ENDED, - mVoiceInteractorUid, totalStreamSizeBytes, mTotalMetadataSizeBytes, - size); - } catch (InterruptedException e) { - // We are including the non-streamed initial audio - // (HotwordAudioStream.getInitialAudio()) bytes in the "stream" size metrics. - int totalStreamSizeBytes = mTotalInitialAudioSizeBytes; - for (SingleAudioStreamCopyTask task : tasks) { - totalStreamSizeBytes += task.mTotalCopiedBytes; - } - - HotwordMetricsLogger.writeAudioEgressEvent(mDetectorType, - HOTWORD_AUDIO_EGRESS_EVENT_REPORTED__EVENT__INTERRUPTED_EXCEPTION, - mVoiceInteractorUid, totalStreamSizeBytes, mTotalMetadataSizeBytes, - size); - Slog.i(TAG, mResultTaskId + ": Task was interrupted. Total bytes egressed: " - + totalStreamSizeBytes + " (including " + mTotalInitialAudioSizeBytes - + " bytes NOT streamed), total metadata bundle size bytes: " - + mTotalMetadataSizeBytes); - bestEffortPropagateError(e.getMessage()); - } finally { - mAppOpsManager.finishOp(AppOpsManager.OPSTR_RECORD_AUDIO_HOTWORD, - mVoiceInteractorUid, mVoiceInteractorPackageName, - mVoiceInteractorAttributionTag); - } - } else { + if (mShouldNotifyAppOpsManager + && mAppOpsManager.startOpNoThrow( + AppOpsManager.OPSTR_RECORD_AUDIO_HOTWORD, + mVoiceInteractorUid, + mVoiceInteractorPackageName, + mVoiceInteractorAttributionTag, + OP_MESSAGE) + != MODE_ALLOWED) { HotwordMetricsLogger.writeAudioEgressEvent(mDetectorType, HOTWORD_AUDIO_EGRESS_EVENT_REPORTED__EVENT__NO_PERMISSION, mVoiceInteractorUid, /* streamSizeBytes= */ 0, /* bundleSizeBytes= */ 0, @@ -258,6 +240,56 @@ final class HotwordAudioStreamCopier { + " uid=" + mVoiceInteractorUid + " packageName=" + mVoiceInteractorPackageName + " attributionTag=" + mVoiceInteractorAttributionTag); + return; + } + try { + HotwordMetricsLogger.writeAudioEgressEvent(mDetectorType, + HOTWORD_AUDIO_EGRESS_EVENT_REPORTED__EVENT__STARTED, + mVoiceInteractorUid, mTotalInitialAudioSizeBytes, + mTotalMetadataSizeBytes, size); + // TODO(b/244599891): Set timeout, close after inactivity + mExecutorService.invokeAll(tasks); + + // We are including the non-streamed initial audio + // (HotwordAudioStream.getInitialAudio()) bytes in the "stream" size metrics. + int totalStreamSizeBytes = mTotalInitialAudioSizeBytes; + for (SingleAudioStreamCopyTask task : tasks) { + totalStreamSizeBytes += task.mTotalCopiedBytes; + } + + Slog.i(TAG, mResultTaskId + ": Task was completed. Total bytes egressed: " + + totalStreamSizeBytes + " (including " + mTotalInitialAudioSizeBytes + + " bytes NOT streamed), total metadata bundle size bytes: " + + mTotalMetadataSizeBytes); + HotwordMetricsLogger.writeAudioEgressEvent(mDetectorType, + HOTWORD_AUDIO_EGRESS_EVENT_REPORTED__EVENT__ENDED, + mVoiceInteractorUid, totalStreamSizeBytes, mTotalMetadataSizeBytes, + size); + } catch (InterruptedException e) { + // We are including the non-streamed initial audio + // (HotwordAudioStream.getInitialAudio()) bytes in the "stream" size metrics. + int totalStreamSizeBytes = mTotalInitialAudioSizeBytes; + for (SingleAudioStreamCopyTask task : tasks) { + totalStreamSizeBytes += task.mTotalCopiedBytes; + } + + HotwordMetricsLogger.writeAudioEgressEvent(mDetectorType, + HOTWORD_AUDIO_EGRESS_EVENT_REPORTED__EVENT__INTERRUPTED_EXCEPTION, + mVoiceInteractorUid, totalStreamSizeBytes, mTotalMetadataSizeBytes, + size); + Slog.i(TAG, mResultTaskId + ": Task was interrupted. Total bytes egressed: " + + totalStreamSizeBytes + " (including " + mTotalInitialAudioSizeBytes + + " bytes NOT streamed), total metadata bundle size bytes: " + + mTotalMetadataSizeBytes); + bestEffortPropagateError(e.getMessage()); + } finally { + if (mShouldNotifyAppOpsManager) { + mAppOpsManager.finishOp( + AppOpsManager.OPSTR_RECORD_AUDIO_HOTWORD, + mVoiceInteractorUid, + mVoiceInteractorPackageName, + mVoiceInteractorAttributionTag); + } } }