From: Michal Bloch <m.bloch@samsung.com>
Date: Tue, 11 Jun 2024 13:00:52 +0000 (+0200)
Subject: Document redundant send/recv checks
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=fb45796b14888b6146b38992d7a92950a9031d04;p=platform%2Fcore%2Fsystem%2Flibdbuspolicy.git

Document redundant send/recv checks

Change-Id: I641c4a60dc5d3b3ad08ab73dd923562e0d1cf59d
---

diff --git a/src/libdbuspolicy1.cpp b/src/libdbuspolicy1.cpp
index 514354a..d20bf00 100644
--- a/src/libdbuspolicy1.cpp
+++ b/src/libdbuspolicy1.cpp
@@ -399,9 +399,39 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_check_out(void* configuration,
 	if (DecisionResult::ALLOW != decision)
 		return decisionToRetCode(decision);
 
-	KdbusConnectionInfo senderInfo(kconn->conn);
-	/* check can recv */
-	/* get sender information from kdbus */
+	/* We checked if we can send the message, but we also need to check if it will be able
+	 * to be received on the other side. This looks like a duplicate check (since receiving
+	 * also performs both checks) but it is done for security reasons.
+	 *
+	 * Consider this example ruleset. It implements a rule that the system overseer,
+	 * and only it, can order hibernation from the freezer service.
+	 *
+	 *  deny send member='hibernate'
+	 * allow send member='hibernate' destination='freezer'
+	 *  deny recv member='hibernate'
+	 * allow recv member='hibernate' sender='overseer'
+	 *
+	 *  ┌──────────┐       ┌─────────┐
+	 *  │ overseer ├──OK──>│ freezer │
+	 *  └─┬────────┘       └─────────┘
+	 *    │                   ^
+	 *    │                   │
+	 *    │     ┌──────bad────┘
+	 *    └─bad─┼────────────┐
+	 *          │            │
+	 *          │            v
+	 *        ┌─┴─┐        ┌───┐
+	 *        │ X │        │ Y │
+	 *        └───┘        └───┘
+	 *
+	 * Now:
+	 * - if receiver didn't check sender, freezer could end up taking orders from some X instead of only overseer.
+	 * - if sender didn't check receiver, overseer could end up talking to some Y instead of only freezer.
+	 *
+	 * So unfortunately it has to be done redundantly. */
+
+	KdbusConnectionInfo senderInfo(kconn->conn); // get sender info from kdbus
+
 	r = prepareNames(sender, senderInfo, KdbusConnection::POLICY_CONN_INFO_NAME);
 	if (r < 0)
 		return r;
@@ -443,6 +473,8 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_check_in(void* configuration,
 	if (DecisionResult::ALLOW != decision)
 		return decisionToRetCode(decision);
 
+	// Check for a 'send' match as well, see the send function above for rationale
+
 	if (!sender)
 		sender = ":";