libbinder: RPC session ID uses the long binder ID
This is 'unguessable' (pending security review and constant time
compare). Right now, it's unclear if we'll go with full TLS for
on-device communication or use some other authentication scheme.
However, this is being used similarly to TLS session tickets.
Bug: 167966510
Test: binderRpcTest
Change-Id: I4c5edd2de6cc3f6ae37b0815e7f45c7a08bac2b1
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index ad377d3..3599427 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -270,14 +270,25 @@
return;
}
- if (header.sessionId == RPC_SESSION_ID_NEW) {
+ RpcAddress sessionId = RpcAddress::fromRawEmbedded(&header.sessionId);
+
+ if (sessionId.isZero()) {
if (reverse) {
ALOGE("Cannot create a new session with a reverse connection, would leak");
return;
}
- LOG_ALWAYS_FATAL_IF(server->mSessionIdCounter >= INT32_MAX, "Out of session IDs");
- server->mSessionIdCounter++;
+ RpcAddress sessionId = RpcAddress::zero();
+ size_t tries = 0;
+ do {
+ // don't block if there is some entropy issue
+ if (tries++ > 5) {
+ ALOGE("Cannot find new address: %s", sessionId.toString().c_str());
+ return;
+ }
+
+ sessionId = RpcAddress::random(true /*forServer*/);
+ } while (server->mSessions.end() != server->mSessions.find(sessionId));
session = RpcSession::make();
session->setMaxThreads(server->mMaxThreads);
@@ -285,16 +296,17 @@
sp<RpcServer::EventListener>::fromExisting(
static_cast<RpcServer::EventListener*>(
server.get())),
- server->mSessionIdCounter)) {
+ sessionId)) {
ALOGE("Failed to attach server to session");
return;
}
- server->mSessions[server->mSessionIdCounter] = session;
+ server->mSessions[sessionId] = session;
} else {
- auto it = server->mSessions.find(header.sessionId);
+ auto it = server->mSessions.find(sessionId);
if (it == server->mSessions.end()) {
- ALOGE("Cannot add thread, no record of session with ID %d", header.sessionId);
+ ALOGE("Cannot add thread, no record of session with ID %s",
+ sessionId.toString().c_str());
return;
}
session = it->second;
@@ -353,12 +365,14 @@
void RpcServer::onSessionLockedAllIncomingThreadsEnded(const sp<RpcSession>& session) {
auto id = session->mId;
LOG_ALWAYS_FATAL_IF(id == std::nullopt, "Server sessions must be initialized with ID");
- LOG_RPC_DETAIL("Dropping session %d", *id);
+ LOG_RPC_DETAIL("Dropping session with address %s", id->toString().c_str());
std::lock_guard<std::mutex> _l(mLock);
auto it = mSessions.find(*id);
- LOG_ALWAYS_FATAL_IF(it == mSessions.end(), "Bad state, unknown session id %d", *id);
- LOG_ALWAYS_FATAL_IF(it->second != session, "Bad state, session has id mismatch %d", *id);
+ LOG_ALWAYS_FATAL_IF(it == mSessions.end(), "Bad state, unknown session id %s",
+ id->toString().c_str());
+ LOG_ALWAYS_FATAL_IF(it->second != session, "Bad state, session has id mismatch %s",
+ id->toString().c_str());
(void)mSessions.erase(it);
}