identity: fix access control checks in libeic.

Also add a new libeic_test binary which has a regression test for this
vulnerability.

Bug: 190757775
Test: atest libeic_test
Test: atest VtsHalIdentityTargetTest
Test: atest CtsIdentityTestCases
Merged-In: I8344655c59930d6bf1baa4e0f8d0f60e4fc9e48d
Change-Id: I8344655c59930d6bf1baa4e0f8d0f60e4fc9e48d

identity/TEST_MAPPING

@@ -8,6 +8,9 @@
     },
     {
       "name": "android.hardware.identity-support-lib-test"
+    },
+    {
+      "name": "libeic_test"
     }
   ]
 }

identity/aidl/default/Android.bp

@@ -114,6 +114,43 @@ cc_binary {
     ],
 }
 
+cc_test {
+    name: "libeic_test",
+    srcs: [
+        "EicTests.cpp",
+        "FakeSecureHardwareProxy.cpp",
+    ],
+    cflags: [
+        "-Wall",
+        "-Wextra",
+        "-g",
+        "-DEIC_DEBUG",
+    ],
+    local_include_dirs: [
+         "common",
+    ],
+    shared_libs: [
+        "liblog",
+        "libcrypto",
+        "libkeymaster_messages",
+    ],
+    static_libs: [
+        "libbase",
+        "libcppbor_external",
+        "libcppcose_rkp",
+        "libutils",
+        "libsoft_attestation_cert",
+        "libkeymaster_portable",
+        "libsoft_attestation_cert",
+        "libpuresoftkeymasterdevice",
+        "android.hardware.identity-support-lib",
+        "android.hardware.identity-libeic-library",
+    ],
+    test_suites: [
+        "general-tests",
+    ],
+}
+
 prebuilt_etc {
     name: "android.hardware.identity_credential.xml",
     sub_dir: "permissions",

identity/aidl/default/EicTests.cpp

@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2021, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+#include <optional>
+#include <string>
+#include <vector>
+
+#include "FakeSecureHardwareProxy.h"
+
+// Most of libeic is tested as part of VTS since there's almost a 1:1 mapping between
+// the HAL and libeic interfaces. This test suite is mainly for the few things which
+// doesn't map directly.
+//
+
+using std::optional;
+using std::string;
+using std::vector;
+
+using android::hardware::identity::AccessCheckResult;
+using android::hardware::identity::FakeSecureHardwarePresentationProxy;
+using android::hardware::identity::FakeSecureHardwareProvisioningProxy;
+
+TEST(EicTest, AccessControlIsEnforced) {
+    // First provision the credential...
+    //
+    FakeSecureHardwareProvisioningProxy provisioningProxy;
+    bool isTestCredential = false;
+    provisioningProxy.initialize(isTestCredential);
+    optional<vector<uint8_t>> credKey =
+            provisioningProxy.createCredentialKey({0x01, 0x02}, {0x03, 0x04});
+    ASSERT_TRUE(credKey.has_value());
+    string docType = "org.iso.18013.5.1.mDL";
+    ASSERT_TRUE(provisioningProxy.startPersonalization(0, {1}, docType, 125));
+
+    vector<int> acpIds = {};
+    string nameSpace = "org.iso.18013.5.1";
+    string name = "NonAccessibleElement";
+    vector<uint8_t> content = {0x63, 0x46, 0x6f, 0x6f};  // "Foo" tstr
+    ASSERT_TRUE(provisioningProxy.beginAddEntry(acpIds, nameSpace, name, content.size()));
+    optional<vector<uint8_t>> encContent =
+            provisioningProxy.addEntryValue(acpIds, nameSpace, name, content);
+    ASSERT_TRUE(encContent.has_value());
+    ASSERT_EQ(encContent->size(), content.size() + 28);
+
+    optional<vector<uint8_t>> signatureOfToBeSigned = provisioningProxy.finishAddingEntries();
+    ASSERT_TRUE(signatureOfToBeSigned.has_value());
+
+    optional<vector<uint8_t>> credData = provisioningProxy.finishGetCredentialData(docType);
+    ASSERT_TRUE(credData.has_value());
+    ASSERT_TRUE(provisioningProxy.shutdown());
+
+    // Then present data from it...
+    //
+    FakeSecureHardwarePresentationProxy presentationProxy;
+    ASSERT_TRUE(presentationProxy.initialize(isTestCredential, docType, credData.value()));
+    AccessCheckResult res =
+            presentationProxy.startRetrieveEntryValue(nameSpace, name, 1, content.size(), acpIds);
+    ASSERT_EQ(res, AccessCheckResult::kNoAccessControlProfiles);
+
+    // Ensure that we can't get the data out if startRetrieveEntryValue() returned
+    // something other than kOk... See b/190757775 for details.
+    //
+    optional<vector<uint8_t>> decContent =
+            presentationProxy.retrieveEntryValue(encContent.value(), nameSpace, name, acpIds);
+    ASSERT_FALSE(decContent.has_value());
+}
+
+int main(int argc, char** argv) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}

identity/aidl/default/libeic/EicPresentation.c

@@ -633,6 +633,8 @@ EicAccessCheckResult eicPresentationStartRetrieveEntryValue(
 
     // We'll need to calc and store a digest of additionalData to check that it's the same
     // additionalData being passed in for every eicPresentationRetrieveEntryValue() call...
+    //
+    ctx->accessCheckOk = false;
     if (!eicCborCalcEntryAdditionalData(accessControlProfileIds, numAccessControlProfileIds,
                                         nameSpace, name, additionalDataCbor,
                                         additionalDataCborBufSize, &additionalDataCborSize,
@@ -680,6 +682,7 @@ EicAccessCheckResult eicPresentationStartRetrieveEntryValue(
 
     if (result == EIC_ACCESS_CHECK_RESULT_OK) {
         eicCborAppendString(&ctx->cbor, name);
+        ctx->accessCheckOk = true;
     }
     return result;
 }
@@ -702,10 +705,15 @@ bool eicPresentationRetrieveEntryValue(EicPresentation* ctx, const uint8_t* encr
                                         calculatedSha256)) {
         return false;
     }
+
     if (eicCryptoMemCmp(calculatedSha256, ctx->additionalDataSha256, EIC_SHA256_DIGEST_SIZE) != 0) {
         eicDebug("SHA-256 mismatch of additionalData");
         return false;
     }
+    if (!ctx->accessCheckOk) {
+        eicDebug("Attempting to retrieve a value for which access is not granted");
+        return false;
+    }
 
     if (!eicOpsDecryptAes128Gcm(ctx->storageKey, encryptedContent, encryptedContentSize,
                                 additionalDataCbor, additionalDataCborSize, content)) {

identity/aidl/default/libeic/EicPresentation.h

@@ -70,6 +70,10 @@ typedef struct {
     // Set to true initialized as a test credential.
     bool testCredential;
 
+    // Set to true if the evaluation of access control checks in
+    // eicPresentationStartRetrieveEntryValue() resulted EIC_ACCESS_CHECK_RESULT_OK
+    bool accessCheckOk;
+
     // These are bitmasks indicating which of the possible 32 access control profiles are
     // authorized. They are built up by eicPresentationValidateAccessControlProfile().
     //