Revert "Change input injection security model to require INJECT_..."
Revert submission 17125748-require-inject-events-permission
Reason for revert: b/228203157, b/228194124
Reverted Changes:
Ib0d66eff3:Change input injection security model to require I...
Ief1a2f10c:Update WindowInputTests after changing injection p...
Ie2d8d0c3d:Change input injection security model to require I...
Change-Id: Iab079575c41821f5330002e23dc13884676a5249
diff --git a/services/inputflinger/dispatcher/InjectionState.cpp b/services/inputflinger/dispatcher/InjectionState.cpp
index c2d3ad6..c8024a6 100644
--- a/services/inputflinger/dispatcher/InjectionState.cpp
+++ b/services/inputflinger/dispatcher/InjectionState.cpp
@@ -20,9 +20,10 @@
namespace android::inputdispatcher {
-InjectionState::InjectionState(const std::optional<int32_t>& targetUid)
+InjectionState::InjectionState(int32_t injectorPid, int32_t injectorUid)
: refCount(1),
- targetUid(targetUid),
+ injectorPid(injectorPid),
+ injectorUid(injectorUid),
injectionResult(android::os::InputEventInjectionResult::PENDING),
injectionIsAsync(false),
pendingForegroundDispatches(0) {}
diff --git a/services/inputflinger/dispatcher/InjectionState.h b/services/inputflinger/dispatcher/InjectionState.h
index 90cf150..0bfafb1 100644
--- a/services/inputflinger/dispatcher/InjectionState.h
+++ b/services/inputflinger/dispatcher/InjectionState.h
@@ -27,12 +27,13 @@
struct InjectionState {
mutable int32_t refCount;
- std::optional<int32_t> targetUid;
+ int32_t injectorPid;
+ int32_t injectorUid;
android::os::InputEventInjectionResult injectionResult; // initially PENDING
bool injectionIsAsync; // set to true if injection is not waiting for the result
int32_t pendingForegroundDispatches; // the number of foreground dispatches in progress
- explicit InjectionState(const std::optional<int32_t>& targetUid);
+ InjectionState(int32_t injectorPid, int32_t injectorUid);
void release();
private:
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp
index f3d0b65..11cceaa 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.cpp
+++ b/services/inputflinger/dispatcher/InputDispatcher.cpp
@@ -578,27 +578,6 @@
return false;
}
-// Checks targeted injection using the window's owner's uid.
-// Returns an empty string if an entry can be sent to the given window, or an error message if the
-// entry is a targeted injection whose uid target doesn't match the window owner.
-std::optional<std::string> verifyTargetedInjection(const sp<WindowInfoHandle>& window,
- const EventEntry& entry) {
- if (entry.injectionState == nullptr || !entry.injectionState->targetUid) {
- // The event was not injected, or the injected event does not target a window.
- return {};
- }
- const int32_t uid = *entry.injectionState->targetUid;
- if (window == nullptr) {
- return StringPrintf("No valid window target for injection into uid %d.", uid);
- }
- if (entry.injectionState->targetUid != window->getInfo()->ownerUid) {
- return StringPrintf("Injected event targeted at uid %d would be dispatched to window '%s' "
- "owned by uid %d.",
- uid, window->getName().c_str(), window->getInfo()->ownerUid);
- }
- return {};
-}
-
} // namespace
// --- InputDispatcher ---
@@ -1057,8 +1036,6 @@
switch (entry.type) {
case EventEntry::Type::KEY: {
- LOG_ALWAYS_FATAL_IF((entry.policyFlags & POLICY_FLAG_TRUSTED) == 0,
- "Unexpected untrusted event.");
// Optimize app switch latency.
// If the application takes too long to catch up then we drop all events preceding
// the app switch key.
@@ -1096,8 +1073,6 @@
}
case EventEntry::Type::MOTION: {
- LOG_ALWAYS_FATAL_IF((entry.policyFlags & POLICY_FLAG_TRUSTED) == 0,
- "Unexpected untrusted event.");
if (shouldPruneInboundQueueLocked(static_cast<MotionEntry&>(entry))) {
mNextUnblockedEvent = mInboundQueue.back();
needWake = true;
@@ -1743,7 +1718,8 @@
}
setInjectionResult(*entry, injectionResult);
- if (injectionResult == InputEventInjectionResult::TARGET_MISMATCH) {
+ if (injectionResult == InputEventInjectionResult::PERMISSION_DENIED) {
+ ALOGW("Permission denied, dropping the motion (isPointer=%s)", toString(isPointerEvent));
return true;
}
if (injectionResult != InputEventInjectionResult::SUCCEEDED) {
@@ -2000,10 +1976,9 @@
// we have a valid, non-null focused window
resetNoFocusedWindowTimeoutLocked();
- // Verify targeted injection.
- if (const auto err = verifyTargetedInjection(focusedWindowHandle, entry); err) {
- ALOGW("Dropping injected event: %s", (*err).c_str());
- return InputEventInjectionResult::TARGET_MISMATCH;
+ // Check permissions.
+ if (!checkInjectionPermission(focusedWindowHandle, entry.injectionState)) {
+ return InputEventInjectionResult::PERMISSION_DENIED;
}
if (focusedWindowHandle->getInfo()->inputConfig.test(
@@ -2069,6 +2044,11 @@
nsecs_t currentTime, const MotionEntry& entry, std::vector<InputTarget>& inputTargets,
nsecs_t* nextWakeupTime, bool* outConflictingPointerActions) {
ATRACE_CALL();
+ enum InjectionPermission {
+ INJECTION_PERMISSION_UNKNOWN,
+ INJECTION_PERMISSION_GRANTED,
+ INJECTION_PERMISSION_DENIED
+ };
// For security reasons, we defer updating the touch state until we are sure that
// event injection will be allowed.
@@ -2078,6 +2058,7 @@
// Update the touch state as needed based on the properties of the touch event.
InputEventInjectionResult injectionResult = InputEventInjectionResult::PENDING;
+ InjectionPermission injectionPermission = INJECTION_PERMISSION_UNKNOWN;
sp<WindowInfoHandle> newHoverWindowHandle(mLastHoverWindowHandle);
sp<WindowInfoHandle> newTouchedWindowHandle;
@@ -2126,7 +2107,7 @@
"in display %" PRId32,
displayId);
// TODO: test multiple simultaneous input streams.
- injectionResult = InputEventInjectionResult::FAILED;
+ injectionResult = InputEventInjectionResult::PERMISSION_DENIED;
switchedDevice = false;
wrongDevice = true;
goto Failed;
@@ -2159,14 +2140,6 @@
newTouchedWindowHandle = tempTouchState.getFirstForegroundWindowHandle();
}
- // Verify targeted injection.
- if (const auto err = verifyTargetedInjection(newTouchedWindowHandle, entry); err) {
- ALOGW("Dropping injected touch event: %s", (*err).c_str());
- injectionResult = os::InputEventInjectionResult::TARGET_MISMATCH;
- newTouchedWindowHandle = nullptr;
- goto Failed;
- }
-
// Figure out whether splitting will be allowed for this window.
if (newTouchedWindowHandle != nullptr) {
if (newTouchedWindowHandle->getInfo()->supportsSplitTouch()) {
@@ -2210,11 +2183,6 @@
for (const sp<WindowInfoHandle>& windowHandle : newTouchedWindows) {
const WindowInfo& info = *windowHandle->getInfo();
- // Skip spy window targets that are not valid for targeted injection.
- if (const auto err = verifyTargetedInjection(windowHandle, entry); err) {
- continue;
- }
-
if (info.inputConfig.test(WindowInfo::InputConfig::PAUSE_DISPATCHING)) {
ALOGI("Not sending touch event to %s because it is paused",
windowHandle->getName().c_str());
@@ -2308,14 +2276,6 @@
newTouchedWindowHandle =
findTouchedWindowAtLocked(displayId, x, y, &tempTouchState, isStylus);
- // Verify targeted injection.
- if (const auto err = verifyTargetedInjection(newTouchedWindowHandle, entry); err) {
- ALOGW("Dropping injected event: %s", (*err).c_str());
- injectionResult = os::InputEventInjectionResult::TARGET_MISMATCH;
- newTouchedWindowHandle = nullptr;
- goto Failed;
- }
-
// Drop touch events if requested by input feature
if (newTouchedWindowHandle != nullptr &&
shouldDropInput(entry, newTouchedWindowHandle)) {
@@ -2407,26 +2367,19 @@
goto Failed;
}
- // Ensure that all touched windows are valid for injection.
- if (entry.injectionState != nullptr) {
- std::string errs;
- for (const TouchedWindow& touchedWindow : tempTouchState.windows) {
- if (touchedWindow.targetFlags & InputTarget::FLAG_DISPATCH_AS_OUTSIDE) {
- // Allow ACTION_OUTSIDE events generated by targeted injection to be
- // dispatched to any uid, since the coords will be zeroed out later.
- continue;
- }
- const auto err = verifyTargetedInjection(touchedWindow.windowHandle, entry);
- if (err) errs += "\n - " + *err;
- }
- if (!errs.empty()) {
- ALOGW("Dropping targeted injection: At least one touched window is not owned by uid "
- "%d:%s",
- *entry.injectionState->targetUid, errs.c_str());
- injectionResult = InputEventInjectionResult::TARGET_MISMATCH;
- goto Failed;
- }
+ // Check permission to inject into all touched foreground windows.
+ if (std::any_of(tempTouchState.windows.begin(), tempTouchState.windows.end(),
+ [this, &entry](const TouchedWindow& touchedWindow) {
+ return (touchedWindow.targetFlags & InputTarget::FLAG_FOREGROUND) != 0 &&
+ !checkInjectionPermission(touchedWindow.windowHandle,
+ entry.injectionState);
+ })) {
+ injectionResult = InputEventInjectionResult::PERMISSION_DENIED;
+ injectionPermission = INJECTION_PERMISSION_DENIED;
+ goto Failed;
}
+ // Permission granted to inject into all touched foreground windows.
+ injectionPermission = INJECTION_PERMISSION_GRANTED;
// Check whether windows listening for outside touches are owned by the same UID. If it is
// set the policy flag that we will not reveal coordinate information to this window.
@@ -2492,6 +2445,19 @@
tempTouchState.filterNonAsIsTouchWindows();
Failed:
+ // Check injection permission once and for all.
+ if (injectionPermission == INJECTION_PERMISSION_UNKNOWN) {
+ if (checkInjectionPermission(nullptr, entry.injectionState)) {
+ injectionPermission = INJECTION_PERMISSION_GRANTED;
+ } else {
+ injectionPermission = INJECTION_PERMISSION_DENIED;
+ }
+ }
+
+ if (injectionPermission != INJECTION_PERMISSION_GRANTED) {
+ return injectionResult;
+ }
+
// Update final pieces of touch state if the injector had permission.
if (!wrongDevice) {
if (switchedDevice) {
@@ -2689,6 +2655,26 @@
}
}
+bool InputDispatcher::checkInjectionPermission(const sp<WindowInfoHandle>& windowHandle,
+ const InjectionState* injectionState) {
+ if (injectionState &&
+ (windowHandle == nullptr ||
+ windowHandle->getInfo()->ownerUid != injectionState->injectorUid) &&
+ !hasInjectionPermission(injectionState->injectorPid, injectionState->injectorUid)) {
+ if (windowHandle != nullptr) {
+ ALOGW("Permission denied: injecting event from pid %d uid %d to window %s "
+ "owned by uid %d",
+ injectionState->injectorPid, injectionState->injectorUid,
+ windowHandle->getName().c_str(), windowHandle->getInfo()->ownerUid);
+ } else {
+ ALOGW("Permission denied: injecting event from pid %d uid %d",
+ injectionState->injectorPid, injectionState->injectorUid);
+ }
+ return false;
+ }
+ return true;
+}
+
/**
* Indicate whether one window handle should be considered as obscuring
* another window handle. We only check a few preconditions. Actually
@@ -4207,20 +4193,20 @@
}
}
-InputEventInjectionResult InputDispatcher::injectInputEvent(const InputEvent* event,
- std::optional<int32_t> targetUid,
- InputEventInjectionSync syncMode,
- std::chrono::milliseconds timeout,
- uint32_t policyFlags) {
+InputEventInjectionResult InputDispatcher::injectInputEvent(
+ const InputEvent* event, int32_t injectorPid, int32_t injectorUid,
+ InputEventInjectionSync syncMode, std::chrono::milliseconds timeout, uint32_t policyFlags) {
if (DEBUG_INBOUND_EVENT_DETAILS) {
- ALOGD("injectInputEvent - eventType=%d, targetUid=%s, syncMode=%d, timeout=%lld, "
- "policyFlags=0x%08x",
- event->getType(), targetUid ? std::to_string(*targetUid).c_str() : "none", syncMode,
- timeout.count(), policyFlags);
+ ALOGD("injectInputEvent - eventType=%d, injectorPid=%d, injectorUid=%d, "
+ "syncMode=%d, timeout=%lld, policyFlags=0x%08x",
+ event->getType(), injectorPid, injectorUid, syncMode, timeout.count(), policyFlags);
}
nsecs_t endTime = now() + std::chrono::duration_cast<std::chrono::nanoseconds>(timeout).count();
- policyFlags |= POLICY_FLAG_INJECTED | POLICY_FLAG_TRUSTED;
+ policyFlags |= POLICY_FLAG_INJECTED;
+ if (hasInjectionPermission(injectorPid, injectorUid)) {
+ policyFlags |= POLICY_FLAG_TRUSTED;
+ }
// For all injected events, set device id = VIRTUAL_KEYBOARD_ID. The only exception is events
// that have gone through the InputFilter. If the event passed through the InputFilter, assign
@@ -4361,7 +4347,7 @@
return InputEventInjectionResult::FAILED;
}
- InjectionState* injectionState = new InjectionState(targetUid);
+ InjectionState* injectionState = new InjectionState(injectorPid, injectorUid);
if (syncMode == InputEventInjectionSync::NONE) {
injectionState->injectionIsAsync = true;
}
@@ -4433,7 +4419,8 @@
} // release lock
if (DEBUG_INJECTION) {
- ALOGD("injectInputEvent - Finished with result %d.", injectionResult);
+ ALOGD("injectInputEvent - Finished with result %d. injectorPid=%d, injectorUid=%d",
+ injectionResult, injectorPid, injectorUid);
}
return injectionResult;
@@ -4472,12 +4459,19 @@
return result;
}
+bool InputDispatcher::hasInjectionPermission(int32_t injectorPid, int32_t injectorUid) {
+ return injectorUid == 0 ||
+ mPolicy->checkInjectEventsPermissionNonReentrant(injectorPid, injectorUid);
+}
+
void InputDispatcher::setInjectionResult(EventEntry& entry,
InputEventInjectionResult injectionResult) {
InjectionState* injectionState = entry.injectionState;
if (injectionState) {
if (DEBUG_INJECTION) {
- ALOGD("Setting input event injection result to %d.", injectionResult);
+ ALOGD("Setting input event injection result to %d. "
+ "injectorPid=%d, injectorUid=%d",
+ injectionResult, injectionState->injectorPid, injectionState->injectorUid);
}
if (injectionState->injectionIsAsync && !(entry.policyFlags & POLICY_FLAG_FILTERED)) {
@@ -4486,12 +4480,12 @@
case InputEventInjectionResult::SUCCEEDED:
ALOGV("Asynchronous input event injection succeeded.");
break;
- case InputEventInjectionResult::TARGET_MISMATCH:
- ALOGV("Asynchronous input event injection target mismatch.");
- break;
case InputEventInjectionResult::FAILED:
ALOGW("Asynchronous input event injection failed.");
break;
+ case InputEventInjectionResult::PERMISSION_DENIED:
+ ALOGW("Asynchronous input event injection permission denied.");
+ break;
case InputEventInjectionResult::TIMED_OUT:
ALOGW("Asynchronous input event injection timed out.");
break;
diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h
index a9fb5c6..ca24f17 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.h
+++ b/services/inputflinger/dispatcher/InputDispatcher.h
@@ -104,7 +104,7 @@
void notifyPointerCaptureChanged(const NotifyPointerCaptureChangedArgs* args) override;
android::os::InputEventInjectionResult injectInputEvent(
- const InputEvent* event, std::optional<int32_t> targetUid,
+ const InputEvent* event, int32_t injectorPid, int32_t injectorUid,
android::os::InputEventInjectionSync syncMode, std::chrono::milliseconds timeout,
uint32_t policyFlags) override;
@@ -275,6 +275,7 @@
// Event injection and synchronization.
std::condition_variable mInjectionResultAvailable;
+ bool hasInjectionPermission(int32_t injectorPid, int32_t injectorUid);
void setInjectionResult(EventEntry& entry,
android::os::InputEventInjectionResult injectionResult);
void transformMotionEntryForInjectionLocked(MotionEntry&,
@@ -550,6 +551,8 @@
void addGlobalMonitoringTargetsLocked(std::vector<InputTarget>& inputTargets, int32_t displayId)
REQUIRES(mLock);
void pokeUserActivityLocked(const EventEntry& eventEntry) REQUIRES(mLock);
+ bool checkInjectionPermission(const sp<android::gui::WindowInfoHandle>& windowHandle,
+ const InjectionState* injectionState);
// Enqueue a drag event if needed, and update the touch state.
// Uses findTouchedWindowTargetsLocked to make the decision
void addDragEventLocked(const MotionEntry& entry) REQUIRES(mLock);
diff --git a/services/inputflinger/dispatcher/include/InputDispatcherInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherInterface.h
index 100bd18..67e1b6f 100644
--- a/services/inputflinger/dispatcher/include/InputDispatcherInterface.h
+++ b/services/inputflinger/dispatcher/include/InputDispatcherInterface.h
@@ -68,16 +68,10 @@
* input injection to proceed.
* Returns one of the INPUT_EVENT_INJECTION_XXX constants.
*
- * If a targetUid is provided, InputDispatcher will only consider injecting the input event into
- * windows owned by the provided uid. If the input event is targeted at a window that is not
- * owned by the provided uid, input injection will fail. If no targetUid is provided, the input
- * event will be dispatched as-is.
- *
- * This method may be called on any thread (usually by the input manager). The caller must
- * perform all necessary permission checks prior to injecting events.
+ * This method may be called on any thread (usually by the input manager).
*/
virtual android::os::InputEventInjectionResult injectInputEvent(
- const InputEvent* event, std::optional<int32_t> targetUid,
+ const InputEvent* event, int32_t injectorPid, int32_t injectorUid,
android::os::InputEventInjectionSync syncMode, std::chrono::milliseconds timeout,
uint32_t policyFlags) = 0;
diff --git a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h
index 575b3d7..de0b6da 100644
--- a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h
+++ b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h
@@ -125,6 +125,15 @@
/* Poke user activity for an event dispatched to a window. */
virtual void pokeUserActivity(nsecs_t eventTime, int32_t eventType, int32_t displayId) = 0;
+ /* Checks whether a given application pid/uid has permission to inject input events
+ * into other applications.
+ *
+ * This method is special in that its implementation promises to be non-reentrant and
+ * is safe to call while holding other locks. (Most other methods make no such guarantees!)
+ */
+ virtual bool checkInjectEventsPermissionNonReentrant(int32_t injectorPid,
+ int32_t injectorUid) = 0;
+
/* Notifies the policy that a pointer down event has occurred outside the current focused
* window.
*