Merge "Make IRPC v3 optionally backwards compatible" am: 79b72fece3

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

Change-Id: Ia0b5645f5eac503012aded507bb1c76f12461f02
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Tri Vo
2022-12-02 04:46:28 +00:00
committed by Automerger Merge Worker
3 changed files with 26 additions and 41 deletions

View File

@@ -181,15 +181,6 @@ class VtsRemotelyProvisionedComponentTests : public testing::TestWithParam<std::
return params;
}
void checkMacedPubkeyVersioned(const MacedPublicKey& macedPubKey, bool testMode,
vector<uint8_t>* payload_value) {
if (rpcHardwareInfo.versionNumber >= VERSION_WITHOUT_TEST_MODE) {
check_maced_pubkey(macedPubKey, false, payload_value);
} else {
check_maced_pubkey(macedPubKey, testMode, payload_value);
}
}
protected:
std::shared_ptr<IRemotelyProvisionedComponent> provisionable_;
RpcHardwareInfo rpcHardwareInfo;
@@ -279,7 +270,7 @@ TEST_P(GenerateKeyTests, generateEcdsaP256Key_prodMode) {
auto status = provisionable_->generateEcdsaP256KeyPair(testMode, &macedPubKey, &privateKeyBlob);
ASSERT_TRUE(status.isOk());
vector<uint8_t> coseKeyData;
checkMacedPubkeyVersioned(macedPubKey, testMode, &coseKeyData);
check_maced_pubkey(macedPubKey, testMode, &coseKeyData);
}
/**
@@ -302,7 +293,7 @@ TEST_P(GenerateKeyTests, generateAndUseEcdsaP256Key_prodMode) {
auto status = provisionable_->generateEcdsaP256KeyPair(testMode, &macedPubKey, &privateKeyBlob);
ASSERT_TRUE(status.isOk());
vector<uint8_t> coseKeyData;
checkMacedPubkeyVersioned(macedPubKey, testMode, &coseKeyData);
check_maced_pubkey(macedPubKey, testMode, &coseKeyData);
AttestationKey attestKey;
attestKey.keyBlob = std::move(privateKeyBlob);
@@ -357,7 +348,7 @@ TEST_P(GenerateKeyTests, generateEcdsaP256Key_testMode) {
bool testMode = true;
auto status = provisionable_->generateEcdsaP256KeyPair(testMode, &macedPubKey, &privateKeyBlob);
ASSERT_TRUE(status.isOk());
checkMacedPubkeyVersioned(macedPubKey, testMode, nullptr);
check_maced_pubkey(macedPubKey, testMode, nullptr);
}
class CertificateRequestTestBase : public VtsRemotelyProvisionedComponentTests {
@@ -382,7 +373,7 @@ class CertificateRequestTestBase : public VtsRemotelyProvisionedComponentTests {
ASSERT_TRUE(status.isOk()) << status.getMessage();
vector<uint8_t> payload_value;
checkMacedPubkeyVersioned(key, testMode, &payload_value);
check_maced_pubkey(key, testMode, &payload_value);
cborKeysToSign_.add(cppbor::EncodedItem(payload_value));
}
}
@@ -401,8 +392,16 @@ class CertificateRequestTest : public CertificateRequestTestBase {
CertificateRequestTestBase::SetUp();
if (rpcHardwareInfo.versionNumber >= VERSION_WITHOUT_TEST_MODE) {
GTEST_SKIP() << "This test case only applies to RKP v1 and v2. "
<< "RKP version discovered: " << rpcHardwareInfo.versionNumber;
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.";
}
}
}
};
@@ -769,30 +768,17 @@ TEST_P(CertificateRequestV2Test, NonEmptyRequestCorruptMac) {
}
/**
* Generate a non-empty certificate request in prod mode, with test keys. Test mode must be
* ignored, i.e. test must pass.
* Generate a non-empty certificate request in prod mode, with test keys. Must fail with
* STATUS_TEST_KEY_IN_PRODUCTION_REQUEST.
*/
TEST_P(CertificateRequestV2Test, NonEmptyRequest_testKeyInProdCert) {
generateKeys(true /* testMode */, 1 /* numKeys */);
bytevec csr;
auto status = provisionable_->generateCertificateRequestV2(keysToSign_, challenge_, &csr);
ASSERT_TRUE(status.isOk()) << status.getMessage();
}
/**
* Call generateCertificateRequest(). Make sure it's removed.
*/
TEST_P(CertificateRequestV2Test, CertificateRequestV1Removed) {
generateTestEekChain(2);
bytevec keysToSignMac;
DeviceInfo deviceInfo;
ProtectedData protectedData;
auto status = provisionable_->generateCertificateRequest(
true /* testMode */, {} /* keysToSign */, testEekChain_.chain, challenge_, &deviceInfo,
&protectedData, &keysToSignMac);
ASSERT_FALSE(status.isOk()) << status.getMessage();
EXPECT_EQ(status.getServiceSpecificError(), BnRemotelyProvisionedComponent::STATUS_REMOVED);
ASSERT_EQ(status.getServiceSpecificError(),
BnRemotelyProvisionedComponent::STATUS_TEST_KEY_IN_PRODUCTION_REQUEST);
}
INSTANTIATE_REM_PROV_AIDL_TEST(CertificateRequestV2Test);

View File

@@ -30,7 +30,8 @@ This document provides an exact description of which changes have occurred in th
* `version` has moved to a top-level field within the CSR generated by the HAL.
* IRemotelyProvisionedComponent
* The need for an EEK has been removed. There is no longer an encrypted portion of the CSR.
* Test mode has been removed.
* 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.
* 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

View File

@@ -132,11 +132,7 @@ interface IRemotelyProvisionedComponent {
* generateKeyPair generates a new ECDSA P-256 key pair that can be attested by the remote
* server.
*
* @param in boolean testMode this field is now deprecated. It is ignored by the implementation
* in v3, but retained to simplify backwards compatibility support. V1 and V2
* implementations must still respect the testMode flag.
*
* testMode indicates whether the generated key is for testing only. Test keys
* @param in boolean testMode indicates whether the generated key is for testing only. Test keys
* are marked (see the definition of PublicKey in the MacedPublicKey structure) to
* prevent them from being confused with production keys.
*
@@ -150,8 +146,8 @@ interface IRemotelyProvisionedComponent {
byte[] generateEcdsaP256KeyPair(in boolean testMode, out MacedPublicKey macedPublicKey);
/**
* This method has been removed in version 3 of the HAL. The header is kept around for
* backwards compatibility purposes. From v3, this method should raise a
* 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.
*
* For v1 and v2 implementations:
@@ -306,7 +302,9 @@ interface IRemotelyProvisionedComponent {
*
* @param in MacedPublicKey[] keysToSign contains the set of keys to certify. The
* IRemotelyProvisionedComponent must validate the MACs on each key. If any entry in the
* array lacks a valid MAC, the method must return STATUS_INVALID_MAC.
* array lacks a valid MAC, the method must return STATUS_INVALID_MAC. This method must
* not accept test keys. If any entry in the array is a test key, the method must return
* STATUS_TEST_KEY_IN_PRODUCTION_REQUEST.
*
* @param in challenge contains a byte string from the provisioning server which will be
* included in the signed data of the CSR structure. Different provisioned backends may