From 09c7473dea4a1cd8233dfd26c3bd82b8d2dbce7c Mon Sep 17 00:00:00 2001 From: Adam Stone Date: Wed, 20 Mar 2019 11:22:04 -0700 Subject: [PATCH 1/2] Add extra OEMCrypto metrics [ Merge from http://go/wvgerrit/74924 ] These were not previously being collected or were collected insufficiently. BUG: http://b/121090396 http://b/112919252 Test: Unit tests, Gplay, Nflix, GTS Change-Id: I32b7206cbe6071519b4a483fbcd0920dc1a26961 --- .../cdm/core/src/crypto_session.cpp | 17 ++++++++-- .../cdm/metrics/include/metrics_collections.h | 20 ++++++++++++ libwvdrmengine/cdm/metrics/src/metrics.proto | 13 +++++++- .../cdm/metrics/src/metrics_collections.cpp | 24 +++++++++++++- .../test/metrics_collections_unittest.cpp | 31 +++++++++++++++++++ 5 files changed, 100 insertions(+), 5 deletions(-) diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index 77bfa357..4b0bdcc3 100644 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -227,8 +227,7 @@ void CryptoSession::Init() { sts = OEMCrypto_SetSandbox( reinterpret_cast(sandbox_id.c_str()), sandbox_id.length()); - // TODO(blueeyes): it might be worth saving the sandbox id in a - // metric. + metrics_->oemcrypto_set_sandbox_.Record(sandbox_id); } M_TIME(sts = OEMCrypto_Initialize(), metrics_, oemcrypto_initialize_, sts); @@ -1868,6 +1867,7 @@ bool CryptoSession::GetResourceRatingTier(SecurityLevel security_level, } WithOecReadLock("GetResourceRatingTier", [&] { *tier = OEMCrypto_ResourceRatingTier(security_level); + metrics_->oemcrypto_resource_rating_tier_.Record(*tier); }); if (*tier < RESOURCE_RATING_TIER_LOW || *tier > RESOURCE_RATING_TIER_HIGH) { uint32_t api_version; @@ -1938,6 +1938,7 @@ CdmResponseType CryptoSession::SetDecryptHash( sts = OEMCrypto_SetDecryptHash( oec_session_id_, frame_number, reinterpret_cast(hash.data()), hash.size()); + metrics_->oemcrypto_set_decrypt_hash_.Increment(sts); }); return MapOEMCryptoResult(sts, SET_DECRYPT_HASH_ERROR, "SetDecryptHash"); @@ -2279,6 +2280,7 @@ CdmResponseType CryptoSession::CreateUsageTableHeader( reinterpret_cast( const_cast(usage_table_header->data())), &usage_table_header_size); + metrics_->oemcrypto_create_usage_table_header_.Increment(result); }); if (result == OEMCrypto_ERROR_SHORT_BUFFER) { @@ -2289,6 +2291,7 @@ CdmResponseType CryptoSession::CreateUsageTableHeader( reinterpret_cast( const_cast(usage_table_header->data())), &usage_table_header_size); + metrics_->oemcrypto_create_usage_table_header_.Increment(result); }); } @@ -2310,6 +2313,7 @@ CdmResponseType CryptoSession::LoadUsageTableHeader( requested_security_level_, reinterpret_cast(usage_table_header.data()), usage_table_header.size()); + metrics_->oemcrypto_load_usage_table_header_.Increment(result); }); if (result != OEMCrypto_SUCCESS) { @@ -2352,6 +2356,7 @@ CdmResponseType CryptoSession::CreateUsageEntry(uint32_t* entry_number) { OEMCryptoResult result; WithOecWriteLock("CreateUsageEntry", [&] { result = OEMCrypto_CreateNewUsageEntry(oec_session_id_, entry_number); + metrics_->oemcrypto_create_new_usage_entry_.Increment(result); }); if (result != OEMCrypto_SUCCESS) { @@ -2382,13 +2387,14 @@ CdmResponseType CryptoSession::LoadUsageEntry( oec_session_id_, entry_number, reinterpret_cast(usage_entry.data()), usage_entry.size()); + metrics_->oemcrypto_load_usage_entry_.Increment(result); }); if (result != OEMCrypto_SUCCESS) { if (result == OEMCrypto_WARNING_GENERATION_SKEW) { LOGW("LoadUsageEntry: OEMCrypto_LoadUsageEntry warning: generation skew"); } else { - LOGE("LoadUsageTableHeader: OEMCrypto_LoadUsageEntry error: %d", result); + LOGE("LoadUsageEntry: OEMCrypto_LoadUsageEntry error: %d", result); } } @@ -2474,6 +2480,7 @@ CdmResponseType CryptoSession::ShrinkUsageTableHeader( result = OEMCrypto_ShrinkUsageTableHeader( requested_security_level_, new_entry_count, NULL, &usage_table_header_len); + metrics_->oemcrypto_shrink_usage_table_header_.Increment(result); }); if (result == OEMCrypto_ERROR_SHORT_BUFFER) { @@ -2485,6 +2492,7 @@ CdmResponseType CryptoSession::ShrinkUsageTableHeader( reinterpret_cast( const_cast(usage_table_header->data())), &usage_table_header_len); + metrics_->oemcrypto_shrink_usage_table_header_.Increment(result); }); } @@ -2502,6 +2510,7 @@ CdmResponseType CryptoSession::MoveUsageEntry(uint32_t new_entry_number) { OEMCryptoResult result; WithOecWriteLock("MoveUsageEntry", [&] { result = OEMCrypto_MoveEntry(oec_session_id_, new_entry_number); + metrics_->oemcrypto_move_entry_.Increment(result); }); return MapOEMCryptoResult( @@ -2550,6 +2559,7 @@ bool CryptoSession::CreateOldUsageEntry( reinterpret_cast(const_cast(client_mac_key.data())), reinterpret_cast(provider_session_token.data()), provider_session_token.size()); + metrics_->oemcrypto_create_old_usage_entry_.Increment(result); }); if (result != OEMCrypto_SUCCESS) { @@ -2570,6 +2580,7 @@ CdmResponseType CryptoSession::CopyOldUsageEntry( oec_session_id_, reinterpret_cast(provider_session_token.data()), provider_session_token.size()); + metrics_->oemcrypto_copy_old_usage_entry_.Increment(result); }); return MapOEMCryptoResult( diff --git a/libwvdrmengine/cdm/metrics/include/metrics_collections.h b/libwvdrmengine/cdm/metrics/include/metrics_collections.h index b80233f7..01a518bf 100644 --- a/libwvdrmengine/cdm/metrics/include/metrics_collections.h +++ b/libwvdrmengine/cdm/metrics/include/metrics_collections.h @@ -254,6 +254,26 @@ class CryptoMetrics { oemcrypto_update_usage_table_; CounterMetric oemcrypto_update_usage_entry_; + CounterMetric + oemcrypto_create_usage_table_header_; + CounterMetric + oemcrypto_load_usage_table_header_; + CounterMetric + oemcrypto_shrink_usage_table_header_; + CounterMetric + oemcrypto_create_new_usage_entry_; + CounterMetric + oemcrypto_load_usage_entry_; + CounterMetric + oemcrypto_move_entry_; + CounterMetric + oemcrypto_create_old_usage_entry_; + CounterMetric + oemcrypto_copy_old_usage_entry_; + ValueMetric oemcrypto_set_sandbox_; + CounterMetric + oemcrypto_set_decrypt_hash_; + ValueMetric oemcrypto_resource_rating_tier_; }; // This class contains session-scoped metrics. All properties and diff --git a/libwvdrmengine/cdm/metrics/src/metrics.proto b/libwvdrmengine/cdm/metrics/src/metrics.proto index e7caa567..993d6f2e 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics.proto +++ b/libwvdrmengine/cdm/metrics/src/metrics.proto @@ -94,7 +94,7 @@ message WvCdmMetrics { // This contains metrics that were captured at the CryptoSession level. These // include CryptoSession metrics and most OEMCrypto metrics. - // next id: 62 + // next id: 73 message CryptoMetrics { // Crypto Session Metrics. optional ValueMetric crypto_session_security_level = 1; @@ -162,6 +162,17 @@ message WvCdmMetrics { optional ValueMetric oemcrypto_usage_table_support = 54; repeated CounterMetric oemcrypto_update_usage_table = 55; repeated CounterMetric oemcrypto_update_usage_entry = 57; + repeated CounterMetric oemcrypto_create_usage_table_header = 62; + repeated CounterMetric oemcrypto_load_usage_table_header = 63; + repeated CounterMetric oemcrypto_shrink_usage_table_header = 64; + repeated CounterMetric oemcrypto_create_new_usage_entry = 65; + repeated CounterMetric oemcrypto_load_usage_entry = 66; + repeated CounterMetric oemcrypto_move_entry = 67; + repeated CounterMetric oemcrypto_create_old_usage_entry = 68; + repeated CounterMetric oemcrypto_copy_old_usage_entry = 69; + optional ValueMetric oemcrypto_set_sandbox = 70; + repeated CounterMetric oemcrypto_set_decrypt_hash = 71; + optional ValueMetric oemcrypto_resource_rating_tier = 72; } // This contains metrics that were captured within a CdmSession. This contains diff --git a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp index e6f4093f..c1da4115 100644 --- a/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp +++ b/libwvdrmengine/cdm/metrics/src/metrics_collections.cpp @@ -161,7 +161,29 @@ void CryptoMetrics::Serialize(WvCdmMetrics::CryptoMetrics *crypto_metrics) oemcrypto_update_usage_table_.ToProto( crypto_metrics->mutable_oemcrypto_update_usage_table()); oemcrypto_update_usage_entry_.ToProto( - crypto_metrics->mutable_oemcrypto_update_usage_entry()); + crypto_metrics->mutable_oemcrypto_update_usage_entry()); + oemcrypto_create_usage_table_header_.ToProto( + crypto_metrics->mutable_oemcrypto_create_usage_table_header()); + oemcrypto_load_usage_table_header_.ToProto( + crypto_metrics->mutable_oemcrypto_load_usage_table_header()); + oemcrypto_shrink_usage_table_header_.ToProto( + crypto_metrics->mutable_oemcrypto_shrink_usage_table_header()); + oemcrypto_create_new_usage_entry_.ToProto( + crypto_metrics->mutable_oemcrypto_create_new_usage_entry()); + oemcrypto_load_usage_entry_.ToProto( + crypto_metrics->mutable_oemcrypto_load_usage_entry()); + oemcrypto_move_entry_.ToProto( + crypto_metrics->mutable_oemcrypto_move_entry()); + oemcrypto_create_old_usage_entry_.ToProto( + crypto_metrics->mutable_oemcrypto_create_old_usage_entry()); + oemcrypto_copy_old_usage_entry_.ToProto( + crypto_metrics->mutable_oemcrypto_copy_old_usage_entry()); + crypto_metrics->set_allocated_oemcrypto_set_sandbox( + oemcrypto_set_sandbox_.ToProto()); + oemcrypto_set_decrypt_hash_.ToProto( + crypto_metrics->mutable_oemcrypto_set_decrypt_hash()); + crypto_metrics->set_allocated_oemcrypto_resource_rating_tier( + oemcrypto_resource_rating_tier_.ToProto()); } SessionMetrics::SessionMetrics() : session_id_(""), completed_(false) {} diff --git a/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp index 52abd85e..801d9fb2 100644 --- a/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp +++ b/libwvdrmengine/cdm/metrics/test/metrics_collections_unittest.cpp @@ -413,6 +413,26 @@ TEST_F(CryptoMetricsTest, AllCryptoMetrics) { .Record(1.0, OEMCrypto_ERROR_INIT_FAILED); crypto_metrics.oemcrypto_update_usage_table_ .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_create_usage_table_header_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_load_usage_table_header_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_shrink_usage_table_header_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_create_new_usage_entry_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_load_usage_entry_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_move_entry_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_create_old_usage_entry_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_copy_old_usage_entry_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_set_sandbox_.Record("sandbox"); + crypto_metrics.oemcrypto_set_decrypt_hash_ + .Increment(OEMCrypto_ERROR_INIT_FAILED); + crypto_metrics.oemcrypto_resource_rating_tier_.Record(123); WvCdmMetrics::CryptoMetrics actual; crypto_metrics.Serialize(&actual); @@ -481,6 +501,17 @@ TEST_F(CryptoMetricsTest, AllCryptoMetrics) { EXPECT_EQ(123, actual.oemcrypto_security_patch_level().int_value()); EXPECT_GT(actual.oemcrypto_select_key_time_us_size(), 0); EXPECT_GT(actual.oemcrypto_update_usage_table_size(), 0); + EXPECT_GT(actual.oemcrypto_create_usage_table_header_size(), 0); + EXPECT_GT(actual.oemcrypto_load_usage_table_header_size(), 0); + EXPECT_GT(actual.oemcrypto_shrink_usage_table_header_size(), 0); + EXPECT_GT(actual.oemcrypto_create_new_usage_entry_size(), 0); + EXPECT_GT(actual.oemcrypto_load_usage_entry_size(), 0); + EXPECT_GT(actual.oemcrypto_move_entry_size(), 0); + EXPECT_GT(actual.oemcrypto_create_old_usage_entry_size(), 0); + EXPECT_GT(actual.oemcrypto_copy_old_usage_entry_size(), 0); + EXPECT_EQ("sandbox", actual.oemcrypto_set_sandbox().string_value()); + EXPECT_GT(actual.oemcrypto_set_decrypt_hash_size(), 0); + EXPECT_EQ(123, actual.oemcrypto_resource_rating_tier().int_value()); } } // namespace metrics From d641797e05fd140e21702e0a95ed1905a325ae0a Mon Sep 17 00:00:00 2001 From: Fred Gylys-Colwell Date: Fri, 22 Mar 2019 11:42:21 -0700 Subject: [PATCH 2/2] Use hex for FDPT hash Merge from Widevine repo of http://go/wvgerrit/75123 Merge from Widevine repo of http://go/wvgerrit/75114 This changes the encoding for the hash to be hex instead of base64. Also, the bad frame number is initialized to 0 to make it easier to debug. And the FDPT test app now uses the correct byte order. Bug: 129100318 Test: unit tests, FDPT test app. Change-Id: I296bab990125a4e18bec92f3316e8289a3b25a6b --- libwvdrmengine/cdm/core/src/cdm_engine.cpp | 2 +- libwvdrmengine/cdm/core/src/crypto_session.cpp | 2 +- libwvdrmengine/cdm/core/test/cdm_engine_test.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libwvdrmengine/cdm/core/src/cdm_engine.cpp b/libwvdrmengine/cdm/core/src/cdm_engine.cpp index bcd11f21..74b9501a 100644 --- a/libwvdrmengine/cdm/core/src/cdm_engine.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_engine.cpp @@ -1810,7 +1810,7 @@ CdmResponseType CdmEngine::ParseDecryptHashString( return INVALID_DECRYPT_HASH_FORMAT; } - std::vector hash_vec = wvcdm::Base64Decode(tokens[2]); + std::vector hash_vec = wvcdm::a2b_hex(tokens[2]); if (hash_vec.empty()) { LOGE("CdmEngine::ParseDecryptHashString: malformed hash: %s", hash_string.c_str()); diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index 77bfa357..126bf590 100644 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -1951,7 +1951,7 @@ CdmResponseType CryptoSession::GetDecryptHashError(std::string* error_string) { } error_string->clear(); - uint32_t failed_frame_number; + uint32_t failed_frame_number = 0; OEMCryptoResult sts; WithOecSessionLock("GetDecryptHashError", [&] { sts = OEMCrypto_GetHashErrorCode(oec_session_id_, &failed_frame_number); diff --git a/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp b/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp index 08459042..87b09f5c 100644 --- a/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp +++ b/libwvdrmengine/cdm/core/test/cdm_engine_test.cpp @@ -405,10 +405,10 @@ TEST_F(WvCdmEngineTest, ParseDecryptHashStringTest) { const std::string test_frame_number_string = std::to_string(test_frame_number); const std::string test_invalid_hash = "an invalid hash"; - std::vector binary_hash{ 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x38 }; + std::vector binary_hash{ 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0xFF }; const std::string test_valid_decoded_hash(binary_hash.begin(), binary_hash.end()); - const std::string test_valid_hash = Base64Encode(binary_hash); + const std::string test_valid_hash = b2a_hex(binary_hash); const std::string test_invalid_hash_string = "sample hash string"; const std::string test_valid_hash_string = test_session_id + kComma + test_frame_number_string + kComma + test_valid_hash;