cameraservice: Copy camera characteristics from session characteristics
The getSessionCharacteristics call is meant to replace the output of
getCameraCharacteristics. The current implementation however passed
the output of HAL directly to the application, which could be missing
quite a few keys.
This CL uses the camera device's characteristics as the base
and copies over any updated values from the HAL instead. This will
ensure the output of getSessionCharacteristics matches the constraints
of getCameraCharacteristics.
Similarly, this CL also filters out any sensitive keys if
getSessionCharactersitics is called from a process that does not have
camera permission granted.
Bug: 303645857
Test: atest android.hardware.camera2.cts.CameraDeviceSetupTest passes
Test: atest android.hardware.camera2.cts.StaticMetadataTest passes
Change-Id: I589c80fc6152ae3cb14c72963953feb94a7f745a
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 20ea5bb..d65fa8b 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -991,12 +991,6 @@
/*out*/ CameraMetadata* outMetadata) {
ATRACE_CALL();
- if (!mInitialized) {
- ALOGE("%s: Camera HAL couldn't be initialized", __FUNCTION__);
- logServiceError("Camera subsystem is not available", ERROR_DISCONNECTED);
- return STATUS_ERROR(ERROR_DISCONNECTED, "Camera subsystem is not available");
- }
-
if (outMetadata == nullptr) {
std::string msg =
fmt::sprintf("Camera %s: Invalid 'outMetadata' input!", unresolvedCameraId.c_str());
@@ -1004,26 +998,35 @@
return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.c_str());
}
+ if (!mInitialized) {
+ ALOGE("%s: Camera HAL couldn't be initialized", __FUNCTION__);
+ logServiceError("Camera subsystem is not available", ERROR_DISCONNECTED);
+ return STATUS_ERROR(ERROR_DISCONNECTED, "Camera subsystem is not available");
+ }
+
std::optional<std::string> cameraIdOptional = resolveCameraId(unresolvedCameraId, deviceId,
- devicePolicy, getCallingUid());
+ devicePolicy, getCallingUid());
if (!cameraIdOptional.has_value()) {
std::string msg = fmt::sprintf("Camera %s: Invalid camera id for device id %d",
- unresolvedCameraId.c_str(), deviceId);
+ unresolvedCameraId.c_str(), deviceId);
ALOGE("%s: %s", __FUNCTION__, msg.c_str());
return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.c_str());
}
std::string cameraId = cameraIdOptional.value();
+ if (shouldRejectSystemCameraConnection(cameraId)) {
+ return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION,
+ "Unable to retrieve camera"
+ "characteristics for system only device %s: ",
+ cameraId.c_str());
+ }
+
bool overrideForPerfClass = SessionConfigurationUtils::targetPerfClassPrimaryCamera(
mPerfClassPrimaryCameraIds, cameraId, targetSdkVersion);
status_t ret = mCameraProviderManager->getSessionCharacteristics(
cameraId, sessionConfiguration, overrideForPerfClass, overrideToPortrait, outMetadata);
- // TODO(b/303645857): Remove fingerprintable metadata if the caller process does not have
- // camera access permission.
-
- Status res = Status::ok();
switch (ret) {
case OK:
// Expected, no handling needed.
@@ -1033,18 +1036,90 @@
"Camera %s: Session characteristics query not supported!",
cameraId.c_str());
ALOGD("%s: %s", __FUNCTION__, msg.c_str());
- res = STATUS_ERROR(CameraService::ERROR_INVALID_OPERATION, msg.c_str());
+ logServiceError(msg, CameraService::ERROR_INVALID_OPERATION);
+ outMetadata->clear();
+ return STATUS_ERROR(CameraService::ERROR_INVALID_OPERATION, msg.c_str());
+ }
+ break;
+ case NAME_NOT_FOUND: {
+ std::string msg = fmt::sprintf(
+ "Camera %s: Unknown camera ID.",
+ cameraId.c_str());
+ ALOGD("%s: %s", __FUNCTION__, msg.c_str());
+ logServiceError(msg, CameraService::ERROR_ILLEGAL_ARGUMENT);
+ outMetadata->clear();
+ return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.c_str());
}
break;
default: {
- std::string msg = fmt::sprintf("Camera %s: Error: %s (%d)", cameraId.c_str(),
- strerror(-ret), ret);
+ std::string msg = fmt::sprintf(
+ "Unable to retrieve session characteristics for camera device %s: "
+ "Error: %s (%d)",
+ cameraId.c_str(), strerror(-ret), ret);
ALOGE("%s: %s", __FUNCTION__, msg.c_str());
- res = STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT, msg.c_str());
+ logServiceError(msg, CameraService::ERROR_INVALID_OPERATION);
+ outMetadata->clear();
+ return STATUS_ERROR(CameraService::ERROR_INVALID_OPERATION, msg.c_str());
}
}
- return res;
+ return filterSensitiveMetadataIfNeeded(cameraId, outMetadata);
+}
+
+Status CameraService::filterSensitiveMetadataIfNeeded(
+ const std::string& cameraId, CameraMetadata* metadata) {
+ int callingPid = getCallingPid();
+ int callingUid = getCallingUid();
+
+ if (callingPid == getpid()) {
+ // Caller is cameraserver; no need to remove keys
+ return Status::ok();
+ }
+
+ SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
+ if (getSystemCameraKind(cameraId, &deviceKind) != OK) {
+ ALOGE("%s: Couldn't get camera kind for camera id %s", __FUNCTION__, cameraId.c_str());
+ metadata->clear();
+ return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION,
+ "Unable to retrieve camera kind for device %s", cameraId.c_str());
+ }
+ if (deviceKind == SystemCameraKind::SYSTEM_ONLY_CAMERA) {
+ // Attempting to query system only camera without system camera permission would have
+ // failed the shouldRejectSystemCameraConnection in the caller. So if we get here
+ // for a system only camera, then the caller has the required permission.
+ // No need to remove keys
+ return Status::ok();
+ }
+
+ std::vector<int32_t> tagsRemoved;
+ bool hasCameraPermission = hasPermissionsForCamera(cameraId, callingPid, callingUid);
+ if (hasCameraPermission) {
+ // Caller has camera permission; no need to remove keys
+ return Status::ok();
+ }
+
+ status_t ret = metadata->removePermissionEntries(
+ mCameraProviderManager->getProviderTagIdLocked(cameraId), &tagsRemoved);
+ if (ret != OK) {
+ metadata->clear();
+ return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION,
+ "Failed to remove camera characteristics needing camera permission "
+ "for device %s:%s (%d)",
+ cameraId.c_str(), strerror(-ret), ret);
+ }
+
+ if (!tagsRemoved.empty()) {
+ ret = metadata->update(ANDROID_REQUEST_CHARACTERISTIC_KEYS_NEEDING_PERMISSION,
+ tagsRemoved.data(), tagsRemoved.size());
+ if (ret != OK) {
+ metadata->clear();
+ return STATUS_ERROR_FMT(
+ ERROR_INVALID_OPERATION,
+ "Failed to insert camera keys needing permission for device %s: %s (%d)",
+ cameraId.c_str(), strerror(-ret), ret);
+ }
+ }
+ return Status::ok();
}
Status CameraService::parseCameraIdRemapping(
@@ -1381,8 +1456,6 @@
"characteristics for system only device %s: ", cameraId.c_str());
}
- Status ret{};
-
bool overrideForPerfClass =
SessionConfigurationUtils::targetPerfClassPrimaryCamera(mPerfClassPrimaryCameraIds,
cameraId, targetSdkVersion);
@@ -1401,45 +1474,8 @@
strerror(-res), res);
}
}
- SystemCameraKind deviceKind = SystemCameraKind::PUBLIC;
- if (getSystemCameraKind(cameraId, &deviceKind) != OK) {
- ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, cameraId.c_str());
- return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION, "Unable to retrieve camera kind "
- "for device %s", cameraId.c_str());
- }
- int callingPid = getCallingPid();
- int callingUid = getCallingUid();
- std::vector<int32_t> tagsRemoved;
- // If it's not calling from cameraserver, check the permission only if
- // android.permission.CAMERA is required. If android.permission.SYSTEM_CAMERA was needed,
- // it would've already been checked in shouldRejectSystemCameraConnection.
- bool checkPermissionForCamera = hasPermissionsForCamera(cameraId, callingPid, callingUid);
- if ((callingPid != getpid()) &&
- (deviceKind != SystemCameraKind::SYSTEM_ONLY_CAMERA) &&
- !checkPermissionForCamera) {
- res = cameraInfo->removePermissionEntries(
- mCameraProviderManager->getProviderTagIdLocked(cameraId),
- &tagsRemoved);
- if (res != OK) {
- cameraInfo->clear();
- return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION, "Failed to remove camera"
- " characteristics needing camera permission for device %s: %s (%d)",
- cameraId.c_str(), strerror(-res), res);
- }
- }
- if (!tagsRemoved.empty()) {
- res = cameraInfo->update(ANDROID_REQUEST_CHARACTERISTIC_KEYS_NEEDING_PERMISSION,
- tagsRemoved.data(), tagsRemoved.size());
- if (res != OK) {
- cameraInfo->clear();
- return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION, "Failed to insert camera "
- "keys needing permission for device %s: %s (%d)", cameraId.c_str(),
- strerror(-res), res);
- }
- }
-
- return ret;
+ return filterSensitiveMetadataIfNeeded(cameraId, cameraInfo);
}
Status CameraService::getTorchStrengthLevel(const std::string& unresolvedCameraId, int32_t deviceId,
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index b4c2edd..e1bc726 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -1529,6 +1529,12 @@
// responsibility to acquire mLogLock before calling this functions.
bool isClientWatchedLocked(const BasicClient *client);
+ // Filters out fingerprintable keys if the calling process does not have CAMERA permission.
+ // Note: function caller should ensure that shouldRejectSystemCameraConnection is checked
+ // for the calling process before calling this function.
+ binder::Status filterSensitiveMetadataIfNeeded(const std::string& cameraId,
+ CameraMetadata* metadata);
+
/**
* Get the current system time as a formatted string.
*/
diff --git a/services/camera/libcameraservice/aidl/AidlUtils.cpp b/services/camera/libcameraservice/aidl/AidlUtils.cpp
index 14e5fad..48d78e1 100644
--- a/services/camera/libcameraservice/aidl/AidlUtils.cpp
+++ b/services/camera/libcameraservice/aidl/AidlUtils.cpp
@@ -18,6 +18,7 @@
#include <aidl/AidlUtils.h>
#include <aidl/ExtensionMetadataTags.h>
+#include <aidl/SessionCharacteristicsTags.h>
#include <aidl/VndkVersionMetadataTags.h>
#include <aidlcommonsupport/NativeHandle.h>
#include <camera/StringUtils.h>
@@ -334,6 +335,49 @@
return OK;
}
+status_t copySessionCharacteristics(const CameraMetadata& from, CameraMetadata* to,
+ int queryVersion) {
+ // Ensure the vendor ID are the same before attempting
+ // anything else. If vendor IDs differ we cannot safely copy the characteristics.
+ if (from.getVendorId() != to->getVendorId()) {
+ ALOGE("%s: Incompatible CameraMetadata objects. Vendor IDs differ. From: %lu; To: %lu",
+ __FUNCTION__, from.getVendorId(), to->getVendorId());
+ return BAD_VALUE;
+ }
+
+ // Allow public tags according to the queryVersion
+ std::unordered_set<uint32_t> validPublicTags;
+ auto last = api_level_to_session_characteristic_keys.upper_bound(queryVersion);
+ for (auto it = api_level_to_session_characteristic_keys.begin(); it != last; it++) {
+ validPublicTags.insert(it->second.cbegin(), it->second.cend());
+ }
+
+ const camera_metadata_t* src = from.getAndLock();
+ camera_metadata_ro_entry_t entry{};
+ for (size_t i = 0; i < get_camera_metadata_entry_count(src); i++) {
+ int ret = get_camera_metadata_ro_entry(src, i, &entry);
+ if (ret != OK) {
+ ALOGE("%s: Could not fetch entry at index %lu. Error: %d", __FUNCTION__, i, ret);
+ from.unlock(src);
+ return BAD_VALUE;
+ }
+
+ if (entry.tag < (uint32_t)VENDOR_SECTION_START &&
+ validPublicTags.find(entry.tag) == validPublicTags.end()) {
+ ALOGI("%s: Session Characteristics contains tag %s but not supported by query version "
+ "(%d)",
+ __FUNCTION__, get_camera_metadata_tag_name(entry.tag), queryVersion);
+ continue;
+ }
+
+ // The entry is either a vendor tag, or a valid session characteristic key.
+ // Copy over the value
+ to->update(entry);
+ }
+ from.unlock(src);
+ return OK;
+}
+
bool areExtensionKeysSupported(const CameraMetadata& metadata) {
auto requestKeys = metadata.find(ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS);
if (requestKeys.count == 0) {
diff --git a/services/camera/libcameraservice/aidl/AidlUtils.h b/services/camera/libcameraservice/aidl/AidlUtils.h
index 562aa70..92e878e 100644
--- a/services/camera/libcameraservice/aidl/AidlUtils.h
+++ b/services/camera/libcameraservice/aidl/AidlUtils.h
@@ -122,6 +122,9 @@
status_t filterVndkKeys(int vndkVersion, CameraMetadata &metadata, bool isStatic = true);
+status_t copySessionCharacteristics(const CameraMetadata& from, CameraMetadata* to,
+ int queryVersion);
+
bool areExtensionKeysSupported(const CameraMetadata& metadata);
status_t filterExtensionKeys(CameraMetadata* metadata /*out*/);
diff --git a/services/camera/libcameraservice/aidl/SessionCharacteristicsTags.h b/services/camera/libcameraservice/aidl/SessionCharacteristicsTags.h
new file mode 100644
index 0000000..cefb8a6
--- /dev/null
+++ b/services/camera/libcameraservice/aidl/SessionCharacteristicsTags.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <map>
+#include <vector>
+#pragma once
+/**
+ * ! Do not edit this file directly !
+ *
+ * Generated automatically from session_characteristics_tags.mako. To be included in
+ * libcameraservice only by aidl/AidlUtils.cpp.
+ */
+
+/**
+ * Mapping of session characteristics to the INFO_SESSION_CONFIGURATION_QUERY_VERSION value
+ * at which they were introduced.
+ */
+std::map<int, std::vector<camera_metadata_tag>> api_level_to_session_characteristic_keys {
+ {35,
+ {
+ ANDROID_CONTROL_ZOOM_RATIO_RANGE,
+ ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM,
+ }},
+};
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index d4b32e8..89e785a 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -1858,7 +1858,7 @@
auto& c = mCameraCharacteristics;
status_t res = c.update(ANDROID_INFO_SESSION_CONFIGURATION_QUERY_VERSION, &versionCode, 1);
-
+ mSessionConfigQueryVersion = versionCode;
return res;
}
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index 1a686bd..2e97207 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -654,8 +654,7 @@
virtual status_t getSessionCharacteristics(
const SessionConfiguration &/*configuration*/,
bool /*overrideForPerfClass*/,
- camera3::metadataGetter /*getMetadata*/,
- CameraMetadata* /*sessionCharacteristics*/) {
+ camera3::metadataGetter /*getMetadata*/, CameraMetadata* /*outChars*/) {
return INVALID_OPERATION;
}
@@ -737,6 +736,10 @@
// Only contains characteristics for hidden physical cameras,
// not for public physical cameras.
std::unordered_map<std::string, CameraMetadata> mPhysicalCameraCharacteristics;
+ // Value filled in from addSessionConfigQueryVersionTag.
+ // Cached to make lookups faster
+ int mSessionConfigQueryVersion = 0;
+
void queryPhysicalCameraIds();
SystemCameraKind getSystemCameraKind();
status_t fixupMonochromeTags();
diff --git a/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp b/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp
index 8eaa7b4..5b24227 100644
--- a/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp
+++ b/services/camera/libcameraservice/common/aidl/AidlProviderInfo.cpp
@@ -17,6 +17,8 @@
#include "common/HalConversionsTemplated.h"
#include "common/CameraProviderInfoTemplated.h"
+#include <aidl/AidlUtils.h>
+
#include <com_android_internal_camera_flags.h>
#include <cutils/properties.h>
@@ -43,6 +45,7 @@
using namespace aidl::android::hardware;
using namespace hardware::camera;
+using android::hardware::cameraservice::utils::conversion::aidl::copySessionCharacteristics;
using hardware::camera2::utils::CameraIdAndSessionConfiguration;
using hardware::ICameraService;
using SessionConfigurationUtils::overrideDefaultRequestKeys;
@@ -927,7 +930,7 @@
status_t AidlProviderInfo::AidlDeviceInfo3::getSessionCharacteristics(
const SessionConfiguration &configuration, bool overrideForPerfClass,
- camera3::metadataGetter getMetadata, CameraMetadata *sessionCharacteristics) {
+ camera3::metadataGetter getMetadata, CameraMetadata* outChars) {
camera::device::StreamConfiguration streamConfiguration;
bool earlyExit = false;
auto res = SessionConfigurationUtils::convertToHALStreamCombination(configuration,
@@ -953,24 +956,32 @@
aidl::android::hardware::camera::device::CameraMetadata chars;
::ndk::ScopedAStatus ret =
interface->getSessionCharacteristics(streamConfiguration, &chars);
- std::vector<uint8_t> &metadata = chars.metadata;
+ if (!ret.isOk()) {
+ ALOGE("%s: Unexpected binder error while getting session characteristics (%d): %s",
+ __FUNCTION__, ret.getExceptionCode(), ret.getMessage());
+ return mapToStatusT(ret);
+ }
- camera_metadata_t *buffer = reinterpret_cast<camera_metadata_t*>(metadata.data());
+ std::vector<uint8_t> &metadata = chars.metadata;
+ auto *buffer = reinterpret_cast<camera_metadata_t*>(metadata.data());
size_t expectedSize = metadata.size();
int resV = validate_camera_metadata_structure(buffer, &expectedSize);
if (resV == OK || resV == CAMERA_METADATA_VALIDATION_SHIFTED) {
set_camera_metadata_vendor_id(buffer, mProviderTagid);
- *sessionCharacteristics = buffer;
} else {
ALOGE("%s: Malformed camera metadata received from HAL", __FUNCTION__);
return BAD_VALUE;
}
- if (!ret.isOk()) {
- ALOGE("%s: Unexpected binder error: %s", __FUNCTION__, ret.getMessage());
- return mapToStatusT(ret);
- }
- return OK;
+ CameraMetadata rawSessionChars;
+ rawSessionChars = buffer; // clone buffer
+ rawSessionChars.sort(); // sort for faster lookups
+
+ *outChars = mCameraCharacteristics;
+ outChars->sort(); // sort for faster reads and (hopefully!) writes
+
+ return copySessionCharacteristics(/*from=*/rawSessionChars, /*to=*/outChars,
+ mSessionConfigQueryVersion);
}
status_t AidlProviderInfo::convertToAidlHALStreamCombinationAndCameraIdsLocked(
diff --git a/services/camera/libcameraservice/common/aidl/AidlProviderInfo.h b/services/camera/libcameraservice/common/aidl/AidlProviderInfo.h
index 38ebc94..f5e0db0 100644
--- a/services/camera/libcameraservice/common/aidl/AidlProviderInfo.h
+++ b/services/camera/libcameraservice/common/aidl/AidlProviderInfo.h
@@ -137,7 +137,7 @@
virtual status_t getSessionCharacteristics(
const SessionConfiguration &/*configuration*/,
bool overrideForPerfClass, camera3::metadataGetter /*getMetadata*/,
- CameraMetadata *sessionCharacteristics /*sessionCharacteristics*/);
+ CameraMetadata */*outChars*/);
std::shared_ptr<aidl::android::hardware::camera::device::ICameraDevice>
startDeviceInterface();