From d29372909dd5aafde44f00fe323278ed2c13af1c Mon Sep 17 00:00:00 2001 From: Jeff Tinker Date: Fri, 19 Apr 2013 16:40:38 -0700 Subject: [PATCH] Don't add offsets to ion handles Fixes a secure buffer addressing offset error in the Widevine CENC drm engine. bug: 8667527 Merges the following from Widevine CDM repository: Allow specification of offset into secure buffer https://widevine-internal-review.googlesource.com/#/c/5100/ Update WVCryptoPlugin to Pass Output Offset as a Separate Parameter https://widevine-internal-review.googlesource.com/#/c/5120/ Add offset to secure data buffer in OEMCrypto DecryptCTR https://widevine-internal-review.googlesource.com/#/c/5110/ Change-Id: Ic3e4b35304c8fbae4aebe4c495285eb787e8c205 --- libwvdrmengine/cdm/core/include/cdm_engine.h | 1 + libwvdrmengine/cdm/core/include/cdm_session.h | 1 + .../cdm/core/include/crypto_session.h | 1 + libwvdrmengine/cdm/core/src/cdm_engine.cpp | 4 +++- libwvdrmengine/cdm/core/src/cdm_session.cpp | 4 +++- .../cdm/core/src/crypto_session.cpp | 4 +++- .../include/wv_content_decryption_module.h | 3 ++- .../cdm/src/wv_content_decryption_module.cpp | 6 +++-- .../cdm/test/request_license_test.cpp | 18 +++++++++----- .../mediacrypto/src/WVCryptoPlugin.cpp | 5 ++-- .../mediacrypto/test/WVCryptoPlugin_test.cpp | 12 +++++----- .../oemcrypto/include/OEMCryptoCENC.h | 24 ++++++++++--------- .../mock/src/oemcrypto_engine_mock.cpp | 10 ++++---- .../mock/src/oemcrypto_engine_mock.h | 2 +- .../oemcrypto/mock/src/oemcrypto_mock.cpp | 7 +++--- 15 files changed, 62 insertions(+), 40 deletions(-) diff --git a/libwvdrmengine/cdm/core/include/cdm_engine.h b/libwvdrmengine/cdm/core/include/cdm_engine.h index 88cc0d72..ae6099c9 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine.h +++ b/libwvdrmengine/cdm/core/include/cdm_engine.h @@ -91,6 +91,7 @@ class CdmEngine : public TimerHandler { const std::vector& iv, size_t block_offset, void* decrypt_buffer, + size_t decrypt_buffer_offset, bool is_video); // Is the key known to any session? diff --git a/libwvdrmengine/cdm/core/include/cdm_session.h b/libwvdrmengine/cdm/core/include/cdm_session.h index 6993f1ed..9d01a852 100644 --- a/libwvdrmengine/cdm/core/include/cdm_session.h +++ b/libwvdrmengine/cdm/core/include/cdm_session.h @@ -58,6 +58,7 @@ class CdmSession { const std::vector& iv, size_t block_offset, void* decrypt_buffer, + size_t decrypt_buffer_offset, bool is_video); // License renewal diff --git a/libwvdrmengine/cdm/core/include/crypto_session.h b/libwvdrmengine/cdm/core/include/crypto_session.h index f41f3849..8e68b266 100644 --- a/libwvdrmengine/cdm/core/include/crypto_session.h +++ b/libwvdrmengine/cdm/core/include/crypto_session.h @@ -71,6 +71,7 @@ class CryptoSession { const std::vector& iv, size_t block_offset, void* decrypt_buffer, + size_t decrypt_buffer_offset, bool is_video); private: diff --git a/libwvdrmengine/cdm/core/src/cdm_engine.cpp b/libwvdrmengine/cdm/core/src/cdm_engine.cpp index 2a996707..66e2c56d 100644 --- a/libwvdrmengine/cdm/core/src/cdm_engine.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_engine.cpp @@ -676,6 +676,7 @@ CdmResponseType CdmEngine::Decrypt( const std::vector& iv, size_t block_offset, void* decrypt_buffer, + size_t decrypt_buffer_offset, bool is_video) { CdmSessionIter iter = sessions_.find(session_id); if (iter == sessions_.end()) { @@ -685,7 +686,8 @@ CdmResponseType CdmEngine::Decrypt( return iter->second->Decrypt(is_encrypted, key_id, encrypt_buffer, encrypt_length, iv, block_offset, - decrypt_buffer, is_video); + decrypt_buffer, decrypt_buffer_offset, + is_video); } bool CdmEngine::IsKeyValid(const KeyId& key_id) { diff --git a/libwvdrmengine/cdm/core/src/cdm_session.cpp b/libwvdrmengine/cdm/core/src/cdm_session.cpp index 205c4721..bf19575e 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session.cpp +++ b/libwvdrmengine/cdm/core/src/cdm_session.cpp @@ -151,6 +151,7 @@ CdmResponseType CdmSession::Decrypt(bool is_encrypted, const std::vector& iv, size_t block_offset, void* decrypt_buffer, + size_t decrypt_buffer_offset, bool is_video) { if (!crypto_session_ || !crypto_session_->IsOpen()) return UNKNOWN_ERROR; @@ -168,7 +169,8 @@ CdmResponseType CdmSession::Decrypt(bool is_encrypted, } return crypto_session_->Decrypt(is_encrypted, encrypt_buffer, encrypt_length, - iv, block_offset, decrypt_buffer, is_video); + iv, block_offset, decrypt_buffer, + decrypt_buffer_offset, is_video); } // License renewal diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index 62e87ac6..ceca0493 100755 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -392,6 +392,7 @@ CdmResponseType CryptoSession::Decrypt(bool is_encrypted, const std::vector& iv, size_t block_offset, void* decrypt_buffer, + size_t decrypt_buffer_offset, bool is_video) { if (!is_destination_buffer_type_valid_) { if (!SetDestinationBufferType()) @@ -404,11 +405,12 @@ CdmResponseType CryptoSession::Decrypt(bool is_encrypted, switch (buffer_descriptor.type) { case OEMCrypto_BufferType_Clear: buffer_descriptor.buffer.clear.address = - static_cast(decrypt_buffer); + static_cast(decrypt_buffer) + decrypt_buffer_offset; buffer_descriptor.buffer.clear.max_length = encrypt_length; break; case OEMCrypto_BufferType_Secure: buffer_descriptor.buffer.secure.handle = decrypt_buffer; + buffer_descriptor.buffer.secure.offset = decrypt_buffer_offset; buffer_descriptor.buffer.secure.max_length = encrypt_length; break; case OEMCrypto_BufferType_Direct: diff --git a/libwvdrmengine/cdm/include/wv_content_decryption_module.h b/libwvdrmengine/cdm/include/wv_content_decryption_module.h index aeb86fed..7343fdb0 100644 --- a/libwvdrmengine/cdm/include/wv_content_decryption_module.h +++ b/libwvdrmengine/cdm/include/wv_content_decryption_module.h @@ -68,7 +68,8 @@ class WvContentDecryptionModule { size_t encrypt_length, const std::vector& iv, size_t block_offset, - void* decrypt_buffer); + void* decrypt_buffer, + size_t decrypt_buffer_offset); // Event listener related methods virtual bool AttachEventListener(const CdmSessionId& session_id, diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index 0a273f7c..8ce82340 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -102,10 +102,12 @@ CdmResponseType WvContentDecryptionModule::Decrypt( size_t encrypt_length, const std::vector& iv, size_t block_offset, - void* decrypt_buffer) { + void* decrypt_buffer, + size_t decrypt_buffer_offset) { return cdm_engine_->Decrypt(session_id, is_encrypted, key_id, encrypt_buffer, encrypt_length, iv, - block_offset, decrypt_buffer, true); + block_offset, decrypt_buffer, + decrypt_buffer_offset, true); } bool WvContentDecryptionModule::AttachEventListener( diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index 66900262..635316a3 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -289,7 +289,8 @@ TEST_F(WvCdmRequestLicenseTest, ClearDecryptionTest) { encrypt_length, data.iv, data.block_offset, - &decrypt_buffer.front())); + &decrypt_buffer.front(), + 0)); EXPECT_TRUE(std::equal(data.decrypt_data.begin(), data.decrypt_data.end(), decrypt_buffer.begin())); @@ -335,7 +336,8 @@ TEST_F(WvCdmRequestLicenseTest, ClearDecryptionNoKeyTest) { encrypt_length, data.iv, data.block_offset, - &decrypt_buffer.front())); + &decrypt_buffer.front(), + 0)); EXPECT_TRUE(std::equal(data.decrypt_data.begin(), data.decrypt_data.end(), decrypt_buffer.begin())); @@ -383,7 +385,8 @@ TEST_F(WvCdmRequestLicenseTest, DecryptionTest) { encrypt_length, data.iv, data.block_offset, - &decrypt_buffer.front())); + &decrypt_buffer.front(), + 0)); EXPECT_TRUE(std::equal(data.decrypt_data.begin(), data.decrypt_data.end(), decrypt_buffer.begin())); @@ -458,7 +461,8 @@ TEST_F(WvCdmRequestLicenseTest, SwitchKeyDecryptionTest) { encrypt_length, data[i].iv, data[i].block_offset, - &decrypt_buffer.front())); + &decrypt_buffer.front(), + 0)); EXPECT_TRUE(std::equal(data[i].decrypt_data.begin(), data[i].decrypt_data.end(), @@ -500,7 +504,8 @@ TEST_F(WvCdmRequestLicenseTest, PartialBlockDecryptionTest) { encrypt_length, data.iv, data.block_offset, - &decrypt_buffer.front())); + &decrypt_buffer.front(), + 0)); EXPECT_TRUE(std::equal(data.decrypt_data.begin(), data.decrypt_data.end(), decrypt_buffer.begin())); @@ -540,7 +545,8 @@ TEST_F(WvCdmRequestLicenseTest, PartialBlockWithOffsetDecryptionTest) { encrypt_length, data.iv, data.block_offset, - &decrypt_buffer.front())); + &decrypt_buffer.front(), + 0)); EXPECT_TRUE(std::equal(data.decrypt_data.begin(), data.decrypt_data.end(), decrypt_buffer.begin())); diff --git a/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp b/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp index a8ae4511..a781b239 100644 --- a/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp +++ b/libwvdrmengine/mediacrypto/src/WVCryptoPlugin.cpp @@ -118,7 +118,7 @@ ssize_t WVCryptoPlugin::decrypt(bool secure, const uint8_t key[KEY_ID_SIZE], CdmResponseType res = mCDM->Decrypt(mSessionId, false, keyId, source + offset, subSample.mNumBytesOfClearData, - ivVector, 0, dest + offset); + ivVector, 0, dest, offset); if (!isCdmResponseTypeSuccess(res)) { ALOGE("Decrypt error result in session %s during unencrypted block: %d", @@ -136,7 +136,8 @@ ssize_t WVCryptoPlugin::decrypt(bool secure, const uint8_t key[KEY_ID_SIZE], CdmResponseType res = mCDM->Decrypt(mSessionId, true, keyId, source + offset, subSample.mNumBytesOfEncryptedData, - ivVector, encrypted_offset % 16, dest + offset); + ivVector, encrypted_offset % 16, dest, + offset); if (!isCdmResponseTypeSuccess(res)) { ALOGE("Decrypt error result in session %s during encrypted block: %d", diff --git a/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp b/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp index 936b2414..e62b5c14 100644 --- a/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp +++ b/libwvdrmengine/mediacrypto/test/WVCryptoPlugin_test.cpp @@ -23,10 +23,10 @@ using namespace wvdrm; class MockCDM : public WvContentDecryptionModule { public: - MOCK_METHOD8(Decrypt, CdmResponseType(const CdmSessionId&, bool, const KeyId&, + MOCK_METHOD9(Decrypt, CdmResponseType(const CdmSessionId&, bool, const KeyId&, const uint8_t*, size_t, const std::vector&, size_t, - void*)); + void*, size_t)); MOCK_METHOD1(QueryStatus, CdmResponseType(CdmQueryMap*)); }; @@ -130,22 +130,22 @@ TEST_F(WVCryptoPluginTest, AttemptsToDecrypt) { EXPECT_CALL(cdm, Decrypt(ElementsAreArray(sessionId, kSessionIdSize), true, ElementsAreArray(keyId, KEY_ID_SIZE), in, 16, - ElementsAreArray(iv, KEY_IV_SIZE), 0, out)) + ElementsAreArray(iv, KEY_IV_SIZE), 0, out, 0)) .WillOnce(Return(wvcdm::NO_ERROR)); EXPECT_CALL(cdm, Decrypt(ElementsAreArray(sessionId, kSessionIdSize), false, ElementsAreArray(keyId, KEY_ID_SIZE), in + 16, 16, - ElementsAreArray(iv, KEY_IV_SIZE), 0, out + 16)) + ElementsAreArray(iv, KEY_IV_SIZE), 0, out, 16)) .WillOnce(Return(wvcdm::NO_ERROR)); EXPECT_CALL(cdm, Decrypt(ElementsAreArray(sessionId, kSessionIdSize), true, ElementsAreArray(keyId, KEY_ID_SIZE), in + 32, 24, - ElementsAreArray(iv, KEY_IV_SIZE), 0, out + 32)) + ElementsAreArray(iv, KEY_IV_SIZE), 0, out, 32)) .WillOnce(Return(wvcdm::NO_ERROR)); EXPECT_CALL(cdm, Decrypt(ElementsAreArray(sessionId, kSessionIdSize), true, ElementsAreArray(keyId, KEY_ID_SIZE), in + 56, 8, - ElementsAreArray(iv, KEY_IV_SIZE), 8, out + 56)) + ElementsAreArray(iv, KEY_IV_SIZE), 8, out, 56)) .WillOnce(Return(wvcdm::NO_ERROR)); } diff --git a/libwvdrmengine/oemcrypto/include/OEMCryptoCENC.h b/libwvdrmengine/oemcrypto/include/OEMCryptoCENC.h index ef0bdb83..97253c32 100644 --- a/libwvdrmengine/oemcrypto/include/OEMCryptoCENC.h +++ b/libwvdrmengine/oemcrypto/include/OEMCryptoCENC.h @@ -113,6 +113,7 @@ typedef struct { struct { // type == OEMCrypto_BufferType_Secure void* handle; size_t max_length; + size_t offset; } secure; struct { // type == OEMCrypto_BufferType_Direct bool is_video; @@ -638,11 +639,13 @@ OEMCryptoResult OEMCrypto_SelectKey(const OEMCrypto_SESSION session, * arguments are ignored. * iv (in) - The initial value block to be used for content decryption. * This is discussed further below. - * offset (in) - If non-zero, the decryption block boundary is different - * from the start of the data. offset should be subtracted from + * block_offset (in) - If non-zero, the decryption block boundary is + * different from the start of the data. offset should be subtracted from * data_addr to compute the starting address of the first decrypted * block. The bytes between the decryption block start address and - * data_addr are discarded after decryption. + * data_addr are discarded after decryption. This is only used to adjust + * the start of decryption block. It does not adjust the beginning of the + * source or destination data. 0 <= block_offset < 16. * out_buffer (in) - A caller-owned descriptor that specifies the * handling of the decrypted byte stream. See OEMCrypto_DestbufferDesc * for details. @@ -690,14 +693,13 @@ OEMCryptoResult OEMCrypto_SelectKey(const OEMCrypto_SESSION session, * Version: * This method changed in API version 5. */ -OEMCryptoResult -OEMCrypto_DecryptCTR(OEMCrypto_SESSION session, - const uint8_t *data_addr, - size_t data_length, - bool is_encrypted, - const uint8_t *iv, - size_t offset, - const OEMCrypto_DestBufferDesc* out_buffer); +OEMCryptoResult OEMCrypto_DecryptCTR(OEMCrypto_SESSION session, + const uint8_t *data_addr, + size_t data_length, + bool is_encrypted, + const uint8_t *iv, + size_t block_offset, + const OEMCrypto_DestBufferDesc* out_buffer); /* * OEMCrypto_InstallKeybox diff --git a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.cpp b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.cpp index fcc8795f..a7711df7 100644 --- a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.cpp +++ b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.cpp @@ -856,7 +856,7 @@ bool CryptoEngine::DecryptMessage(SessionContext* session, bool CryptoEngine::DecryptCTR(SessionContext* session, const uint8_t* iv, - size_t byte_offset, + size_t block_offset, const uint8_t* cipher_data, size_t cipher_data_length, bool is_encrypted, @@ -922,18 +922,18 @@ bool CryptoEngine::DecryptCTR(SessionContext* session, // Encrypt the IV. uint8_t ecount_buf[AES_BLOCK_SIZE]; - if (byte_offset != 0) { + if (block_offset != 0) { // The context is needed only when not starting a new block. AES_encrypt(aes_iv, ecount_buf, &aes_key); ctr128_inc(aes_iv); } // Decryption. - unsigned int byte_offset_cur = byte_offset; + unsigned int block_offset_cur = block_offset; AES_ctr128_encrypt( cipher_data, reinterpret_cast(clear_data), cipher_data_length, - &aes_key, aes_iv, ecount_buf, &byte_offset_cur); - if (byte_offset_cur != ((byte_offset + cipher_data_length) % AES_BLOCK_SIZE)) { + &aes_key, aes_iv, ecount_buf, &block_offset_cur); + if (block_offset_cur != ((block_offset + cipher_data_length) % AES_BLOCK_SIZE)) { LOGE("[DecryptCTR(): FAILURE: byte offset wrong.]"); return false; } diff --git a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.h b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.h index d6eb8863..7db606b9 100644 --- a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.h +++ b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_engine_mock.h @@ -234,7 +234,7 @@ class CryptoEngine { bool DecryptCTR(SessionContext* session, const uint8_t* iv, - size_t byte_offset, + size_t block_offset, const uint8_t* cipher_data, size_t cipher_data_length, bool is_encrypted, diff --git a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_mock.cpp b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_mock.cpp index e882ce2a..a6292a25 100644 --- a/libwvdrmengine/oemcrypto/mock/src/oemcrypto_mock.cpp +++ b/libwvdrmengine/oemcrypto/mock/src/oemcrypto_mock.cpp @@ -501,7 +501,7 @@ OEMCryptoResult OEMCrypto_DecryptCTR(OEMCrypto_SESSION session, size_t data_length, bool is_encrypted, const uint8_t* iv, - size_t offset, + size_t block_offset, const OEMCrypto_DestBufferDesc* out_buffer) { if (trace_all_calls) { printf("-- OEMCryptoResult OEMCrypto_DecryptCTR(OEMCrypto_SESSION session,\n"); @@ -517,7 +517,8 @@ OEMCryptoResult OEMCrypto_DecryptCTR(OEMCrypto_SESSION session, break; case OEMCrypto_BufferType_Secure: buffer_type = kBufferTypeSecure; - destination = out_buffer->buffer.secure.handle; + destination = (out_buffer->buffer.secure.handle + + out_buffer->buffer.secure.offset); max_length = out_buffer->buffer.secure.max_length; break; default: @@ -551,7 +552,7 @@ OEMCryptoResult OEMCrypto_DecryptCTR(OEMCrypto_SESSION session, return OEMCrypto_ERROR_INVALID_CONTEXT; } - if (!crypto_engine->DecryptCTR(session_ctx, iv, (int)offset, + if (!crypto_engine->DecryptCTR(session_ctx, iv, (int)block_offset, data_addr, data_length, is_encrypted, destination, buffer_type)) { LOGE("[OEMCrypto_DecryptCTR(): OEMCrypto_ERROR_DECRYPT_FAILED]");