Replace the loopback port and abstract socket with pathname sockets - #264
Merged
Conversation
…ds cannot reach them The daemon exposes two local control channels -- the WebUI's key-management endpoint (KeyAdmin) and the daemon<->interceptor config channel -- and both were bound on transports any uid on the device can reach. KeyAdmin listened on `127.0.0.1:8790`; a loopback TCP port has no owner, so authentication can only sit above the transport and the socket itself stays open for anyone to connect and speak to. The config channel bound the [abstract unix name](https://man7.org/linux/man-pages/man7/unix.7.html) `\0teesim`; the abstract namespace ignores filesystem permissions and is a flat global address space, so any process can address it, and even its existence leaks through `connect()` -- a bound name is refused with `EACCES`, an unbound one with `ECONNREFUSED`, which separates "present" from "absent" without any access at all. A pathname socket instead inherits the access control of the directory that holds it. Under a directory whose search bit is denied to unprivileged uids the path cannot be traversed, so `connect()` returns `EACCES` whether or not the node exists -- nothing to enumerate, and nothing to tell apart from an empty directory. KeyAdmin now binds `/data/adb/teesim/admin.sock` inside the module's 0700 directory. A WebView cannot open a unix socket, so the WebUI reaches it through the manager's root shell and a small HTTP-over-UDS client (`teesim-uds`, built per abi and staged at a fixed path); the request and response wire format is unchanged, only the socket underneath moved. The endpoint keeps its random admin token and now also refuses any peer whose uid is not root at `accept`. The config channel (`common/control.cpp`) now binds `/data/misc/keystore/.teesim-ctl`. It has to live in keystore's own directory rather than the module's: the interceptor binds from inside keystore (uid 1017, which cannot traverse root's 0700 `/data/adb`), and `/data/misc/keystore` is a 0700 directory keystore owns. sepolicy grants keystore the `sock_file` create and bind and the root daemon the traversal and connect; the existing `SO_PEERCRED` root check on the server is untouched.
… behind Moving the two control channels onto filesystem sockets updated the code but not the prose around it. The daemon<->interceptor channel was still documented as the abstract name `@teesim` in control.h, control.cpp, both entry points, App.kt and three READMEs, and KeyAdmin was still described as a loopback listener in the webroot transport seam and in KeyAdmin's own body-size comment -- neither address exists any more, so a reader following the comments would look for a socket that was never bound. Each now names the control socket, or the root-only socket, and lets Const.kt and control.cpp be the single places a path is written down. Control.kt also spelled keystore's uid as the literal 1017 next to the constant that already holds it, so it now reads Const.AID_KEYSTORE. That constant's own doc says where the name comes from: AOSP's AID_KEYSTORE in android_filesystem_config.h, mirrored here because the framework's Process.KEYSTORE_UID is @hide and the public SDK exposes only the root/system/phone/shell uids and the application-uid bounds. Comments and documentation only; no behaviour change.
JingMatrix
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The daemon exposes two local control channels -- the WebUI's key-management endpoint (KeyAdmin) and the daemon<->interceptor config channel -- and both were bound on transports any uid on the device can reach. KeyAdmin listened on
127.0.0.1:8790; a loopback TCP port has no owner, so authentication can only sit above the transport and the socket itself stays open for anyone to connect and speak to. The config channel bound the abstract unix name\0teesim; the abstract namespace ignores filesystem permissions and is a flat global address space, so any process can address it, and even its existence leaks throughconnect()-- a bound name is refused withEACCES, an unbound one withECONNREFUSED, which separates "present" from "absent" without any access at all.A pathname socket instead inherits the access control of the directory that holds it. Under a directory whose search bit is denied to unprivileged uids the path cannot be traversed, so
connect()returnsEACCESwhether or not the node exists -- nothing to enumerate, and nothing to tell apart from an empty directory.KeyAdmin now binds
/data/adb/teesim/admin.sockinside the module's 0700 directory. A WebView cannot open a unix socket, so the WebUI reaches it through the manager's root shell and a small HTTP-over-UDS client (teesim-uds, built per abi and staged at a fixed path); the request and response wire format is unchanged, only the socket underneath moved. The endpoint keeps its random admin token and now also refuses any peer whose uid is not root ataccept.The config channel (
common/control.cpp) now binds/data/misc/keystore/.teesim-ctl. It has to live in keystore's own directory rather than the module's: the interceptor binds from inside keystore (uid 1017, which cannot traverse root's 0700/data/adb), and/data/misc/keystoreis a 0700 directory keystore owns. sepolicy grants keystore thesock_filecreate and bind and the root daemon the traversal and connect; the existingSO_PEERCREDroot check on the server is untouched.