Camera: Fix client permission check
Modify StageFright's CameraSource to forward calling PID as
client PID when connecting to CameraService so CameraService
can check if the client PID has permission to use camera.
Change CameraService to check calling UID is trusted before
using the passed in client PID and client UID to verify permission.
Bug: 24511454
Change-Id: I4906ab73510e2c75714690bed675e3c13aca3ccf
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 4be2c92..be66ef8 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -46,6 +46,7 @@
#include <utils/Log.h>
#include <utils/String16.h>
#include <utils/Trace.h>
+#include <private/android_filesystem_config.h>
#include <system/camera_vendor_tags.h>
#include <system/camera_metadata.h>
#include <system/camera.h>
@@ -779,13 +780,13 @@
status_t CameraService::initializeShimMetadata(int cameraId) {
int uid = getCallingUid();
- String16 internalPackageName("media");
+ String16 internalPackageName("cameraserver");
String8 id = String8::format("%d", cameraId);
status_t ret = NO_ERROR;
sp<Client> tmp = nullptr;
if ((ret = connectHelper<ICameraClient,Client>(sp<ICameraClient>{nullptr}, id,
- static_cast<int>(CAMERA_HAL_API_VERSION_UNSPECIFIED), internalPackageName, uid, API_1,
- false, true, tmp)) != NO_ERROR) {
+ static_cast<int>(CAMERA_HAL_API_VERSION_UNSPECIFIED), internalPackageName, uid,
+ USE_CALLING_PID, API_1, false, true, tmp)) != NO_ERROR) {
ALOGE("%s: Error %d (%s) initializing shim metadata.", __FUNCTION__, ret, strerror(ret));
return ret;
}
@@ -852,22 +853,52 @@
return INVALID_OPERATION;
}
-status_t CameraService::validateConnectLocked(const String8& cameraId, /*inout*/int& clientUid)
- const {
+// Can camera service trust the caller based on the calling UID?
+static bool isTrustedCallingUid(uid_t uid) {
+ switch (uid) {
+ case AID_MEDIA: // mediaserver
+ case AID_CAMERASERVER: // cameraserver
+ return true;
+ default:
+ return false;
+ }
+}
+
+status_t CameraService::validateConnectLocked(const String8& cameraId, /*inout*/int& clientUid,
+ /*inout*/int& clientPid) const {
int callingPid = getCallingPid();
+ int callingUid = getCallingUid();
+ // Check if we can trust clientUid
if (clientUid == USE_CALLING_UID) {
- clientUid = getCallingUid();
- } else {
- // We only trust our own process to forward client UIDs
- if (callingPid != getpid()) {
- ALOGE("CameraService::connect X (PID %d) rejected (don't trust clientUid %d)",
- callingPid, clientUid);
- return PERMISSION_DENIED;
- }
+ clientUid = callingUid;
+ } else if (!isTrustedCallingUid(callingUid)) {
+ ALOGE("CameraService::connect X (calling PID %d, calling UID %d) rejected "
+ "(don't trust clientUid %d)", callingPid, callingUid, clientUid);
+ return PERMISSION_DENIED;
}
+ // Check if we can trust clientPid
+ if (clientPid == USE_CALLING_PID) {
+ clientPid = callingPid;
+ } else if (!isTrustedCallingUid(callingUid)) {
+ ALOGE("CameraService::connect X (calling PID %d, calling UID %d) rejected "
+ "(don't trust clientPid %d)", callingPid, callingUid, clientPid);
+ return PERMISSION_DENIED;
+ }
+
+ // If it's not calling from cameraserver, check the permission.
+ if (callingPid != getpid() &&
+ !checkPermission(String16("android.permission.CAMERA"), clientPid, clientUid)) {
+ ALOGE("Permission Denial: can't use the camera pid=%d, uid=%d", clientPid, clientUid);
+ return PERMISSION_DENIED;
+ }
+
+ // Only use passed in clientPid to check permission. Use calling PID as the client PID that's
+ // connected to camera service directly.
+ clientPid = callingPid;
+
if (!mModule) {
ALOGE("CameraService::connect X (PID %d) rejected (camera HAL module not loaded)",
callingPid);
@@ -1140,6 +1171,7 @@
int cameraId,
const String16& clientPackageName,
int clientUid,
+ int clientPid,
/*out*/
sp<ICamera>& device) {
@@ -1148,7 +1180,7 @@
String8 id = String8::format("%d", cameraId);
sp<Client> client = nullptr;
ret = connectHelper<ICameraClient,Client>(cameraClient, id, CAMERA_HAL_API_VERSION_UNSPECIFIED,
- clientPackageName, clientUid, API_1, false, false, /*out*/client);
+ clientPackageName, clientUid, clientPid, API_1, false, false, /*out*/client);
if(ret != NO_ERROR) {
logRejected(id, getCallingPid(), String8(clientPackageName),
@@ -1189,7 +1221,7 @@
status_t ret = NO_ERROR;
sp<Client> client = nullptr;
ret = connectHelper<ICameraClient,Client>(cameraClient, id, halVersion, clientPackageName,
- clientUid, API_1, true, false, /*out*/client);
+ clientUid, USE_CALLING_PID, API_1, true, false, /*out*/client);
if(ret != NO_ERROR) {
logRejected(id, getCallingPid(), String8(clientPackageName),
@@ -1214,8 +1246,8 @@
String8 id = String8::format("%d", cameraId);
sp<CameraDeviceClient> client = nullptr;
ret = connectHelper<ICameraDeviceCallbacks,CameraDeviceClient>(cameraCb, id,
- CAMERA_HAL_API_VERSION_UNSPECIFIED, clientPackageName, clientUid, API_2, false, false,
- /*out*/client);
+ CAMERA_HAL_API_VERSION_UNSPECIFIED, clientPackageName, clientUid, USE_CALLING_PID,
+ API_2, false, false, /*out*/client);
if(ret != NO_ERROR) {
logRejected(id, getCallingPid(), String8(clientPackageName),
@@ -1781,21 +1813,6 @@
// Permission checks
switch (code) {
- case BnCameraService::CONNECT:
- case BnCameraService::CONNECT_DEVICE:
- case BnCameraService::CONNECT_LEGACY: {
- if (pid != selfPid) {
- // we're called from a different process, do the real check
- if (!checkCallingPermission(
- String16("android.permission.CAMERA"))) {
- const int uid = getCallingUid();
- ALOGE("Permission Denial: "
- "can't use the camera pid=%d, uid=%d", pid, uid);
- return PERMISSION_DENIED;
- }
- }
- break;
- }
case BnCameraService::NOTIFY_SYSTEM_EVENT: {
if (pid != selfPid) {
// Ensure we're being called by system_server, or similar process with
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index 5877c65..aa1e1e8 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -116,7 +116,7 @@
virtual status_t getCameraVendorTagDescriptor(/*out*/ sp<VendorTagDescriptor>& desc);
virtual status_t connect(const sp<ICameraClient>& cameraClient, int cameraId,
- const String16& clientPackageName, int clientUid,
+ const String16& clientPackageName, int clientUid, int clientPid,
/*out*/
sp<ICamera>& device);
@@ -488,7 +488,8 @@
virtual void onFirstRef();
// Check if we can connect, before we acquire the service lock.
- status_t validateConnectLocked(const String8& cameraId, /*inout*/int& clientUid) const;
+ status_t validateConnectLocked(const String8& cameraId, /*inout*/int& clientUid,
+ /*inout*/int& clientPid) const;
// Handle active client evictions, and update service state.
// Only call with with mServiceLock held.
@@ -501,8 +502,9 @@
// Single implementation shared between the various connect calls
template<class CALLBACK, class CLIENT>
status_t connectHelper(const sp<CALLBACK>& cameraCb, const String8& cameraId, int halVersion,
- const String16& clientPackageName, int clientUid, apiLevel effectiveApiLevel,
- bool legacyMode, bool shimUpdateOnly, /*out*/sp<CLIENT>& device);
+ const String16& clientPackageName, int clientUid, int clientPid,
+ apiLevel effectiveApiLevel, bool legacyMode, bool shimUpdateOnly,
+ /*out*/sp<CLIENT>& device);
// Lock guarding camera service state
Mutex mServiceLock;
@@ -801,12 +803,11 @@
template<class CALLBACK, class CLIENT>
status_t CameraService::connectHelper(const sp<CALLBACK>& cameraCb, const String8& cameraId,
- int halVersion, const String16& clientPackageName, int clientUid,
+ int halVersion, const String16& clientPackageName, int clientUid, int clientPid,
apiLevel effectiveApiLevel, bool legacyMode, bool shimUpdateOnly,
/*out*/sp<CLIENT>& device) {
status_t ret = NO_ERROR;
String8 clientName8(clientPackageName);
- int clientPid = getCallingPid();
ALOGI("CameraService::connect call (PID %d \"%s\", camera ID %s) for HAL version %s and "
"Camera API version %d", clientPid, clientName8.string(), cameraId.string(),
@@ -826,7 +827,8 @@
}
// Enforce client permissions and do basic sanity checks
- if((ret = validateConnectLocked(cameraId, /*inout*/clientUid)) != NO_ERROR) {
+ if((ret = validateConnectLocked(cameraId, /*inout*/clientUid, /*inout*/clientPid)) !=
+ NO_ERROR) {
return ret;
}
diff --git a/services/camera/libcameraservice/api1/Camera2Client.cpp b/services/camera/libcameraservice/api1/Camera2Client.cpp
index c17fc65..5ac5743 100644
--- a/services/camera/libcameraservice/api1/Camera2Client.cpp
+++ b/services/camera/libcameraservice/api1/Camera2Client.cpp
@@ -371,7 +371,7 @@
ATRACE_CALL();
Mutex::Autolock icl(mBinderSerializationLock);
- // Allow both client and the media server to disconnect at all times
+ // Allow both client and the cameraserver to disconnect at all times
int callingPid = getCallingPid();
if (callingPid != mClientPid && callingPid != mServicePid) return;
@@ -1233,6 +1233,7 @@
}
void Camera2Client::releaseRecordingFrame(const sp<IMemory>& mem) {
+ (void)mem;
ATRACE_CALL();
ALOGW("%s: Not supported in buffer queue mode.", __FUNCTION__);
}
diff --git a/services/camera/libcameraservice/api1/CameraClient.cpp b/services/camera/libcameraservice/api1/CameraClient.cpp
index cba4590..6aeab98 100644
--- a/services/camera/libcameraservice/api1/CameraClient.cpp
+++ b/services/camera/libcameraservice/api1/CameraClient.cpp
@@ -234,7 +234,7 @@
LOG1("disconnect E (pid %d)", callingPid);
Mutex::Autolock lock(mLock);
- // Allow both client and the media server to disconnect at all times
+ // Allow both client and the cameraserver to disconnect at all times
if (callingPid != mClientPid && callingPid != mServicePid) {
ALOGW("different client - don't disconnect");
return;
@@ -1001,6 +1001,7 @@
}
status_t CameraClient::setVideoTarget(const sp<IGraphicBufferProducer>& bufferProducer) {
+ (void)bufferProducer;
ALOGE("%s: %d: CameraClient doesn't support setting a video target.", __FUNCTION__, __LINE__);
return INVALID_OPERATION;
}