Merge "Strictly deprecate IRPC test mode key generation" am: 29b1d626ba

Original change: https://android-review.googlesource.com/c/platform/hardware/interfaces/+/2559010

Change-Id: I8a5d8bb7e5eaddc4e1823645dd4569687eb57a74
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl b/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl
index 461357d..2a4cba1 100644
--- a/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl
+++ b/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl
@@ -134,6 +134,10 @@
      *        are marked (see the definition of PublicKey in the MacedPublicKey structure) to
      *        prevent them from being confused with production keys.
      *
+     *        This parameter has been deprecated since version 3 of the HAL and will always be
+     *        false. From v3, if this parameter is true, the method must raise a
+     *        ServiceSpecificException with an error of code of STATUS_REMOVED.
+     *
      * @param out MacedPublicKey macedPublicKey contains the public key of the generated key pair,
      *        MACed so that generateCertificateRequest can easily verify, without the
      *        privateKeyHandle, that the contained public key is for remote certification.
diff --git a/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
index 94bfbb4..8906543 100644
--- a/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
+++ b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
@@ -47,7 +47,11 @@
 namespace {
 
 constexpr int32_t VERSION_WITH_UNIQUE_ID_SUPPORT = 2;
+
+constexpr int32_t VERSION_WITHOUT_EEK = 3;
 constexpr int32_t VERSION_WITHOUT_TEST_MODE = 3;
+constexpr int32_t VERSION_WITH_CERTIFICATE_REQUEST_V2 = 3;
+constexpr int32_t VERSION_WITH_SUPPORTED_NUM_KEYS_IN_CSR = 3;
 
 constexpr uint8_t MIN_CHALLENGE_SIZE = 0;
 constexpr uint8_t MAX_CHALLENGE_SIZE = 64;
@@ -226,21 +230,13 @@
     RpcHardwareInfo hwInfo;
     ASSERT_TRUE(provisionable_->getHardwareInfo(&hwInfo).isOk());
 
-    const std::set<int> validCurves = {RpcHardwareInfo::CURVE_P256, RpcHardwareInfo::CURVE_25519};
-    // First check for the implementations that supports only IRPC V3+.
-    if (rpcHardwareInfo.versionNumber >= VERSION_WITHOUT_TEST_MODE) {
-        bytevec keysToSignMac;
-        DeviceInfo deviceInfo;
-        ProtectedData protectedData;
-        auto status = provisionable_->generateCertificateRequest(false, {}, {}, {}, &deviceInfo,
-                                                                 &protectedData, &keysToSignMac);
-        if (!status.isOk() &&
-            (status.getServiceSpecificError() == BnRemotelyProvisionedComponent::STATUS_REMOVED)) {
-            ASSERT_EQ(hwInfo.supportedEekCurve, RpcHardwareInfo::CURVE_NONE)
-                    << "Invalid curve: " << hwInfo.supportedEekCurve;
-            return;
-        }
+    if (rpcHardwareInfo.versionNumber >= VERSION_WITHOUT_EEK) {
+        ASSERT_EQ(hwInfo.supportedEekCurve, RpcHardwareInfo::CURVE_NONE)
+                << "Invalid curve: " << hwInfo.supportedEekCurve;
+        return;
     }
+
+    const std::set<int> validCurves = {RpcHardwareInfo::CURVE_P256, RpcHardwareInfo::CURVE_25519};
     ASSERT_EQ(validCurves.count(hwInfo.supportedEekCurve), 1)
             << "Invalid curve: " << hwInfo.supportedEekCurve;
 }
@@ -264,7 +260,7 @@
  * Verify implementation supports at least MIN_SUPPORTED_NUM_KEYS_IN_CSR keys in a CSR.
  */
 TEST_P(GetHardwareInfoTests, supportedNumKeysInCsr) {
-    if (rpcHardwareInfo.versionNumber < VERSION_WITHOUT_TEST_MODE) {
+    if (rpcHardwareInfo.versionNumber < VERSION_WITH_SUPPORTED_NUM_KEYS_IN_CSR) {
         return;
     }
 
@@ -365,6 +361,13 @@
     bytevec privateKeyBlob;
     bool testMode = true;
     auto status = provisionable_->generateEcdsaP256KeyPair(testMode, &macedPubKey, &privateKeyBlob);
+
+    if (rpcHardwareInfo.versionNumber >= VERSION_WITHOUT_TEST_MODE) {
+        ASSERT_FALSE(status.isOk());
+        EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_REMOVED);
+        return;
+    }
+
     ASSERT_TRUE(status.isOk());
     check_maced_pubkey(macedPubKey, testMode, nullptr);
 }
@@ -410,7 +413,7 @@
         CertificateRequestTestBase::SetUp();
         ASSERT_FALSE(HasFatalFailure());
 
-        if (rpcHardwareInfo.versionNumber >= VERSION_WITHOUT_TEST_MODE) {
+        if (rpcHardwareInfo.versionNumber >= VERSION_WITH_CERTIFICATE_REQUEST_V2) {
             GTEST_SKIP() << "This test case only applies to RKP v1 and v2. "
                          << "RKP version discovered: " << rpcHardwareInfo.versionNumber;
         }
@@ -688,7 +691,7 @@
         CertificateRequestTestBase::SetUp();
         ASSERT_FALSE(HasFatalFailure());
 
-        if (rpcHardwareInfo.versionNumber < VERSION_WITHOUT_TEST_MODE) {
+        if (rpcHardwareInfo.versionNumber < VERSION_WITH_CERTIFICATE_REQUEST_V2) {
             GTEST_SKIP() << "This test case only applies to RKP v3 and above. "
                          << "RKP version discovered: " << rpcHardwareInfo.versionNumber;
         }
@@ -802,23 +805,23 @@
 }
 
 /**
- * Generate a non-empty certificate request in prod mode, with test keys.  Must fail with
- * STATUS_TEST_KEY_IN_PRODUCTION_REQUEST.
+ * Call generateCertificateRequest(). Make sure it's removed.
  */
-TEST_P(CertificateRequestV2Test, NonEmptyRequest_testKeyInProdCert) {
-    generateKeys(true /* testMode */, 1 /* numKeys */);
-
-    bytevec csr;
-    auto status = provisionable_->generateCertificateRequestV2(keysToSign_, challenge_, &csr);
+TEST_P(CertificateRequestV2Test, CertificateRequestV1Removed_prodMode) {
+    bytevec keysToSignMac;
+    DeviceInfo deviceInfo;
+    ProtectedData protectedData;
+    auto status = provisionable_->generateCertificateRequest(
+            false /* testMode */, {} /* keysToSign */, {} /* EEK chain */, challenge_, &deviceInfo,
+            &protectedData, &keysToSignMac);
     ASSERT_FALSE(status.isOk()) << status.getMessage();
-    ASSERT_EQ(status.getServiceSpecificError(),
-              BnRemotelyProvisionedComponent::STATUS_TEST_KEY_IN_PRODUCTION_REQUEST);
+    EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_REMOVED);
 }
 
 /**
- * Call generateCertificateRequest(). Make sure it's removed.
+ * Call generateCertificateRequest() in test mode. Make sure it's removed.
  */
-TEST_P(CertificateRequestV2Test, CertificateRequestV1Removed) {
+TEST_P(CertificateRequestV2Test, CertificateRequestV1Removed_testMode) {
     bytevec keysToSignMac;
     DeviceInfo deviceInfo;
     ProtectedData protectedData;