From d2d320ab0b442a887a0ef2457d28ab4cdd214f28 Mon Sep 17 00:00:00 2001
From: Pawan Wagh <waghpawan@google.com>
Date: Mon, 15 May 2023 22:25:55 +0000
Subject: [PATCH] Return error from gatekeeperd

Instead of crashing the gatekeeperd on incorrect
inputs to AIDL interface, return errors from the
service.

Test: m gatekeeperd, booted device
Bug: 279970163
Change-Id: Ifd3330e749f4ce147db5886f1f2dbb00c322bed2
---
 gatekeeperd/gatekeeperd.cpp | 41 +++++++++++++++++++++++++++++--------
 gatekeeperd/gatekeeperd.h   |  2 +-
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/gatekeeperd/gatekeeperd.cpp b/gatekeeperd/gatekeeperd.cpp
index e5241b584..798716766 100644
--- a/gatekeeperd/gatekeeperd.cpp
+++ b/gatekeeperd/gatekeeperd.cpp
@@ -144,14 +144,22 @@ void GateKeeperProxy::clear_sid(uint32_t userId) {
     }
 }
 
-uint32_t GateKeeperProxy::adjust_userId(uint32_t userId) {
+Status GateKeeperProxy::adjust_userId(uint32_t userId, uint32_t* hw_userId) {
     static constexpr uint32_t kGsiOffset = 1000000;
-    CHECK(userId < kGsiOffset);
-    CHECK((aidl_hw_device != nullptr) || (hw_device != nullptr));
-    if (is_running_gsi) {
-        return userId + kGsiOffset;
+    if (userId >= kGsiOffset) {
+        return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT);
     }
-    return userId;
+
+    if ((aidl_hw_device == nullptr) && (hw_device == nullptr)) {
+        return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE);
+    }
+
+    if (is_running_gsi) {
+        *hw_userId = userId + kGsiOffset;
+        return Status::ok();
+    }
+    *hw_userId = userId;
+    return Status::ok();
 }
 
 #define GK_ERROR *gkResponse = GKResponse::error(), Status::ok()
@@ -201,7 +209,12 @@ Status GateKeeperProxy::enroll(int32_t userId,
     android::hardware::hidl_vec<uint8_t> newPwd;
     newPwd.setToExternal(const_cast<uint8_t*>(desiredPassword.data()), desiredPassword.size());
 
-    uint32_t hw_userId = adjust_userId(userId);
+    uint32_t hw_userId = 0;
+    Status result = adjust_userId(userId, &hw_userId);
+    if (!result.isOk()) {
+        return result;
+    }
+
     uint64_t secureUserId = 0;
     if (aidl_hw_device) {
         // AIDL gatekeeper service
@@ -300,7 +313,12 @@ Status GateKeeperProxy::verifyChallenge(int32_t userId, int64_t challenge,
         }
     }
 
-    uint32_t hw_userId = adjust_userId(userId);
+    uint32_t hw_userId = 0;
+    Status result = adjust_userId(userId, &hw_userId);
+    if (!result.isOk()) {
+        return result;
+    }
+
     android::hardware::hidl_vec<uint8_t> curPwdHandle;
     curPwdHandle.setToExternal(const_cast<uint8_t*>(enrolledPasswordHandle.data()),
                                enrolledPasswordHandle.size());
@@ -410,7 +428,12 @@ Status GateKeeperProxy::clearSecureUserId(int32_t userId) {
     }
     clear_sid(userId);
 
-    uint32_t hw_userId = adjust_userId(userId);
+    uint32_t hw_userId = 0;
+    Status result = adjust_userId(userId, &hw_userId);
+    if (!result.isOk()) {
+        return result;
+    }
+
     if (aidl_hw_device) {
         aidl_hw_device->deleteUser(hw_userId);
     } else if (hw_device) {
diff --git a/gatekeeperd/gatekeeperd.h b/gatekeeperd/gatekeeperd.h
index 29873da29..b1f08c611 100644
--- a/gatekeeperd/gatekeeperd.h
+++ b/gatekeeperd/gatekeeperd.h
@@ -47,7 +47,7 @@ class GateKeeperProxy : public BnGateKeeperService {
 
     // This should only be called on userIds being passed to the GateKeeper HAL. It ensures that
     // secure storage shared across a GSI image and a host image will not overlap.
-    uint32_t adjust_userId(uint32_t userId);
+    Status adjust_userId(uint32_t userId, uint32_t* hw_userId);
 
 #define GK_ERROR *gkResponse = GKResponse::error(), Status::ok()