From 14a034209c9e1a89c7bd96a308113fbd12261a59 Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Thu, 15 Apr 2021 21:57:28 -0700 Subject: [PATCH 1/7] Turn on ODK tests in CE CDM test and fix test helper Merge from Widevine repo of http://go/wvgerrit/122223 This adds the ODK unit tests to the CE CDM tests so that they run as part of the presubmit tests. The test helper had some pointer problems converting a bool to a uint32, so it has been updated to handle this correctly. Some other tests failed comparing signed to unsigned, to these have also been fixed. test: ran odk_test bug: 118657876 Change-Id: I744a1e89f4e4729c31d3f53e729984ffac1d96fd --- .../oemcrypto/odk/src/odk_structs_priv.h | 22 +++++++++---------- .../oemcrypto/odk/test/odk_test.cpp | 8 +++---- .../oemcrypto/odk/test/odk_test_helper.cpp | 20 +++++++++++++++-- .../oemcrypto/odk/test/odk_test_helper.h | 7 +++++- 4 files changed, 39 insertions(+), 18 deletions(-) diff --git a/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h b/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h index 6b138f4d..dd9eb20d 100644 --- a/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h +++ b/libwvdrmengine/oemcrypto/odk/src/odk_structs_priv.h @@ -74,26 +74,26 @@ typedef struct { // 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 +#define ODK_LICENSE_REQUEST_SIZE 20u +#define ODK_RENEWAL_REQUEST_SIZE 28u +#define ODK_PROVISIONING_REQUEST_SIZE 88u // These are the possible timer status values. -#define ODK_CLOCK_TIMER_STATUS_UNDEFINED 0 // Should not happen. +#define ODK_CLOCK_TIMER_STATUS_UNDEFINED 0u // Should not happen. // When the structure has been initialized, but no license is loaded. -#define ODK_CLOCK_TIMER_STATUS_LICENSE_NOT_LOADED 1 +#define ODK_CLOCK_TIMER_STATUS_LICENSE_NOT_LOADED 1u // After the license is loaded, before a successful decrypt. -#define ODK_CLOCK_TIMER_STATUS_LICENSE_LOADED 2 +#define ODK_CLOCK_TIMER_STATUS_LICENSE_LOADED 2u // After the license is loaded, if a renewal has also been loaded. -#define ODK_CLOCK_TIMER_STATUS_RENEWAL_LOADED 3 +#define ODK_CLOCK_TIMER_STATUS_RENEWAL_LOADED 3u // The first decrypt has occurred and the timer is active. -#define ODK_CLOCK_TIMER_STATUS_ACTIVE 4 +#define ODK_CLOCK_TIMER_STATUS_ACTIVE 4u // The first decrypt has occurred and the timer is unlimited. -#define ODK_CLOCK_TIMER_STATUS_UNLIMITED 5 +#define ODK_CLOCK_TIMER_STATUS_UNLIMITED 5u // The timer has transitioned from active to expired. -#define ODK_CLOCK_TIMER_STATUS_EXPIRED 6 +#define ODK_CLOCK_TIMER_STATUS_EXPIRED 6u // The license has been marked as inactive. -#define ODK_CLOCK_TIMER_STATUS_LICENSE_INACTIVE 7 +#define ODK_CLOCK_TIMER_STATUS_LICENSE_INACTIVE 7u // A helper function for computing timer limits when a renewal is loaded. OEMCryptoResult ODK_ComputeRenewalDuration(const ODK_TimerLimits* timer_limits, diff --git a/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp b/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp index cf41b77a..11113434 100644 --- a/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp +++ b/libwvdrmengine/oemcrypto/odk/test/odk_test.cpp @@ -176,15 +176,15 @@ TEST(OdkTest, SerializeFieldsStress) { std::srand(0); size_t total_size = 0; for (int i = 0; i < n; i++) { - fields[i].type = static_cast(std::rand() % - static_cast(ODK_NUMTYPES)); + fields[i].type = static_cast( + std::rand() % static_cast(ODK_LAST_STRESSABLE_TYPE)); fields[i].value = malloc(ODK_AllocSize(fields[i].type)); fields[i].name = "stress"; total_size += ODK_FieldLength(fields[i].type); } uint8_t* buf = new uint8_t[total_size]{}; - for (int i = 0; i < total_size; i++) { + for (size_t i = 0; i < total_size; i++) { buf[i] = std::rand() & 0xff; } @@ -701,7 +701,7 @@ TEST(OdkSizeTest, ReleaseRequest) { &core_message_length, &nonce_values, &clock_values, system_time_seconds)); // Release requests do not have a core message. - EXPECT_GE(core_message_length, 0); + EXPECT_GE(core_message_length, 0u); } TEST(OdkSizeTest, ProvisioningRequest) { diff --git a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp index 9eeb49c4..d96d3c45 100644 --- a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp +++ b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.cpp @@ -86,10 +86,10 @@ void ODK_SetDefaultLicenseResponseParams(ODK_LicenseResponseParams* params) { ".srm_restriction_data"}, {ODK_UINT32, &(params->parsed_license.license_type), ".license_type"}, {ODK_UINT32, &(params->parsed_license.nonce_required), ".nonce_required"}, - {ODK_UINT32, + {ODK_BOOL, &(params->parsed_license.timer_limits.soft_enforce_rental_duration), ".soft_enforce_rental_duration"}, - {ODK_UINT32, + {ODK_BOOL, &(params->parsed_license.timer_limits.soft_enforce_playback_duration), ".soft_enforce_playback_duration"}, {ODK_UINT64, @@ -201,6 +201,8 @@ size_t ODK_FieldLength(ODK_FieldType type) { return sizeof(uint32_t); case ODK_UINT64: return sizeof(uint64_t); + case ODK_BOOL: // Booleans are stored in the message as 32 bit ints. + return sizeof(uint32_t); case ODK_SUBSTRING: return sizeof(uint32_t) + sizeof(uint32_t); case ODK_DEVICEID: @@ -242,6 +244,12 @@ OEMCryptoResult ODK_WriteSingleField(uint8_t* buf, const ODK_Field* field) { memcpy(buf, &u64, sizeof(u64)); break; } + case ODK_BOOL: { + const bool value = *static_cast(field->value); + const uint32_t u32 = oemcrypto_htobe32(value ? 1 : 0); + memcpy(buf, &u32, sizeof(u32)); + break; + } case ODK_SUBSTRING: { OEMCrypto_Substring* s = static_cast(field->value); const uint32_t off = oemcrypto_htobe32(s->offset); @@ -288,6 +296,13 @@ OEMCryptoResult ODK_ReadSingleField(const uint8_t* buf, *u64p = oemcrypto_be64toh(*u64p); break; } + case ODK_BOOL: { + uint32_t value; + memcpy(&value, buf, sizeof(uint32_t)); + value = oemcrypto_be32toh(value); + *static_cast(field->value) = (value != 0); + break; + } case ODK_SUBSTRING: { OEMCrypto_Substring* s = static_cast(field->value); uint32_t off = 0; @@ -325,6 +340,7 @@ OEMCryptoResult ODK_DumpSingleField(const uint8_t* buf, << "\n"; break; } + case ODK_BOOL: case ODK_UINT32: { uint32_t val; memcpy(&val, buf, sizeof(uint32_t)); diff --git a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h index c32318e4..f29c9b8a 100644 --- a/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h +++ b/libwvdrmengine/oemcrypto/odk/test/odk_test_helper.h @@ -21,7 +21,12 @@ enum ODK_FieldType { ODK_SUBSTRING, ODK_DEVICEID, ODK_HASH, - ODK_NUMTYPES, + // The "stressable" types are the ones we can put in a stress test that packs + // and unpacks random data and can expect to get back the same thing. + ODK_LAST_STRESSABLE_TYPE, + // Put boolean after ODK_LAST_STRESSABLE_TYPE, so that we skip boolean type in + // SerializeFieldsStress because we unpack any nonzero to 'true'. + ODK_BOOL, }; enum ODK_FieldMode { From 5a58b6d8d2cd154754425c6125bdbb3ff6381c8d Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Thu, 15 Apr 2021 21:59:06 -0700 Subject: [PATCH 2/7] Add http socket tests to other tests Merge from Widevine repo of http://go/wvgerrit/122224 This CL removes the separate main() in http_socket_test that allowed the user to set the server on the command line. I don't think anybody was using this, and it conflicted with our desire to include this suite of tests with the other CE CDM tests running on Luci. test: ran http_socket_test bug: 118657876 Change-Id: I9228b9cc97a0af2afd1bb4a99bc40b88ce956d67 --- .../cdm/core/test/http_socket_test.cpp | 40 ------------------- libwvdrmengine/cdm/test/Android.mk | 3 +- 2 files changed, 1 insertion(+), 42 deletions(-) diff --git a/libwvdrmengine/cdm/core/test/http_socket_test.cpp b/libwvdrmengine/cdm/core/test/http_socket_test.cpp index 8de2d0d3..34a9f375 100644 --- a/libwvdrmengine/cdm/core/test/http_socket_test.cpp +++ b/libwvdrmengine/cdm/core/test/http_socket_test.cpp @@ -206,43 +206,3 @@ TEST_F(HttpSocketTest, RoundTripTest) { } } // namespace wvcdm - -int main(int argc, char** argv) { - using namespace wvcdm; - - ::testing::InitGoogleTest(&argc, argv); - - std::string temp; - std::string test_server(kHttpsTestServer); - std::string test_data(gTestData); - for (int i = 1; i < argc; i++) { - temp.assign(argv[i]); - if (temp.find("--server=") == 0) { - gTestServer.assign(temp.substr(strlen("--server="))); - } else if (temp.find("--data=") == 0) { - gTestData.assign(temp.substr(strlen("--data="))); - } else { - std::cout << "error: unknown option '" << argv[i] << "'" << std::endl; - std::cout << "usage: http_socket_test [options]" << std::endl - << std::endl; - std::cout << std::setw(30) << std::left << " --server="; - std::cout - << "configure the test server url, please include http[s] in the url" - << std::endl; - std::cout << std::setw(30) << std::left << " "; - std::cout << "default: " << test_server << std::endl; - std::cout << std::setw(30) << std::left << " --data="; - std::cout << "configure data to send, in ascii string format" - << std::endl; - std::cout << std::setw(30) << std::left << " "; - std::cout << "default: " << test_data << std::endl << std::endl; - return 0; - } - } - - std::cout << std::endl; - std::cout << "Server: " << gTestServer << std::endl; - std::cout << "Data: " << gTestData << std::endl; - - return RUN_ALL_TESTS(); -} diff --git a/libwvdrmengine/cdm/test/Android.mk b/libwvdrmengine/cdm/test/Android.mk index 734e1dd8..d37ee0f6 100644 --- a/libwvdrmengine/cdm/test/Android.mk +++ b/libwvdrmengine/cdm/test/Android.mk @@ -91,8 +91,7 @@ include $(LOCAL_PATH)/integration-test.mk test_name := http_socket_test test_src_dir := ../core/test -test_main := -include $(LOCAL_PATH)/integration-test.mk +include $(LOCAL_PATH)/unit-test.mk test_name := initialization_data_unittest test_src_dir := ../core/test From a87eec804c665034abf0571db1eafe9790d09f6b Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Thu, 15 Apr 2021 22:01:24 -0700 Subject: [PATCH 3/7] Fix unused param warnings in oemcrypto fuzz test Merge from Widevine repo of http://go/wvgerrit/122403 Re-merge of http://go/wvgerrit/105184 which was accidentally undone by http://go/wvgerrit/107063. Unused params are reported as warning when built in Android. Test: Ran oemcrypto unit tests Bug: 160734070 Change-Id: Id8384c58c8ace0b214464380fb961d108f1b5c3b --- .../fuzzing/odk_provisioning_response_fuzz_with_mutator.cpp | 4 +++- .../test/fuzzing/odk_renewal_response_fuzz_with_mutator.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_provisioning_response_fuzz_with_mutator.cpp b/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_provisioning_response_fuzz_with_mutator.cpp index 52492534..4ad8ca40 100644 --- a/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_provisioning_response_fuzz_with_mutator.cpp +++ b/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_provisioning_response_fuzz_with_mutator.cpp @@ -6,13 +6,15 @@ #include #include "fuzzing/odk_fuzz_helper.h" +#include "odk_attributes.h" namespace oemcrypto_core_message { // The custom mutator: Ensure that each input can be deserialized properly // by ODK function after mutation. extern "C" size_t LLVMFuzzerCustomMutator(uint8_t* data, size_t size, - size_t max_size, unsigned int seed) { + size_t max_size, + unsigned int seed UNUSED) { const size_t kProvisioningResponseArgsSize = sizeof(ODK_ParseProvisioning_Args); if (size < kProvisioningResponseArgsSize) { diff --git a/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_renewal_response_fuzz_with_mutator.cpp b/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_renewal_response_fuzz_with_mutator.cpp index d6a9dd4a..2502ab80 100644 --- a/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_renewal_response_fuzz_with_mutator.cpp +++ b/libwvdrmengine/oemcrypto/odk/test/fuzzing/odk_renewal_response_fuzz_with_mutator.cpp @@ -6,13 +6,15 @@ #include #include "fuzzing/odk_fuzz_helper.h" +#include "odk_attributes.h" namespace oemcrypto_core_message { // The custom mutator: Ensure that each input can be deserialized properly // by ODK function after mutation. extern "C" size_t LLVMFuzzerCustomMutator(uint8_t* data, size_t size, - size_t max_size, unsigned int seed) { + size_t max_size, + unsigned int seed UNUSED) { const size_t kRenewalResponseArgsSize = sizeof(ODK_ParseRenewal_Args); if (size < kRenewalResponseArgsSize) { return 0; From ebb7d45a257b8af2f4f0bacfbac45aeeed8b6fe9 Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Fri, 16 Apr 2021 10:43:32 -0700 Subject: [PATCH 4/7] Fix CDM Builds w/ OEMCrypto_ERROR_INVALID_ENTITLED_KEY_SESSION Merge from Widevine repo of http://go/wvgerrit/108224 A recent Copybara merge to master broke CDM builds by introducing a new OEMCrypto error code. This patch adds it to the test printers so that the build can pass again. Bug: 185597829 Test: CE CDM Build Change-Id: I6dd829f4c618c9ebec937cf711ff57b7f1678994 --- libwvdrmengine/cdm/core/test/test_printers.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libwvdrmengine/cdm/core/test/test_printers.cpp b/libwvdrmengine/cdm/core/test/test_printers.cpp index 267444bd..d7e99b5b 100644 --- a/libwvdrmengine/cdm/core/test/test_printers.cpp +++ b/libwvdrmengine/cdm/core/test/test_printers.cpp @@ -74,7 +74,6 @@ void PrintTo(const enum CdmResponseType& value, ::std::ostream* os) { case CERT_PROVISIONING_RESPONSE_ERROR_10: *os << "CERT_PROVISIONING_RESPONSE_ERROR_10"; break; - break; case CLIENT_ID_AES_ENCRYPT_ERROR: *os << "CLIENT_ID_AES_ENCRYPT_ERROR"; break; @@ -1184,6 +1183,9 @@ void PrintTo(const enum OEMCryptoResult& value, ::std::ostream* os) { case OEMCrypto_WARNING_MIXED_OUTPUT_PROTECTION: *os << "MIXED_OUTPUT_PROTECTION"; break; + case OEMCrypto_ERROR_INVALID_ENTITLED_KEY_SESSION: + *os << "OEMCrypto_ERROR_INVALID_ENTITLED_KEY_SESSION"; + break; // ODK Values. case ODK_ERROR_CORE_MESSAGE: *os << "CORE_MESSAGE"; From c21d40c68f9b04ab118f59586b9c027885958b01 Mon Sep 17 00:00:00 2001 From: Rahul Frias Date: Fri, 23 Apr 2021 12:21:43 -0700 Subject: [PATCH 5/7] Address CE CDM code review comments Undoes a change to the buffer size from http://ag/13865723 Bug: 184813991 Test: WV unit/integration tests Change-Id: I40cf786f149626ff65a3362020b3da859bb86159 --- libwvdrmengine/cdm/util/src/string_conversions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libwvdrmengine/cdm/util/src/string_conversions.cpp b/libwvdrmengine/cdm/util/src/string_conversions.cpp index 3faa2fe5..9e822102 100644 --- a/libwvdrmengine/cdm/util/src/string_conversions.cpp +++ b/libwvdrmengine/cdm/util/src/string_conversions.cpp @@ -208,7 +208,7 @@ std::string b2a_hex(const std::string& byte) { std::string HexEncode(const uint8_t* in_buffer, size_t size) { static const char kHexChars[] = "0123456789ABCDEF"; if (size == 0) return ""; - constexpr unsigned int kMaxSafeSize = 3072; + constexpr unsigned int kMaxSafeSize = 2048; if (size > kMaxSafeSize) size = kMaxSafeSize; // Each input byte creates two output hex characters. std::string out_buffer(size * 2, '\0'); From 3a503278e6213f0c37e88451fbf6def8b301f85c Mon Sep 17 00:00:00 2001 From: Robert Shih Date: Wed, 20 Jan 2021 18:21:42 -0800 Subject: [PATCH 6/7] Update Android WVCdm version to 16.1.0 TODO: update VersionNumberTest.VersionNumberChangeCanary [ Merge of http://go/wvgerrit/123403 ] Bug: 183073374 Test: GtsMediaTestCases Change-Id: I1773416b6f3f212bead5e6b7749da52883041d07 --- libwvdrmengine/cdm/include/wv_android_constants.h | 2 +- libwvdrmengine/cdm/test/request_license_test.cpp | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/libwvdrmengine/cdm/include/wv_android_constants.h b/libwvdrmengine/cdm/include/wv_android_constants.h index e69913bb..ac7c6627 100644 --- a/libwvdrmengine/cdm/include/wv_android_constants.h +++ b/libwvdrmengine/cdm/include/wv_android_constants.h @@ -9,7 +9,7 @@ namespace wvcdm { -static const std::string kWVAndroidCdmVersion = "16.0.0"; +static const std::string kWVAndroidCdmVersion = "16.1.0"; } // namespace wvcdm diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index e5dcf686..c60349b4 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -5952,6 +5952,11 @@ TEST(VersionNumberTest, VersionNumberChangeCanary) { android::base::GetProperty("ro.build.version.release", ""); ASSERT_TRUE(release_number.size() > 0); // Reminder to change the Widevine CDM version number. + // + // TODO(robertshih): + // update expected release_number to 12 once Android version has changed; + // no need to update Widevine version number in wv_android_constants.h + // because it is already at 16.1.0. EXPECT_STREQ("11", release_number.c_str()) << "The Android version number has changed. You need to update this test " "and also possibly update the Widevine version number in " From 63eae1f4a4e4e98f6c152aed0196fdb46acbacb2 Mon Sep 17 00:00:00 2001 From: Ereth McKnight-MacNeil Date: Wed, 20 Jan 2021 12:52:57 -0800 Subject: [PATCH 7/7] Shell quoting in move_widevine_data.sh Add double quotes to prevent globbing and word splitting. Bug: crbug.com/1168550 Test: Create /data/mediadrm files and observe they are moved Change-Id: I8d1cd70971588f903657825ea6e10c019954f403 (cherry picked from commit 8649f7a952b7e4f8edd17edfca0b0962e2fb2662) (cherry picked from commit 84d0d73be533d7c5916830e6559888433c3b19bf) --- libwvdrmengine/move_widevine_data.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libwvdrmengine/move_widevine_data.sh b/libwvdrmengine/move_widevine_data.sh index 56c1e704..dfdd852a 100755 --- a/libwvdrmengine/move_widevine_data.sh +++ b/libwvdrmengine/move_widevine_data.sh @@ -9,8 +9,8 @@ if [ ! -f "$FILES_MOVED" ]; then for i in "$SRC_PATH/IDM"*; do dest_path=$DEST_PATH/"${i#$SRC_PATH/}" if [ -d "$i" ]; then - mkdir -p $dest_path -m 700 - mv $i "$DEST_PATH" + mkdir -p "$dest_path" -m 700 + mv "$i" "$DEST_PATH" for file in ${dest_path}/${L3_FILE_PATTERN}; do [ -e "${file}" ] || continue # We need to move any L3 files in the IDM folder to the L3 subfolder. @@ -18,7 +18,7 @@ if [ ! -f "$FILES_MOVED" ]; then mkdir -p "${l3_dir}" -m 700 mv "${file}" "${l3_dir}" done - find $dest_path -print0 | while IFS= read -r -d '' file + find "$dest_path" -print0 | while IFS= read -r -d '' file do chgrp media "$file" done