From e0b8ccd1a3db7059c56322ad6cbcb7d7c34c2954 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Thu, 27 Oct 2016 08:21:35 -0700 Subject: logd: inherit android_get_control_file() Setup and then collect from the environment /proc/kmsg and /dev/kmsg file descriptors. Do not do so for logcat --reinit. Test: gTest logd-unit-tests, liblog-unit-tests and logcat-unit-tests Bug: 32450474 Change-Id: Ied537ca561fcd4e71a9ad9c57398a23ba23f6ced --- logd/logd.rc | 2 ++ logd/main.cpp | 44 ++++++++++++++++++++++++++++---------------- 2 files changed, 30 insertions(+), 16 deletions(-) (limited to 'logd') diff --git a/logd/logd.rc b/logd/logd.rc index 31ed4df34..5fc92e1b0 100644 --- a/logd/logd.rc +++ b/logd/logd.rc @@ -2,6 +2,8 @@ service logd /system/bin/logd socket logd stream 0666 logd logd socket logdr seqpacket 0666 logd logd socket logdw dgram 0222 logd logd + file /proc/kmsg r + file /dev/kmsg w group root system readproc writepid /dev/cpuset/system-background/tasks diff --git a/logd/main.cpp b/logd/main.cpp index 1ac1415ed..7c71e7d5c 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -189,11 +190,11 @@ static void *reinit_thread_start(void * /*obj*/) { set_sched_policy(0, SP_BACKGROUND); setpriority(PRIO_PROCESS, 0, ANDROID_PRIORITY_BACKGROUND); - // If we are AID_ROOT, we should drop to AID_SYSTEM, if we are anything - // else, we have even lesser privileges and accept our fate. Not worth - // checking for error returns setting this thread's privileges. - (void)setgid(AID_SYSTEM); - (void)setuid(AID_SYSTEM); + // If we are AID_ROOT, we should drop to AID_LOGD+AID_SYSTEM, if we are + // anything else, we have even lesser privileges and accept our fate. Not + // worth checking for error returns setting this thread's privileges. + (void)setgid(AID_SYSTEM); // readonly access to /data/system/packages.list + (void)setuid(AID_LOGD); // access to everything logd. while (reinit_running && !sem_wait(&reinit) && reinit_running) { @@ -318,17 +319,6 @@ static void readDmesg(LogAudit *al, LogKlog *kl) { // logging plugins like auditd and restart control. Additional // transitory per-client threads are created for each reader. int main(int argc, char *argv[]) { - int fdPmesg = -1; - bool klogd = __android_logger_property_get_bool("logd.kernel", - BOOL_DEFAULT_TRUE | - BOOL_DEFAULT_FLAG_PERSIST | - BOOL_DEFAULT_FLAG_ENG | - BOOL_DEFAULT_FLAG_SVELTE); - if (klogd) { - fdPmesg = open("/proc/kmsg", O_RDONLY | O_NDELAY); - } - fdDmesg = open("/dev/kmsg", O_WRONLY); - // issue reinit command. KISS argument parsing. if ((argc > 1) && argv[1] && !strcmp(argv[1], "--reinit")) { int sock = TEMP_FAILURE_RETRY( @@ -364,6 +354,28 @@ int main(int argc, char *argv[]) { return strncmp(buffer, success, sizeof(success) - 1) != 0; } + static const char dev_kmsg[] = "/dev/kmsg"; + fdDmesg = android_get_control_file(dev_kmsg); + if (fdDmesg < 0) { + fdDmesg = TEMP_FAILURE_RETRY(open(dev_kmsg, O_WRONLY | O_CLOEXEC)); + } + + int fdPmesg = -1; + bool klogd = __android_logger_property_get_bool("logd.kernel", + BOOL_DEFAULT_TRUE | + BOOL_DEFAULT_FLAG_PERSIST | + BOOL_DEFAULT_FLAG_ENG | + BOOL_DEFAULT_FLAG_SVELTE); + if (klogd) { + static const char proc_kmsg[] = "/proc/kmsg"; + fdPmesg = android_get_control_file(proc_kmsg); + if (fdPmesg < 0) { + fdPmesg = TEMP_FAILURE_RETRY(open(proc_kmsg, + O_RDONLY | O_NDELAY | O_CLOEXEC)); + } + if (fdPmesg < 0) android::prdebug("Failed to open %s\n", proc_kmsg); + } + // Reinit Thread sem_init(&reinit, 0, 0); sem_init(&uidName, 0, 0); -- cgit v1.2.3 From 5b44340acb9793945b97c4a9b6e5a1e9516ea33e Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Thu, 27 Oct 2016 08:34:12 -0700 Subject: logd: start logd services in logd gid logd - start as root:logd+system+readproc logd-reinit - start as logd:logd ToDo: start as logd:logd+system+readproc (libminijail) Test: gTest logd-unit-tests, liblog-unit-tests and logcat-unit-tests Bug: 32450474 Change-Id: I42c806ca1730a7f9eb9e34f064ae31a2ef9fc678 --- logd/logd.rc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'logd') diff --git a/logd/logd.rc b/logd/logd.rc index 5fc92e1b0..aecc4be47 100644 --- a/logd/logd.rc +++ b/logd/logd.rc @@ -4,10 +4,12 @@ service logd /system/bin/logd socket logdw dgram 0222 logd logd file /proc/kmsg r file /dev/kmsg w - group root system readproc + group logd system readproc writepid /dev/cpuset/system-background/tasks service logd-reinit /system/bin/logd --reinit oneshot disabled + user logd + group logd writepid /dev/cpuset/system-background/tasks -- cgit v1.2.3 From f0b8e1bce61e839d5f94fb0918423b0eda14c779 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Fri, 28 Oct 2016 14:49:53 -0700 Subject: logd: drop libminijail dependency Use libcap instead of libminijail. Set CAP_SETGID before setgroups, then clear it afterwards. Test: gTest logd-unit-tests, liblog-unit-tests and logcat-unit-tests Bug: 32450474 Change-Id: I2ed027fd5efd95f76b1dd4c5791bae5f2ea94c28 --- logd/Android.mk | 2 +- logd/main.cpp | 54 ++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 9 deletions(-) (limited to 'logd') diff --git a/logd/Android.mk b/logd/Android.mk index 81637d21b..7fe48d746 100644 --- a/logd/Android.mk +++ b/logd/Android.mk @@ -29,7 +29,7 @@ LOCAL_SHARED_LIBRARIES := \ libcutils \ libbase \ libpackagelistparser \ - libminijail + libcap # This is what we want to do: # event_logtags = $(shell \ diff --git a/logd/main.cpp b/logd/main.cpp index 7c71e7d5c..920b1bad2 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -41,12 +41,10 @@ #include #include #include -#include #include #include #include #include -#include #include #include "CommandListener.h" @@ -112,13 +110,53 @@ static int drop_privs() { return -1; } + if (prctl(PR_SET_KEEPCAPS, 1) < 0) { + android::prdebug("failed to set PR_SET_KEEPCAPS"); + return -1; + } + + std::unique_ptr caps(cap_init(), cap_free); + if (cap_clear(caps.get()) < 0) return -1; + cap_value_t cap_value[] = { + CAP_SETGID, // must be first for below + CAP_SYSLOG, + CAP_AUDIT_CONTROL + }; + if (cap_set_flag(caps.get(), CAP_PERMITTED, + arraysize(cap_value), cap_value, + CAP_SET) < 0) return -1; + if (cap_set_flag(caps.get(), CAP_EFFECTIVE, + arraysize(cap_value), cap_value, + CAP_SET) < 0) return -1; + if (cap_set_proc(caps.get()) < 0) { + android::prdebug("failed to set CAP_SETGID, CAP_SYSLOG or CAP_AUDIT_CONTROL (%d)", errno); + return -1; + } + gid_t groups[] = { AID_READPROC }; - ScopedMinijail j(minijail_new()); - minijail_set_supplementary_gids(j.get(), arraysize(groups), groups); - minijail_change_uid(j.get(), AID_LOGD); - minijail_change_gid(j.get(), AID_LOGD); - minijail_use_caps(j.get(), CAP_TO_MASK(CAP_SYSLOG) | CAP_TO_MASK(CAP_AUDIT_CONTROL)); - minijail_enter(j.get()); + + if (setgroups(arraysize(groups), groups) == -1) { + android::prdebug("failed to set AID_READPROC groups"); + return -1; + } + + if (setgid(AID_LOGD) != 0) { + android::prdebug("failed to set AID_LOGD gid"); + return -1; + } + + if (setuid(AID_LOGD) != 0) { + android::prdebug("failed to set AID_LOGD uid"); + return -1; + } + + if (cap_set_flag(caps.get(), CAP_PERMITTED, 1, cap_value, CAP_CLEAR) < 0) return -1; + if (cap_set_flag(caps.get(), CAP_EFFECTIVE, 1, cap_value, CAP_CLEAR) < 0) return -1; + if (cap_set_proc(caps.get()) < 0) { + android::prdebug("failed to clear CAP_SETGID (%d)", errno); + return -1; + } + return 0; } -- cgit v1.2.3 From d8f01807b8a49496256ccd75d49e0fd6be576424 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Mon, 31 Oct 2016 13:49:44 -0700 Subject: logd: drop capabilities in logd --reinit and logd.daemon Test: gTest logd-unit-tests, liblog-unit-tests and logcat-unit-tests Bug: 32450474 Change-Id: I842a7a64f0ba695acef66caf54270f9475c9f9ac --- logd/main.cpp | 70 +++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 31 deletions(-) (limited to 'logd') diff --git a/logd/main.cpp b/logd/main.cpp index 920b1bad2..770aa25c9 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -228,6 +228,11 @@ static void *reinit_thread_start(void * /*obj*/) { set_sched_policy(0, SP_BACKGROUND); setpriority(PRIO_PROCESS, 0, ANDROID_PRIORITY_BACKGROUND); + cap_t caps = cap_init(); + (void)cap_clear(caps); + (void)cap_set_proc(caps); + (void)cap_free(caps); + // If we are AID_ROOT, we should drop to AID_LOGD+AID_SYSTEM, if we are // anything else, we have even lesser privileges and accept our fate. Not // worth checking for error returns setting this thread's privileges. @@ -350,6 +355,39 @@ static void readDmesg(LogAudit *al, LogKlog *kl) { } } +static int issueReinit() { + cap_t caps = cap_init(); + (void)cap_clear(caps); + (void)cap_set_proc(caps); + (void)cap_free(caps); + + int sock = TEMP_FAILURE_RETRY( + socket_local_client("logd", + ANDROID_SOCKET_NAMESPACE_RESERVED, + SOCK_STREAM)); + if (sock < 0) return -errno; + + static const char reinitStr[] = "reinit"; + ssize_t ret = TEMP_FAILURE_RETRY(write(sock, reinitStr, sizeof(reinitStr))); + if (ret < 0) return -errno; + + struct pollfd p; + memset(&p, 0, sizeof(p)); + p.fd = sock; + p.events = POLLIN; + ret = TEMP_FAILURE_RETRY(poll(&p, 1, 1000)); + if (ret < 0) return -errno; + if ((ret == 0) || !(p.revents & POLLIN)) return -ETIME; + + static const char success[] = "success"; + char buffer[sizeof(success) - 1]; + memset(buffer, 0, sizeof(buffer)); + ret = TEMP_FAILURE_RETRY(read(sock, buffer, sizeof(buffer))); + if (ret < 0) return -errno; + + return strncmp(buffer, success, sizeof(success) - 1) != 0; +} + // Foreground waits for exit of the main persistent threads // that are started here. The threads are created to manage // UNIX domain client sockets for writing, reading and @@ -359,37 +397,7 @@ static void readDmesg(LogAudit *al, LogKlog *kl) { int main(int argc, char *argv[]) { // issue reinit command. KISS argument parsing. if ((argc > 1) && argv[1] && !strcmp(argv[1], "--reinit")) { - int sock = TEMP_FAILURE_RETRY( - socket_local_client("logd", - ANDROID_SOCKET_NAMESPACE_RESERVED, - SOCK_STREAM)); - if (sock < 0) { - return -errno; - } - static const char reinit[] = "reinit"; - ssize_t ret = TEMP_FAILURE_RETRY(write(sock, reinit, sizeof(reinit))); - if (ret < 0) { - return -errno; - } - struct pollfd p; - memset(&p, 0, sizeof(p)); - p.fd = sock; - p.events = POLLIN; - ret = TEMP_FAILURE_RETRY(poll(&p, 1, 1000)); - if (ret < 0) { - return -errno; - } - if ((ret == 0) || !(p.revents & POLLIN)) { - return -ETIME; - } - static const char success[] = "success"; - char buffer[sizeof(success) - 1]; - memset(buffer, 0, sizeof(buffer)); - ret = TEMP_FAILURE_RETRY(read(sock, buffer, sizeof(buffer))); - if (ret < 0) { - return -errno; - } - return strncmp(buffer, success, sizeof(success) - 1) != 0; + return issueReinit(); } static const char dev_kmsg[] = "/dev/kmsg"; -- cgit v1.2.3 From d2b3291ffa1cd9c2214b4a68d72508461de57e48 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Fri, 28 Oct 2016 15:11:46 -0700 Subject: logd: auditd + klogd control CAPS Test: gTest logd-unit-tests, liblog-unit-tests and logcat-unit-testsa Bug: 32450474 Change-Id: Icdaf9e352e86c9e140928509201da743004aeedb --- logd/main.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'logd') diff --git a/logd/main.cpp b/logd/main.cpp index 770aa25c9..99ad08023 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -89,7 +89,7 @@ // logd // -static int drop_privs() { +static int drop_privs(bool klogd, bool auditd) { struct sched_param param; memset(¶m, 0, sizeof(param)); @@ -119,8 +119,8 @@ static int drop_privs() { if (cap_clear(caps.get()) < 0) return -1; cap_value_t cap_value[] = { CAP_SETGID, // must be first for below - CAP_SYSLOG, - CAP_AUDIT_CONTROL + klogd ? CAP_SYSLOG : CAP_SETGID, + auditd ? CAP_AUDIT_CONTROL : CAP_SETGID }; if (cap_set_flag(caps.get(), CAP_PERMITTED, arraysize(cap_value), cap_value, @@ -444,7 +444,10 @@ int main(int argc, char *argv[]) { pthread_attr_destroy(&attr); } - if (drop_privs() != 0) { + bool auditd = __android_logger_property_get_bool("logd.auditd", + BOOL_DEFAULT_TRUE | + BOOL_DEFAULT_FLAG_PERSIST); + if (drop_privs(klogd, auditd) != 0) { return -1; } @@ -499,9 +502,6 @@ int main(int argc, char *argv[]) { // initiated log messages. New log entries are added to LogBuffer // and LogReader is notified to send updates to connected clients. - bool auditd = __android_logger_property_get_bool("logd.auditd", - BOOL_DEFAULT_TRUE | - BOOL_DEFAULT_FLAG_PERSIST); LogAudit *al = NULL; if (auditd) { al = new LogAudit(logBuf, reader, -- cgit v1.2.3 From 77fdb22cf686002dfba8a24cf36666d7257b3034 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Fri, 28 Oct 2016 14:36:36 -0700 Subject: logd: start logd service in logd uid Test: gTest logd-unit-tests, liblog-unit-tests and logcat-unit-tests Manual inspect grep '^Cap' /proc//status for correct capabilities Bug: 32450474 Change-Id: Ia6a3872901969a789d4309d410dbfd5f5d17b3ce --- logd/logd.rc | 1 + 1 file changed, 1 insertion(+) (limited to 'logd') diff --git a/logd/logd.rc b/logd/logd.rc index aecc4be47..54349dd67 100644 --- a/logd/logd.rc +++ b/logd/logd.rc @@ -4,6 +4,7 @@ service logd /system/bin/logd socket logdw dgram 0222 logd logd file /proc/kmsg r file /dev/kmsg w + user logd group logd system readproc writepid /dev/cpuset/system-background/tasks -- cgit v1.2.3 From 107e29ac1b1c297a0d4ee35c4978e79f47013e2c Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Fri, 28 Oct 2016 15:51:03 -0700 Subject: logd: if eng build, be a bit more permissive about failures Allows us some leaway to investigate logd issues on eng builds Test: gTests logd-unit-tests, liblog-unit-tests and logcat-unit-tests Manual on eng builds, bad logd.rc to fake permission issues Bug: 32450474 Change-Id: I432016e29e5601d67c502076ead941cecdcbebe7 --- logd/main.cpp | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) (limited to 'logd') diff --git a/logd/main.cpp b/logd/main.cpp index 99ad08023..d698976d0 100644 --- a/logd/main.cpp +++ b/logd/main.cpp @@ -90,29 +90,36 @@ // static int drop_privs(bool klogd, bool auditd) { + // Tricky, if ro.build.type is "eng" then this is true because of the + // side effect that ro.debuggable == 1 as well, else it is false. + bool eng = __android_logger_property_get_bool("ro.build.type", BOOL_DEFAULT_FALSE); + struct sched_param param; memset(¶m, 0, sizeof(param)); if (set_sched_policy(0, SP_BACKGROUND) < 0) { - return -1; + android::prdebug("failed to set background scheduling policy"); + if (!eng) return -1; } if (sched_setscheduler((pid_t) 0, SCHED_BATCH, ¶m) < 0) { - return -1; + android::prdebug("failed to set batch scheduler"); + if (!eng) return -1; } if (setpriority(PRIO_PROCESS, 0, ANDROID_PRIORITY_BACKGROUND) < 0) { - return -1; + android::prdebug("failed to set background cgroup"); + if (!eng) return -1; } - if (prctl(PR_SET_DUMPABLE, 0) < 0) { + if (!eng && (prctl(PR_SET_DUMPABLE, 0) < 0)) { android::prdebug("failed to clear PR_SET_DUMPABLE"); return -1; } if (prctl(PR_SET_KEEPCAPS, 1) < 0) { android::prdebug("failed to set PR_SET_KEEPCAPS"); - return -1; + if (!eng) return -1; } std::unique_ptr caps(cap_init(), cap_free); @@ -130,31 +137,31 @@ static int drop_privs(bool klogd, bool auditd) { CAP_SET) < 0) return -1; if (cap_set_proc(caps.get()) < 0) { android::prdebug("failed to set CAP_SETGID, CAP_SYSLOG or CAP_AUDIT_CONTROL (%d)", errno); - return -1; + if (!eng) return -1; } gid_t groups[] = { AID_READPROC }; if (setgroups(arraysize(groups), groups) == -1) { android::prdebug("failed to set AID_READPROC groups"); - return -1; + if (!eng) return -1; } if (setgid(AID_LOGD) != 0) { android::prdebug("failed to set AID_LOGD gid"); - return -1; + if (!eng) return -1; } if (setuid(AID_LOGD) != 0) { android::prdebug("failed to set AID_LOGD uid"); - return -1; + if (!eng) return -1; } if (cap_set_flag(caps.get(), CAP_PERMITTED, 1, cap_value, CAP_CLEAR) < 0) return -1; if (cap_set_flag(caps.get(), CAP_EFFECTIVE, 1, cap_value, CAP_CLEAR) < 0) return -1; if (cap_set_proc(caps.get()) < 0) { android::prdebug("failed to clear CAP_SETGID (%d)", errno); - return -1; + if (!eng) return -1; } return 0; -- cgit v1.2.3