Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 01a6bad2 authored by Steven Moreland's avatar Steven Moreland
Browse files

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
parent 91538243

libs/binder/RpcServer.cpp

+24 −10
Original line number Diff line number Diff line
@@ -270,14 +270,25 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie 
            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 @@ void RpcServer::establishConnection(sp<RpcServer>&& server, base::unique_fd clie 
                                       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 @@ bool RpcServer::setupSocketServer(const RpcSocketAddress& addr) { 
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);

}

libs/binder/RpcSession.cpp

+11 −11
Original line number Diff line number Diff line
@@ -218,18 +218,17 @@ status_t RpcSession::readId() { 
        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 @@ bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) { 
                            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 @@ bool RpcSession::setupSocketClient(const RpcSocketAddress& addr) { 
    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 @@ bool RpcSession::setupOneSocketConnection(const RpcSocketAddress& addr, int32_t 
            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::addOutgoingConnection(unique_fd fd) { 
}



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);

libs/binder/RpcState.cpp

+5 −10
Original line number Diff line number Diff line
@@ -369,7 +369,7 @@ status_t RpcState::getMaxThreads(const sp<RpcSession::RpcConnection>& connection 
}



status_t RpcState::getSessionId(const sp<RpcSession::RpcConnection>& connection,

                                const sp<RpcSession>& session, int32_t* sessionIdOut) {

                                const sp<RpcSession>& session, RpcAddress* sessionIdOut) {

    Parcel data;

    data.markForRpc(session);

    Parcel reply;
@@ -382,12 +382,7 @@ status_t RpcState::getSessionId(const sp<RpcSession::RpcConnection>& connection, 
        return status;

    }



    int32_t sessionId;

    status = reply.readInt32(&sessionId);

    if (status != OK) return status;



    *sessionIdOut = sessionId;

    return OK;

    return sessionIdOut->readFromParcel(reply);

}



status_t RpcState::transact(const sp<RpcSession::RpcConnection>& connection,
@@ -767,9 +762,9 @@ processTransactInternalTailCall: 
                }

                case RPC_SPECIAL_TRANSACT_GET_SESSION_ID: {

                    // for client connections, this should always report the value

                    // originally returned from the server

                    int32_t id = session->mId.value();

                    replyStatus = reply.writeInt32(id);

                    // originally returned from the server, so this is asserting

                    // that it exists

                    replyStatus = session->mId.value().writeToParcel(&reply);

                    break;

                }

                default: {

libs/binder/RpcState.h

+1 −1
Original line number Diff line number Diff line
@@ -62,7 +62,7 @@ public: 
    status_t getMaxThreads(const sp<RpcSession::RpcConnection>& connection,

                           const sp<RpcSession>& session, size_t* maxThreadsOut);

    status_t getSessionId(const sp<RpcSession::RpcConnection>& connection,

                          const sp<RpcSession>& session, int32_t* sessionIdOut);

                          const sp<RpcSession>& session, RpcAddress* sessionIdOut);



    [[nodiscard]] status_t transact(const sp<RpcSession::RpcConnection>& connection,

                                    const sp<IBinder>& address, uint32_t code, const Parcel& data,

libs/binder/RpcWireFormat.h

+10 −12
Original line number Diff line number Diff line
@@ -20,20 +20,26 @@ namespace android { 
#pragma clang diagnostic push

#pragma clang diagnostic error "-Wpadded"



constexpr int32_t RPC_SESSION_ID_NEW = -1;



enum : uint8_t {

    RPC_CONNECTION_OPTION_REVERSE = 0x1,

};



constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_CREATED = 1 << 0; // distinguish from '0' address

constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_FOR_SERVER = 1 << 1;



struct RpcWireAddress {

    uint64_t options;

    uint8_t address[32];

};



/**

 * This is sent to an RpcServer in order to request a new connection is created,

 * either as part of a new session or an existing session

 */

struct RpcConnectionHeader {

    int32_t sessionId;

    RpcWireAddress sessionId;

    uint8_t options;

    uint8_t reserved[3];

    uint8_t reserved[7];

};



#define RPC_CONNECTION_INIT_OKAY "cci"
@@ -89,14 +95,6 @@ struct RpcWireHeader { 
    uint32_t reserved[2];

};



constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_CREATED = 1 << 0; // distinguish from '0' address

constexpr uint64_t RPC_WIRE_ADDRESS_OPTION_FOR_SERVER = 1 << 1;



struct RpcWireAddress {

    uint64_t options;

    uint8_t address[32];

};



struct RpcWireTransaction {

    RpcWireAddress address;

    uint32_t code;