From 935df08b15d291e9696159526fd067a37a1b66fb Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 Apr 2019 12:37:32 -0700 Subject: [PATCH 1/3] Refactor security checks to register policies There is no functional changes in this patch The checks were much more complex then needed. The behaviour on empty mix was not explicit, it depended on non trivial behaviour of anyMatch and allMatch and the code flow. There was an early positive return leading to the rest of the permission check to be skipped. Which was correct but prone to bugs if the code was changed. The code in general was hard to change without introducing security bug as well as difficult to review. Change it to a two part section, first the permissions needed are determined explicitly. Then those permissions are checked. Early return is only used for permission denial. Test: atest android.media.cts.AudioPlaybackCaptureTest#testCaptureMediaUsage Bug: 129948989 Change-Id: I6b4177e47025af983ba6c980d58ea308c4c378f1 Signed-off-by: Kevin Rocard --- .../android/server/audio/AudioService.java | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 4cde931d2ef5a..20b0deb930e6e 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -6741,40 +6741,58 @@ public class AudioService extends IAudioService.Stub private boolean isPolicyRegisterAllowed(AudioPolicyConfig policyConfig, IMediaProjection projection) { - boolean isLoopbackRenderPolicy = policyConfig.getMixes().stream().allMatch( - mix -> mix.getRouteFlags() == (mix.ROUTE_FLAG_RENDER | mix.ROUTE_FLAG_LOOP_BACK)); + boolean requireValidProjection = false; + boolean requireCaptureAudioOrMediaOutputPerm = false; + boolean requireModifyRouting = false; - if (isLoopbackRenderPolicy) { - boolean allowPrivilegedPlaybackCapture = policyConfig.getMixes().stream().anyMatch( - mix -> mix.getRule().allowPrivilegedPlaybackCapture()); - if (allowPrivilegedPlaybackCapture - && !(hasPermission(android.Manifest.permission.CAPTURE_AUDIO_OUTPUT) - || hasPermission(android.Manifest.permission.CAPTURE_MEDIA_OUTPUT))) { - // Opt-out can not be bypassed without a system permission - return false; + if (policyConfig.getMixes().isEmpty()) { + requireModifyRouting |= true; + } + for (AudioMix mix : policyConfig.getMixes()) { + // If mix is requesting a privileged capture + if (mix.getRule().allowPrivilegedPlaybackCapture()) { + // then it must have CAPTURE_MEDIA_OUTPUT or CAPTURE_AUDIO_OUTPUT permission + requireCaptureAudioOrMediaOutputPerm |= true; } - if (canProjectAudio(projection)) { - // Policy that do not modify the audio routing only need an audio projection - return true; + // If mix is RENDER|LOOPBACK, then an audio MediaProjection is enough + // otherwise MODIFY_AUDIO_ROUTING permission is required + if (mix.getRouteFlags() == mix.ROUTE_FLAG_LOOP_BACK_RENDER && projection != null) { + requireValidProjection |= true; + } else { + requireModifyRouting |= true; } } - boolean hasPermissionModifyAudioRouting = - (PackageManager.PERMISSION_GRANTED == mContext.checkCallingPermission( - android.Manifest.permission.MODIFY_AUDIO_ROUTING)); - if (hasPermissionModifyAudioRouting) { - return true; + if (requireCaptureAudioOrMediaOutputPerm + && !callerHasPermission(android.Manifest.permission.CAPTURE_MEDIA_OUTPUT) + && !callerHasPermission(android.Manifest.permission.CAPTURE_AUDIO_OUTPUT)) { + Log.e(TAG, "Privileged audio capture requires CAPTURE_MEDIA_OUTPUT or " + + "CAPTURE_AUDIO_OUTPUT system permission"); + return false; } - return false; + + if (requireValidProjection && !canProjectAudio(projection)) { + return false; + } + + if (requireModifyRouting + && !callerHasPermission(android.Manifest.permission.MODIFY_AUDIO_ROUTING)) { + Log.e(TAG, "Can not capture audio without MODIFY_AUDIO_ROUTING"); + return false; + } + + return true; } - private boolean hasPermission(String permission) { - return PackageManager.PERMISSION_GRANTED == mContext.checkCallingPermission(permission); + + private boolean callerHasPermission(String permission) { + return mContext.checkCallingPermission(permission) == PackageManager.PERMISSION_GRANTED; } /** @return true if projection is a valid MediaProjection that can project audio. */ private boolean canProjectAudio(IMediaProjection projection) { if (projection == null) { + Log.e(TAG, "MediaProjection is null"); return false; } From 61164db786006d10f84a9a905ce7a3051561e6e8 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Fri, 17 May 2019 16:58:22 -0700 Subject: [PATCH 2/3] Fix permission check when registering an audio policy The current checks were allowing public apps to register an audio policy with an APC mix as intended, but the policy could also have focus listener and volume controller which are not intended to be used by public apps. Test: atest android.media.cts.AudioPlaybackCaptureTest Bug: 129948989 Change-Id: I97d41ed08faff05e59ae6b495ee62ae11f4a09c1 --- .../com/android/server/audio/AudioService.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 20b0deb930e6e..2631c23da8e1e 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -6698,7 +6698,10 @@ public class AudioService extends IAudioService.Stub boolean isVolumeController, IMediaProjection projection) { AudioSystem.setDynamicPolicyCallback(mDynPolicyCallback); - if (!isPolicyRegisterAllowed(policyConfig, projection)) { + if (!isPolicyRegisterAllowed(policyConfig, + isFocusPolicy || isTestFocusPolicy || hasFocusListener, + isVolumeController, + projection)) { Slog.w(TAG, "Permission denied to register audio policy for pid " + Binder.getCallingPid() + " / uid " + Binder.getCallingUid() + ", need MODIFY_AUDIO_ROUTING or MediaProjection that can project audio"); @@ -6739,13 +6742,18 @@ public class AudioService extends IAudioService.Stub * as those policy do not modify the audio routing. */ private boolean isPolicyRegisterAllowed(AudioPolicyConfig policyConfig, - IMediaProjection projection) { + boolean hasFocusAccess, + boolean isVolumeController, + IMediaProjection projection) { boolean requireValidProjection = false; boolean requireCaptureAudioOrMediaOutputPerm = false; boolean requireModifyRouting = false; - if (policyConfig.getMixes().isEmpty()) { + if (hasFocusAccess || isVolumeController) { + requireModifyRouting |= true; + } else if (policyConfig.getMixes().isEmpty()) { + // An empty policy could be used to lock the focus or add mixes later requireModifyRouting |= true; } for (AudioMix mix : policyConfig.getMixes()) { From e5b29c98ee6a8e51ebacab9ea0ae4fd9067fef29 Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Wed, 3 Apr 2019 12:37:32 -0700 Subject: [PATCH 3/3] System apps are not allowed to record better then 16kHz mono This privacy requirement was documented but was not implemented. Test: atest android.media.cts.AudioPlaybackCaptureTest#testCaptureMediaUsage Bug: 129948989 Change-Id: I256e6ed1965263416a893393d5462ec73099751e Signed-off-by: Kevin Rocard --- media/java/android/media/AudioAttributes.java | 10 ++-- .../android/media/audiopolicy/AudioMix.java | 57 ++++++++++++++++++- .../media/audiopolicy/AudioMixingRule.java | 4 ++ .../android/server/audio/AudioService.java | 6 ++ 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/media/java/android/media/AudioAttributes.java b/media/java/android/media/AudioAttributes.java index a8f313b9df8a8..dc3041fb4878e 100644 --- a/media/java/android/media/AudioAttributes.java +++ b/media/java/android/media/AudioAttributes.java @@ -400,7 +400,7 @@ public final class AudioAttributes implements Parcelable { /** * Indicates that the audio may be captured by any app. * - * For privacy, the following usages can not be recorded: VOICE_COMMUNICATION*, + * For privacy, the following usages cannot be recorded: VOICE_COMMUNICATION*, * USAGE_NOTIFICATION*, USAGE_ASSISTANCE* and USAGE_ASSISTANT. * * On {@link android.os.Build.VERSION_CODES#Q}, this means only {@link #USAGE_UNKNOWN}, @@ -413,11 +413,11 @@ public final class AudioAttributes implements Parcelable { /** * Indicates that the audio may only be captured by system apps. * - * System apps can capture for many purposes like accessibility, user guidance... + * System apps can capture for many purposes like accessibility, live captions, user guidance... * but abide to the following restrictions: - * - the audio can not leave the device - * - the audio can not be passed to a third party app - * - the audio can not be recorded at a higher quality then 16kHz 16bit mono + * - the audio cannot leave the device + * - the audio cannot be passed to a third party app + * - the audio cannot be recorded at a higher quality than 16kHz 16bit mono * * See {@link Builder#setAllowedCapturePolicy}. */ diff --git a/media/java/android/media/audiopolicy/AudioMix.java b/media/java/android/media/audiopolicy/AudioMix.java index 2f03d26830f2b..a82c78fad2710 100644 --- a/media/java/android/media/audiopolicy/AudioMix.java +++ b/media/java/android/media/audiopolicy/AudioMix.java @@ -91,6 +91,20 @@ public class AudioMix { */ public static final int ROUTE_FLAG_LOOP_BACK = 0x1 << 1; + /** + * An audio mix behavior where the targeted audio is played unaffected but a copy is + * accessible for capture through {@link AudioRecord}. + * + * Only capture of playback is supported, not capture of capture. + * Use concurrent capture instead to capture what is captured by other apps. + * + * The captured audio is an approximation of the played audio. + * Effects and volume are not applied, and track are mixed with different delay then in the HAL. + * As a result, this API is not suitable for echo cancelling. + * @hide + */ + public static final int ROUTE_FLAG_LOOP_BACK_RENDER = ROUTE_FLAG_LOOP_BACK | ROUTE_FLAG_RENDER; + private static final int ROUTE_FLAG_SUPPORTED = ROUTE_FLAG_RENDER | ROUTE_FLAG_LOOP_BACK; // MIX_TYPE_* values to keep in sync with frameworks/av/include/media/AudioPolicy.h @@ -125,6 +139,15 @@ public class AudioMix { */ public static final int MIX_STATE_MIXING = 1; + /** Maximum sampling rate for privileged playback capture*/ + private static final int PRIVILEDGED_CAPTURE_MAX_SAMPLE_RATE = 16000; + + /** Maximum channel number for privileged playback capture*/ + private static final int PRIVILEDGED_CAPTURE_MAX_CHANNEL_NUMBER = 1; + + /** Maximum channel number for privileged playback capture*/ + private static final int PRIVILEDGED_CAPTURE_MAX_BYTES_PER_SAMPLE = 2; + /** * The current mixing state. * @return one of {@link #MIX_STATE_DISABLED}, {@link #MIX_STATE_IDLE}, @@ -140,7 +163,8 @@ public class AudioMix { return mRouteFlags; } - AudioFormat getFormat() { + /** @hide */ + public AudioFormat getFormat() { return mFormat; } @@ -182,6 +206,31 @@ public class AudioMix { return true; } + /** @return an error string if the format would not allow Privileged playbackCapture + * null otherwise + * @hide */ + public static String canBeUsedForPrivilegedCapture(AudioFormat format) { + int sampleRate = format.getSampleRate(); + if (sampleRate > PRIVILEDGED_CAPTURE_MAX_SAMPLE_RATE || sampleRate <= 0) { + return "Privileged audio capture sample rate " + sampleRate + + " can not be over " + PRIVILEDGED_CAPTURE_MAX_SAMPLE_RATE + "kHz"; + } + int channelCount = format.getChannelCount(); + if (channelCount > PRIVILEDGED_CAPTURE_MAX_CHANNEL_NUMBER || channelCount <= 0) { + return "Privileged audio capture channel count " + channelCount + " can not be over " + + PRIVILEDGED_CAPTURE_MAX_CHANNEL_NUMBER; + } + int encoding = format.getEncoding(); + if (!format.isPublicEncoding(encoding) || !format.isEncodingLinearPcm(encoding)) { + return "Privileged audio capture encoding " + encoding + "is not linear"; + } + if (format.getBytesPerSample(encoding) > PRIVILEDGED_CAPTURE_MAX_BYTES_PER_SAMPLE) { + return "Privileged audio capture encoding " + encoding + " can not be over " + + PRIVILEDGED_CAPTURE_MAX_BYTES_PER_SAMPLE + " bytes per sample"; + } + return null; + } + /** @hide */ @Override public boolean equals(Object o) { @@ -390,6 +439,12 @@ public class AudioMix { } } } + if (mRule.allowPrivilegedPlaybackCapture()) { + String error = AudioMix.canBeUsedForPrivilegedCapture(mFormat); + if (error != null) { + throw new IllegalArgumentException(error); + } + } return new AudioMix(mRule, mFormat, mRouteFlags, mCallbackFlags, mDeviceSystemType, mDeviceAddress); } diff --git a/media/java/android/media/audiopolicy/AudioMixingRule.java b/media/java/android/media/audiopolicy/AudioMixingRule.java index ed2fdae71fb0e..c4afd95be9ac6 100644 --- a/media/java/android/media/audiopolicy/AudioMixingRule.java +++ b/media/java/android/media/audiopolicy/AudioMixingRule.java @@ -365,6 +365,10 @@ public class AudioMixingRule { /** * Set if the audio of app that opted out of audio playback capture should be captured. * + * Caller of this method with true, MUST abide to the restriction listed in + * {@link ALLOW_CAPTURE_BY_SYSTEM}, including but not limited to the captured audio + * can not leave the capturing app, and the quality is limited to 16k mono. + * * The permission {@link CAPTURE_AUDIO_OUTPUT} or {@link CAPTURE_MEDIA_OUTPUT} is needed * to ignore the opt-out. * diff --git a/services/core/java/com/android/server/audio/AudioService.java b/services/core/java/com/android/server/audio/AudioService.java index 2631c23da8e1e..86832f444ff0b 100644 --- a/services/core/java/com/android/server/audio/AudioService.java +++ b/services/core/java/com/android/server/audio/AudioService.java @@ -6761,6 +6761,12 @@ public class AudioService extends IAudioService.Stub if (mix.getRule().allowPrivilegedPlaybackCapture()) { // then it must have CAPTURE_MEDIA_OUTPUT or CAPTURE_AUDIO_OUTPUT permission requireCaptureAudioOrMediaOutputPerm |= true; + // and its format must be low quality enough + String error = mix.canBeUsedForPrivilegedCapture(mix.getFormat()); + if (error != null) { + Log.e(TAG, error); + return false; + } } // If mix is RENDER|LOOPBACK, then an audio MediaProjection is enough