From 48122e2c11f32d1e3090436cf0201a27739fc583 Mon Sep 17 00:00:00 2001 From: "John W. Bruce" Date: Tue, 6 Oct 2020 14:40:18 -0700 Subject: [PATCH 1/2] Fix TimeRollbackPrevention Test (This is a merge of http://go/wvgerrit/107243.) This code is based on a bug report and patch from Sony. The TimeRollbackPrevention test was failing when run with CE CDM and the OEC Ref, although it passed in some other configurations. The cause was twofold: 1) The test sleep code was not accounting for rollback when calculating the clock drift, causing incorrect time values to elapse. 2) Fixing the previous exposed a bug in the CE CDM test host where it did not handle negative time passing correctly. This patch expands Sony's fix with additional comments and some code cleanup to try to make the code clearer and more robust against future errors, particularly in the error-prone TestHost code. Bug: 169942369 Test: jenkins/ce_cdm_tests Test: build_and_run_all_unit_tests.sh Test: x86-64, all CE CDM unit tests Change-Id: Id52b8c38255f70b04bc2735c4e309fb90992f53e --- libwvdrmengine/cdm/util/test/test_sleep.cpp | 17 ++++++++++------- libwvdrmengine/cdm/util/test/test_sleep.h | 6 +++--- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/libwvdrmengine/cdm/util/test/test_sleep.cpp b/libwvdrmengine/cdm/util/test/test_sleep.cpp index 1656381a..4893ca74 100644 --- a/libwvdrmengine/cdm/util/test/test_sleep.cpp +++ b/libwvdrmengine/cdm/util/test/test_sleep.cpp @@ -23,7 +23,7 @@ namespace wvcdm { bool TestSleep::real_sleep_ = true; TestSleep::CallBack* TestSleep::callback_ = nullptr; -int TestSleep::total_clock_rollback_ = 0; +int TestSleep::total_clock_rollback_seconds_ = 0; void TestSleep::Sleep(unsigned int seconds) { int64_t milliseconds = 1000 * seconds; @@ -33,11 +33,15 @@ void TestSleep::Sleep(unsigned int seconds) { // total since the start, and then compare to a running total of sleep // calls. We sleep for approximately x second, and then advance the clock by // the amount of time that has actually passed. - static auto start_real = std::chrono::system_clock().now(); - static int64_t fake_clock = 0; + static const auto start_real = std::chrono::system_clock().now(); sleep(seconds); - auto now_real = std::chrono::system_clock().now(); - int64_t total_real = (now_real - start_real) / std::chrono::milliseconds(1); + const auto now_real = std::chrono::system_clock().now(); + const int64_t rollback_adjustment = 1000 * total_clock_rollback_seconds_; + const int64_t total_real = + (now_real - start_real) / std::chrono::milliseconds(1) + + rollback_adjustment; + + static int64_t fake_clock = 0; // We want to advance the fake clock by the difference between the real // clock, and the previous value on the fake clock. milliseconds = total_real - fake_clock; @@ -90,7 +94,7 @@ bool TestSleep::RollbackSystemTime(int seconds) { // For both real and fake sleep we still update the callback and we still keep // track of the total amount of time slept. - total_clock_rollback_ += seconds; + total_clock_rollback_seconds_ += seconds; if (callback_ != nullptr) callback_->ElapseTime(-1000 * seconds); return true; } @@ -98,7 +102,6 @@ bool TestSleep::RollbackSystemTime(int seconds) { bool TestSleep::CanChangeSystemTime() { // If we are using a fake clock, then we can move the clock backwards by // just going backwards. - // ElapseTime. if (!real_sleep_) { return true; } diff --git a/libwvdrmengine/cdm/util/test/test_sleep.h b/libwvdrmengine/cdm/util/test/test_sleep.h index 34c4b3f6..35dcdc70 100644 --- a/libwvdrmengine/cdm/util/test/test_sleep.h +++ b/libwvdrmengine/cdm/util/test/test_sleep.h @@ -44,8 +44,8 @@ class TestSleep { // Roll the system clock forward to undo all previous calls to // RollBackSystemTime. Returns true on success. static bool ResetRollback() { - return total_clock_rollback_ == 0 || - RollbackSystemTime(-total_clock_rollback_); + return total_clock_rollback_seconds_ == 0 || + RollbackSystemTime(-total_clock_rollback_seconds_); } // Returns true if the system time can be rolled back. This is true on some @@ -67,7 +67,7 @@ class TestSleep { static CallBack* callback_; // The sum of all calls to RollBackSystemTime. Kept so we can undo all changes // at the end of a test. - static int total_clock_rollback_; + static int total_clock_rollback_seconds_; }; } // namespace wvcdm From 25489dfa5b1e69ae3f9f1fb980927edd5e195665 Mon Sep 17 00:00:00 2001 From: "John W. Bruce" Date: Tue, 6 Oct 2020 14:42:54 -0700 Subject: [PATCH 2/2] Allow 1 or 2 GetOEMPublicCertificate Calls in Metrics Tests (This is a merge of http://go/wvgerrit/107263.) The CryptoSessionMetricsTest suite assumed that GetOEMPublicCertificate would only be called once, but in practice, it may be called twice, since the first call can return OEMCrypto_ERROR_SHORT_BUFFER. This patch updates the tests to accept 1 or 2 calls. This patch also updates a few EXPECTs on vector lengths that should have been ASSERTs, to avoid problems when later accessing the vector. Bug: 169111969 Test: jenkins/ce_cdm_tests Test: build_and_run_all_unit_tests.sh Change-Id: I9432dd2694c7181ab57ed55f66ff6c8be0c867f9 --- .../cdm/core/test/crypto_session_unittest.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/libwvdrmengine/cdm/core/test/crypto_session_unittest.cpp b/libwvdrmengine/cdm/core/test/crypto_session_unittest.cpp index b42015da..96bba6af 100644 --- a/libwvdrmengine/cdm/core/test/crypto_session_unittest.cpp +++ b/libwvdrmengine/cdm/core/test/crypto_session_unittest.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include "crypto_session.h" @@ -19,6 +20,10 @@ #include "wv_cdm_types.h" #include "wv_metrics.pb.h" +using ::testing::AllOf; +using ::testing::Ge; +using ::testing::Le; + namespace { const uint8_t kOemCert[] = { @@ -296,7 +301,7 @@ TEST_F(CryptoSessionMetricsTest, OpenSessionValidMetrics) { // lite, convert these tests to use Message-based convenience functions. // Validate common metrics regardless of provisioning type. - EXPECT_EQ(1, metrics_proto.oemcrypto_initialize_time_us().size()); + ASSERT_EQ(1, metrics_proto.oemcrypto_initialize_time_us().size()); EXPECT_TRUE(metrics_proto.oemcrypto_initialize_time_us(0) .attributes() .has_oem_crypto_result()); @@ -327,7 +332,8 @@ TEST_F(CryptoSessionMetricsTest, OpenSessionValidMetrics) { EXPECT_EQ(OEMCrypto_OEMCertificate, metrics_proto.oemcrypto_provisioning_method().int_value()); ASSERT_EQ(1, metrics_proto.oemcrypto_get_oem_public_certificate().size()); - EXPECT_EQ(1, metrics_proto.oemcrypto_get_oem_public_certificate(0).count()); + EXPECT_THAT(metrics_proto.oemcrypto_get_oem_public_certificate(0).count(), + AllOf(Ge(1), Le(2))); } else if (token_type == kClientTokenDrmCert) { // TODO(blueeyes): Add support for getting the system id from a // pre-installed DRM certificate.. @@ -373,8 +379,9 @@ TEST_F(CryptoSessionMetricsTest, GetProvisioningTokenValidMetrics) { EXPECT_EQ(OEMCrypto_OEMCertificate, metrics_proto.oemcrypto_provisioning_method().int_value()); - EXPECT_EQ(1, metrics_proto.oemcrypto_get_oem_public_certificate().size()); - EXPECT_EQ(1, metrics_proto.oemcrypto_get_oem_public_certificate(0).count()); + ASSERT_EQ(1, metrics_proto.oemcrypto_get_oem_public_certificate().size()); + EXPECT_THAT(metrics_proto.oemcrypto_get_oem_public_certificate(0).count(), + AllOf(Ge(1), Le(2))); ASSERT_EQ(1, metrics_proto.crypto_session_get_token().size()); EXPECT_EQ(1, metrics_proto.crypto_session_get_token(0).count());