From 95b313137073dfb2332875349cf6d6d8d5fd70ef Mon Sep 17 00:00:00 2001
From: Andrew Scull <ascull@google.com>
Date: Mon, 27 Mar 2023 19:16:07 +0000
Subject: [PATCH] Deprecate CSR v1 and v2 in IRPC v3

Deprecate the CSR format from v1 and v2 of the HAL, again. The older CSR
versions were allowed in order to ease migration from the
RemoteProvisioner app over to rkpd and that has now been completed.

Bug: 260920864
Test: atest VtsHalRemotelyProvisionedComponentTargetTest
Change-Id: I4d16eb64e4ffe602b4b252159202a4ddb56d63d7
---
 security/rkp/CHANGELOG.md                     |  2 +-
 .../IRemotelyProvisionedComponent.aidl        |  6 ++---
 .../VtsRemotelyProvisionedComponentTests.cpp  | 26 ++++++++++++-------
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/security/rkp/CHANGELOG.md b/security/rkp/CHANGELOG.md
index 9409a6db0c..f425284362 100644
--- a/security/rkp/CHANGELOG.md
+++ b/security/rkp/CHANGELOG.md
@@ -31,7 +31,7 @@ This document provides an exact description of which changes have occurred in th
 * IRemotelyProvisionedComponent
   * The need for an EEK has been removed. There is no longer an encrypted portion of the CSR.
   * Keys for new CSR format must be generated with test mode set to false, effectively removing test
-    mode in the new CSR flow. Old behavior is kept unchanged for backwards compatibility.
+    mode in the new CSR flow.
   * The schema for the CSR itself has been significantly simplified, please see
     IRemotelyProvisionedComponent.aidl for more details. Notably,
     * the chain of signing, MACing, and encryption operations has been replaced with a single
diff --git a/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl b/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl
index 35b83ddbfc..7960c7f930 100644
--- a/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl
+++ b/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl
@@ -144,9 +144,9 @@ interface IRemotelyProvisionedComponent {
     byte[] generateEcdsaP256KeyPair(in boolean testMode, out MacedPublicKey macedPublicKey);
 
     /**
-     * This method can be removed in version 3 of the HAL. The header is kept around for
-     * backwards compatibility purposes. From v3, this method is allowed to raise a
-     * ServiceSpecificException with an error code of STATUS_REMOVED.
+     * This method has been deprecated since version 3 of the HAL. The header is kept around for
+     * backwards compatibility purposes. From v3, this method must raise a ServiceSpecificException
+     * with an error code of STATUS_REMOVED.
      *
      * For v1 and v2 implementations:
      * generateCertificateRequest creates a certificate request to be sent to the provisioning
diff --git a/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
index bf40976ec3..9f68bfa847 100644
--- a/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
+++ b/security/rkp/aidl/vts/functional/VtsRemotelyProvisionedComponentTests.cpp
@@ -408,16 +408,8 @@ class CertificateRequestTest : public CertificateRequestTestBase {
         ASSERT_FALSE(HasFatalFailure());
 
         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)) {
-                GTEST_SKIP() << "This test case applies to RKP v3+ only if "
-                             << "generateCertificateRequest() is implemented.";
-            }
+            GTEST_SKIP() << "This test case only applies to RKP v1 and v2. "
+                         << "RKP version discovered: " << rpcHardwareInfo.versionNumber;
         }
     }
 };
@@ -798,6 +790,20 @@ TEST_P(CertificateRequestV2Test, NonEmptyRequest_testKeyInProdCert) {
               BnRemotelyProvisionedComponent::STATUS_TEST_KEY_IN_PRODUCTION_REQUEST);
 }
 
+/**
+ * Call generateCertificateRequest(). Make sure it's removed.
+ */
+TEST_P(CertificateRequestV2Test, CertificateRequestV1Removed) {
+    bytevec keysToSignMac;
+    DeviceInfo deviceInfo;
+    ProtectedData protectedData;
+    auto status = provisionable_->generateCertificateRequest(
+            true /* testMode */, {} /* keysToSign */, {} /* EEK chain */, challenge_, &deviceInfo,
+            &protectedData, &keysToSignMac);
+    ASSERT_FALSE(status.isOk()) << status.getMessage();
+    EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_REMOVED);
+}
+
 void parse_root_of_trust(const vector<uint8_t>& attestation_cert,
                          vector<uint8_t>* verified_boot_key, VerifiedBoot* verified_boot_state,
                          bool* device_locked, vector<uint8_t>* verified_boot_hash) {