Add commands for clearing NVRAM on full reset. am: 0b833364a3 am: 84cff3f1a4 am: c537226ff1

am: b463b5ad37

Change-Id: Iddfe922ad4042b92824fcf40395b7da357e5c938

7 files changed, 280 insertions, 6 deletions

diff --git a/core/Android.mk b/core/Android.mk
index 4da770c..6a705ea 100644
--- a/core/Android.mk
+++ b/core/Android.mk
@@ -35,7 +35,7 @@ LOCAL_SRC_FILES := \
 	crypto_boringssl.cpp \
 	nvram_manager.cpp \
 	persistence.cpp
-LOCAL_CFLAGS := -Wall -Werror -Wextra
+LOCAL_CFLAGS := -Wall -Werror -Wextra -DNVRAM_WIPE_STORAGE_SUPPORT
 LOCAL_CLANG := true
 LOCAL_C_INCLUDES := $(LOCAL_PATH)/include
 LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
diff --git a/core/include/nvram/core/nvram_manager.h b/core/include/nvram/core/nvram_manager.h
index 97ee424..ac03e09 100644
--- a/core/include/nvram/core/nvram_manager.h
+++ b/core/include/nvram/core/nvram_manager.h
@@ -56,6 +56,33 @@ class NvramManager {
   nvram_result_t LockSpaceRead(const LockSpaceReadRequest& request,
                                LockSpaceReadResponse* response);
 
+  // The wipe functions are meant for use by firmware after determining the
+  // device's mode of operation. These can be used to clear access-controlled
+  // NVRAM when a user invokes a full hardware reset. Note that in regular
+  // operation, the user *MUST BE PREVENTED* from wiping access-controlled
+  // NVRAM.
+  //
+  // If a full hardware reset can conveniently clear the access-controlled NVRAM
+  // storage area out of band, it's fine to do so. In this case, the
+  // wiping-related commands should not be exposed at all. Note that this is the
+  // default behavior - the reference implementation will ignore all wipe
+  // requests unless compiled with NVRAM_WIPE_STORAGE_SUPPORT=1.
+  //
+  // For devices where firmware doesn't have direct control over the storage
+  // area used by access-controlled NVRAM, the wiping commands are provided to
+  // facilitate clearing storage:
+  //   1. Determine boot mode.
+  //   2. If not in recovery mode, call DisableWipe(). All further wipe requests
+  //      will be rejected. A reboot (or TEE restart for that matter) is
+  //      required before a new decision can be made.
+  //   3. If operating in recovery mode, forgo calling DisableWipe(). The
+  //      recovery process will then be able to invoke WipeStorage() later as
+  //      needed.
+  nvram_result_t WipeStorage(const WipeStorageRequest& request,
+                             WipeStorageResponse* response);
+  nvram_result_t DisableWipe(const DisableWipeRequest& request,
+                             DisableWipeResponse* response);
+
  private:
   // Holds transient state corresponding to an allocated NVRAM space, i.e. meta
   // data valid for a single boot. One instance of this struct is kept in memory
@@ -116,6 +143,7 @@ class NvramManager {
 
   bool initialized_ = false;
   bool disable_create_ = false;
+  bool disable_wipe_ = false;
 
   // Bookkeeping information for allocated spaces.
   size_t num_spaces_ = 0;
diff --git a/core/nvram_manager.cpp b/core/nvram_manager.cpp
index a1eb9c1..9ea3d65 100644
--- a/core/nvram_manager.cpp
+++ b/core/nvram_manager.cpp
@@ -135,6 +135,14 @@ void NvramManager::Dispatch(const nvram::Request& request,
       result = LockSpaceRead(*input.get<COMMAND_LOCK_SPACE_READ>(),
                              &output->Activate<COMMAND_LOCK_SPACE_READ>());
       break;
+    case nvram::COMMAND_WIPE_STORAGE:
+      result = WipeStorage(*input.get<COMMAND_WIPE_STORAGE>(),
+                           &output->Activate<COMMAND_WIPE_STORAGE>());
+      break;
+    case nvram::COMMAND_DISABLE_WIPE:
+      result = DisableWipe(*input.get<COMMAND_DISABLE_WIPE>(),
+                           &output->Activate<COMMAND_DISABLE_WIPE>());
+      break;
   }
 
   response->result = result;
@@ -161,6 +169,7 @@ nvram_result_t NvramManager::GetInfo(const GetInfoRequest& /* request */,
   for (size_t i = 0; i < num_spaces_; ++i) {
     space_list[i] = spaces_[i].index;
   }
+  response->wipe_disabled = disable_wipe_;
 
   return NV_RESULT_SUCCESS;
 }
@@ -501,6 +510,76 @@ nvram_result_t NvramManager::LockSpaceRead(
   return NV_RESULT_INVALID_PARAMETER;
 }
 
+nvram_result_t NvramManager::WipeStorage(
+    const WipeStorageRequest& /* request */,
+    WipeStorageResponse* /* response */) {
+  if (!Initialize())
+    return NV_RESULT_INTERNAL_ERROR;
+
+#ifdef NVRAM_WIPE_STORAGE_SUPPORT
+  if (disable_wipe_) {
+    return NV_RESULT_OPERATION_DISABLED;
+  }
+
+  // Go through all spaces and wipe the corresponding data. Note that the header
+  // is only updated once all space data is gone. This will "break" all spaces
+  // that are left declared but don't have data. This situation can be observed
+  // if we crash somewhere during the wiping process before clearing the header.
+  //
+  // Note that we deliberately choose this wiping sequence so we can never end
+  // up in a state where the header appears clean but existing space data
+  // remains.
+  //
+  // As a final note, the ideal solution would be to atomically clear the header
+  // and delete all space data. While more desirable from an operational point
+  // of view, this would drastically complicate storage layer requirements to
+  // support cross-object atomicity instead of per-object atomicity.
+  for (size_t i = 0; i < num_spaces_; ++i) {
+    const uint32_t index = spaces_[i].index;
+    switch (persistence::DeleteSpace(index)) {
+      case storage::Status::kStorageError:
+        NVRAM_LOG_ERR("Failed to wipe space 0x%" PRIx32 " data.", index);
+        return NV_RESULT_INTERNAL_ERROR;
+      case storage::Status::kNotFound:
+        // The space was missing even if it shouldn't have been. This may occur
+        // if a previous wiping attempt was aborted half-way. Log an error, but
+        // return success as we're in the desired state.
+        NVRAM_LOG_WARN("Space 0x%" PRIx32 " data missing on wipe.", index);
+        break;
+      case storage::Status::kSuccess:
+        break;
+    }
+  }
+
+  // All spaces are gone, clear the header.
+  num_spaces_ = 0;
+  return WriteHeader(Optional<uint32_t>());
+#else  // NVRAM_WIPE_STORAGE_SUPPORT
+  // We're not accessing the flag member, so prevent a compiler warning. The
+  // alternative of conditionally including the member in the class declaration
+  // looks cleaner at first sight, but comes with the risk of
+  // NVRAM_WIPE_STORAGE_SUPPORT polarity mismatches between compilation units,
+  // which is more subtly dangerous, so we rather keep the member even for the
+  // case in which it is not used.
+  (void)disable_wipe_;
+  return NV_RESULT_OPERATION_DISABLED;
+#endif  // NVRAM_WIPE_STORAGE_SUPPORT
+}
+
+nvram_result_t NvramManager::DisableWipe(
+    const DisableWipeRequest& /* request */,
+    DisableWipeResponse* /* response */) {
+  if (!Initialize())
+    return NV_RESULT_INTERNAL_ERROR;
+
+#ifdef NVRAM_WIPE_STORAGE_SUPPORT
+  disable_wipe_ = true;
+  return NV_RESULT_SUCCESS;
+#else  // NVRAM_WIPE_STORAGE_SUPPORT
+  return NV_RESULT_OPERATION_DISABLED;
+#endif  // NVRAM_WIPE_STORAGE_SUPPORT
+}
+
 nvram_result_t NvramManager::SpaceRecord::CheckWriteAccess(
     const Blob& authorization_value) {
   if (persistent.HasControl(NV_CONTROL_PERSISTENT_WRITE_LOCK)) {
diff --git a/core/rules.mk b/core/rules.mk
index ed2f0a4..2afd570 100644
--- a/core/rules.mk
+++ b/core/rules.mk
@@ -38,6 +38,10 @@ ifneq ($(NVRAM_LOG_LEVEL),)
 GLOBAL_DEFINES += NVRAM_LOG_LEVEL=$(NVRAM_LOG_LEVEL)
 endif
 
+ifneq ($(NVRAM_WIPE_STORAGE_SUPPORT),)
+GLOBAL_DEFINES += NVRAM_WIPE_STORAGE_SUPPORT=$(NVRAM_WIPE_STORAGE_SUPPORT)
+endif
+
 GLOBAL_INCLUDES += $(LOCAL_DIR)/include/
 
 include make/module.mk
diff --git a/core/tests/nvram_manager_test.cpp b/core/tests/nvram_manager_test.cpp
index b42f26a..8b00440 100644
--- a/core/tests/nvram_manager_test.cpp
+++ b/core/tests/nvram_manager_test.cpp
@@ -1106,5 +1106,126 @@ TEST_F(NvramManagerTest, LockSpaceRead_NotLockable) {
       nvram.LockSpaceRead(lock_space_read_request, &lock_space_read_response));
 }
 
+TEST_F(NvramManagerTest, WipeStorage_Success) {
+  // Set up an NVRAM space.
+  NvramSpace space;
+  ASSERT_TRUE(space.contents.Resize(10));
+  ASSERT_EQ(storage::Status::kSuccess, persistence::StoreSpace(17, space));
+  SetupHeader(NvramHeader::kVersion, 17);
+
+  // Check that the space is visible.
+  NvramManager nvram;
+  GetSpaceInfoRequest get_space_info_request;
+  get_space_info_request.index = 17;
+  GetSpaceInfoResponse get_space_info_response;
+  EXPECT_EQ(NV_RESULT_SUCCESS, nvram.GetSpaceInfo(get_space_info_request,
+                                                  &get_space_info_response));
+  EXPECT_EQ(10U, get_space_info_response.size);
+
+  // Request a wipe.
+  WipeStorageRequest wipe_storage_request;
+  WipeStorageResponse wipe_storage_response;
+  EXPECT_EQ(NV_RESULT_SUCCESS,
+            nvram.WipeStorage(wipe_storage_request, &wipe_storage_response));
+
+  // The space should no longer be declared.
+  GetInfoRequest get_info_request;
+  GetInfoResponse get_info_response;
+  EXPECT_EQ(NV_RESULT_SUCCESS,
+            nvram.GetInfo(get_info_request, &get_info_response));
+  EXPECT_EQ(0U, get_info_response.space_list.size());
+
+  // Accessing the space should fail.
+  EXPECT_EQ(
+      NV_RESULT_SPACE_DOES_NOT_EXIST,
+      nvram.GetSpaceInfo(get_space_info_request, &get_space_info_response));
+}
+
+TEST_F(NvramManagerTest, WipeStorage_Abort) {
+  // Set up two pre-existing spaces and a matching header.
+  NvramSpace space;
+  ASSERT_TRUE(space.contents.Resize(10));
+  ASSERT_EQ(storage::Status::kSuccess, persistence::StoreSpace(1, space));
+  ASSERT_TRUE(space.contents.Resize(20));
+  ASSERT_EQ(storage::Status::kSuccess, persistence::StoreSpace(2, space));
+  NvramHeader header;
+  header.version = NvramHeader::kVersion;
+  ASSERT_TRUE(header.allocated_indices.Resize(2));
+  header.allocated_indices[0] = 1;
+  header.allocated_indices[1] = 2;
+  ASSERT_EQ(storage::Status::kSuccess, persistence::StoreHeader(header));
+
+  // Check that the spaces are visible.
+  NvramManager nvram;
+  GetInfoRequest get_info_request;
+  GetInfoResponse get_info_response;
+  EXPECT_EQ(NV_RESULT_SUCCESS,
+            nvram.GetInfo(get_info_request, &get_info_response));
+  EXPECT_EQ(2U, get_info_response.space_list.size());
+  int space_mask = 0;
+  for (size_t i = 0; i < get_info_response.space_list.size(); ++i) {
+    space_mask |= (1 << get_info_response.space_list[i]);
+  }
+  EXPECT_EQ(0x6, space_mask);
+
+  // Set things up so the deletion request for the second space fails.
+  storage::SetSpaceWriteError(2, true);
+
+  // The wipe request should fail now.
+  WipeStorageRequest wipe_storage_request;
+  WipeStorageResponse wipe_storage_response;
+  EXPECT_EQ(NV_RESULT_INTERNAL_ERROR,
+            nvram.WipeStorage(wipe_storage_request, &wipe_storage_response));
+
+  // New wipe attempt with a fresh instance after clearing the error.
+  storage::SetSpaceWriteError(2, false);
+  NvramManager nvram2;
+  EXPECT_EQ(NV_RESULT_SUCCESS,
+            nvram2.WipeStorage(wipe_storage_request, &wipe_storage_response));
+
+  // No spaces should remain.
+  EXPECT_EQ(NV_RESULT_SUCCESS,
+            nvram2.GetInfo(get_info_request, &get_info_response));
+  EXPECT_EQ(0U, get_info_response.space_list.size());
+}
+
+TEST_F(NvramManagerTest, WipeStorage_Disable) {
+  // Set up an NVRAM space.
+  NvramSpace space;
+  ASSERT_TRUE(space.contents.Resize(10));
+  ASSERT_EQ(storage::Status::kSuccess, persistence::StoreSpace(17, space));
+  SetupHeader(NvramHeader::kVersion, 17);
+
+  NvramManager nvram;
+
+  // Disable wiping.
+  DisableWipeRequest disable_wipe_request;
+  DisableWipeResponse disable_wipe_response;
+  EXPECT_EQ(NV_RESULT_SUCCESS,
+            nvram.DisableWipe(disable_wipe_request, &disable_wipe_response));
+
+  // A wipe request should fail.
+  WipeStorageRequest wipe_storage_request;
+  WipeStorageResponse wipe_storage_response;
+  EXPECT_EQ(NV_RESULT_OPERATION_DISABLED,
+            nvram.WipeStorage(wipe_storage_request, &wipe_storage_response));
+
+  // The space should remain declared.
+  GetInfoRequest get_info_request;
+  GetInfoResponse get_info_response;
+  EXPECT_EQ(NV_RESULT_SUCCESS,
+            nvram.GetInfo(get_info_request, &get_info_response));
+  ASSERT_EQ(1U, get_info_response.space_list.size());
+  EXPECT_EQ(17U, get_info_response.space_list[0]);
+
+  // The space data should remain present.
+  GetSpaceInfoRequest get_space_info_request;
+  get_space_info_request.index = 17;
+  GetSpaceInfoResponse get_space_info_response;
+  EXPECT_EQ(NV_RESULT_SUCCESS, nvram.GetSpaceInfo(get_space_info_request,
+                                                  &get_space_info_response));
+  EXPECT_EQ(10U, get_space_info_response.size);
+}
+
 }  // namespace
 }  // namespace nvram
diff --git a/messages/include/nvram/messages/nvram_messages.h b/messages/include/nvram/messages/nvram_messages.h
index e574707..c2691f7 100644
--- a/messages/include/nvram/messages/nvram_messages.h
+++ b/messages/include/nvram/messages/nvram_messages.h
@@ -28,6 +28,8 @@
 namespace nvram {
 
 enum Command {
+  // Commands corresponding to the API defined in the access-controlled NVRAM
+  // HAL spec. Note that some commands service multiple HAL API calls.
   COMMAND_GET_INFO = 1,
   COMMAND_CREATE_SPACE = 2,
   COMMAND_GET_SPACE_INFO = 3,
@@ -37,6 +39,12 @@ enum Command {
   COMMAND_READ_SPACE = 7,
   COMMAND_LOCK_SPACE_WRITE = 8,
   COMMAND_LOCK_SPACE_READ = 9,
+
+  // The wipe commands are provided as a utility for clearing NVRAM during
+  // hardware reset. These are not accessible via the HAL API, but may be used
+  // by implementations to implement NVRAM clearing on full device reset.
+  COMMAND_WIPE_STORAGE = 10,
+  COMMAND_DISABLE_WIPE = 11,
 };
 
 // COMMAND_GET_INFO request/response.
@@ -48,6 +56,7 @@ struct GetInfoResponse {
   uint64_t max_space_size = 0;
   uint32_t max_spaces = 0;
   Vector<uint32_t> space_list;
+  bool wipe_disabled = false;
 };
 
 // COMMAND_CREATE_SPACE request/response.
@@ -120,6 +129,14 @@ struct LockSpaceReadRequest {
 
 struct LockSpaceReadResponse {};
 
+// COMMAND_WIPE request/response.
+struct WipeStorageRequest {};
+struct WipeStorageResponse {};
+
+// COMMAND_DISABLE_WIPE request/response.
+struct DisableWipeRequest {};
+struct DisableWipeResponse {};
+
 // Generic request message, carrying command-specific payload. The slot set in
 // the payload determines the requested command.
 using RequestUnion = TaggedUnion<
@@ -132,7 +149,9 @@ using RequestUnion = TaggedUnion<
     TaggedUnionMember<COMMAND_WRITE_SPACE, WriteSpaceRequest>,
     TaggedUnionMember<COMMAND_READ_SPACE, ReadSpaceRequest>,
     TaggedUnionMember<COMMAND_LOCK_SPACE_WRITE, LockSpaceWriteRequest>,
-    TaggedUnionMember<COMMAND_LOCK_SPACE_READ, LockSpaceReadRequest>>;
+    TaggedUnionMember<COMMAND_LOCK_SPACE_READ, LockSpaceReadRequest>,
+    TaggedUnionMember<COMMAND_WIPE_STORAGE, WipeStorageRequest>,
+    TaggedUnionMember<COMMAND_DISABLE_WIPE, DisableWipeRequest>>;
 struct Request {
   RequestUnion payload;
 };
@@ -149,7 +168,9 @@ using ResponseUnion = TaggedUnion<
     TaggedUnionMember<COMMAND_WRITE_SPACE, WriteSpaceResponse>,
     TaggedUnionMember<COMMAND_READ_SPACE, ReadSpaceResponse>,
     TaggedUnionMember<COMMAND_LOCK_SPACE_WRITE, LockSpaceWriteResponse>,
-    TaggedUnionMember<COMMAND_LOCK_SPACE_READ, LockSpaceReadResponse>>;
+    TaggedUnionMember<COMMAND_LOCK_SPACE_READ, LockSpaceReadResponse>,
+    TaggedUnionMember<COMMAND_WIPE_STORAGE, WipeStorageResponse>,
+    TaggedUnionMember<COMMAND_DISABLE_WIPE, DisableWipeResponse>>;
 struct Response {
   nvram_result_t result = NV_RESULT_SUCCESS;
   ResponseUnion payload;
diff --git a/messages/nvram_messages.cpp b/messages/nvram_messages.cpp
index 7b1cafb..37fe7af 100644
--- a/messages/nvram_messages.cpp
+++ b/messages/nvram_messages.cpp
@@ -37,7 +37,8 @@ template<> struct DescriptorForType<GetInfoResponse> {
                     MakeField(2, &GetInfoResponse::available_size),
                     MakeField(3, &GetInfoResponse::max_spaces),
                     MakeField(4, &GetInfoResponse::space_list),
-                    MakeField(5, &GetInfoResponse::max_space_size));
+                    MakeField(5, &GetInfoResponse::max_space_size),
+                    MakeField(6, &GetInfoResponse::wipe_disabled));
 };
 
 template<> struct DescriptorForType<CreateSpaceRequest> {
@@ -125,6 +126,22 @@ template<> struct DescriptorForType<LockSpaceReadResponse> {
   static constexpr auto kFields = MakeFieldList();
 };
 
+template<> struct DescriptorForType<WipeStorageRequest> {
+  static constexpr auto kFields = MakeFieldList();
+};
+
+template<> struct DescriptorForType<WipeStorageResponse> {
+  static constexpr auto kFields = MakeFieldList();
+};
+
+template<> struct DescriptorForType<DisableWipeRequest> {
+  static constexpr auto kFields = MakeFieldList();
+};
+
+template<> struct DescriptorForType<DisableWipeResponse> {
+  static constexpr auto kFields = MakeFieldList();
+};
+
 template<> struct DescriptorForType<Request> {
   static constexpr auto kFields = MakeFieldList(
       MakeOneOfField(1, &Request::payload, COMMAND_GET_INFO),
@@ -135,7 +152,9 @@ template<> struct DescriptorForType<Request> {
       MakeOneOfField(6, &Request::payload, COMMAND_WRITE_SPACE),
       MakeOneOfField(7, &Request::payload, COMMAND_READ_SPACE),
       MakeOneOfField(8, &Request::payload, COMMAND_LOCK_SPACE_WRITE),
-      MakeOneOfField(9, &Request::payload, COMMAND_LOCK_SPACE_READ));
+      MakeOneOfField(9, &Request::payload, COMMAND_LOCK_SPACE_READ),
+      MakeOneOfField(10, &Request::payload, COMMAND_WIPE_STORAGE),
+      MakeOneOfField(11, &Request::payload, COMMAND_DISABLE_WIPE));
 };
 
 template<> struct DescriptorForType<Response> {
@@ -149,7 +168,9 @@ template<> struct DescriptorForType<Response> {
       MakeOneOfField(7, &Response::payload, COMMAND_WRITE_SPACE),
       MakeOneOfField(8, &Response::payload, COMMAND_READ_SPACE),
       MakeOneOfField(9, &Response::payload, COMMAND_LOCK_SPACE_WRITE),
-      MakeOneOfField(10, &Response::payload, COMMAND_LOCK_SPACE_READ));
+      MakeOneOfField(10, &Response::payload, COMMAND_LOCK_SPACE_READ),
+      MakeOneOfField(11, &Response::payload, COMMAND_WIPE_STORAGE),
+      MakeOneOfField(12, &Response::payload, COMMAND_DISABLE_WIPE));
 };
 
 template <typename Message>