From f6848802cbc4f2f9d54831ba8441a38b6f3f8767 Mon Sep 17 00:00:00 2001 From: Christian Frank Date: Thu, 25 Feb 2021 10:09:15 +0100 Subject: [PATCH] Add attribution context for MusicRecognitionManager attribution. In addition: 1) Use attribution tag specified in the MusicRecognitionService's manifest. 2) Explicitly check for RECORD_AUDIO permission of receiving service. Example audio attribution from dumpsys: RECORD_AUDIO (allow): MusicRecognitionDoneByServiceX=[ Access: [fg-tpd] 2021-02-23 18:21:01.753 (-14s877ms) duration=+8s250ms proxy[uid=1000, pkg=android, attributionTag=MusicRecognitionManagerService] ] Test: atest and manual inspection of attributions with dumpsys. Change-Id: Ie415104580a1814b0b74f2d5b489bf8247d9238d Bug: 178174412 Bug: 169403302 --- core/res/AndroidManifest.xml | 4 ++ core/res/res/values/strings.xml | 3 + ...usicRecognitionAttributionTagCallback.aidl | 10 +++ .../IMusicRecognitionService.aidl | 3 + .../IMusicRecognitionServiceCallback.aidl | 2 +- .../MusicRecognitionService.java | 11 +++- ...MusicRecognitionManagerPerUserService.java | 65 ++++++++++++++----- .../RemoteMusicRecognitionService.java | 24 ++++++- 8 files changed, 100 insertions(+), 22 deletions(-) create mode 100644 media/java/android/media/musicrecognition/IMusicRecognitionAttributionTagCallback.aidl diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index 8e1da08195158..c2f824f2e0c36 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -5592,6 +5592,10 @@ + + GNSS Time Update Service + + Music Recognition Manager Service + Your device will be erased diff --git a/media/java/android/media/musicrecognition/IMusicRecognitionAttributionTagCallback.aidl b/media/java/android/media/musicrecognition/IMusicRecognitionAttributionTagCallback.aidl new file mode 100644 index 0000000000000..ceef73cd194a2 --- /dev/null +++ b/media/java/android/media/musicrecognition/IMusicRecognitionAttributionTagCallback.aidl @@ -0,0 +1,10 @@ +package android.media.musicrecognition; + +/** + * Interface from {@link MusicRecognitionService} to system to pass attribution tag. + * + * @hide + */ +oneway interface IMusicRecognitionAttributionTagCallback { + void onAttributionTag(in String attributionTag); +} \ No newline at end of file diff --git a/media/java/android/media/musicrecognition/IMusicRecognitionService.aidl b/media/java/android/media/musicrecognition/IMusicRecognitionService.aidl index 26543ed8bf8c5..c970161a115b5 100644 --- a/media/java/android/media/musicrecognition/IMusicRecognitionService.aidl +++ b/media/java/android/media/musicrecognition/IMusicRecognitionService.aidl @@ -4,6 +4,7 @@ import android.media.AudioFormat; import android.os.ParcelFileDescriptor; import android.os.IBinder; import android.media.musicrecognition.IMusicRecognitionServiceCallback; +import android.media.musicrecognition.IMusicRecognitionAttributionTagCallback; /** * Interface from the system to a {@link MusicRecognitionService}. @@ -15,4 +16,6 @@ oneway interface IMusicRecognitionService { in ParcelFileDescriptor fd, in AudioFormat audioFormat, in IMusicRecognitionServiceCallback callback); + + void getAttributionTag(in IMusicRecognitionAttributionTagCallback callback); } \ No newline at end of file diff --git a/media/java/android/media/musicrecognition/IMusicRecognitionServiceCallback.aidl b/media/java/android/media/musicrecognition/IMusicRecognitionServiceCallback.aidl index 15215c4e15f1e..10a65545cc7e9 100644 --- a/media/java/android/media/musicrecognition/IMusicRecognitionServiceCallback.aidl +++ b/media/java/android/media/musicrecognition/IMusicRecognitionServiceCallback.aidl @@ -4,7 +4,7 @@ import android.os.Bundle; import android.media.MediaMetadata; /** - * Interface from a {@MusicRecognitionService} the system. + * Interface from a {@MusicRecognitionService} to the system. * * @hide */ diff --git a/media/java/android/media/musicrecognition/MusicRecognitionService.java b/media/java/android/media/musicrecognition/MusicRecognitionService.java index 04b4c39bf0fac..385aff01d45a6 100644 --- a/media/java/android/media/musicrecognition/MusicRecognitionService.java +++ b/media/java/android/media/musicrecognition/MusicRecognitionService.java @@ -90,7 +90,7 @@ public abstract class MusicRecognitionService extends Service { try { callback.onRecognitionSucceeded(result, extras); } catch (RemoteException e) { - e.rethrowFromSystemServer(); + throw e.rethrowFromSystemServer(); } } @@ -99,11 +99,18 @@ public abstract class MusicRecognitionService extends Service { try { callback.onRecognitionFailed(failureCode); } catch (RemoteException e) { - e.rethrowFromSystemServer(); + throw e.rethrowFromSystemServer(); } } })); } + + @Override + public void getAttributionTag( + IMusicRecognitionAttributionTagCallback callback) throws RemoteException { + String tag = MusicRecognitionService.this.getAttributionTag(); + callback.onAttributionTag(tag); + } }; @Override diff --git a/services/musicrecognition/java/com/android/server/musicrecognition/MusicRecognitionManagerPerUserService.java b/services/musicrecognition/java/com/android/server/musicrecognition/MusicRecognitionManagerPerUserService.java index 87b2c84a30f7f..4c5bbebdfd45d 100644 --- a/services/musicrecognition/java/com/android/server/musicrecognition/MusicRecognitionManagerPerUserService.java +++ b/services/musicrecognition/java/com/android/server/musicrecognition/MusicRecognitionManagerPerUserService.java @@ -48,6 +48,8 @@ import com.android.server.infra.AbstractPerUserSystemService; import java.io.IOException; import java.io.OutputStream; import java.util.Objects; +import java.util.concurrent.CompletableFuture; + /** * Handles per-user requests received by @@ -60,6 +62,11 @@ public final class MusicRecognitionManagerPerUserService extends implements RemoteMusicRecognitionService.Callbacks { private static final String TAG = MusicRecognitionManagerPerUserService.class.getSimpleName(); + private static final String MUSIC_RECOGNITION_MANAGER_ATTRIBUTION_TAG = + "MusicRecognitionManagerService"; + private static final String KEY_MUSIC_RECOGNITION_SERVICE_ATTRIBUTION_TAG = + "android.media.musicrecognition.attributiontag"; + // Number of bytes per sample of audio (which is a short). private static final int BYTES_PER_SAMPLE = 2; private static final int MAX_STREAMING_SECONDS = 24; @@ -68,18 +75,24 @@ public final class MusicRecognitionManagerPerUserService extends @GuardedBy("mLock") private RemoteMusicRecognitionService mRemoteService; private final AppOpsManager mAppOpsManager; + private final String mAttributionMessage; - private String mAttributionTag; - private String mAttributionMessage; + // Service info of the remote MusicRecognitionService (which the audio gets forwarded to). private ServiceInfo mServiceInfo; + private CompletableFuture mAttributionTagFuture; MusicRecognitionManagerPerUserService( @NonNull MusicRecognitionManagerService primary, @NonNull Object lock, int userId) { super(primary, lock, userId); - mAppOpsManager = getContext().getSystemService(AppOpsManager.class); + + // When attributing audio-access, this establishes that audio access is performed by + // MusicRecognitionManager (on behalf of the receiving service, whose attribution tag, + // provided by mAttributionTagFuture, is used for the actual calls to startProxyOp(...). + mAppOpsManager = getContext().createAttributionContext( + MUSIC_RECOGNITION_MANAGER_ATTRIBUTION_TAG).getSystemService(AppOpsManager.class); mAttributionMessage = String.format("MusicRecognitionManager.invokedByUid.%s", userId); - mAttributionTag = null; + mAttributionTagFuture = null; mServiceInfo = null; } @@ -126,10 +139,13 @@ public final class MusicRecognitionManagerPerUserService extends new MusicRecognitionServiceCallback(clientCallback), mMaster.isBindInstantServiceAllowed(), mMaster.verbose); + try { mServiceInfo = getContext().getPackageManager().getServiceInfo( - mRemoteService.getComponentName(), 0); + mRemoteService.getComponentName(), PackageManager.GET_META_DATA); + mAttributionTagFuture = mRemoteService.getAttributionTag(); + Slog.i(TAG, "Remote service bound: " + mRemoteService.getComponentName()); } catch (PackageManager.NameNotFoundException e) { Slog.e(TAG, "Service was not found.", e); } @@ -172,11 +188,13 @@ public final class MusicRecognitionManagerPerUserService extends ParcelFileDescriptor audioSink = clientPipe.second; ParcelFileDescriptor clientRead = clientPipe.first; - mMaster.mExecutorService.execute(() -> { - streamAudio(recognitionRequest, clientCallback, audioSink); - }); + mAttributionTagFuture.thenAcceptAsync( + tag -> { + streamAudio(tag, recognitionRequest, clientCallback, audioSink); + }, mMaster.mExecutorService); + // Send the pipe down to the lookup service while we write to it asynchronously. - mRemoteService.writeAudioToPipe(clientRead, recognitionRequest.getAudioFormat()); + mRemoteService.onAudioStreamStarted(clientRead, recognitionRequest.getAudioFormat()); } /** @@ -186,10 +204,12 @@ public final class MusicRecognitionManagerPerUserService extends * @param clientCallback the callback to notify on errors. * @param audioSink the sink to which to stream audio to. */ - private void streamAudio(@NonNull RecognitionRequest recognitionRequest, - IMusicRecognitionManagerCallback clientCallback, ParcelFileDescriptor audioSink) { + private void streamAudio(@Nullable String attributionTag, + @NonNull RecognitionRequest recognitionRequest, + IMusicRecognitionManagerCallback clientCallback, + ParcelFileDescriptor audioSink) { try { - startRecordAudioOp(); + startRecordAudioOp(attributionTag); } catch (SecurityException e) { // A security exception can occur if the MusicRecognitionService (receiving the audio) // does not (or does no longer) hold the necessary permissions to record audio. @@ -214,7 +234,7 @@ public final class MusicRecognitionManagerPerUserService extends Slog.e(TAG, "Audio streaming stopped.", e); } finally { audioRecord.release(); - finishRecordAudioOp(); + finishRecordAudioOp(attributionTag); try { clientCallback.onAudioStreamClosed(); } catch (RemoteException ignored) { @@ -323,23 +343,32 @@ public final class MusicRecognitionManagerPerUserService extends * Tracks that the RECORD_AUDIO operation started (attributes it to the service receiving the * audio). */ - private void startRecordAudioOp() { - mAppOpsManager.startProxyOp( + private void startRecordAudioOp(@Nullable String attributionTag) { + int status = mAppOpsManager.startProxyOp( Objects.requireNonNull(AppOpsManager.permissionToOp(RECORD_AUDIO)), mServiceInfo.applicationInfo.uid, mServiceInfo.packageName, - mAttributionTag, + attributionTag, mAttributionMessage); + // The above should already throw a SecurityException. This is just a fallback. + if (status != AppOpsManager.MODE_ALLOWED) { + throw new SecurityException(String.format( + "Failed to obtain RECORD_AUDIO permission (status: %d) for " + + "receiving service: %s", status, mServiceInfo.getComponentName())); + } + Slog.i(TAG, String.format( + "Starting audio streaming. Attributing to %s (%d) with tag '%s'", + mServiceInfo.packageName, mServiceInfo.applicationInfo.uid, attributionTag)); } /** Tracks that the RECORD_AUDIO operation finished. */ - private void finishRecordAudioOp() { + private void finishRecordAudioOp(@Nullable String attributionTag) { mAppOpsManager.finishProxyOp( Objects.requireNonNull(AppOpsManager.permissionToOp(RECORD_AUDIO)), mServiceInfo.applicationInfo.uid, mServiceInfo.packageName, - mAttributionTag); + attributionTag); } /** Establishes an audio stream from the DSP audio source. */ diff --git a/services/musicrecognition/java/com/android/server/musicrecognition/RemoteMusicRecognitionService.java b/services/musicrecognition/java/com/android/server/musicrecognition/RemoteMusicRecognitionService.java index 6c7d673ffe11d..99b4482114923 100644 --- a/services/musicrecognition/java/com/android/server/musicrecognition/RemoteMusicRecognitionService.java +++ b/services/musicrecognition/java/com/android/server/musicrecognition/RemoteMusicRecognitionService.java @@ -20,15 +20,20 @@ import android.annotation.NonNull; import android.content.ComponentName; import android.content.Context; import android.media.AudioFormat; +import android.media.musicrecognition.IMusicRecognitionAttributionTagCallback; import android.media.musicrecognition.IMusicRecognitionService; import android.media.musicrecognition.MusicRecognitionService; import android.os.IBinder; import android.os.ParcelFileDescriptor; +import android.os.RemoteException; import android.text.format.DateUtils; import com.android.internal.infra.AbstractMultiplePendingRequestsRemoteService; import com.android.server.musicrecognition.MusicRecognitionManagerPerUserService.MusicRecognitionServiceCallback; +import java.util.concurrent.CompletableFuture; + + /** Remote connection to an instance of {@link MusicRecognitionService}. */ public class RemoteMusicRecognitionService extends AbstractMultiplePendingRequestsRemoteService binder.onAudioStreamStarted(fd, audioFormat, mServerCallback)); } + + + /** + * Returns the name of the tag defined in the remote service's manifest. + */ + public CompletableFuture getAttributionTag() { + CompletableFuture attributionTagFuture = new CompletableFuture(); + scheduleAsyncRequest( + binder -> binder.getAttributionTag( + new IMusicRecognitionAttributionTagCallback.Stub() { + @Override + public void onAttributionTag(String tag) throws RemoteException { + attributionTagFuture.complete(tag); + } + })); + return attributionTagFuture; + } }