commit 01a6bad2e1441c4ec89d6157dc663cb43c6d9cf9
author Steven Moreland <smoreland@google.com> Fri Jun 11 00:59:20 2021 +0000
committer Steven Moreland <smoreland@google.com> Sat Jun 12 00:22:10 2021 +0000
tree a669a7d880d669a23d509ac27a357868119da2b8
parent 915382439c1db7768c48cbecb235bfc57a9b6437

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

libs/binder/RpcSession.cpp

diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index f953a05..931a876 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -218,18 +218,17 @@
         LOG_ALWAYS_FATAL_IF(mForServer != nullptr, "Can only update ID for client.");
     }
 
-    int32_t id;
-
     ExclusiveConnection connection;
     status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this),
                                                 ConnectionUse::CLIENT, &connection);
     if (status != OK) return status;
 
-    status = state()->getSessionId(connection.get(), sp<RpcSession>::fromExisting(this), &id);
+    mId = RpcAddress::zero();
+    status = state()->getSessionId(connection.get(), sp<RpcSession>::fromExisting(this),
+                                   &mId.value());
     if (status != OK) return status;
 
-    LOG_RPC_DETAIL("RpcSession %p has id %d", this, id);
-    mId = id;
+    LOG_RPC_DETAIL("RpcSession %p has id %s", this, mId->toString().c_str());
     return OK;
 }
 
@@ -329,7 +328,7 @@
                             mOutgoingConnections.size());
     }
 
-    if (!setupOneSocketConnection(addr, RPC_SESSION_ID_NEW, false /*reverse*/)) return false;
+    if (!setupOneSocketConnection(addr, RpcAddress::zero(), false /*reverse*/)) return false;
 
     // TODO(b/189955605): we should add additional sessions dynamically
     // instead of all at once.
@@ -366,7 +365,8 @@
     return true;
 }
 
-bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t id, bool reverse) {
+bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, const RpcAddress& id,
+                                          bool reverse) {
     for (size_t tries = 0; tries < 5; tries++) {
         if (tries > 0) usleep(10000);
 
@@ -390,9 +390,9 @@
             return false;
         }
 
-        RpcConnectionHeader header{
-                .sessionId = id,
-        };
+        RpcConnectionHeader header{.options = 0};
+        memcpy(&header.sessionId, &id.viewRawEmbedded(), sizeof(RpcWireAddress));
+
         if (reverse) header.options |= RPC_CONNECTION_OPTION_REVERSE;
 
         if (sizeof(header) != TEMP_FAILURE_RETRY(write(serverFd.get(), &header, sizeof(header)))) {
@@ -469,7 +469,7 @@
 }
 
 bool RpcSession::setForServer(const wp<RpcServer>& server, const wp<EventListener>& eventListener,
-                              int32_t sessionId) {
+                              const RpcAddress& sessionId) {
     LOG_ALWAYS_FATAL_IF(mForServer != nullptr);
     LOG_ALWAYS_FATAL_IF(server == nullptr);
     LOG_ALWAYS_FATAL_IF(mEventListener != nullptr);