Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit db6ef1eb authored by Atneya Nair's avatar Atneya Nair
Browse files

Fix audio AppOps refcount mismatch 

The state of the appop corresponding to a recording must be consistent
with the APS notion of whether a recording is silenced.

Ensure that when we start recording, we only un-silence the track if the
op is allowed from the appop side. This requires plumbing the exact perm
state through the startRecording API.

Test: atest AudioRecordPermissionTest.java
Bug: 293603271
Flag: EXEMPT security
Change-Id: Iea630bb2bf1b54697ab6a95ee9f65db5cc4d134a
parent ac10f323

media/utils/ServiceUtilities.cpp

+13 −9
Original line number Diff line number Diff line
@@ -41,6 +41,10 @@ 


namespace android {



namespace {

constexpr auto PERMISSION_HARD_DENIED = permission::PermissionChecker::PERMISSION_HARD_DENIED;

}



using content::AttributionSourceState;



static const String16 sAndroidPermissionRecordAudio("android.permission.RECORD_AUDIO");
@@ -115,7 +119,7 @@ std::optional<AttributionSourceState> resolveAttributionSource( 
    return std::optional<AttributionSourceState>{myAttributionSource};

}



    static bool checkRecordingInternal(const AttributionSourceState &attributionSource,

    static int checkRecordingInternal(const AttributionSourceState &attributionSource,

                                       const uint32_t virtualDeviceId,

                                       const String16 &msg, bool start, audio_source_t source) {

    // Okay to not track in app ops as audio server or media server is us and if
@@ -138,15 +142,15 @@ std::optional<AttributionSourceState> resolveAttributionSource( 
    const int32_t attributedOpCode = getOpForSource(source);



    permission::PermissionChecker permissionChecker;

    bool permitted = false;

    int permitted;

    if (start) {

        permitted = (permissionChecker.checkPermissionForStartDataDeliveryFromDatasource(

        permitted = permissionChecker.checkPermissionForStartDataDeliveryFromDatasource(

                sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,

                attributedOpCode) != permission::PermissionChecker::PERMISSION_HARD_DENIED);

                attributedOpCode);

    } else {

        permitted = (permissionChecker.checkPermissionForPreflightFromDatasource(

        permitted = permissionChecker.checkPermissionForPreflightFromDatasource(

                sAndroidPermissionRecordAudio, resolvedAttributionSource.value(), msg,

                attributedOpCode) != permission::PermissionChecker::PERMISSION_HARD_DENIED);

                attributedOpCode);

    }



    return permitted;
@@ -156,17 +160,17 @@ static constexpr int DEVICE_ID_DEFAULT = 0; 


bool recordingAllowed(const AttributionSourceState &attributionSource, audio_source_t source) {

    return checkRecordingInternal(attributionSource, DEVICE_ID_DEFAULT, String16(), /*start*/ false,

                                  source);

                                  source) != PERMISSION_HARD_DENIED;

}



bool recordingAllowed(const AttributionSourceState &attributionSource,

                      const uint32_t virtualDeviceId,

                      audio_source_t source) {

    return checkRecordingInternal(attributionSource, virtualDeviceId,

                                  String16(), /*start*/ false, source);

                                  String16(), /*start*/ false, source) != PERMISSION_HARD_DENIED;

}



bool startRecording(const AttributionSourceState& attributionSource,

int startRecording(const AttributionSourceState& attributionSource,

                    const uint32_t virtualDeviceId,

                    const String16& msg,

                    audio_source_t source) {

media/utils/include/mediautils/ServiceUtilities.h

+1 −1
Original line number Diff line number Diff line
@@ -92,7 +92,7 @@ bool recordingAllowed(const AttributionSourceState& attributionSource, 
bool recordingAllowed(const AttributionSourceState &attributionSource,

                      uint32_t virtualDeviceId,

                      audio_source_t source);

bool startRecording(const AttributionSourceState& attributionSource, uint32_t virtualDeviceId,

int startRecording(const AttributionSourceState& attributionSource, uint32_t virtualDeviceId,

                    const String16& msg, audio_source_t source);

void finishRecording(const AttributionSourceState& attributionSource, uint32_t virtualDeviceId,

                     audio_source_t source);

services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp

+28 −14
Original line number Diff line number Diff line
@@ -86,6 +86,10 @@ using media::audio::common::AudioUuid; 
using media::audio::common::Int;



constexpr int kDefaultVirtualDeviceId = 0;

namespace {

constexpr auto PERMISSION_HARD_DENIED = permission::PermissionChecker::PERMISSION_HARD_DENIED;

constexpr auto PERMISSION_GRANTED = permission::PermissionChecker::PERMISSION_GRANTED;

}



const std::vector<audio_usage_t>& SYSTEM_USAGES = {

    AUDIO_USAGE_CALL_ASSISTANT,
@@ -900,13 +904,13 @@ Status AudioPolicyService::startInput(int32_t portIdAidl) 


    std::stringstream msg;

    msg << "Audio recording on session " << client->session;

    const auto permitted = startRecording(client->attributionSource, client->virtualDeviceId,

            String16(msg.str().c_str()), client->attributes.source);



    // check calling permissions

    if (!(startRecording(client->attributionSource, client->virtualDeviceId,

                         String16(msg.str().c_str()), client->attributes.source)

            || client->attributes.source == AUDIO_SOURCE_FM_TUNER

            || client->attributes.source == AUDIO_SOURCE_REMOTE_SUBMIX

            || client->attributes.source == AUDIO_SOURCE_ECHO_REFERENCE)) {

    if (permitted == PERMISSION_HARD_DENIED && client->attributes.source != AUDIO_SOURCE_FM_TUNER

            && client->attributes.source != AUDIO_SOURCE_REMOTE_SUBMIX

            && client->attributes.source != AUDIO_SOURCE_ECHO_REFERENCE) {

        ALOGE("%s permission denied: recording not allowed for attribution source %s",

                __func__, client->attributionSource.toString().c_str());

        return binderStatusFromStatusT(PERMISSION_DENIED);
@@ -926,13 +930,17 @@ Status AudioPolicyService::startInput(int32_t portIdAidl) 
        return binderStatusFromStatusT(INVALID_OPERATION);

    }



    // Force the possibly silenced client to be unsilenced since we just called

    // startRecording (i.e. we have assumed it is unsilenced).

    // At this point in time, the client is inactive, so no calls to appops are sent in

    // setAppState_l.

    // This ensures existing clients have the same behavior as new clients (starting unsilenced).

    // Force the possibly silenced client to match the state on the appops side

    // following the call to startRecording (i.e. unsilenced iff call succeeded)

    // At this point in time, the client is inactive, so no calls to appops are

    // sent in setAppState_l. This ensures existing clients have the same

    // behavior as new clients.

    // TODO(b/282076713)

    if (permitted == PERMISSION_GRANTED) {

        setAppState_l(client, APP_STATE_TOP);

    } else {

        setAppState_l(client, APP_STATE_IDLE);

    }



    client->active = true;

    client->startTimeNs = systemTime();
@@ -1018,9 +1026,11 @@ Status AudioPolicyService::startInput(int32_t portIdAidl) 
        client->active = false;

        client->startTimeNs = 0;

        updateUidStates_l();

        if (!client->silenced) {

            finishRecording(client->attributionSource, client->virtualDeviceId,

                    client->attributes.source);

        }

    }



    return binderStatusFromStatusT(status);

}
@@ -1048,7 +1058,11 @@ Status AudioPolicyService::stopInput(int32_t portIdAidl) 
    updateUidStates_l();



    // finish the recording app op

    finishRecording(client->attributionSource, client->virtualDeviceId, client->attributes.source);

    if (!client->silenced) {

        finishRecording(client->attributionSource, client->virtualDeviceId,

                client->attributes.source);

    }



    AutoCallerClear acc;

    return binderStatusFromStatusT(mAudioPolicyManager->stopInput(portId));

}

services/audiopolicy/service/AudioPolicyService.cpp

+8 −3
Original line number Diff line number Diff line
@@ -61,6 +61,10 @@ static const nsecs_t kAudioCommandTimeoutNs = seconds(3); // 3 seconds 


static const String16 sManageAudioPolicyPermission("android.permission.MANAGE_AUDIO_POLICY");



namespace {

constexpr auto PERMISSION_GRANTED = permission::PermissionChecker::PERMISSION_GRANTED;

}



// Creates an association between Binder code to name for IAudioPolicyService.

#define IAUDIOPOLICYSERVICE_BINDER_METHOD_MACRO_LIST \

BINDER_METHOD_ENTRY(onNewAudioModulesAvailable) \
@@ -1218,9 +1222,10 @@ void AudioPolicyService::setAppState_l(sp<AudioRecordClient> client, app_state_t 
                } else {

                    std::stringstream msg;

                    msg << "Audio recording un-silenced on session " << client->session;

                    if (!startRecording(client->attributionSource, client->virtualDeviceId,

                                        String16(msg.str().c_str()), client->attributes.source)) {

                        silenced = true;

                    if (startRecording(client->attributionSource, client->virtualDeviceId,

                                String16(msg.str().c_str()), client->attributes.source)

                                != PERMISSION_GRANTED) {

                        return;

                    }

                }

            }