binder: Delete addTrustedPeerCertificate.
For RpcServer and RpcSession, caller should retain
an ownership of the verifier when creating
RpcTransportCtxFactory, so APIs to addTrustedPeerCertificate
are deleted.
The logic to add certificates should be in the implementation
of RpcCertificateVerifier. See follow-up CLs.
Test: binderRpcTest
Bug: 198833574
Change-Id: If0ed87adfeee2ee48582604881ee335b5d689589
diff --git a/libs/binder/RpcServer.cpp b/libs/binder/RpcServer.cpp
index ad9ba96..ba723e6 100644
--- a/libs/binder/RpcServer.cpp
+++ b/libs/binder/RpcServer.cpp
@@ -144,15 +144,6 @@
return mCtx->getCertificate(format);
}
-status_t RpcServer::addTrustedPeerCertificate(CertificateFormat format, std::string_view cert) {
- std::lock_guard<std::mutex> _l(mLock);
- // Ensure that join thread is not running or shutdown trigger is not set up. In either case,
- // it means there are child threads running. It is invalid to add trusted peer certificates
- // after join thread and/or child threads are running to avoid race condition.
- if (mJoinThreadRunning || mShutdownTrigger != nullptr) return INVALID_OPERATION;
- return mCtx->addTrustedPeerCertificate(format, cert);
-}
-
static void joinRpcServer(sp<RpcServer>&& thiz) {
thiz->join();
}
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index c57b749..8da3fa3 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -64,23 +64,12 @@
sp<RpcSession> RpcSession::make() {
// Default is without TLS.
- return make(RpcTransportCtxFactoryRaw::make(), std::nullopt, std::nullopt);
+ return make(RpcTransportCtxFactoryRaw::make());
}
-sp<RpcSession> RpcSession::make(std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory,
- std::optional<CertificateFormat> serverCertificateFormat,
- std::optional<std::string> serverCertificate) {
+sp<RpcSession> RpcSession::make(std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory) {
auto ctx = rpcTransportCtxFactory->newClientCtx();
if (ctx == nullptr) return nullptr;
- LOG_ALWAYS_FATAL_IF(serverCertificateFormat.has_value() != serverCertificate.has_value());
- if (serverCertificateFormat.has_value() && serverCertificate.has_value()) {
- status_t status =
- ctx->addTrustedPeerCertificate(*serverCertificateFormat, *serverCertificate);
- if (status != OK) {
- ALOGE("Cannot add trusted server certificate: %s", statusToString(status).c_str());
- return nullptr;
- }
- }
return sp<RpcSession>::make(std::move(ctx));
}
diff --git a/libs/binder/RpcTransportRaw.cpp b/libs/binder/RpcTransportRaw.cpp
index 930df12..62c9530 100644
--- a/libs/binder/RpcTransportRaw.cpp
+++ b/libs/binder/RpcTransportRaw.cpp
@@ -112,7 +112,6 @@
return std::make_unique<RpcTransportRaw>(std::move(fd));
}
std::string getCertificate(CertificateFormat) const override { return {}; }
- status_t addTrustedPeerCertificate(CertificateFormat, std::string_view) override { return OK; }
};
} // namespace
diff --git a/libs/binder/RpcTransportTls.cpp b/libs/binder/RpcTransportTls.cpp
index bcf3254..e9a494f 100644
--- a/libs/binder/RpcTransportTls.cpp
+++ b/libs/binder/RpcTransportTls.cpp
@@ -450,7 +450,6 @@
std::unique_ptr<RpcTransport> newTransport(android::base::unique_fd fd,
FdTrigger* fdTrigger) const override;
std::string getCertificate(CertificateFormat) const override;
- status_t addTrustedPeerCertificate(CertificateFormat, std::string_view cert) override;
protected:
virtual void preHandshake(Ssl* ssl) const = 0;
@@ -462,11 +461,6 @@
return {};
}
-status_t RpcTransportCtxTls::addTrustedPeerCertificate(CertificateFormat, std::string_view) {
- // TODO(b/195166979): set certificate here
- return OK;
-}
-
// Common implementation for creating server and client contexts. The child class, |Impl|, is
// provided as a template argument so that this function can initialize an |Impl| object.
template <typename Impl, typename>
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index d0e4e27..da1f79b 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -139,12 +139,6 @@
std::string getCertificate(CertificateFormat);
/**
- * See RpcTransportCtx::addTrustedPeerCertificate.
- * Thread-safe. This is only possible before the server is join()-ing.
- */
- status_t addTrustedPeerCertificate(CertificateFormat, std::string_view cert);
-
- /**
* Runs join() in a background thread. Immediately returns.
*/
void start();
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index d92af0a..a40116f 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -57,9 +57,7 @@
// Create an RpcSession with the given configuration. |serverCertificateFormat| and
// |serverCertificate| must have values or be nullopt simultaneously. If they have values, set
// server certificate.
- static sp<RpcSession> make(std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory,
- std::optional<CertificateFormat> serverCertificateFormat,
- std::optional<std::string> serverCertificate);
+ static sp<RpcSession> make(std::unique_ptr<RpcTransportCtxFactory> rpcTransportCtxFactory);
/**
* Set the maximum number of threads allowed to be made (for things like callbacks).
diff --git a/libs/binder/include/binder/RpcTransport.h b/libs/binder/include/binder/RpcTransport.h
index 9e6faf6..da132a1 100644
--- a/libs/binder/include/binder/RpcTransport.h
+++ b/libs/binder/include/binder/RpcTransport.h
@@ -75,18 +75,6 @@
// - For TLS, this returns the certificate. See RpcTransportTls for details.
[[nodiscard]] virtual std::string getCertificate(CertificateFormat format) const = 0;
- // Add a trusted peer certificate. Peers presenting this certificate are accepted.
- //
- // Caller must ensure that newTransport() are called after all trusted peer certificates
- // are added. Otherwise, RpcTransport-s created before may not trust peer certificates
- // added later.
- //
- // Implementation details:
- // - For raw sockets, this always returns OK.
- // - For TLS, this adds trusted peer certificate. See RpcTransportTls for details.
- [[nodiscard]] virtual status_t addTrustedPeerCertificate(CertificateFormat format,
- std::string_view cert) = 0;
-
protected:
RpcTransportCtx() = default;
};
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 8a03de2..dbf8899 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -529,8 +529,7 @@
status_t status;
for (size_t i = 0; i < options.numSessions; i++) {
- sp<RpcSession> session =
- RpcSession::make(newFactory(rpcSecurity), std::nullopt, std::nullopt);
+ sp<RpcSession> session = RpcSession::make(newFactory(rpcSecurity));
session->setMaxThreads(options.numIncomingConnections);
switch (socketType) {
@@ -1215,8 +1214,7 @@
}
server->start();
- sp<RpcSession> session =
- RpcSession::make(RpcTransportCtxFactoryRaw::make(), std::nullopt, std::nullopt);
+ sp<RpcSession> session = RpcSession::make(RpcTransportCtxFactoryRaw::make());
status_t status = session->setupVsockClient(VMADDR_CID_LOCAL, vsockPort);
while (!server->shutdown()) usleep(10000);
ALOGE("Detected vsock loopback supported: %s", statusToString(status).c_str());
diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
index 7fd9f6b..8bf04cc 100644
--- a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
+++ b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
@@ -36,8 +36,7 @@
void fillRandomParcel(Parcel* p, FuzzedDataProvider&& provider) {
if (provider.ConsumeBool()) {
- auto session =
- RpcSession::make(RpcTransportCtxFactoryRaw::make(), std::nullopt, std::nullopt);
+ auto session = RpcSession::make(RpcTransportCtxFactoryRaw::make());
CHECK_EQ(OK, session->addNullDebuggingClient());
p->markForRpc(session);
fillRandomParcelData(p, std::move(provider));