From c78f53cd0e9ce68cc52a851584b6ce5b34baef7d Mon Sep 17 00:00:00 2001 From: Chavi Weingarten Date: Fri, 14 Apr 2023 18:50:53 +0000 Subject: Cleaned up transaction sanitize calls Exposed a way for a client to invoke sanitize with a uid and pid to ensure we don't remove states when the process that added it was privileged. Added a helper function to get the permission ints based on the String permission values so SF and clients can call the same API. In SF, call sanitize as soon as setTransactionState is called since that's the point where the Transaction has been passed over binder so we can identify the calling uid. This allows us to remove the permission values passed to applyTransactionState and unifies the places that were calling sanitize. Test: CredentialsTest Bug: 267794530 Change-Id: I30c1800f0fee43df1cee82464139db7b56a7d911 --- libs/gui/Android.bp | 1 + libs/gui/ISurfaceComposer.cpp | 8 ++-- libs/gui/LayerStatePermissions.cpp | 58 ++++++++++++++++++++++++++++ libs/gui/SurfaceComposerClient.cpp | 12 ++++-- libs/gui/include/gui/ISurfaceComposer.h | 2 +- libs/gui/include/gui/LayerStatePermissions.h | 29 ++++++++++++++ libs/gui/include/gui/SurfaceComposerClient.h | 2 +- libs/gui/tests/Surface_test.cpp | 2 +- 8 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 libs/gui/LayerStatePermissions.cpp create mode 100644 libs/gui/include/gui/LayerStatePermissions.h (limited to 'libs') diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index 33bb343c9d..80fed98434 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -226,6 +226,7 @@ cc_library_shared { "ITransactionCompletedListener.cpp", "LayerDebugInfo.cpp", "LayerMetadata.cpp", + "LayerStatePermissions.cpp", "LayerState.cpp", "OccupancyTracker.cpp", "StreamSplitter.cpp", diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index cefb9a71d6..d72f65eb7a 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -62,7 +62,7 @@ public: status_t setTransactionState(const FrameTimelineInfo& frameTimelineInfo, Vector& state, const Vector& displays, uint32_t flags, const sp& applyToken, - const InputWindowCommands& commands, int64_t desiredPresentTime, + InputWindowCommands commands, int64_t desiredPresentTime, bool isAutoTimestamp, const std::vector& uncacheBuffers, bool hasListenerCallbacks, @@ -188,9 +188,9 @@ status_t BnSurfaceComposer::onTransact( SAFE_PARCEL(data.readUint64, &transactionId); return setTransactionState(frameTimelineInfo, state, displays, stateFlags, applyToken, - inputWindowCommands, desiredPresentTime, isAutoTimestamp, - uncacheBuffers, hasListenerCallbacks, listenerCallbacks, - transactionId); + std::move(inputWindowCommands), desiredPresentTime, + isAutoTimestamp, uncacheBuffers, hasListenerCallbacks, + listenerCallbacks, transactionId); } default: { return BBinder::onTransact(code, data, reply, flags); diff --git a/libs/gui/LayerStatePermissions.cpp b/libs/gui/LayerStatePermissions.cpp new file mode 100644 index 0000000000..28697ca953 --- /dev/null +++ b/libs/gui/LayerStatePermissions.cpp @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2023 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 +#include +#include +#ifndef __ANDROID_VNDK__ +#include +#endif // __ANDROID_VNDK__ +#include + +namespace android { +std::unordered_map LayerStatePermissions::mPermissionMap = { + // If caller has ACCESS_SURFACE_FLINGER, they automatically get ROTATE_SURFACE_FLINGER + // permission, as well + {"android.permission.ACCESS_SURFACE_FLINGER", + layer_state_t::Permission::ACCESS_SURFACE_FLINGER | + layer_state_t::Permission::ROTATE_SURFACE_FLINGER}, + {"android.permission.ROTATE_SURFACE_FLINGER", + layer_state_t::Permission::ROTATE_SURFACE_FLINGER}, + {"android.permission.INTERNAL_SYSTEM_WINDOW", + layer_state_t::Permission::INTERNAL_SYSTEM_WINDOW}, +}; + +static bool callingThreadHasPermission(const std::string& permission __attribute__((unused)), + int pid __attribute__((unused)), + int uid __attribute__((unused))) { +#ifndef __ANDROID_VNDK__ + return uid == AID_GRAPHICS || uid == AID_SYSTEM || + PermissionCache::checkPermission(String16(permission.c_str()), pid, uid); +#endif // __ANDROID_VNDK__ + return false; +} + +uint32_t LayerStatePermissions::getTransactionPermissions(int pid, int uid) { + uint32_t permissions = 0; + for (auto [permissionName, permissionVal] : mPermissionMap) { + if (callingThreadHasPermission(permissionName, pid, uid)) { + permissions |= permissionVal; + } + } + + return permissions; +} +} // namespace android diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index eb5cc4f8ab..1b13ec1c06 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -54,6 +54,7 @@ #include #include +#include #include #include @@ -716,11 +717,16 @@ SurfaceComposerClient::Transaction::Transaction(const Transaction& other) mListenerCallbacks = other.mListenerCallbacks; } -void SurfaceComposerClient::Transaction::sanitize() { +void SurfaceComposerClient::Transaction::sanitize(int pid, int uid) { + uint32_t permissions = LayerStatePermissions::getTransactionPermissions(pid, uid); for (auto & [handle, composerState] : mComposerStates) { - composerState.state.sanitize(0 /* permissionMask */); + composerState.state.sanitize(permissions); + } + if (!mInputWindowCommands.empty() && + (permissions & layer_state_t::Permission::ACCESS_SURFACE_FLINGER) == 0) { + ALOGE("Only privileged callers are allowed to send input commands."); + mInputWindowCommands.clear(); } - mInputWindowCommands.clear(); } std::unique_ptr diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h index 1e67225a4e..bd21851c14 100644 --- a/libs/gui/include/gui/ISurfaceComposer.h +++ b/libs/gui/include/gui/ISurfaceComposer.h @@ -113,7 +113,7 @@ public: virtual status_t setTransactionState( const FrameTimelineInfo& frameTimelineInfo, Vector& state, const Vector& displays, uint32_t flags, const sp& applyToken, - const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime, + InputWindowCommands inputWindowCommands, int64_t desiredPresentTime, bool isAutoTimestamp, const std::vector& uncacheBuffer, bool hasListenerCallbacks, const std::vector& listenerCallbacks, uint64_t transactionId) = 0; diff --git a/libs/gui/include/gui/LayerStatePermissions.h b/libs/gui/include/gui/LayerStatePermissions.h new file mode 100644 index 0000000000..a90f30c621 --- /dev/null +++ b/libs/gui/include/gui/LayerStatePermissions.h @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2023 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 +#include +#include + +namespace android { +class LayerStatePermissions { +public: + static uint32_t getTransactionPermissions(int pid, int uid); + +private: + static std::unordered_map mPermissionMap; +}; +} // namespace android \ No newline at end of file diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 945b164fdc..8d2cdaf5b8 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -744,7 +744,7 @@ public: * * TODO (b/213644870): Remove all permissioned things from Transaction */ - void sanitize(); + void sanitize(int pid, int uid); static sp getDefaultApplyToken(); static void setDefaultApplyToken(sp applyToken); diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index fccc408473..5bc6904563 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -699,7 +699,7 @@ public: Vector& /*state*/, const Vector& /*displays*/, uint32_t /*flags*/, const sp& /*applyToken*/, - const InputWindowCommands& /*inputWindowCommands*/, + InputWindowCommands /*inputWindowCommands*/, int64_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/, const std::vector& /*cachedBuffer*/, bool /*hasListenerCallbacks*/, -- cgit v1.2.3-59-g8ed1b