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

Commit d55533c7 authored by Devin Moore's avatar Devin Moore Committed by Android (Google) Code Review
Browse files

Merge "Address AServiceManager_checkServiceAccess API feedback" into main

parents 29f1102a be9603f7
Loading
Loading
Loading
Loading
+28 −3
Original line number Diff line number Diff line
@@ -50,6 +50,28 @@ enum AServiceManager_AddServiceFlag : uint32_t {
    // All other bits are reserved for internal usage
};

/**
 * These are the different SELinux permissions that processes need to have for
 * different operations with servicemanager.
 */
enum AServiceManager_PermissionType : uint32_t {
    /**
     * Permission for a process to "find" this service through ServiceManager
     * APIs like AServiceManager_getService or AServiceManager_waitForService
     */
    CHECK_ACCESS_PERMISSION_FIND,
    /**
     * Permission for a process to "list" services with
     * libbinder's IServiceManager::listServices
     */
    CHECK_ACCESS_PERMISSION_LIST,
    /**
     * Permission for a process to "add", or register, a service with
     * servicemanager through AServiceManager_addService
     */
    CHECK_ACCESS_PERMISSION_ADD,
};

/**
 * This registers the service with the default service manager under this instance name. This does
 * not take ownership of binder.
@@ -329,6 +351,8 @@ void AServiceManager_reRegister() __INTRODUCED_IN(31);
 *
 * This is useful when a process will be making calls to servicemanager on behalf of another
 * process (callerCtx).
 * The direct caller of this function also needs to have the permissions it is
 * checking on behalf of the other process.
 *
 * \param caller_sid - UTF-8 encoded string. SELinux context of the process that is being checked.
 * \param caller_debug_pid - PID of the process that is being checked. This can
@@ -338,8 +362,8 @@ void AServiceManager_reRegister() __INTRODUCED_IN(31);
 *                     only uses this for logging denials for better debugging.
 * \param instance - UTF-8 encoded string. Instance name of the service that the caller
 *                   wants to interact with.
 * \param permission - UTF-8 encoded string. The servicemanager SELinux permission that the process
 *                     is interested in for the service. This is either "find", "list", or "add".
 * \param permission - The servicemanager SELinux permission that the process
 *                     is interested in for the service.
 *
 * \return True if the process with `caller_sid` has the SELinux `permission`
 *         for the given service `instance`. False if it does not have
@@ -347,6 +371,7 @@ void AServiceManager_reRegister() __INTRODUCED_IN(31);
 */
bool AServiceManager_checkServiceAccess(const char* caller_sid, pid_t caller_debug_pid,
                                        uid_t caller_uid, const char* instance,
                                        const char* permission) __INTRODUCED_IN(37);
                                        AServiceManager_PermissionType permission)
        __INTRODUCED_IN(37);

