Avoid GK memory leak

GetAuthTokenKey is intended to retain ownership of the returned key. To
follow this, we cache the result of speaking to keymaster.

Bug: 129768470
Bug: 140434850
Bug: 131618642
Change-Id: I36965eed01296c0f13d93f9dac7520bdc8006fb3

2 files changed, 40 insertions, 17 deletions

diff --git a/trusty_gatekeeper.cpp b/trusty_gatekeeper.cpp
index 3d5d717..45448a8 100644
--- a/trusty_gatekeeper.cpp
+++ b/trusty_gatekeeper.cpp
@@ -43,7 +43,8 @@ namespace gatekeeper {
 static const uint8_t DERIVATION_DATA[HMAC_SHA_256_KEY_SIZE] =
         "TrustyGateKeeperDerivationData0";
 
-TrustyGateKeeper::TrustyGateKeeper() : GateKeeper() {
+TrustyGateKeeper::TrustyGateKeeper() : GateKeeper(),
+    cached_auth_token_key_len_(0) {
     rng_initialized_ = false;
     calls_since_reseed_ = 0;
     num_mem_records_ = 0;
@@ -114,31 +115,42 @@ void TrustyGateKeeper::ClearMasterKey() {
     master_key_.reset();
 }
 
+/*
+ * While the GetAuthTokenKey header file says this value cannot be cached,
+ * after consulting with the GK/KM team this is incorrect - this is a per-boot
+ * key, and so in-memory caching is acceptable.
+ */
 bool TrustyGateKeeper::GetAuthTokenKey(const uint8_t** auth_token_key,
                                        uint32_t* length) const {
     *length = 0;
-    *auth_token_key = NULL;
+    *auth_token_key = nullptr;
 
-    long rc = keymaster_open();
-    if (rc < 0) {
-        return false;
-    }
+    if (!cached_auth_token_key_) {
+        long rc = keymaster_open();
+        if (rc < 0) {
+            return false;
+        }
 
-    keymaster_session_t session = (keymaster_session_t)rc;
+        keymaster_session_t session = (keymaster_session_t)rc;
 
-    uint8_t* key = NULL;
-    uint32_t local_length = 0;
+        uint8_t* key = nullptr;
+        uint32_t local_length = 0;
 
-    rc = keymaster_get_auth_token_key(session, &key, &local_length);
-    keymaster_close(session);
+        rc = keymaster_get_auth_token_key(session, &key, &local_length);
+        keymaster_close(session);
 
-    if (rc == NO_ERROR) {
-        *auth_token_key = key;
-        *length = local_length;
-        return true;
-    } else {
-        return false;
+        if (rc == NO_ERROR) {
+            cached_auth_token_key_.reset(key);
+            cached_auth_token_key_len_ = local_length;
+        } else {
+            return false;
+        }
     }
+
+    *auth_token_key = cached_auth_token_key_.get();
+    *length = cached_auth_token_key_len_;
+
+    return true;
 }
 
 void TrustyGateKeeper::GetPasswordKey(const uint8_t** password_key,
diff --git a/trusty_gatekeeper.h b/trusty_gatekeeper.h
index 5d45f50..4decf46 100644
--- a/trusty_gatekeeper.h
+++ b/trusty_gatekeeper.h
@@ -26,6 +26,13 @@
 
 namespace gatekeeper {
 
+template <typename T>
+struct FreeDeleter {
+    inline void operator()(T* p) const {
+        free(p);
+    }
+};
+
 class TrustyGateKeeper : public GateKeeper {
 public:
     TrustyGateKeeper();
@@ -93,6 +100,10 @@ private:
 
     int num_mem_records_;
     UniquePtr<failure_record_t[]> mem_records_;
+
+    mutable UniquePtr<uint8_t, FreeDeleter<uint8_t>>
+            cached_auth_token_key_;
+    mutable size_t cached_auth_token_key_len_;
 };
 
 }  // namespace gatekeeper