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

Commit 38558e18 authored by Atneya Nair's avatar Atneya Nair
Browse files

Appops refcount fixup 

Fix two issues in Iea630bb2bf1b54697ab6a95ee9f65db5cc4d134a

1. Fix boolean return values in the startRecording method, which
   implicitly converted incorrectly.
2. Correctly handle virtual sources by not deriving their silence state
   from the existing checkPerm method, which checks OP_RECORD/RECORD
   perm. These sources should instead check only their associated op
   state.

For the second, update the appop callback listening/handling as well, by
only following/evaluating OP_RECORD for relevant sources.

Test: CtsMediaAudioPermissionTestCases
Bug: 293603271
Bug: 374800336
Bug: 374869085
Flag: EXEMPT security
Change-Id: I8a3498a08a31543219279cb063c043fd0c767cd7
parent 508c20be

media/utils/ServiceUtilities.cpp

+79 −36
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ 
namespace android {



namespace {

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

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

}
@@ -78,19 +79,32 @@ static String16 resolveCallingPackage(PermissionController& permissionController 


int32_t getOpForSource(audio_source_t source) {

  switch (source) {

    case AUDIO_SOURCE_HOTWORD:

      return AppOpsManager::OP_RECORD_AUDIO_HOTWORD;

    case AUDIO_SOURCE_FM_TUNER:

        return AppOpsManager::OP_NONE;

    case AUDIO_SOURCE_ECHO_REFERENCE: // fallthrough

    case AUDIO_SOURCE_REMOTE_SUBMIX:

      return AppOpsManager::OP_RECORD_AUDIO_OUTPUT;

    case AUDIO_SOURCE_VOICE_DOWNLINK:

      return AppOpsManager::OP_RECORD_INCOMING_PHONE_AUDIO;

    case AUDIO_SOURCE_HOTWORD:

      return AppOpsManager::OP_RECORD_AUDIO_HOTWORD;

    case AUDIO_SOURCE_DEFAULT:

    default:

      return AppOpsManager::OP_RECORD_AUDIO;

  }

}



bool isRecordOpRequired(audio_source_t source) {

  switch (source) {

    case AUDIO_SOURCE_FM_TUNER:

    case AUDIO_SOURCE_ECHO_REFERENCE: // fallthrough

    case AUDIO_SOURCE_REMOTE_SUBMIX:

        return false;

    default:

      return true;

  }

}



std::optional<AttributionSourceState> resolveAttributionSource(

        const AttributionSourceState& callerAttributionSource, const uint32_t virtualDeviceId) {

    AttributionSourceState nextAttributionSource = callerAttributionSource;
@@ -122,6 +136,7 @@ std::optional<AttributionSourceState> resolveAttributionSource( 
    return std::optional<AttributionSourceState>{myAttributionSource};

}





static int checkRecordingInternal(const AttributionSourceState &attributionSource,

                                       const uint32_t virtualDeviceId,

                                       const String16 &msg, bool start, audio_source_t source) {
@@ -131,19 +146,19 @@ std::optional<AttributionSourceState> resolveAttributionSource( 
    // user is active, but it is a core system service so let it through.

    // TODO(b/141210120): UserManager.DISALLOW_RECORD_AUDIO should not affect system user 0

    uid_t uid = VALUE_OR_FATAL(aidl2legacy_int32_t_uid_t(attributionSource.uid));

    if (isAudioServerOrMediaServerOrSystemServerOrRootUid(uid)) return true;

    if (isAudioServerOrMediaServerOrSystemServerOrRootUid(uid)) return PERMISSION_GRANTED;



    const int32_t attributedOpCode = getOpForSource(source);

    if (isRecordOpRequired(source)) {

        // We specify a pid and uid here as mediaserver (aka MediaRecorder or StageFrightRecorder)

        // may open a record track on behalf of a client. Note that pid may be a tid.

        // IMPORTANT: DON'T USE PermissionCache - RUNTIME PERMISSIONS CHANGE.

        std::optional<AttributionSourceState> resolvedAttributionSource =

                resolveAttributionSource(attributionSource, virtualDeviceId);

        if (!resolvedAttributionSource.has_value()) {

        return false;

            return PERMISSION_HARD_DENIED;

        }



    const int32_t attributedOpCode = getOpForSource(source);



        permission::PermissionChecker permissionChecker;

        int permitted;

        if (start) {
@@ -157,6 +172,21 @@ std::optional<AttributionSourceState> resolveAttributionSource( 
        }



        return permitted;

    } else {

        if (attributedOpCode == AppOpsManager::OP_NONE) return PERMISSION_GRANTED;  // nothing to do

        AppOpsManager ap{};

        PermissionController pc{};

        return ap.startOpNoThrow(

                attributedOpCode, attributionSource.uid,

                resolveCallingPackage(pc,

                                      String16{attributionSource.packageName.value_or("").c_str()},

                                      attributionSource.uid),

                false,

                attributionSource.attributionTag.has_value()

                        ? String16{attributionSource.attributionTag.value().c_str()}

                        : String16{},

                msg);

    }

}



static constexpr int DEVICE_ID_DEFAULT = 0;
@@ -188,6 +218,8 @@ void finishRecording(const AttributionSourceState &attributionSource, uint32_t v 
    uid_t uid = VALUE_OR_FATAL(aidl2legacy_int32_t_uid_t(attributionSource.uid));

    if (isAudioServerOrMediaServerOrSystemServerOrRootUid(uid)) return;



    const int32_t attributedOpCode = getOpForSource(source);

    if (isRecordOpRequired(source)) {

        // We specify a pid and uid here as mediaserver (aka MediaRecorder or StageFrightRecorder)

        // may open a record track on behalf of a client. Note that pid may be a tid.

        // IMPORTANT: DON'T USE PermissionCache - RUNTIME PERMISSIONS CHANGE.
@@ -197,10 +229,21 @@ void finishRecording(const AttributionSourceState &attributionSource, uint32_t v 
            return;

        }



    const int32_t attributedOpCode = getOpForSource(source);

        permission::PermissionChecker permissionChecker;

        permissionChecker.finishDataDeliveryFromDatasource(attributedOpCode,

                resolvedAttributionSource.value());

    } else {

        if (attributedOpCode == AppOpsManager::OP_NONE) return;  // nothing to do

        AppOpsManager ap{};

        PermissionController pc{};

        ap.finishOp(attributedOpCode, attributionSource.uid,

                    resolveCallingPackage(

                            pc, String16{attributionSource.packageName.value_or("").c_str()},

                            attributionSource.uid),

                    attributionSource.attributionTag.has_value()

                            ? String16{attributionSource.attributionTag.value().c_str()}

                            : String16{});

    }

}



bool captureAudioOutputAllowed(const AttributionSourceState& attributionSource) {

media/utils/include/mediautils/ServiceUtilities.h

+1 −0
Original line number Diff line number Diff line
@@ -118,6 +118,7 @@ bool mustAnonymizeBluetoothAddress( 
        const AttributionSourceState& attributionSource, const String16& caller);

void anonymizeBluetoothAddress(char *address);



bool isRecordOpRequired(audio_source_t source);

int32_t getOpForSource(audio_source_t source);



AttributionSourceState getCallingAttributionSource();

services/audiopolicy/service/AudioPolicyInterfaceImpl.cpp

+2 −3
Original line number Diff line number Diff line
@@ -909,13 +909,12 @@ 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 (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) {

    if (permitted == PERMISSION_HARD_DENIED) {

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

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

        return binderStatusFromStatusT(PERMISSION_DENIED);

services/audiopolicy/service/AudioRecordClient.cpp

+8 −3
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 
#include "AudioRecordClient.h"

#include "AudioPolicyService.h"

#include "binder/AppOpsManager.h"

#include "mediautils/ServiceUtilities.h"

#include <android_media_audiopolicy.h>



#include <algorithm>
@@ -118,16 +119,20 @@ OpRecordAudioMonitor::createIfNeeded( 
    }



    return new OpRecordAudioMonitor(attributionSource, virtualDeviceId, attr,

                                    getOpForSource(attr.source), commandThread);

                                    getOpForSource(attr.source),

                                    isRecordOpRequired(attr.source),

                                    commandThread);

}



OpRecordAudioMonitor::OpRecordAudioMonitor(

        const AttributionSourceState &attributionSource,

        const uint32_t virtualDeviceId, const audio_attributes_t &attr,

        int32_t appOp,

        bool shouldMonitorRecord,

        wp<AudioPolicyService::AudioCommandThread> commandThread) :

        mHasOp(true), mAttributionSource(attributionSource),

        mVirtualDeviceId(virtualDeviceId), mAttr(attr), mAppOp(appOp),

        mShouldMonitorRecord(shouldMonitorRecord),

        mCommandThread(commandThread) {

}
@@ -160,7 +165,7 @@ void OpRecordAudioMonitor::onFirstRef() 
                      });

    };

    reg(mAppOp);

    if (mAppOp != AppOpsManager::OP_RECORD_AUDIO) {

    if (mAppOp != AppOpsManager::OP_RECORD_AUDIO && mShouldMonitorRecord) {

        reg(AppOpsManager::OP_RECORD_AUDIO);

    }

}
@@ -186,7 +191,7 @@ void OpRecordAudioMonitor::checkOp(bool updateUidStates) { 
                });

    };

    bool hasIt = check(mAppOp);

    if (mAppOp != AppOpsManager::OP_RECORD_AUDIO) {

    if (mAppOp != AppOpsManager::OP_RECORD_AUDIO && mShouldMonitorRecord) {

        hasIt = hasIt && check(AppOpsManager::OP_RECORD_AUDIO);

    }

services/audiopolicy/service/AudioRecordClient.h

+2 −0
Original line number Diff line number Diff line
@@ -47,6 +47,7 @@ private: 
                         uint32_t virtualDeviceId,

                         const audio_attributes_t &attr,

                         int32_t appOp,

                         bool shouldMonitorRecord,

                         wp<AudioPolicyService::AudioCommandThread> commandThread);



    void onFirstRef() override;
@@ -74,6 +75,7 @@ private: 
    const uint32_t mVirtualDeviceId;

    const audio_attributes_t mAttr;

    const int32_t mAppOp;

    const bool mShouldMonitorRecord;

    wp<AudioPolicyService::AudioCommandThread> mCommandThread;

};