summaryrefslogtreecommitdiff
path: root/libs/binder/RpcState.cpp
diff options
context:
space:
mode:
author Steven Moreland <smoreland@google.com> 2021-06-10 23:35:35 +0000
committer Steven Moreland <smoreland@google.com> 2021-06-12 00:22:10 +0000
commit915382439c1db7768c48cbecb235bfc57a9b6437 (patch)
tree93d00fc64c4a223d1c197aa19c2cee293e06c3fd /libs/binder/RpcState.cpp
parent7b8bc4c6997c45c8f896e16537f7abc61fe3ac2e (diff)
libbinder: RPC prevent binder address collision
It's not clear that we're going to continue using such large addresses (they are expensive, and we don't actually need addresses themselves to be cryptographically unguessable - also since we are reading from urandom, we're using a lot of entropy - ...), but just in case, clearing out TODOs (and also in preparation of using RpcAddress for something else, which we probably will continue using). This prevents address collision by doing two things: - create a bitspace for server vs client addresses (one bit in the address, in a newly defined header, determines the originating side of the connection for the address). - instead of aborting when a duplicated address is created, try to create a new one. As a side-effect, this also adds a header to binder RPC addresses. Bug: 182939933 Test: binderRpcTest Change-Id: I8ff0d29ca6df25b3f1d9662978fccbb3eb76c8ad
Diffstat (limited to 'libs/binder/RpcState.cpp')
-rw-r--r--libs/binder/RpcState.cpp54
1 files changed, 43 insertions, 11 deletions
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 967610977d..15eec20d1d 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -83,21 +83,45 @@ status_t RpcState::onBinderLeaving(const sp<RpcSession>& session, const sp<IBind
}
LOG_ALWAYS_FATAL_IF(isRpc, "RPC binder must have known address at this point");
- auto&& [it, inserted] = mNodeForAddress.insert({RpcAddress::unique(),
- BinderNode{
- .binder = binder,
- .timesSent = 1,
- .sentRef = binder,
- }});
- // TODO(b/182939933): better organization could avoid needing this log
- LOG_ALWAYS_FATAL_IF(!inserted);
-
- *outAddress = it->first;
- return OK;
+ bool forServer = session->server() != nullptr;
+
+ for (size_t tries = 0; tries < 5; tries++) {
+ auto&& [it, inserted] = mNodeForAddress.insert({RpcAddress::random(forServer),
+ BinderNode{
+ .binder = binder,
+ .timesSent = 1,
+ .sentRef = binder,
+ }});
+ if (inserted) {
+ *outAddress = it->first;
+ return OK;
+ }
+
+ // well, we don't have visibility into the header here, but still
+ static_assert(sizeof(RpcWireAddress) == 40, "this log needs updating");
+ ALOGW("2**256 is 1e77. If you see this log, you probably have some entropy issue, or maybe "
+ "you witness something incredible!");
+ }
+
+ ALOGE("Unable to create an address in order to send out %p", binder.get());
+ return WOULD_BLOCK;
}
status_t RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcAddress& address,
sp<IBinder>* out) {
+ // ensure that: if we want to use addresses for something else in the future (for
+ // instance, allowing transitive binder sends), that we don't accidentally
+ // send those addresses to old server. Accidentally ignoring this in that
+ // case and considering the binder to be recognized could cause this
+ // process to accidentally proxy transactions for that binder. Of course,
+ // if we communicate with a binder, it could always be proxying
+ // information. However, we want to make sure that isn't done on accident
+ // by a client.
+ if (!address.isRecognizedType()) {
+ ALOGE("Address is of an unknown type, rejecting: %s", address.toString().c_str());
+ return BAD_VALUE;
+ }
+
std::unique_lock<std::mutex> _l(mNodeMutex);
if (mTerminated) return DEAD_OBJECT;
@@ -117,6 +141,14 @@ status_t RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcAddr
return OK;
}
+ // we don't know about this binder, so the other side of the connection
+ // should have created it.
+ if (address.isForServer() == !!session->server()) {
+ ALOGE("Server received unrecognized address which we should own the creation of %s.",
+ address.toString().c_str());
+ return BAD_VALUE;
+ }
+
auto&& [it, inserted] = mNodeForAddress.insert({address, BinderNode{}});
LOG_ALWAYS_FATAL_IF(!inserted, "Failed to insert binder when creating proxy");