Revert "Revert "Revert "Revert "SF: Updating permissions checking in Surface Flinger.""""
This reverts commit e334440508f8e36729f04e80d963d84eebb5ad59.
Reason for revert: Try #3. Anyone should be able to check whether they can access SF
throught AUTHENTICATE_SURFACE. If they are on the list of producers the call will return
true, otherwise false, but the call itself should not be protected.
Test: SF tests updated. They all pass.
Change-Id: Ie53f34aff6bb25e5976278ed59b03a0d56eb6633
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index bf5c28e..337f89d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1091,15 +1091,6 @@
status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const
NO_THREAD_SAFETY_ANALYSIS {
- IPCThreadState* ipc = IPCThreadState::self();
- const int pid = ipc->getCallingPid();
- const int uid = ipc->getCallingUid();
- if ((uid != AID_SHELL) &&
- !PermissionCache::checkPermission(sDump, pid, uid)) {
- ALOGE("Layer debug info permission denied for pid=%d, uid=%d", pid, uid);
- return PERMISSION_DENIED;
- }
-
// Try to acquire a lock for 1s, fail gracefully
const status_t err = mStateLock.timedLock(s2ns(1));
const bool locked = (err == NO_ERROR);
@@ -3258,9 +3249,8 @@
IPCThreadState* ipc = IPCThreadState::self();
const int pid = ipc->getCallingPid();
const int uid = ipc->getCallingUid();
-
if ((uid != AID_GRAPHICS && uid != AID_SYSTEM) &&
- !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
+ !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
return false;
}
return true;
@@ -4446,51 +4436,64 @@
}
status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {
- switch (code) {
- case CREATE_CONNECTION:
- case CREATE_DISPLAY:
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wswitch-enum"
+ switch (static_cast<ISurfaceComposerTag>(code)) {
+ // These methods should at minimum make sure that the client requested
+ // access to SF.
case BOOT_FINISHED:
case CLEAR_ANIMATION_FRAME_STATS:
- case GET_ANIMATION_FRAME_STATS:
- case SET_POWER_MODE:
- case GET_HDR_CAPABILITIES:
+ case CREATE_CONNECTION:
+ case CREATE_DISPLAY:
+ case DESTROY_DISPLAY:
case ENABLE_VSYNC_INJECTIONS:
+ case GET_ACTIVE_COLOR_MODE:
+ case GET_ANIMATION_FRAME_STATS:
+ case GET_HDR_CAPABILITIES:
+ case SET_ACTIVE_CONFIG:
+ case SET_ACTIVE_COLOR_MODE:
case INJECT_VSYNC:
- {
- // codes that require permission check
+ case SET_POWER_MODE: {
if (!callingThreadHasUnscopedSurfaceFlingerAccess()) {
IPCThreadState* ipc = IPCThreadState::self();
ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d",
ipc->getCallingPid(), ipc->getCallingUid());
return PERMISSION_DENIED;
}
- break;
- }
- /*
- * Calling setTransactionState is safe, because you need to have been
- * granted a reference to Client* and Handle* to do anything with it.
- *
- * Creating a scoped connection is safe, as per discussion in ISurfaceComposer.h
- */
- case SET_TRANSACTION_STATE:
- case CREATE_SCOPED_CONNECTION:
- {
return OK;
}
- case CAPTURE_SCREEN:
- {
- // codes that require permission check
+ case GET_LAYER_DEBUG_INFO: {
IPCThreadState* ipc = IPCThreadState::self();
const int pid = ipc->getCallingPid();
const int uid = ipc->getCallingUid();
- if ((uid != AID_GRAPHICS) &&
- !PermissionCache::checkPermission(sReadFramebuffer, pid, uid)) {
- ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);
+ if ((uid != AID_SHELL) && !PermissionCache::checkPermission(sDump, pid, uid)) {
+ ALOGE("Layer debug info permission denied for pid=%d, uid=%d", pid, uid);
return PERMISSION_DENIED;
}
- break;
+ return OK;
}
- case CAPTURE_LAYERS: {
+ // Used by apps to hook Choreographer to SurfaceFlinger.
+ case CREATE_DISPLAY_EVENT_CONNECTION:
+ // The following calls are currently used by clients that do not
+ // request necessary permissions. However, they do not expose any secret
+ // information, so it is OK to pass them.
+ case AUTHENTICATE_SURFACE:
+ case GET_ACTIVE_CONFIG:
+ case GET_BUILT_IN_DISPLAY:
+ case GET_DISPLAY_COLOR_MODES:
+ case GET_DISPLAY_CONFIGS:
+ case GET_DISPLAY_STATS:
+ case GET_SUPPORTED_FRAME_TIMESTAMPS:
+ // Calling setTransactionState is safe, because you need to have been
+ // granted a reference to Client* and Handle* to do anything with it.
+ case SET_TRANSACTION_STATE:
+ // Creating a scoped connection is safe, as per discussion in ISurfaceComposer.h
+ case CREATE_SCOPED_CONNECTION: {
+ return OK;
+ }
+ case CAPTURE_LAYERS:
+ case CAPTURE_SCREEN: {
+ // codes that require permission check
IPCThreadState* ipc = IPCThreadState::self();
const int pid = ipc->getCallingPid();
const int uid = ipc->getCallingUid();
@@ -4499,15 +4502,37 @@
ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);
return PERMISSION_DENIED;
}
- break;
+ return OK;
+ }
+ // The following codes are deprecated and should never be allowed to access SF.
+ case CONNECT_DISPLAY_UNUSED:
+ case CREATE_GRAPHIC_BUFFER_ALLOC_UNUSED: {
+ ALOGE("Attempting to access SurfaceFlinger with unused code: %u", code);
+ return PERMISSION_DENIED;
}
}
- return OK;
+
+ // These codes are used for the IBinder protocol to either interrogate the recipient
+ // side of the transaction for its canonical interface descriptor or to dump its state.
+ // We let them pass by default.
+ if (code == IBinder::INTERFACE_TRANSACTION || code == IBinder::DUMP_TRANSACTION ||
+ code == IBinder::PING_TRANSACTION || code == IBinder::SHELL_COMMAND_TRANSACTION ||
+ code == IBinder::SYSPROPS_TRANSACTION) {
+ return OK;
+ }
+ // Numbers from 1000 to 1029 are currently use for backdoors. The code
+ // in onTransact verifies that the user is root, and has access to use SF.
+ if (code >= 1000 && code <= 1029) {
+ ALOGV("Accessing SurfaceFlinger through backdoor code: %u", code);
+ return OK;
+ }
+ ALOGE("Permission Denial: SurfaceFlinger did not recognize request code: %u", code);
+ return PERMISSION_DENIED;
+#pragma clang diagnostic pop
}
-status_t SurfaceFlinger::onTransact(
- uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
-{
+status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* reply,
+ uint32_t flags) {
status_t credentialCheck = CheckTransactCodeCredentials(code);
if (credentialCheck != OK) {
return credentialCheck;