__END_DECLS
+16 −3
Original line number Diff line number Diff line
@@ -253,12 +253,25 @@ void AServiceManager_reRegister() {

bool AServiceManager_checkServiceAccess(const char* caller_sid, pid_t caller_debug_pid,
                                        uid_t caller_uid, const char* instance,
                                        const char* permission) {
                                        AServiceManager_PermissionType permission) {
    LOG_ALWAYS_FATAL_IF(caller_sid == nullptr, "caller_sid == nullptr");
    LOG_ALWAYS_FATAL_IF(instance == nullptr, "instance == nullptr");
    LOG_ALWAYS_FATAL_IF(permission == nullptr, "permission == nullptr");
    String16 permissionString;
    switch (permission) {
        case AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_FIND:
            permissionString = String16("find");
            break;
        case AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_LIST:
            permissionString = String16("list");
            break;
        case AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_ADD:
            permissionString = String16("add");
            break;
        default:
            LOG_ALWAYS_FATAL("Unknown value for permission argument! permission: %d", permission);
    }

    sp<IServiceManager> sm = defaultServiceManager();
    return sm->checkServiceAccess(String16(caller_sid), caller_debug_pid, caller_uid,
                                  String16(instance), String16(permission));
                                  String16(instance), permissionString);
}
+22 −8
Original line number Diff line number Diff line
@@ -1087,21 +1087,35 @@ TEST(NdkBinder, GetClassInterfaceDescriptor) {

TEST(NdkBinder, CheckServiceAccessOk) {
    // This test case runs as su which has access to all services
    EXPECT_TRUE(AServiceManager_checkServiceAccess("u:r:su:s0", 0, 0, "adb", "find"));
    EXPECT_TRUE(AServiceManager_checkServiceAccess(
            "u:r:su:s0", 0, 0, "adb",
            AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_FIND));
    EXPECT_TRUE(AServiceManager_checkServiceAccess(
            "u:r:su:s0", 0, 0, "adb",
            AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_LIST));
    EXPECT_TRUE(AServiceManager_checkServiceAccess(
            "u:r:su:s0", 0, 0, "adb", AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_ADD));
}

TEST(NdkBinder, CheckServiceAccessNotOk) {
    EXPECT_FALSE(
            AServiceManager_checkServiceAccess("u:r:some_unknown_sid:s0", 0, 0, "adb", "find"));
    EXPECT_FALSE(AServiceManager_checkServiceAccess(
            "u:r:some_unknown_sid:s0", 0, 0, "adb",
            AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_FIND));
}

TEST(NdkBinder, InvalidCheckServiceAccessArgs) {
    EXPECT_DEATH(AServiceManager_checkServiceAccess(nullptr, 0, 0, nullptr, nullptr), "nullptr");
    EXPECT_DEATH(AServiceManager_checkServiceAccess("u:r:su:s0", 0, 0, nullptr, nullptr),
    constexpr AServiceManager_PermissionType kUnknownPermission =
            static_cast<AServiceManager_PermissionType>(8000);
    EXPECT_DEATH(AServiceManager_checkServiceAccess(nullptr, 0, 0, nullptr, kUnknownPermission),
                 "nullptr");
    EXPECT_DEATH(AServiceManager_checkServiceAccess("u:r:su:s0", 0, 0, nullptr, kUnknownPermission),
                 "nullptr");
    EXPECT_DEATH(AServiceManager_checkServiceAccess("u:r:su:s0", 0, 0, "adb", kUnknownPermission),
                 "Unknown value for permission argument! permission: 8000");
    EXPECT_DEATH(AServiceManager_checkServiceAccess(
                         "u:r:su:s0", 0, 0, nullptr,
                         AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_FIND),
                 "nullptr");
    EXPECT_DEATH(AServiceManager_checkServiceAccess("u:r:su:s0", 0, 0, "adb", nullptr), "nullptr");
    EXPECT_DEATH(AServiceManager_checkServiceAccess("u:r:su:s0", 0, 0, nullptr, "find"), "nullptr");
    EXPECT_FALSE(AServiceManager_checkServiceAccess("u:r:su:s0", 0, 0, "adb", "unknown"));
}

static void addOne(int* to) {
+4 −1
Original line number Diff line number Diff line
@@ -127,7 +127,10 @@ pub use service::{
// TODO(b/402766978) Once LLDNK symbols are supported in rust, this can be along with the rest
// of the service symbols in vendor variants.
#[cfg(not(any(trusty, android_ndk, android_vendor, android_vndk)))]
pub use service::check_service_access;
pub use service::{
    check_service_access, CHECK_ACCESS_PERMISSION_ADD, CHECK_ACCESS_PERMISSION_FIND,
    CHECK_ACCESS_PERMISSION_LIST,
};

#[cfg(not(any(trusty, android_ndk)))]
#[allow(deprecated)]
+34 −4
Original line number Diff line number Diff line
@@ -26,6 +26,21 @@ use std::ffi::{c_void, CStr, CString};
use std::os::raw::c_char;
use std::sync::Mutex;

/// Value to use with check_service_access for permission to "find" a service
#[cfg(not(any(trusty, android_ndk, android_vendor, android_vndk)))]
pub const CHECK_ACCESS_PERMISSION_FIND: u32 =
    sys::AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_FIND as u32;

/// Value to use with check_service_access for permission to "list" services
#[cfg(not(any(trusty, android_ndk, android_vendor, android_vndk)))]
pub const CHECK_ACCESS_PERMISSION_LIST: u32 =
    sys::AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_LIST as u32;

/// Value to use with check_service_access for permission to "add" a service
#[cfg(not(any(trusty, android_ndk, android_vendor, android_vndk)))]
pub const CHECK_ACCESS_PERMISSION_ADD: u32 =
    sys::AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_ADD as u32;

/// Register a new service with the default service manager.
///
/// Registers the given binder object with the given identifier. If successful,
@@ -260,7 +275,10 @@ pub fn get_declared_instances(interface: &str) -> Result<Vec<String>> {
/// callerUid - UID process that is being checked. Used for logging denials
/// name - name of the service that the caller wants to interact with
/// permission - the servicemanager SELinux permission that the process is
///              interested in for the service. This is either "find", "list", or "add".
///              interested in for the service. This is one of:
///                 CHECK_ACCESS_PERMISSION_FIND,
///                 CHECK_ACCESS_PERMISSION_LIST,
///                 CHECK_ACCESS_PERMISSION_ADD
// TODO(b/402766978) Add this back into vendor variants when the LLNDK symbols are supported
// with something like __builtin_available
#[cfg(not(any(trusty, android_ndk, android_vendor, android_vndk)))]
@@ -269,11 +287,23 @@ pub fn check_service_access(
    caller_debug_pid: pid_t,
    caller_uid: uid_t,
    name: &str,
    permission: &str,
    permission: u32,
) -> Result<bool> {
    let caller_sid = CString::new(caller_sid).or(Err(StatusCode::UNEXPECTED_NULL))?;
    let name = CString::new(name).or(Err(StatusCode::UNEXPECTED_NULL))?;
    let permission = CString::new(permission).or(Err(StatusCode::UNEXPECTED_NULL))?;
    let permission = match permission {
        CHECK_ACCESS_PERMISSION_FIND => {
            sys::AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_FIND
        }
        CHECK_ACCESS_PERMISSION_LIST => {
            sys::AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_LIST
        }
        CHECK_ACCESS_PERMISSION_ADD => {
            sys::AServiceManager_PermissionType::CHECK_ACCESS_PERMISSION_ADD
        }
        _ => return Err(StatusCode::BAD_TYPE),
    };

    // Safety: The CStrings are valid at this point and are only used during the duration
    // of the call.
    unsafe {
@@ -282,7 +312,7 @@ pub fn check_service_access(
            caller_debug_pid,
            caller_uid,
            name.as_ptr(),
            permission.as_ptr(),
            permission,
        ))
    }
}