host_init_verifier: add check for root services and linux capabilities
If a service that runs under root doesn't have the capabilities field in
it's definition, then it will inherit all the capabilities that init
has.
This change adds a linter to detect such services and ask developers to
explicitly specify capabilities that their service needs. If service
doesn't require any capabilities then empty capabilities fields should
be added in the service definition.
The actual access control list on what capabilities a process can use is
controlled by the SELinux, so inheriting all the init capabilities is
not a security issue here. However, asking services to explicitly
specify the capabilities they need is a good defense-in-depth mechanism.
So far this linter only checks the services on /system partition.
All currently offending services are added to the exempt list. I will
work on fixing some of them in the follow-up changes.
Bug: 249796710
Test: m dist
Change-Id: I2db06af165ae320a9c5086756067dceef20cd28d
diff --git a/init/host_init_verifier.cpp b/init/host_init_verifier.cpp
index db127d3..d015ae9 100644
--- a/init/host_init_verifier.cpp
+++ b/init/host_init_verifier.cpp
@@ -22,6 +22,7 @@
#include <stdio.h>
#include <stdlib.h>
+#include <cstdlib>
#include <fstream>
#include <iostream>
#include <iterator>
@@ -216,6 +217,80 @@
}
}
+bool CheckServiceCapabilities(const ServiceList& service_list,
+ const std::set<std::string>& system_services) {
+ static const std::set<std::string> kExemptList = {
+ "apexd",
+ "apexd-bootstrap",
+ "apexd-snapshotde",
+ "adbd",
+ "boottrace",
+ "boringssl_self_test32",
+ "boringssl_self_test64",
+ "boringssl_self_test_apex32",
+ "boringssl_self_test_apex64",
+ "bsplogstart",
+ "bugreportd",
+ "charger",
+ "clear-bcb",
+ "composd",
+ "dumpstate",
+ "dumpstatez",
+ "fastbootd",
+ "gsid",
+ "installd",
+ "mmedialogstart",
+ "mobile_log_d",
+ // Yes, it's contorl, not control :(
+ "mobile_log_d_contorl",
+ "mobile_log_d_sublog_config",
+ "odsign",
+ "profcollectd",
+ "recovery",
+ "recovery-console",
+ "servicemanager",
+ "setup-bcb",
+ "snapuserd",
+ "snapuserd_proxy",
+ "sysproxyd",
+ "trace_buf_off",
+ "ueventd",
+ "uncrypt",
+ "update_engine",
+ "update_verifier",
+ "update_verifier_nonencrypted",
+ "usbd",
+ "vold",
+ "zygote",
+ "zygote_secondary",
+ };
+ bool found_error = false;
+ for (const auto& service : service_list) {
+ if (service->uid() != 0) {
+ continue;
+ }
+ // TODO(b/249796710): enable this linter for other partitions as well
+ if (system_services.count(service->name()) == 0) {
+ LOG(DEBUG) << "Skipping capabilities check for '" << service->name()
+ << "' because it doesn't belong to system partition";
+ continue;
+ }
+ if (!service->capabilities().has_value() && kExemptList.count(service->name()) == 0) {
+ LOG(ERROR) << "Service '" << service->name() << "' (defined in " << service->filename()
+ << ") runs under 'root' user but does not "
+ << "specify capabiltiies it needs. This will result in service inheriting "
+ "all the "
+ << "capabilities that 'init' has. Please explicitly specify the "
+ "capabilities that '"
+ << service->name()
+ << "' need. If it doesn't need any capabilities then leave the "
+ "'capabilities' field empty.";
+ found_error = true;
+ }
+ }
+ return !found_error;
+}
+
int main(int argc, char** argv) {
android::base::InitLogging(argv, &android::base::StdioLogger);
android::base::SetMinimumLogSeverity(android::base::ERROR);
@@ -319,11 +394,17 @@
parser.AddSectionParser("on", std::make_unique<ActionParser>(&am, GetSubcontext()));
parser.AddSectionParser("import", std::make_unique<HostImportParser>());
+ std::set<std::string> system_services;
if (!partition_map.empty()) {
for (const auto& p : partition_search_order) {
if (partition_map.find(p) != partition_map.end()) {
parser.ParseConfig(partition_map.at(p) + "etc/init");
}
+ if (p == "system") {
+ for (const auto& service : ServiceList::GetInstance()) {
+ system_services.insert(service->name());
+ }
+ }
}
} else {
if (!parser.ParseConfigFileInsecure(*argv)) {
@@ -336,6 +417,9 @@
LOG(ERROR) << "Failed to parse init scripts with " << failures << " error(s).";
return EXIT_FAILURE;
}
+ if (!CheckServiceCapabilities(sl, system_services)) {
+ return EXIT_FAILURE;
+ }
return EXIT_SUCCESS;
}
diff --git a/init/service.h b/init/service.h
index f9749d2..9cc2920 100644
--- a/init/service.h
+++ b/init/service.h
@@ -145,6 +145,8 @@
const std::string& filename() const { return filename_; }
void set_filename(const std::string& name) { filename_ = name; }
+ const std::optional<CapSet>& capabilities() const { return capabilities_; }
+
private:
void NotifyStateChange(const std::string& new_state) const;
void StopOrReset(int how);