Merge "host_init_verifier: add check for root services and linux capabilities"
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);