Resolve uid / pid early during connection
This CL further clusters identity resolution in a single place in each
of connect / connectDevice / initializeShimMetadata. This will mean that
checking the validity of the provided AttributionSource can occur in
the same place as legacy caller identity resolution, and one flag check
will be needed in each caller to connectHelper.
Flag: EXEMPT Strict mechanical refactor
Bug: 190657833
Test: atest CtsCameraTestCases, Camera1 + Camera2 apps
Change-Id: I2ba8e78e7419e956151ed54d6a2a88f7946774ce
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index f94300e..fb6a8a7 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -1612,11 +1612,12 @@
Status ret = Status::ok();
sp<Client> tmp = nullptr;
- logConnectionAttempt(getCallingPid(), kServiceName, cameraIdStr, API_1);
+ int callingPid = getCallingPid();
+ logConnectionAttempt(callingPid, kServiceName, cameraIdStr, API_1);
if (!(ret = connectHelper<ICameraClient,Client>(
sp<ICameraClient>{nullptr}, cameraIdStr, cameraId,
- kServiceName, /*systemNativeClient*/ false, {}, uid, USE_CALLING_PID,
+ kServiceName, /*systemNativeClient*/ false, {}, uid, callingPid,
API_1, /*shimUpdateOnly*/ true, /*oomScoreOffset*/ 0,
/*targetSdkVersion*/ __ANDROID_API_FUTURE__,
/*rotationOverride*/hardware::ICameraService::ROTATION_OVERRIDE_OVERRIDE_TO_PORTRAIT,
@@ -1690,7 +1691,7 @@
}
Status CameraService::validateConnectLocked(const std::string& cameraId,
- const std::string& clientName8, /*inout*/int& clientUid, /*inout*/int& clientPid) const {
+ const std::string& clientName8, int clientUid, int clientPid) const {
#ifdef __BRILLO__
UNUSED(clientName8);
@@ -1734,36 +1735,24 @@
return Status::ok();
}
-Status CameraService::validateClientPermissionsLocked(const std::string& cameraId,
- const std::string& clientName, int& clientUid, int& clientPid) const {
+Status CameraService::errorNotTrusted(int clientPid, int clientUid, const std::string& cameraId,
+ const std::string& clientName, bool isPid) const {
int callingPid = getCallingPid();
int callingUid = getCallingUid();
+ ALOGE("CameraService::connect X (calling PID %d, calling UID %d) rejected "
+ "(don't trust %s %d)", callingPid, callingUid, isPid ? "clientPid" : "clientUid",
+ isPid ? clientPid : clientUid);
+ return STATUS_ERROR_FMT(ERROR_PERMISSION_DENIED,
+ "Untrusted caller (calling PID %d, UID %d) trying to "
+ "forward camera access to camera %s for client %s (PID %d, UID %d)",
+ getCallingPid(), getCallingUid(), cameraId.c_str(),
+ clientName.c_str(), clientPid, clientUid);
+}
- // Check if we can trust clientUid
- if (clientUid == USE_CALLING_UID) {
- 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 STATUS_ERROR_FMT(ERROR_PERMISSION_DENIED,
- "Untrusted caller (calling PID %d, UID %d) trying to "
- "forward camera access to camera %s for client %s (PID %d, UID %d)",
- callingPid, callingUid, cameraId.c_str(),
- clientName.c_str(), clientPid, clientUid);
- }
-
- // 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 STATUS_ERROR_FMT(ERROR_PERMISSION_DENIED,
- "Untrusted caller (calling PID %d, UID %d) trying to "
- "forward camera access to camera %s for client %s (PID %d, UID %d)",
- callingPid, callingUid, cameraId.c_str(),
- clientName.c_str(), clientPid, clientUid);
- }
+Status CameraService::validateClientPermissionsLocked(const std::string& cameraId,
+ const std::string& clientName, int clientUid, int clientPid) const {
+ int callingPid = getCallingPid();
+ int callingUid = getCallingUid();
if (shouldRejectSystemCameraConnection(cameraId)) {
ALOGW("Attempting to connect to system-only camera id %s, connection rejected",
@@ -2156,10 +2145,28 @@
clientPackageNameMaybe);
logConnectionAttempt(clientAttribution.pid, clientPackageName, cameraIdStr, API_1);
+ int clientUid = clientAttribution.uid;
+ int clientPid = clientAttribution.pid;
+
+ // Resolve the client identity. In the near future, we will no longer rely on USE_CALLING_*, and
+ // need a way to guarantee the caller identity early.
+
+ // Check if we can trust clientUid
+ if (!resolveClientUid(clientUid)) {
+ return errorNotTrusted(clientPid, clientUid, cameraIdStr, clientPackageName,
+ /* isPid=*/ false);
+ }
+
+ // Check if we can trust clientUid
+ if (!resolveClientPid(clientPid)) {
+ return errorNotTrusted(clientPid, clientUid, cameraIdStr, clientPackageName,
+ /* isPid= */ true);
+ }
+
sp<Client> client = nullptr;
ret = connectHelper<ICameraClient,Client>(cameraClient, cameraIdStr, api1CameraId,
clientPackageName, /*systemNativeClient*/ false, {},
- clientAttribution.uid, clientAttribution.pid, API_1,
+ clientUid, clientPid, API_1,
/*shimUpdateOnly*/ false, /*oomScoreOffset*/ 0, targetSdkVersion,
rotationOverride, forceSlowJpegMode, cameraIdStr, isNonSystemNdk, /*out*/client);
@@ -2275,6 +2282,22 @@
clientPackageNameMaybe);
logConnectionAttempt(clientAttribution.pid, clientPackageName, cameraId, API_2);
+ userid_t clientUserId = multiuser_get_user_id(clientAttribution.uid);
+ if (clientAttribution.uid == USE_CALLING_UID) {
+ clientUserId = multiuser_get_user_id(callingUid);
+ }
+
+ // Resolve the client identity. In the near future, we will no longer rely on USE_CALLING_*, and
+ // need a way to guarantee the caller identity early.
+
+ int clientUid = clientAttribution.uid;
+ int clientPid = callingPid;
+ // Check if we can trust clientUid
+ if (!resolveClientUid(clientUid)) {
+ return errorNotTrusted(clientPid, clientUid, cameraId, clientPackageName,
+ /* isPid= */ false);
+ }
+
if (oomScoreOffset < 0) {
std::string msg =
fmt::sprintf("Cannot increase the priority of a client %s pid %d for "
@@ -2284,11 +2307,6 @@
return STATUS_ERROR(ERROR_ILLEGAL_ARGUMENT, msg.c_str());
}
- userid_t clientUserId = multiuser_get_user_id(clientAttribution.uid);
- if (clientAttribution.uid == USE_CALLING_UID) {
- clientUserId = multiuser_get_user_id(callingUid);
- }
-
// Automotive privileged client AID_AUTOMOTIVE_EVS using exterior system camera for use cases
// such as rear view and surround view cannot be disabled.
if ((!isAutomotivePrivilegedClient(callingUid) || !isAutomotiveExteriorSystemCamera(cameraId))
@@ -2312,7 +2330,7 @@
ret = connectHelper<hardware::camera2::ICameraDeviceCallbacks,CameraDeviceClient>(cameraCb,
cameraId, /*api1CameraId*/-1, clientPackageName, systemNativeClient,
- clientAttribution.attributionTag, clientAttribution.uid, USE_CALLING_PID, API_2,
+ clientAttribution.attributionTag, clientUid, clientPid, API_2,
/*shimUpdateOnly*/ false, oomScoreOffset, targetSdkVersion, rotationOverride,
/*forceSlowJpegMode*/false, unresolvedCameraId, isNonSystemNdk, /*out*/client);
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index d5c57cb..0ac391d 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -950,14 +950,16 @@
// If clientPid/clientUid are USE_CALLING_PID/USE_CALLING_UID, they will be overwritten with
// the calling pid/uid.
binder::Status validateConnectLocked(const std::string& cameraId, const std::string& clientName,
- /*inout*/int& clientUid, /*inout*/int& clientPid) const;
+ int clientUid, int clientPid) const;
binder::Status validateClientPermissionsLocked(const std::string& cameraId,
- const std::string& clientName, /*inout*/int& clientUid, /*inout*/int& clientPid) const;
+ const std::string& clientName, int clientUid, int clientPid) const;
// If clientPackageNameMaybe is empty, attempts to resolve the package name.
std::string resolvePackageName(int clientUid, const std::string& clientPackageNameMaybe) const;
void logConnectionAttempt(int clientPid, const std::string& clientPackageName,
const std::string& cameraId, apiLevel effectiveApiLevel) const;
+ binder::Status errorNotTrusted(int clientPid, int clientUid, const std::string& cameraId,
+ const std::string& clientName, bool isPid) const;
bool isCameraPrivacyEnabled(const String16& packageName,const std::string& cameraId,
int clientPid, int ClientUid);
diff --git a/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.cpp b/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.cpp
index 93b440b..72f8c4b 100644
--- a/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.cpp
+++ b/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.cpp
@@ -80,6 +80,33 @@
return;
}
+// TODO(362551824): Make USE_CALLING_UID more explicit with a scoped enum.
+bool AttributionAndPermissionUtils::resolveClientUid(/*inout*/ int& clientUid) {
+ int callingUid = getCallingUid();
+
+ if (clientUid == hardware::ICameraService::USE_CALLING_UID) {
+ clientUid = callingUid;
+ } else if (!isTrustedCallingUid(callingUid)) {
+ return false;
+ }
+
+ return true;
+}
+
+// TODO(362551824): Make USE_CALLING_UID more explicit with a scoped enum.
+bool AttributionAndPermissionUtils::resolveClientPid(/*inout*/ int& clientPid) {
+ int callingUid = getCallingUid();
+ int callingPid = getCallingPid();
+
+ if (clientPid == hardware::ICameraService::USE_CALLING_PID) {
+ clientPid = callingPid;
+ } else if (!isTrustedCallingUid(callingUid)) {
+ return false;
+ }
+
+ return true;
+}
+
bool AttributionAndPermissionUtils::checkAutomotivePrivilegedClient(const std::string &cameraId,
const AttributionSourceState &attributionSource) {
if (isAutomotivePrivilegedClient(attributionSource.uid)) {
diff --git a/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.h b/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.h
index 4daab0f..a23fba7 100644
--- a/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.h
+++ b/services/camera/libcameraservice/utils/AttributionAndPermissionUtils.h
@@ -61,6 +61,9 @@
virtual int64_t clearCallingIdentity();
virtual void restoreCallingIdentity(int64_t token);
+ virtual bool resolveClientUid(/*inout*/ int& clientUid);
+ virtual bool resolveClientPid(/*inout*/ int& clientPid);
+
/**
* Pre-grants the permission if the attribution source uid is for an automotive
* privileged client. Otherwise uses system service permission checker to check
@@ -170,6 +173,14 @@
mAttributionAndPermissionUtils->restoreCallingIdentity(token);
}
+ bool resolveClientUid(/*inout*/ int& clientUid) const {
+ return mAttributionAndPermissionUtils->resolveClientUid(clientUid);
+ }
+
+ bool resolveClientPid(/*inout*/ int& clientPid) const {
+ return mAttributionAndPermissionUtils->resolveClientPid(clientPid);
+ }
+
// The word 'System' here does not refer to callers only on the system
// partition. They just need to have an android system uid.
bool callerHasSystemUid() const {