ODK: Address review comments

Merge of http://go/wvgerrit/95666

Mostly fixing coding styles and a few vulnerability check.
Updating tests according to the fix.

Bug: 150614088
Bug: 150881959
Test: Ran cdm and odk unit tests
Change-Id: I109a96ee8ded089d59ab49c2f94b6833c932fd1e
This commit is contained in:
Cong Lin
2020-03-12 18:25:46 -07:00
parent fae5d3f7a9
commit 5a6a2075f5
8 changed files with 129 additions and 119 deletions

View File

@@ -33,6 +33,8 @@ using oemcrypto_core_message::serialize::CreateCoreLicenseResponse;
using oemcrypto_core_message::serialize::CreateCoreProvisioningResponse;
using oemcrypto_core_message::serialize::CreateCoreRenewalResponse;
constexpr uint32_t kExtraPayloadSize = 128u;
template <typename T, typename F, typename G>
void ValidateRequest(uint32_t message_type,
const std::vector<ODK_Field>& extra_fields,
@@ -65,16 +67,15 @@ void ValidateRequest(uint32_t message_type,
EXPECT_EQ(OEMCrypto_ERROR_SHORT_BUFFER,
odk_prepare_func(buf_empty, &core_message_length, &nonce_values));
EXPECT_EQ(core_message_length, message_size);
EXPECT_EQ(nullptr, buf_empty);
// non-empty buf, expect core message length to be set correctly, and buf is
// filled with ODK_Field values appropriately
uint8_t* buf = new uint8_t[message_size]();
uint8_t* buf = new uint8_t[message_size];
EXPECT_EQ(OEMCrypto_SUCCESS,
odk_prepare_func(buf, &core_message_length, &nonce_values));
EXPECT_EQ(core_message_length, message_size);
uint8_t* buf_expected = new uint8_t[message_size]();
uint8_t* buf_expected = new uint8_t[message_size];
size_t buf_len_expected = 0;
EXPECT_EQ(OEMCrypto_SUCCESS, ODK_IterFields(ODK_WRITE, buf_expected, SIZE_MAX,
&buf_len_expected, total_fields));
@@ -93,7 +94,7 @@ void ValidateRequest(uint32_t message_type,
nonce_values.api_major_version = t.api_major_version;
nonce_values.nonce = t.nonce;
nonce_values.session_id = t.session_id;
uint8_t* buf2 = new uint8_t[message_size]();
uint8_t* buf2 = new uint8_t[message_size];
EXPECT_EQ(OEMCrypto_SUCCESS,
odk_prepare_func(buf2, &core_message_length, &nonce_values));
EXPECT_EQ(core_message_length, message_size);
@@ -123,9 +124,9 @@ void ValidateResponse(ODK_CoreMessage* core_message,
uint8_t* buf = nullptr;
uint32_t buf_size = 0;
ODK_BuildMessageBuffer(core_message, extra_fields, buf, &buf_size);
ODK_BuildMessageBuffer(core_message, extra_fields, &buf, &buf_size);
uint8_t* zero = new uint8_t[buf_size]();
uint8_t* zero = new uint8_t[buf_size];
size_t bytes_read = 0;
// zero-out input
EXPECT_EQ(OEMCrypto_SUCCESS, ODK_IterFields(ODK_READ, zero, buf_size,
@@ -151,7 +152,7 @@ void ValidateResponse(ODK_CoreMessage* core_message,
TEST(OdkTest, SerializeFields) {
uint32_t x[] = {0, 1, 2};
uint64_t y[] = {3ll << 32, 4ll << 32, 5ll << 32};
uint64_t y[] = {3LL << 32, 4LL << 32, 5LL << 32};
OEMCrypto_Substring s = {.offset = 6, .length = 7};
std::vector<ODK_Field> fields = {
{ODK_UINT32, &x[0], "x[0]"}, {ODK_UINT32, &x[1], "x[1]"},
@@ -245,7 +246,7 @@ TEST(OdkTest, NullRequestTest) {
}
TEST(OdkTest, NullResponseTest) {
const size_t message_size = 64;
constexpr size_t message_size = 64;
uint8_t message[message_size] = {0};
size_t core_message_length = message_size;
uint8_t request_hash[ODK_SHA256_HASH_SIZE] = {0};
@@ -280,7 +281,7 @@ TEST(OdkTest, NullResponseTest) {
true, request_hash, &timer_limits, &clock_values,
&nonce_values, &parsed_license));
uint64_t system_time = 0;
constexpr uint64_t system_time = 0;
uint64_t timer_value = 0;
EXPECT_EQ(ODK_ERROR_CORE_MESSAGE,
ODK_ParseRenewal(message, message_size, core_message_length,
@@ -350,7 +351,7 @@ TEST(OdkTest, PrepareCoreRenewalRequest) {
size_t core_message_length = sizeof(renewal_message);
ODK_NonceValues nonce_values{0};
ODK_ClockValues clock_values{0};
uint64_t system_time_seconds = 10;
constexpr uint64_t system_time_seconds = 10;
EXPECT_EQ(OEMCrypto_SUCCESS,
ODK_PrepareCoreRenewalRequest(
renewal_message, sizeof(renewal_message), &core_message_length,
@@ -361,7 +362,7 @@ TEST(OdkTest, PrepareCoreRenewalRequestTimer) {
uint8_t renewal_message[ODK_RENEWAL_REQUEST_SIZE] = {0};
size_t core_message_length = sizeof(renewal_message);
ODK_NonceValues nonce_values{2, 16, 0, 0};
uint64_t system_time_seconds = 10;
constexpr uint64_t system_time_seconds = 10;
ODK_ClockValues clock_values_updated{0};
// system time smaller than first decrypt time
clock_values_updated.time_of_first_decrypt = system_time_seconds + 1;
@@ -469,19 +470,16 @@ TEST(OdkTest, ParseLicenseErrorNonce) {
ODK_SetDefaultLicenseResponseParams(&params);
uint8_t* buf = nullptr;
uint32_t buf_size = 0;
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, buf,
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, &buf,
&buf_size);
// temporarily mess up with nonce
uint32_t nonce = params.core_message.nonce_values.nonce;
params.core_message.nonce_values.nonce = 0;
OEMCryptoResult err = ODK_ParseLicense(
buf, buf_size + 128, buf_size, params.initial_license_load,
buf, buf_size + kExtraPayloadSize, buf_size, params.initial_license_load,
params.usage_entry_present, params.request_hash, &(params.timer_limits),
&(params.clock_values), &(params.core_message.nonce_values),
&(params.parsed_license));
EXPECT_EQ(OEMCrypto_ERROR_INVALID_NONCE, err);
// restore value
params.core_message.nonce_values.nonce = nonce;
delete[] buf;
}
@@ -490,17 +488,15 @@ TEST(OdkTest, ParseLicenseErrorUsageEntry) {
ODK_SetDefaultLicenseResponseParams(&params);
uint8_t* buf = nullptr;
uint32_t buf_size = 0;
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, buf,
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, &buf,
&buf_size);
bool usage_entry_present = params.usage_entry_present;
params.usage_entry_present = false;
OEMCryptoResult err = ODK_ParseLicense(
buf, buf_size + 128, buf_size, params.initial_license_load,
buf, buf_size + kExtraPayloadSize, buf_size, params.initial_license_load,
params.usage_entry_present, params.request_hash, &(params.timer_limits),
&(params.clock_values), &(params.core_message.nonce_values),
&(params.parsed_license));
EXPECT_EQ(ODK_ERROR_CORE_MESSAGE, err);
params.usage_entry_present = usage_entry_present;
delete[] buf;
}
@@ -509,19 +505,16 @@ TEST(OdkTest, ParseLicenseErrorRequestHash) {
ODK_SetDefaultLicenseResponseParams(&params);
uint8_t* buf = nullptr;
uint32_t buf_size = 0;
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, buf,
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, &buf,
&buf_size);
// temporarily mess up with request hash
uint32_t first_byte = params.request_hash[0];
params.request_hash[0] = 0xff;
OEMCryptoResult err = ODK_ParseLicense(
buf, buf_size + 128, buf_size, params.initial_license_load,
buf, buf_size + kExtraPayloadSize, buf_size, params.initial_license_load,
params.usage_entry_present, params.request_hash, &(params.timer_limits),
&(params.clock_values), &(params.core_message.nonce_values),
&(params.parsed_license));
EXPECT_EQ(ODK_ERROR_CORE_MESSAGE, err);
// restore value
params.request_hash[0] = first_byte;
delete[] buf;
}
@@ -530,19 +523,14 @@ TEST(OdkTest, ParseRenewalErrorTimer) {
ODK_SetDefaultRenewalResponseParams(&params);
uint8_t* buf = nullptr;
uint32_t buf_size = 0;
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, buf,
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, &buf,
&buf_size);
// temporarily mess up with time_of_renewal_request
uint64_t time_of_renewal_request =
params.clock_values.time_of_renewal_request;
params.clock_values.time_of_renewal_request = 0;
OEMCryptoResult err = ODK_ParseRenewal(
buf, buf_size, buf_size, &(params.core_message.nonce_values),
params.system_time, &(params.timer_limits), &(params.clock_values),
&(params.playback_timer));
EXPECT_EQ(ODK_STALE_RENEWAL, err);
// restore value
params.clock_values.time_of_renewal_request = time_of_renewal_request;
delete[] buf;
}
@@ -551,17 +539,14 @@ TEST(OdkTest, ParsePrivisioningErrorDeviceId) {
ODK_SetDefaultProvisioningResponseParams(&params);
uint8_t* buf = nullptr;
uint32_t buf_size = 0;
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, buf,
ODK_BuildMessageBuffer(&(params.core_message), params.extra_fields, &buf,
&buf_size);
// temporarily mess up with device_id
uint8_t first_byte = params.device_id[0];
params.device_id[0] = 0;
OEMCryptoResult err = ODK_ParseProvisioning(
buf, buf_size + 16, buf_size, &(params.core_message.nonce_values),
params.device_id, params.device_id_length, &(params.parsed_provisioning));
EXPECT_EQ(ODK_ERROR_CORE_MESSAGE, err);
// restore value
params.device_id[0] = first_byte;
delete[] buf;
}
@@ -573,11 +558,11 @@ TEST(OdkTest, LicenseResponseRoundtrip) {
uint8_t request_hash_read[ODK_SHA256_HASH_SIZE];
memcpy(request_hash_read, params.request_hash, sizeof(request_hash_read));
auto odk_parse_func = [&](const uint8_t* buf, size_t size) {
return ODK_ParseLicense(buf, size + 128, size, params.initial_license_load,
params.usage_entry_present, request_hash_read,
&(params.timer_limits), &(params.clock_values),
&(params.core_message.nonce_values),
&(params.parsed_license));
return ODK_ParseLicense(
buf, size + kExtraPayloadSize, size, params.initial_license_load,
params.usage_entry_present, request_hash_read, &(params.timer_limits),
&(params.clock_values), &(params.core_message.nonce_values),
&(params.parsed_license));
};
const std::string request_hash_string(
reinterpret_cast<const char*>(request_hash_read),
@@ -596,8 +581,8 @@ TEST(OdkTest, LicenseResponseRoundtrip) {
TEST(OdkTest, RenewalResponseRoundtrip) {
ODK_RenewalResponseParams params;
ODK_SetDefaultRenewalResponseParams(&params);
uint64_t playback_clock = params.playback_clock;
uint64_t renewal_duration = params.renewal_duration;
const uint64_t playback_clock = params.playback_clock;
const uint64_t renewal_duration = params.renewal_duration;
auto odk_parse_func = [&](const uint8_t* buf, size_t size) {
OEMCryptoResult err =
ODK_ParseRenewal(buf, size, size, &(params.core_message.nonce_values),
@@ -626,7 +611,7 @@ TEST(OdkTest, ProvisionResponseRoundtrip) {
ODK_ProvisioningResponseParams params;
ODK_SetDefaultProvisioningResponseParams(&params);
// save a copy of params.device_id as it will be zero out during the test
uint32_t device_id_length = params.device_id_length;
const uint32_t device_id_length = params.device_id_length;
uint8_t device_id[ODK_DEVICE_ID_LEN_MAX] = {0};
memcpy(device_id, params.device_id, device_id_length);
@@ -661,9 +646,8 @@ TEST(OdkSizeTest, LicenseRequest) {
EXPECT_EQ(OEMCrypto_ERROR_SHORT_BUFFER,
ODK_PrepareCoreLicenseRequest(message, message_length,
&core_message_length, &nonce_values));
// All messages have at least a five 4-byte fields.
size_t minimum_message_size = 5 * 4;
EXPECT_GE(core_message_length, minimum_message_size);
// the core_message_length should be appropriately set
EXPECT_EQ(ODK_LICENSE_REQUEST_SIZE, core_message_length);
}
TEST(OdkSizeTest, RenewalRequest) {
@@ -684,9 +668,8 @@ TEST(OdkSizeTest, RenewalRequest) {
ODK_PrepareCoreRenewalRequest(message, message_length,
&core_message_length, &nonce_values,
&clock_values, system_time_seconds));
// All messages have at least a five 4-byte fields.
size_t minimum_message_size = 5 * 4;
EXPECT_GE(core_message_length, minimum_message_size);
// the core_message_length should be appropriately set
EXPECT_EQ(ODK_RENEWAL_REQUEST_SIZE, core_message_length);
}
TEST(OdkSizeTest, ReleaseRequest) {
@@ -726,9 +709,8 @@ TEST(OdkSizeTest, ProvisioningRequest) {
ODK_PrepareCoreProvisioningRequest(
message, message_length, &core_message_length, &nonce_values,
nullptr, device_id_length));
// All messages have at least a five 4-byte fields.
size_t minimum_message_size = 5 * 4;
EXPECT_GE(core_message_length, minimum_message_size);
// the core_message_length should be appropriately set
EXPECT_EQ(ODK_PROVISIONING_REQUEST_SIZE, core_message_length);
}
} // namespace