DO NOT MERGE: Check system permission for tellMediatorAlive and tellDumpFinished CarWatchdog API calls.
Change-Id: Ia628577be4eafffcde663154da882f7b1e9c5897
Test: Tested with unit tests.
Bug: 171334071
diff --git a/watchdog/server/src/WatchdogBinderMediator.cpp b/watchdog/server/src/WatchdogBinderMediator.cpp
index 1dc6873..fd0dd8e 100644
--- a/watchdog/server/src/WatchdogBinderMediator.cpp
+++ b/watchdog/server/src/WatchdogBinderMediator.cpp
@@ -54,7 +54,7 @@
"%s or %s: Displays this help text.\n"
"When no options are specified, carwatchdog report is generated.\n";
-Status checkSystemPermission() {
+Status checkSystemUser() {
if (IPCThreadState::self()->getCallingUid() != AID_SYSTEM) {
return Status::fromExceptionCode(Status::EX_SECURITY,
"Calling process does not have proper privilege");
@@ -151,7 +151,7 @@
}
Status WatchdogBinderMediator::registerMediator(const sp<ICarWatchdogClient>& mediator) {
- Status status = checkSystemPermission();
+ Status status = checkSystemUser();
if (!status.isOk()) {
return status;
}
@@ -159,29 +159,47 @@
}
Status WatchdogBinderMediator::unregisterMediator(const sp<ICarWatchdogClient>& mediator) {
- Status status = checkSystemPermission();
+ Status status = checkSystemUser();
if (!status.isOk()) {
return status;
}
return mWatchdogProcessService->unregisterMediator(mediator);
}
Status WatchdogBinderMediator::registerMonitor(const sp<ICarWatchdogMonitor>& monitor) {
- Status status = checkSystemPermission();
+ Status status = checkSystemUser();
if (!status.isOk()) {
return status;
}
return mWatchdogProcessService->registerMonitor(monitor);
}
Status WatchdogBinderMediator::unregisterMonitor(const sp<ICarWatchdogMonitor>& monitor) {
- Status status = checkSystemPermission();
+ Status status = checkSystemUser();
if (!status.isOk()) {
return status;
}
return mWatchdogProcessService->unregisterMonitor(monitor);
}
+Status WatchdogBinderMediator::tellMediatorAlive(const sp<ICarWatchdogClient>& mediator,
+ const std::vector<int32_t>& clientsNotResponding,
+ int32_t sessionId) {
+ Status status = checkSystemUser();
+ if (!status.isOk()) {
+ return status;
+ }
+ return mWatchdogProcessService->tellMediatorAlive(mediator, clientsNotResponding, sessionId);
+}
+Status WatchdogBinderMediator::tellDumpFinished(const android::sp<ICarWatchdogMonitor>& monitor,
+ int32_t pid) {
+ Status status = checkSystemUser();
+ if (!status.isOk()) {
+ return status;
+ }
+ return mWatchdogProcessService->tellDumpFinished(monitor, pid);
+}
+
Status WatchdogBinderMediator::notifySystemStateChange(StateType type, int32_t arg1, int32_t arg2) {
- Status status = checkSystemPermission();
+ Status status = checkSystemUser();
if (!status.isOk()) {
return status;
}
diff --git a/watchdog/server/src/WatchdogBinderMediator.h b/watchdog/server/src/WatchdogBinderMediator.h
index 530659b..dbc26a3 100644
--- a/watchdog/server/src/WatchdogBinderMediator.h
+++ b/watchdog/server/src/WatchdogBinderMediator.h
@@ -61,14 +61,9 @@
}
binder::Status tellMediatorAlive(const sp<ICarWatchdogClient>& mediator,
const std::vector<int32_t>& clientsNotResponding,
- int32_t sessionId) override {
- return mWatchdogProcessService->tellMediatorAlive(mediator, clientsNotResponding,
- sessionId);
- }
+ int32_t sessionId) override;
binder::Status tellDumpFinished(const android::sp<ICarWatchdogMonitor>& monitor,
- int32_t pid) override {
- return mWatchdogProcessService->tellDumpFinished(monitor, pid);
- }
+ int32_t pid) override;
binder::Status notifySystemStateChange(StateType type, int32_t arg1, int32_t arg2) override;
protected:
diff --git a/watchdog/server/tests/WatchdogBinderMediatorTest.cpp b/watchdog/server/tests/WatchdogBinderMediatorTest.cpp
index a32dcca..7e44893 100644
--- a/watchdog/server/tests/WatchdogBinderMediatorTest.cpp
+++ b/watchdog/server/tests/WatchdogBinderMediatorTest.cpp
@@ -270,7 +270,16 @@
ASSERT_TRUE(status.isOk()) << status;
}
+TEST_F(WatchdogBinderMediatorTest, TestErrorOnTellMediatorAliveWithNonSystemCallingUid) {
+ sp<ICarWatchdogClient> mediator = new MockICarWatchdogClient();
+ std::vector clientsNotResponding = {123};
+ EXPECT_CALL(*mMockWatchdogProcessService, tellMediatorAlive(_, _, _)).Times(0);
+ Status status = mWatchdogBinderMediator->tellMediatorAlive(mediator, clientsNotResponding, 456);
+ ASSERT_FALSE(status.isOk()) << status;
+}
+
TEST_F(WatchdogBinderMediatorTest, TestTellMediatorAlive) {
+ setSystemCallingUid();
sp<ICarWatchdogClient> mediator = new MockICarWatchdogClient();
std::vector clientsNotResponding = {123};
EXPECT_CALL(*mMockWatchdogProcessService,
@@ -280,7 +289,15 @@
ASSERT_TRUE(status.isOk()) << status;
}
+TEST_F(WatchdogBinderMediatorTest, TestErrorOnTellDumpFinishedWithNonSystemCallingUid) {
+ sp<ICarWatchdogMonitor> monitor = new MockICarWatchdogMonitor();
+ EXPECT_CALL(*mMockWatchdogProcessService, tellDumpFinished(_, _)).Times(0);
+ Status status = mWatchdogBinderMediator->tellDumpFinished(monitor, 456);
+ ASSERT_FALSE(status.isOk()) << status;
+}
+
TEST_F(WatchdogBinderMediatorTest, TestTellDumpFinished) {
+ setSystemCallingUid();
sp<ICarWatchdogMonitor> monitor = new MockICarWatchdogMonitor();
EXPECT_CALL(*mMockWatchdogProcessService, tellDumpFinished(monitor, 456))
.WillOnce(Return(Status::ok()));