From 5a6a2075f5109629659c09761508b4710514e9b3 Mon Sep 17 00:00:00 2001 From: Cong Lin Date: Thu, 12 Mar 2020 18:25:46 -0700 Subject: [PATCH] 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 --- libwvdrmengine/oemcrypto/odk/README | 4 +- .../odk/src/core_message_serialize.cpp | 2 +- libwvdrmengine/oemcrypto/odk/src/odk.c | 7 +- .../oemcrypto/odk/src/odk_structs_priv.h | 5 + .../oemcrypto/odk/test/odk_fuzz.cpp | 120 +++++++++++------- .../oemcrypto/odk/test/odk_test.cpp | 86 +++++-------- .../oemcrypto/odk/test/odk_test_helper.cpp | 22 ++-- .../oemcrypto/odk/test/odk_test_helper.h | 2 +- 8 files changed, 129 insertions(+), 119 deletions(-) diff --git a/libwvdrmengine/oemcrypto/odk/README b/libwvdrmengine/oemcrypto/odk/README index 6b4cdff6..44dac83f 100644 --- a/libwvdrmengine/oemcrypto/odk/README +++ b/libwvdrmengine/oemcrypto/odk/README @@ -1,7 +1,7 @@ -The ODK Library is used to generate and parse core OEMCrypto messages for +The ODK Library is used to generate and parse core OEMCrypto messages for OEMCrypto v16 and above. -This library is used by both OEMCrypto on a device, and by Widvine license and +This library is used by both OEMCrypto on a device, and by Widevine license and provisioning servers. The source of truth for these files is in the server code base on piper. Do not diff --git a/libwvdrmengine/oemcrypto/odk/src/core_message_serialize.cpp b/libwvdrmengine/oemcrypto/odk/src/core_message_serialize.cpp index 1ce827e4..cdd97447 100644 --- a/libwvdrmengine/oemcrypto/odk/src/core_message_serialize.cpp +++ b/libwvdrmengine/oemcrypto/odk/src/core_message_serialize.cpp @@ -35,7 +35,7 @@ bool CreateResponse(uint32_t message_type, const S& core_request, return false; } - auto* header = reinterpret_cast(&response); + auto* header = &response.request.core_message; header->message_type = message_type; header->nonce_values.api_major_version = core_request.api_major_version; header->nonce_values.api_minor_version = core_request.api_minor_version; diff --git a/libwvdrmengine/oemcrypto/odk/src/odk.c b/libwvdrmengine/oemcrypto/odk/src/odk.c index d23efd7e..be34f879 100644 --- a/libwvdrmengine/oemcrypto/odk/src/odk.c +++ b/libwvdrmengine/oemcrypto/odk/src/odk.c @@ -248,7 +248,7 @@ OEMCryptoResult ODK_PrepareCoreProvisioningRequest( sizeof(ODK_PreparedProvisioningRequest)); } -/* @@ parse request functions */ +/* @@ parse response functions */ OEMCryptoResult ODK_ParseLicense( const uint8_t* message, size_t message_length, size_t core_message_length, @@ -384,8 +384,9 @@ OEMCryptoResult ODK_ParseProvisioning( const uint8_t zero[ODK_DEVICE_ID_LEN_MAX] = {0}; /* check bytes beyond device_id_length are 0 */ - if (memcmp(zero, provisioning_response.request.device_id + device_id_length, - ODK_DEVICE_ID_LEN_MAX - device_id_length) != 0) { + if (crypto_memcmp(zero, + provisioning_response.request.device_id + device_id_length, + ODK_DEVICE_ID_LEN_MAX - device_id_length) != 0) { return ODK_ERROR_CORE_MESSAGE; } diff --git a/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h b/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h index a9e39703..991f59d0 100644 --- a/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h +++ b/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h @@ -60,6 +60,11 @@ typedef struct { ODK_ParsedProvisioning* parsed_provisioning; } ODK_ProvisioningResponse; +/* These are the sum of sizeof of each individual member of the request structs + */ +/* without any padding added by the compiler. Make sure they get updated when */ +/* request structs change. Refer to test suite OdkSizeTest in */ +/* ../test/odk_test.cpp for validations of each of the defined request sizes. */ #define ODK_LICENSE_REQUEST_SIZE 20 #define ODK_RENEWAL_REQUEST_SIZE 28 #define ODK_PROVISIONING_REQUEST_SIZE 88 diff --git a/libwvdrmengine/oemcrypto/odk/test/odk_fuzz.cpp b/libwvdrmengine/oemcrypto/odk/test/odk_fuzz.cpp index d129e694..3da6744f 100644 --- a/libwvdrmengine/oemcrypto/odk/test/odk_fuzz.cpp +++ b/libwvdrmengine/oemcrypto/odk/test/odk_fuzz.cpp @@ -20,7 +20,7 @@ #include "odk_structs.h" #include "odk_structs_priv.h" -typedef std::function +typedef std::function roundtrip_fun; using oemcrypto_core_message::ODK_LicenseRequest; @@ -74,35 +74,32 @@ static OEMCryptoResult odk_serialize_ProvisioningRequest( template static roundtrip_fun kdo_odk(const F& kdo_fun, const G& odk_fun) { auto roundtrip = [&](const uint8_t* in, uint8_t* out, size_t size, - size_t clock_value_size) -> size_t { + size_t clock_value_size) -> void { if (size <= clock_value_size) { - return 0; + return; } // Input byte array format: [Clock Values][data to parse] std::string input(reinterpret_cast(in) + clock_value_size, size - clock_value_size); T t = {}; if (!kdo_fun(input, &t)) { - return 0; + return; } ODK_NonceValues nonce_values = {t.api_minor_version, t.api_major_version, t.nonce, t.session_id}; OEMCryptoResult err = odk_fun(in, out, &size, t, &nonce_values); - return OEMCrypto_SUCCESS == err ? size : 0; + if (OEMCrypto_SUCCESS != err) { + return; + } + assert(0 == memcmp(in + clock_value_size, out, size)); }; return roundtrip; } // @ odk deserialize; kdo serialize namespace { -struct ODK_Common_Args { - uint16_t api_minor_version; - uint16_t api_major_version; - uint32_t nonce; - uint32_t session_id; -}; struct ODK_ParseLicense_Args { - ODK_Common_Args common; + ODK_NonceValues nonce_values; uint8_t initial_license_load; uint8_t usage_entry_present; uint8_t request_hash[32]; @@ -110,22 +107,21 @@ struct ODK_ParseLicense_Args { ODK_ClockValues clock_values; }; struct ODK_ParseRenewal_Args { - ODK_Common_Args common; + ODK_NonceValues nonce_values; uint64_t system_time; ODK_TimerLimits timer_limits; ODK_ClockValues clock_values; }; struct ODK_ParseProvisioning_Args { - ODK_Common_Args common; + ODK_NonceValues nonce_values; size_t device_id_length; uint8_t device_id[64]; }; } // namespace -uint8_t convert_byte_to_valid_boolean(const bool* in) { - uint8_t boolean_value; - memcpy(&boolean_value, in, 1); - return boolean_value % 2; +bool convert_byte_to_valid_boolean(const bool* in) { + const int value = *reinterpret_cast(in); + return value != 0; } static OEMCryptoResult odk_deserialize_LicenseResponse( @@ -142,10 +138,10 @@ static OEMCryptoResult odk_deserialize_LicenseResponse( static bool kdo_serialize_LicenseResponse(const ODK_ParseLicense_Args* args, const ODK_ParsedLicense& parsed_lic, std::string* oemcrypto_core_message) { - const auto& common = args->common; - ODK_LicenseRequest core_request{common.api_minor_version, - common.api_major_version, common.nonce, - common.session_id}; + const auto& nonce_values = args->nonce_values; + ODK_LicenseRequest core_request{nonce_values.api_minor_version, + nonce_values.api_major_version, + nonce_values.nonce, nonce_values.session_id}; std::string core_request_sha_256( reinterpret_cast(args->request_hash), 32); return CreateCoreLicenseResponse( @@ -184,10 +180,10 @@ static bool kdo_serialize_RenewalResponse( const ODK_ParseRenewal_Args* args, const ODK_PreparedRenewalRequest& renewal_msg, std::string* oemcrypto_core_message) { - const auto& common = args->common; - ODK_RenewalRequest core_request{common.api_minor_version, - common.api_major_version, common.nonce, - common.session_id, renewal_msg.playback_time}; + const auto& nonce_values = args->nonce_values; + ODK_RenewalRequest core_request{ + nonce_values.api_minor_version, nonce_values.api_major_version, + nonce_values.nonce, nonce_values.session_id, renewal_msg.playback_time}; return CreateCoreRenewalResponse( core_request, args->timer_limits.initial_renewal_duration_seconds, oemcrypto_core_message); @@ -204,11 +200,13 @@ static bool kdo_serialize_ProvisioningResponse( const ODK_ParseProvisioning_Args* args, const ODK_ParsedProvisioning& parsed_prov, std::string* oemcrypto_core_message) { - const auto& common = args->common; - assert(args->device_id_length <= sizeof(args->device_id)); + const auto& nonce_values = args->nonce_values; + if (args->device_id_length > sizeof(args->device_id)) { + return false; + } ODK_ProvisioningRequest core_request{ - common.api_minor_version, common.api_major_version, common.nonce, - common.session_id, + nonce_values.api_minor_version, nonce_values.api_major_version, + nonce_values.nonce, nonce_values.session_id, std::string(reinterpret_cast(args->device_id), args->device_id_length)}; return CreateCoreProvisioningResponse(parsed_prov, core_request, @@ -227,35 +225,60 @@ static bool kdo_serialize_ProvisioningResponse( template static roundtrip_fun odk_kdo(const F& odk_fun, const G& kdo_fun) { auto roundtrip = [&](const uint8_t* in, uint8_t* out, size_t size, - size_t args_size) -> size_t { + size_t args_size) -> void { // Input byte array format: [function arguments][data to parse] if (args_size > size) { - return 0; + return; } - T t = {}; const uint8_t* buf = in + args_size; - size_t len = size - args_size; std::shared_ptr _args(new A()); A* args = _args.get(); memcpy(args, in, args_size); - const auto& common = args->common; - ODK_NonceValues nonce_values = {common.api_minor_version, - common.api_major_version, common.nonce, - common.session_id}; - OEMCryptoResult err = odk_fun(buf, len, args, &nonce_values, &t); - if (err != OEMCrypto_SUCCESS) { - return 0; - } - + args->nonce_values.api_major_version = ODK_MAJOR_VERSION; + args->nonce_values.api_minor_version = ODK_MINOR_VERSION; + /* + * Input random bytes from autofuzz are interpreted by this script as + * [function args][data to parse]. Odk deserialize functions + * expect the nonce values in function args to match with those + * in data to parse which is not possible with random bytes. + * We follow two pass approach. + * + * 1st pass - We copy random bytes into struct t and call kdo serialize + * with function args which will create oemcrypto core message using nonce + * from function args. Now we have a valid oemcrypto core message which is + * formed using nonce_values from function args which acts as input bytes + * for 2nd pass + * + * 2nd pass - oemcrypto core message from 1st pass guarantees that + * nonce_values in [function args] and core message match. we call + * odk_deserialize using nonce from function args and oemcrypto core message + * from 1st pass. Then we call kdo function which generates oemcrypto core + * message2, which should be equal to oemcrypto_core_message which was input + * to 2nd pass + */ + // TODO(ellurubharath): Use structure aware fuzzing + // 1st pass + memcpy(&t, buf, sizeof(t)); std::string oemcrypto_core_message; if (!kdo_fun(args, t, &oemcrypto_core_message)) { - return 0; + return; } - assert(oemcrypto_core_message.size() <= size); - memcpy(out, oemcrypto_core_message.data(), oemcrypto_core_message.size()); - return oemcrypto_core_message.size(); + + // 2nd pass + ODK_NonceValues nonce_values = args->nonce_values; + OEMCryptoResult result = + odk_fun(reinterpret_cast(oemcrypto_core_message.data()), + oemcrypto_core_message.size(), args, &nonce_values, &t); + if (result != OEMCrypto_SUCCESS) { + return; + } + std::string oemcrypto_core_message2; + if (!kdo_fun(args, t, &oemcrypto_core_message2)) { + return; + } + assert(oemcrypto_core_message == oemcrypto_core_message2); }; return roundtrip; } @@ -265,8 +288,7 @@ static void verify_roundtrip(const uint8_t* in, size_t size, roundtrip_fun roundtrip, size_t args_size) { std::vector _out(size); auto out = _out.data(); - size_t n = roundtrip(in, out, size, args_size); - assert(n <= size && 0 == memcmp(in + args_size, out, n)); + roundtrip(in, out, size, args_size); } // Entry point for fuzzer, data is random bytes program gets from autofuzzer diff --git a/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp b/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp index 22ee24d0..1da4cc88 100644 --- a/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp +++ b/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp @@ -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 void ValidateRequest(uint32_t message_type, const std::vector& 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 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(¶ms); 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(¶ms); 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(¶ms); 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(¶ms); 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(¶ms); 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(request_hash_read), @@ -596,8 +581,8 @@ TEST(OdkTest, LicenseResponseRoundtrip) { TEST(OdkTest, RenewalResponseRoundtrip) { ODK_RenewalResponseParams params; ODK_SetDefaultRenewalResponseParams(¶ms); - 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(¶ms); // 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 diff --git a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp index 02962307..fd240648 100644 --- a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp +++ b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp @@ -226,24 +226,24 @@ OEMCryptoResult ODK_WriteSingleField(uint8_t* buf, const ODK_Field* field) { } switch (field->type) { case ODK_UINT16: { - uint16_t u16 = htobe16(*static_cast(field->value)); + const uint16_t u16 = htobe16(*static_cast(field->value)); memcpy(buf, &u16, sizeof(u16)); break; } case ODK_UINT32: { - uint32_t u32 = htobe32(*static_cast(field->value)); + const uint32_t u32 = htobe32(*static_cast(field->value)); memcpy(buf, &u32, sizeof(u32)); break; } case ODK_UINT64: { - uint64_t u64 = htobe64(*static_cast(field->value)); + const uint64_t u64 = htobe64(*static_cast(field->value)); memcpy(buf, &u64, sizeof(u64)); break; } case ODK_SUBSTRING: { OEMCrypto_Substring* s = static_cast(field->value); - uint32_t off = htobe32(s->offset); - uint32_t len = htobe32(s->length); + const uint32_t off = htobe32(s->offset); + const uint32_t len = htobe32(s->length); memcpy(buf, &off, sizeof(off)); memcpy(buf + sizeof(off), &len, sizeof(len)); break; @@ -264,7 +264,7 @@ OEMCryptoResult ODK_WriteSingleField(uint8_t* buf, const ODK_Field* field) { OEMCryptoResult ODK_ReadSingleField(const uint8_t* buf, const ODK_Field* field) { - if (field == nullptr || field->value == nullptr) { + if (buf == nullptr || field == nullptr || field->value == nullptr) { return ODK_ERROR_CORE_MESSAGE; } switch (field->type) { @@ -311,7 +311,7 @@ OEMCryptoResult ODK_ReadSingleField(const uint8_t* buf, OEMCryptoResult ODK_DumpSingleField(const uint8_t* buf, const ODK_Field* field) { - if (field == nullptr || field->value == nullptr) { + if (buf == nullptr || field == nullptr || field->value == nullptr) { return ODK_ERROR_CORE_MESSAGE; } switch (field->type) { @@ -442,7 +442,7 @@ void ODK_ResetOdkFields(std::vector* fields) { } for (auto& field : *fields) { if (field.value != nullptr) { - size_t size = ODK_AllocSize(field.type); + const size_t size = ODK_AllocSize(field.type); memset(field.value, 0, size); } } @@ -450,7 +450,7 @@ void ODK_ResetOdkFields(std::vector* fields) { void ODK_BuildMessageBuffer(ODK_CoreMessage* core_message, const std::vector& extra_fields, - uint8_t*& buf, uint32_t* buf_size) { + uint8_t** buf, uint32_t* buf_size) { ASSERT_TRUE(core_message != nullptr); ASSERT_TRUE(buf_size != nullptr); std::vector total_fields = { @@ -477,10 +477,10 @@ void ODK_BuildMessageBuffer(ODK_CoreMessage* core_message, // update message_size *(reinterpret_cast(total_fields[1].value)) = *buf_size; - buf = new uint8_t[*buf_size](); + *buf = new uint8_t[*buf_size]; size_t bytes_written = 0; // serialize ODK fields to message buffer - EXPECT_EQ(OEMCrypto_SUCCESS, ODK_IterFields(ODK_WRITE, buf, SIZE_MAX, + EXPECT_EQ(OEMCrypto_SUCCESS, ODK_IterFields(ODK_WRITE, *buf, SIZE_MAX, &bytes_written, total_fields)); EXPECT_EQ(bytes_written, *buf_size); } diff --git a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h index 95d56638..ad29e89d 100644 --- a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h +++ b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h @@ -92,7 +92,7 @@ void ODK_ResetOdkFields(std::vector* fields); /* Serialize core_message and extra_fields into buf */ void ODK_BuildMessageBuffer(ODK_CoreMessage* core_message, const std::vector& extra_fields, - uint8_t*& buf, uint32_t* buf_size); + uint8_t** buf, uint32_t* buf_size); } /* namespace wvodk_test */