From 35cf9c2f9998934df40096200ef479d6ec5c8404 Mon Sep 17 00:00:00 2001 From: Ian Benz Date: Fri, 19 Jan 2024 23:28:28 +0000 Subject: [PATCH] Fix OEMCrypto test issues identified by Coverity Change-Id: Ic9f4982bf022292d10a0a88f10648a46077ec0cf --- .../oemcrypto/test/oec_session_util.cpp | 35 ++++++++++--------- .../oemcrypto/test/oemcrypto_basic_test.cpp | 4 +-- .../oemcrypto/test/oemcrypto_decrypt_test.cpp | 6 ++-- .../oemcrypto/test/oemcrypto_decrypt_test.h | 2 +- .../test/oemcrypto_generic_crypto_test.cpp | 10 +++--- .../oemcrypto/test/oemcrypto_license_test.h | 2 +- .../test/oemcrypto_provisioning_test.cpp | 15 ++++---- 7 files changed, 38 insertions(+), 36 deletions(-) diff --git a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp index 2517ffea..833f5825 100644 --- a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp +++ b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp @@ -114,7 +114,6 @@ OEMCryptoResult DecryptCTR(const vector& key_handle, } // namespace - // Encrypt a block of data using CTR mode. void EncryptCTR(const vector& in_buffer, const uint8_t* key, const uint8_t* starting_iv, vector* out_buffer) { @@ -219,9 +218,12 @@ class boringssl_ptr { Test_PST_Report::Test_PST_Report(const std::string& pst_in, OEMCrypto_Usage_Entry_Status status_in) - : status(status_in), pst(pst_in) { - time_created = wvutil::Clock().GetCurrentTime(); -} + : status(status_in), + seconds_since_license_received(0), + seconds_since_first_decrypt(0), + seconds_since_last_decrypt(0), + pst(pst_in), + time_created(wvutil::Clock().GetCurrentTime()) {} template @@ -561,12 +563,12 @@ void Provisioning40RoundTrip::PrepareSession(bool is_oem_key) { public_key.resize(public_key_size); if (is_oem_key) { - wrapped_oem_key_ = wrapped_private_key; - oem_public_key_ = public_key; + wrapped_oem_key_ = std::move(wrapped_private_key); + oem_public_key_ = std::move(public_key); oem_key_type_ = key_type; } else { - wrapped_drm_key_ = wrapped_private_key; - drm_public_key_ = public_key; + wrapped_drm_key_ = std::move(wrapped_private_key); + drm_public_key_ = std::move(public_key); drm_key_type_ = key_type; } } @@ -622,8 +624,8 @@ void Provisioning40CastRoundTrip::PrepareSession() { wrapped_private_key.resize(wrapped_private_key_size); public_key.resize(public_key_size); - wrapped_drm_key_ = wrapped_private_key; - drm_public_key_ = public_key; + wrapped_drm_key_ = std::move(wrapped_private_key); + drm_public_key_ = std::move(public_key); drm_key_type_ = key_type; } @@ -1233,11 +1235,11 @@ void EntitledMessage::MakeOneKey(size_t entitlement_key_index) { offsets->content_key_data_iv = FindSubstring( key_data->content_key_data_iv, sizeof(key_data->content_key_data_iv)); - EXPECT_EQ(1, GetRandBytes(key_data->content_iv, - sizeof(key_data->content_iv))); + EXPECT_EQ(1, + GetRandBytes(key_data->content_iv, sizeof(key_data->content_iv))); key_data->content_iv_length = sizeof(key_data->content_iv); - offsets->content_iv = FindSubstring( - key_data->content_iv, key_data->content_iv_length); + offsets->content_iv = + FindSubstring(key_data->content_iv, key_data->content_iv_length); } OEMCrypto_EntitledContentKeyObject* EntitledMessage::entitled_key_array() { @@ -1387,7 +1389,7 @@ void EntitledMessage::LoadCasKeys(bool load_even, bool load_odd, odd_key.content_key_id = entitled_key_array_[1].content_key_id; odd_key.content_key_data_iv = entitled_key_array_[1].content_key_data_iv; odd_key.content_key_data = entitled_key_array_[1].content_key_data; - even_key.content_iv = entitled_key_array_[1].content_iv; + odd_key.content_iv = entitled_key_array_[1].content_iv; } OEMCryptoResult sts = OEMCrypto_LoadCasECMKeys( @@ -1591,7 +1593,7 @@ OEMCryptoResult RenewalRoundTrip::LoadResponse(Session* session) { GetFileName("oemcrypto_load_renewal_fuzz_seed_corpus"); // Corpus for renewal response fuzzer should be in the format: // OEMCrypto_Renewal_Response_Fuzz + license_renewal_response. - OEMCrypto_Renewal_Response_Fuzz renewal_response_fuzz; + OEMCrypto_Renewal_Response_Fuzz renewal_response_fuzz = {}; renewal_response_fuzz.core_request = core_request_; renewal_response_fuzz.renewal_duration_seconds = renewal_duration_seconds_; AppendToFile(file_name, @@ -1870,7 +1872,6 @@ void Session::LoadOEMCert(bool verify_cert) { util::RsaPublicKey::FromSslHandle(EVP_PKEY_get0_RSA(pubkey.get())); ASSERT_TRUE(public_rsa_) << "Failed to extract public RSA key from OEM certificate"; - return; } if (verify_cert) { vector buffer(80); diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_basic_test.cpp b/libwvdrmengine/oemcrypto/test/oemcrypto_basic_test.cpp index 365cfec4..a17c750b 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_basic_test.cpp +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_basic_test.cpp @@ -359,7 +359,7 @@ TEST_F(OEMCryptoClientTest, CheckJsonBuildInformationAPI18) { // check for existence in map // check if value matches expectation // remove from map - for (int i = 0; i < jsmn_result; i++) { + for (int32_t i = 0; i < jsmn_result; i++) { jsmntok_t token = tokens[i]; std::string key = build_info.substr(token.start, token.end - token.start); if (expected.find(key) != expected.end()) { @@ -372,7 +372,7 @@ TEST_F(OEMCryptoClientTest, CheckJsonBuildInformationAPI18) { // if map is not empty, return false if (expected.size() > 0) { std::string missing; - for (auto e : expected) { + for (const auto& e : expected) { missing.append(e.first); missing.append(" "); } diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.cpp b/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.cpp index a61c8933..00df038d 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.cpp +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.cpp @@ -463,7 +463,7 @@ TEST_P(OEMCryptoSessionTestsDecryptTests, DecryptMaxSampleAPI16) { subsample_sizes.push_back({clear_size, encrypted_size}); bytes_remaining -= this_subsample_size; } - ASSERT_NO_FATAL_FAILURE(SetSubsampleSizes(subsample_sizes)); + ASSERT_NO_FATAL_FAILURE(SetSubsampleSizes(std::move(subsample_sizes))); ASSERT_NO_FATAL_FAILURE(LoadLicense()); ASSERT_NO_FATAL_FAILURE(MakeBuffers()); ASSERT_NO_FATAL_FAILURE(EncryptData()); @@ -477,7 +477,7 @@ TEST_P(OEMCryptoSessionTestsDecryptTests, while (number_of_subsamples-- > 0) { subsample_sizes.push_back({100, 100}); } - SetSubsampleSizes(subsample_sizes); + SetSubsampleSizes(std::move(subsample_sizes)); LoadLicense(); MakeBuffers(); EncryptData(); @@ -501,7 +501,7 @@ TEST_P(OEMCryptoSessionTestsDecryptTests, OEMCryptoMemoryCheckDecryptCENCStatusForHugeSubSample) { std::vector subsample_sizes; subsample_sizes.push_back({100000, 100000}); - SetSubsampleSizes(subsample_sizes); + SetSubsampleSizes(std::move(subsample_sizes)); LoadLicense(); MakeBuffers(); EncryptData(); diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.h b/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.h index 36a249c8..556d08ea 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.h +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_decrypt_test.h @@ -120,7 +120,7 @@ class OEMCryptoSessionTestsDecryptTests void SetSubsampleSizes(std::vector subsample_sizes) { // This is just sugar for having one sample with the given subsamples in it. - SetSampleSizes({subsample_sizes}); + SetSampleSizes({std::move(subsample_sizes)}); } void SetSampleSizes(std::vector> sample_sizes) { diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_generic_crypto_test.cpp b/libwvdrmengine/oemcrypto/test/oemcrypto_generic_crypto_test.cpp index 867b4f8e..83ff4d86 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_generic_crypto_test.cpp +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_generic_crypto_test.cpp @@ -122,12 +122,12 @@ TEST_P(OEMCryptoGenericCryptoTest, GenericKeyDecryptSameBufferAPI12) { session_.license().keys[key_index].key_id, session_.license().keys[key_index].key_id_length, OEMCrypto_CipherMode_CENC, key_handle)); - vector buffer = encrypted; + vector resultant(encrypted.size()); ASSERT_EQ(OEMCrypto_SUCCESS, - GenericDecrypt(key_handle.data(), key_handle.size(), buffer.data(), - buffer.size(), iv_, OEMCrypto_AES_CBC_128_NO_PADDING, - buffer.data())); - ASSERT_EQ(clear_buffer_, buffer); + GenericDecrypt(key_handle.data(), key_handle.size(), + encrypted.data(), encrypted.size(), iv_, + OEMCrypto_AES_CBC_128_NO_PADDING, resultant.data())); + ASSERT_EQ(clear_buffer_, resultant); } // Test that Generic_Decrypt fails to decrypt to an insecure buffer if the key diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_license_test.h b/libwvdrmengine/oemcrypto/test/oemcrypto_license_test.h index 8d7339b2..6e185c32 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_license_test.h +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_license_test.h @@ -329,7 +329,7 @@ class LicenseWithUsageEntry { ASSERT_NO_FATAL_FAILURE(session_.GenerateReport(pst())); Test_PST_Report expected(pst(), status); ASSERT_NO_FATAL_FAILURE( - session_.VerifyReport(expected, time_license_received_, + session_.VerifyReport(std::move(expected), time_license_received_, time_first_decrypt_, time_last_decrypt_)); // The PST report was signed above. Below we verify that the entire message // that is sent to the server will be signed by the right mac keys. diff --git a/libwvdrmengine/oemcrypto/test/oemcrypto_provisioning_test.cpp b/libwvdrmengine/oemcrypto/test/oemcrypto_provisioning_test.cpp index 2cdab5a9..33101bad 100644 --- a/libwvdrmengine/oemcrypto/test/oemcrypto_provisioning_test.cpp +++ b/libwvdrmengine/oemcrypto/test/oemcrypto_provisioning_test.cpp @@ -1269,13 +1269,13 @@ TEST_F(OEMCryptoLoadsCertificate, RSAPerformance) { } auto delta_time = clock.now() - start_time; const double provision_time = - delta_time / std::chrono::milliseconds(1) / count; + delta_time / std::chrono::milliseconds(1) / static_cast(count); Session session; ASSERT_NO_FATAL_FAILURE(CreateWrappedDRMKey()); start_time = clock.now(); count = 0; - while (clock.now() - start_time < kTestDuration) { + do { Session s; ASSERT_NO_FATAL_FAILURE(s.open()); ASSERT_NO_FATAL_FAILURE(s.LoadWrappedRsaDrmKey(wrapped_drm_key_)); @@ -1309,10 +1309,10 @@ TEST_F(OEMCryptoLoadsCertificate, RSAPerformance) { signature.data(), &signature_length, kSign_RSASSA_PSS); ASSERT_EQ(OEMCrypto_SUCCESS, sts); count++; - } + } while (clock.now() - start_time < kTestDuration); delta_time = clock.now() - start_time; const double license_request_time = - delta_time / std::chrono::milliseconds(1) / count; + delta_time / std::chrono::milliseconds(1) / static_cast(count); Session s; ASSERT_NO_FATAL_FAILURE(s.open()); @@ -1345,17 +1345,18 @@ TEST_F(OEMCryptoLoadsCertificate, RSAPerformance) { "b9f58fc1dacbf74b59d354a1e62cfa0e" "bf"); start_time = clock.now(); - while (clock.now() - start_time < kTestDuration) { + count = 0; + do { ASSERT_EQ(OEMCrypto_SUCCESS, OEMCrypto_DeriveKeysFromSessionKey( s.session_id(), enc_session_key.data(), enc_session_key.size(), mac_context.data(), mac_context.size(), enc_context.data(), enc_context.size())); count++; - } + } while (clock.now() - start_time < kTestDuration); delta_time = clock.now() - start_time; const double derive_keys_time = - delta_time / std::chrono::milliseconds(1) / count; + delta_time / std::chrono::milliseconds(1) / static_cast(count); OEMCrypto_Security_Level level = OEMCrypto_SecurityLevel(); printf(