From 8fa8896d2ed97eb274c62f0e386dabf2e2a82a45 Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Tue, 26 Jan 2016 14:32:35 -0800 Subject: [PATCH] logd: security buffer only AID_SYSTEM reader - limit AID_SYSTEM uid or gid to read security buffer messages - adjust liblog tests to reflect the reality of this adjustment To fully test all security buffer paths and modes $ su 0,0,0 /data/nativetest/liblog-unit-tests/liblog-unit-tests --gtest_filter=liblog.__security* $ su 1000,1000,1000 /data/nativetest/liblog-unit-tests/liblog-unit-tests --gtest_filter=liblog.__security* $ su 2000,2000,2000 /data/nativetest/liblog-unit-tests/liblog-unit-tests --gtest_filter=liblog.__security* ToDo: Integrate the above individually into the gTest Q/A testing Bug: 26029733 Change-Id: Idcf5492db78fa6934ef6fb43f3ef861052675651 --- liblog/tests/liblog_test.cpp | 52 +++++++++++++++++++++++++++++++++++- logd/FlushCommand.cpp | 8 ++++++ logd/FlushCommand.h | 1 + logd/LogBuffer.cpp | 7 ++++- logd/LogBuffer.h | 2 +- logd/LogReader.cpp | 1 + logd/LogTimes.cpp | 5 ++-- 7 files changed, 71 insertions(+), 5 deletions(-) diff --git a/liblog/tests/liblog_test.cpp b/liblog/tests/liblog_test.cpp index a936455f3..8517c9f89 100644 --- a/liblog/tests/liblog_test.cpp +++ b/liblog/tests/liblog_test.cpp @@ -18,6 +18,8 @@ #include #include #include +#include +#include #include #include @@ -25,6 +27,7 @@ #include #include #include +#include #include // enhanced version of LOG_FAILURE_RETRY to add support for EAGAIN and @@ -368,6 +371,48 @@ TEST(liblog, __security_buffer) { return; } + /* Matches clientHasLogCredentials() in logd */ + uid_t uid = getuid(); + gid_t gid = getgid(); + bool clientHasLogCredentials = true; + if ((uid != AID_SYSTEM) && (uid != AID_ROOT) && (uid != AID_LOG) + && (gid != AID_SYSTEM) && (gid != AID_ROOT) && (gid != AID_LOG)) { + uid_t euid = geteuid(); + if ((euid != AID_SYSTEM) && (euid != AID_ROOT) && (euid != AID_LOG)) { + gid_t egid = getegid(); + if ((egid != AID_SYSTEM) && (egid != AID_ROOT) && (egid != AID_LOG)) { + int num_groups = getgroups(0, NULL); + if (num_groups > 0) { + gid_t groups[num_groups]; + num_groups = getgroups(num_groups, groups); + while (num_groups > 0) { + if (groups[num_groups - 1] == AID_LOG) { + break; + } + --num_groups; + } + } + if (num_groups <= 0) { + clientHasLogCredentials = false; + } + } + } + } + if (!clientHasLogCredentials) { + fprintf(stderr, "WARNING: " + "not in system context, bypassing end-to-end test\n"); + + log_time ts(CLOCK_MONOTONIC); + + buffer.type = EVENT_TYPE_LONG; + buffer.data = *(static_cast((void *)&ts)); + + // expect failure! + ASSERT_GE(0, __android_log_security_bwrite(0, &buffer, sizeof(buffer))); + + return; + } + pid_t pid = getpid(); ASSERT_TRUE(NULL != (logger_list = android_logger_list_open( @@ -415,7 +460,12 @@ TEST(liblog, __security_buffer) { android_logger_list_close(logger_list); - EXPECT_EQ(1, count); + bool clientHasSecurityCredentials = (uid == AID_SYSTEM) || (gid == AID_SYSTEM); + if (!clientHasSecurityCredentials) { + fprintf(stderr, "WARNING: " + "not system, content submitted but can not check end-to-end\n"); + } + EXPECT_EQ(clientHasSecurityCredentials ? 1 : 0, count); } diff --git a/logd/FlushCommand.cpp b/logd/FlushCommand.cpp index 48036d340..fd45c4a0a 100644 --- a/logd/FlushCommand.cpp +++ b/logd/FlushCommand.cpp @@ -93,3 +93,11 @@ void FlushCommand::runSocketCommand(SocketClient *client) { bool FlushCommand::hasReadLogs(SocketClient *client) { return clientHasLogCredentials(client); } + +static bool clientHasSecurityCredentials(SocketClient *client) { + return (client->getUid() == AID_SYSTEM) || (client->getGid() == AID_SYSTEM); +} + +bool FlushCommand::hasSecurityLogs(SocketClient *client) { + return clientHasSecurityCredentials(client); +} diff --git a/logd/FlushCommand.h b/logd/FlushCommand.h index e0f2212be..92247739a 100644 --- a/logd/FlushCommand.h +++ b/logd/FlushCommand.h @@ -45,6 +45,7 @@ public: virtual void runSocketCommand(SocketClient *client); static bool hasReadLogs(SocketClient *client); + static bool hasSecurityLogs(SocketClient *client); }; #endif diff --git a/logd/LogBuffer.cpp b/logd/LogBuffer.cpp index 9e0d4515b..8c30f79cb 100644 --- a/logd/LogBuffer.cpp +++ b/logd/LogBuffer.cpp @@ -907,7 +907,8 @@ unsigned long LogBuffer::getSize(log_id_t id) { } uint64_t LogBuffer::flushTo( - SocketClient *reader, const uint64_t start, bool privileged, + SocketClient *reader, const uint64_t start, + bool privileged, bool security, int (*filter)(const LogBufferElement *element, void *arg), void *arg) { LogBufferElementCollection::iterator it; uint64_t max = start; @@ -938,6 +939,10 @@ uint64_t LogBuffer::flushTo( continue; } + if (!security && (element->getLogId() == LOG_ID_SECURITY)) { + continue; + } + if (element->getSequence() <= start) { continue; } diff --git a/logd/LogBuffer.h b/logd/LogBuffer.h index 03739c7eb..7e9923606 100644 --- a/logd/LogBuffer.h +++ b/logd/LogBuffer.h @@ -111,7 +111,7 @@ public: uid_t uid, pid_t pid, pid_t tid, const char *msg, unsigned short len); uint64_t flushTo(SocketClient *writer, const uint64_t start, - bool privileged, + bool privileged, bool security, int (*filter)(const LogBufferElement *element, void *arg) = NULL, void *arg = NULL); diff --git a/logd/LogReader.cpp b/logd/LogReader.cpp index c2d65b601..667a3f249 100644 --- a/logd/LogReader.cpp +++ b/logd/LogReader.cpp @@ -163,6 +163,7 @@ bool LogReader::onDataAvailable(SocketClient *cli) { logbuf().isMonotonic() && android::isMonotonic(start)); logbuf().flushTo(cli, sequence, FlushCommand::hasReadLogs(cli), + FlushCommand::hasSecurityLogs(cli), logFindStart.callback, &logFindStart); if (!logFindStart.found()) { diff --git a/logd/LogTimes.cpp b/logd/LogTimes.cpp index b4c97a985..a4b96d33d 100644 --- a/logd/LogTimes.cpp +++ b/logd/LogTimes.cpp @@ -126,6 +126,7 @@ void *LogTimeEntry::threadStart(void *obj) { LogBuffer &logbuf = me->mReader.logbuf(); bool privileged = FlushCommand::hasReadLogs(client); + bool security = FlushCommand::hasSecurityLogs(client); me->leadingDropped = true; @@ -150,10 +151,10 @@ void *LogTimeEntry::threadStart(void *obj) { unlock(); if (me->mTail) { - logbuf.flushTo(client, start, privileged, FilterFirstPass, me); + logbuf.flushTo(client, start, privileged, security, FilterFirstPass, me); me->leadingDropped = true; } - start = logbuf.flushTo(client, start, privileged, FilterSecondPass, me); + start = logbuf.flushTo(client, start, privileged, security, FilterSecondPass, me); lock();