From f5e71b62b5b280031352d70ed9945b253b2c4d9c Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Wed, 15 Oct 2014 14:56:12 -0700 Subject: [PATCH] Fix two packager crash bug with invalid arguments Fix b/18005827 Packager crashes in debug mode with widevine decryption enabled given a non-existing file. Fix b/18005632 Packager crashes with invalid fixed_key_encryption parameters. Also cleans up some code. Change-Id: I097f5c8dc113eb6d33b42b19a4bf5de125c47c30 --- packager/app/packager_util.cc | 2 + packager/app/validate_flag.h | 5 ++ packager/app/widevine_encryption_flags.cc | 3 +- packager/media/base/widevine_key_source.cc | 24 ++----- packager/media/base/widevine_key_source.h | 5 +- packager/media/formats/mp4/box_definitions.cc | 34 ++++++--- .../media/formats/mp4/mp4_media_parser.cc | 6 +- .../media/formats/wvm/wvm_media_parser.cc | 70 +++++++++++-------- packager/media/formats/wvm/wvm_media_parser.h | 4 +- 9 files changed, 90 insertions(+), 63 deletions(-) diff --git a/packager/app/packager_util.cc b/packager/app/packager_util.cc index 8feedb08eb..1314d484d5 100644 --- a/packager/app/packager_util.cc +++ b/packager/app/packager_util.cc @@ -4,6 +4,8 @@ // license that can be found in the LICENSE file or at // https://developers.google.com/open-source/licenses/bsd +#include "packager/app/packager_util.h" + #include #include diff --git a/packager/app/validate_flag.h b/packager/app/validate_flag.h index 280dde70e3..5ccfa6a1dd 100644 --- a/packager/app/validate_flag.h +++ b/packager/app/validate_flag.h @@ -6,6 +6,9 @@ // // Flag validation help functions. +#ifndef APP_VALIDATE_FLAG_H_ +#define APP_VALIDATE_FLAG_H_ + #include namespace edash_packager { @@ -31,3 +34,5 @@ bool ValidateFlag(const char* flag_name, void PrintError(const std::string& error_message); } // namespace edash_packager + +#endif // APP_VALIDATE_FLAG_H_ diff --git a/packager/app/widevine_encryption_flags.cc b/packager/app/widevine_encryption_flags.cc index ed6e6635f3..efa06381b3 100644 --- a/packager/app/widevine_encryption_flags.cc +++ b/packager/app/widevine_encryption_flags.cc @@ -142,7 +142,8 @@ bool ValidateWidevineCryptoFlags() { if (FLAGS_crypto_period_duration < 0) { PrintError("--crypto_period_duration should not be negative."); success = false; - } else if (FLAGS_crypto_period_duration > 0 && !FLAGS_enable_widevine_encryption) { + } else if (FLAGS_crypto_period_duration > 0 && + !FLAGS_enable_widevine_encryption) { PrintError( "--crypto_period_duration should be specified only if " "--enable_widevine_encryption."); diff --git a/packager/media/base/widevine_key_source.cc b/packager/media/base/widevine_key_source.cc index 5288953e86..eca563f996 100644 --- a/packager/media/base/widevine_key_source.cc +++ b/packager/media/base/widevine_key_source.cc @@ -146,6 +146,7 @@ WidevineKeySource::WidevineKeySource(const std::string& server_url) key_production_started_(false), start_key_production_(false, false), first_crypto_period_index_(0) { + key_production_thread_.Start(); } WidevineKeySource::~WidevineKeySource() { @@ -168,7 +169,7 @@ Status WidevineKeySource::FetchKeys(const std::vector& content_id, BytesToBase64String(content_id, &content_id_base64_string); request_dict_.SetString("content_id", content_id_base64_string); request_dict_.SetString("policy", policy); - return FetchKeysCommon(false); + return FetchKeysInternal(!kEnableKeyRotation, 0, false); } Status WidevineKeySource::FetchKeys(const std::vector& pssh_data) { @@ -177,29 +178,17 @@ Status WidevineKeySource::FetchKeys(const std::vector& pssh_data) { std::string pssh_data_base64_string; BytesToBase64String(pssh_data, &pssh_data_base64_string); request_dict_.SetString("pssh_data", pssh_data_base64_string); - return FetchKeysCommon(false); + return FetchKeysInternal(!kEnableKeyRotation, 0, false); } Status WidevineKeySource::FetchKeys(uint32_t asset_id) { base::AutoLock scoped_lock(lock_); request_dict_.Clear(); request_dict_.SetInteger("asset_id", asset_id); - return FetchKeysCommon(true); + return FetchKeysInternal(!kEnableKeyRotation, 0, true); } -Status WidevineKeySource::FetchKeysCommon(bool widevine_classic) { - // TODO(tinskip): Make this method callable multiple times to fetch - // different keys. - DCHECK(!key_production_thread_.HasBeenStarted()); - key_production_thread_.Start(); - - // Perform a fetch request to find out if the key source is healthy. - // It also stores the keys fetched for consumption later. - return FetchKeysInternal(!kEnableKeyRotation, 0, widevine_classic); -} - -Status WidevineKeySource::GetKey(TrackType track_type, - EncryptionKey* key) { +Status WidevineKeySource::GetKey(TrackType track_type, EncryptionKey* key) { DCHECK(key); if (encryption_key_map_.find(track_type) == encryption_key_map_.end()) { return Status(error::INTERNAL_ERROR, @@ -235,7 +224,8 @@ Status WidevineKeySource::GetCryptoPeriodKey(uint32_t crypto_period_index, if (!key_production_started_) { // Another client may have a slightly smaller starting crypto period // index. Set the initial value to account for that. - first_crypto_period_index_ = crypto_period_index ? crypto_period_index - 1 : 0; + first_crypto_period_index_ = + crypto_period_index ? crypto_period_index - 1 : 0; DCHECK(!key_pool_); key_pool_.reset(new EncryptionKeyQueue(crypto_period_count_, first_crypto_period_index_)); diff --git a/packager/media/base/widevine_key_source.h b/packager/media/base/widevine_key_source.h index 090b951b79..9c91948950 100644 --- a/packager/media/base/widevine_key_source.h +++ b/packager/media/base/widevine_key_source.h @@ -26,7 +26,7 @@ template class ProducerConsumerQueue; class WidevineKeySource : public KeySource { public: /// @param server_url is the Widevine common encryption server url. - WidevineKeySource(const std::string& server_url); + explicit WidevineKeySource(const std::string& server_url); virtual ~WidevineKeySource(); @@ -67,9 +67,6 @@ class WidevineKeySource : public KeySource { TrackType track_type, EncryptionKey* key); - // Common implementation of FetchKeys methods above. - Status FetchKeysCommon(bool widevine_classic); - // The closure task to fetch keys repeatedly. void FetchKeysTask(); diff --git a/packager/media/formats/mp4/box_definitions.cc b/packager/media/formats/mp4/box_definitions.cc index 7c4eadd36c..345f12a65f 100644 --- a/packager/media/formats/mp4/box_definitions.cc +++ b/packager/media/formats/mp4/box_definitions.cc @@ -18,6 +18,9 @@ const uint32_t kBoxSize = kFourCCSize + sizeof(uint32_t); // Additional 1-byte version and 3-byte flags. const uint32_t kFullBoxSize = kBoxSize + 4; +// Key Id size as defined in CENC spec. +const uint32_t kCencKeyIdSize = 16; + // 9 uint32_t in big endian formatted array. const uint8_t kUnityMatrix[] = {0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, @@ -226,12 +229,21 @@ TrackEncryption::~TrackEncryption() {} FourCC TrackEncryption::BoxType() const { return FOURCC_TENC; } bool TrackEncryption::ReadWrite(BoxBuffer* buffer) { + if (!buffer->Reading()) { + if (default_kid.size() != kCencKeyIdSize) { + LOG(WARNING) << "CENC defines key id length of " << kCencKeyIdSize + << " bytes; got " << default_kid.size() + << ". Resized accordingly."; + default_kid.resize(kCencKeyIdSize); + } + } + uint8_t flag = is_encrypted ? 1 : 0; RCHECK(FullBox::ReadWrite(buffer) && buffer->IgnoreBytes(2) && // reserved. buffer->ReadWriteUInt8(&flag) && buffer->ReadWriteUInt8(&default_iv_size) && - buffer->ReadWriteVector(&default_kid, 16)); + buffer->ReadWriteVector(&default_kid, kCencKeyIdSize)); if (buffer->Reading()) { is_encrypted = (flag != 0); if (is_encrypted) { @@ -244,7 +256,7 @@ bool TrackEncryption::ReadWrite(BoxBuffer* buffer) { } uint32_t TrackEncryption::ComputeSize() { - atom_size = kFullBoxSize + sizeof(uint32_t) + default_kid.size(); + atom_size = kFullBoxSize + sizeof(uint32_t) + kCencKeyIdSize; return atom_size; } @@ -1674,8 +1686,7 @@ bool SampleGroupDescription::ReadWrite(BoxBuffer* buffer) { return true; } - const size_t kKeyIdSize = 16; - const size_t kEntrySize = sizeof(uint32_t) + kKeyIdSize; + const size_t kEntrySize = sizeof(uint32_t) + kCencKeyIdSize; uint32_t default_length = 0; if (version == 1) { if (buffer->Reading()) { @@ -1699,14 +1710,20 @@ bool SampleGroupDescription::ReadWrite(BoxBuffer* buffer) { } } - if (!buffer->Reading()) - RCHECK(entries[i].key_id.size() == kKeyIdSize); + if (!buffer->Reading()) { + if (entries[i].key_id.size() != kCencKeyIdSize) { + LOG(WARNING) << "CENC defines key id length of " << kCencKeyIdSize + << " bytes; got " << entries[i].key_id.size() + << ". Resized accordingly."; + entries[i].key_id.resize(kCencKeyIdSize); + } + } uint8_t flag = entries[i].is_encrypted ? 1 : 0; RCHECK(buffer->IgnoreBytes(2) && // reserved. buffer->ReadWriteUInt8(&flag) && buffer->ReadWriteUInt8(&entries[i].iv_size) && - buffer->ReadWriteVector(&entries[i].key_id, kKeyIdSize)); + buffer->ReadWriteVector(&entries[i].key_id, kCencKeyIdSize)); if (buffer->Reading()) { entries[i].is_encrypted = (flag != 0); @@ -1726,8 +1743,7 @@ uint32_t SampleGroupDescription::ComputeSize() { // This box is optional. Skip it if it is not used. atom_size = 0; if (!entries.empty()) { - const size_t kKeyIdSize = 16; - const size_t kEntrySize = sizeof(uint32_t) + kKeyIdSize; + const size_t kEntrySize = sizeof(uint32_t) + kCencKeyIdSize; atom_size = kFullBoxSize + sizeof(grouping_type) + (version == 1 ? sizeof(uint32_t) : 0) + sizeof(uint32_t) + entries.size() * kEntrySize; diff --git a/packager/media/formats/mp4/mp4_media_parser.cc b/packager/media/formats/mp4/mp4_media_parser.cc index f48990f38d..4601e1b4a4 100644 --- a/packager/media/formats/mp4/mp4_media_parser.cc +++ b/packager/media/formats/mp4/mp4_media_parser.cc @@ -428,7 +428,11 @@ bool MP4MediaParser::EnqueueSample(bool* err) { << ", cts=" << runs_->cts() << ", size=" << runs_->sample_size(); - new_sample_cb_.Run(runs_->track_id(), stream_sample); + if (!new_sample_cb_.Run(runs_->track_id(), stream_sample)) { + *err = true; + LOG(ERROR) << "Failed to process the sample."; + return false; + } runs_->AdvanceSample(); return true; diff --git a/packager/media/formats/wvm/wvm_media_parser.cc b/packager/media/formats/wvm/wvm_media_parser.cc index dbdc2a56c1..db4c5a2abe 100644 --- a/packager/media/formats/wvm/wvm_media_parser.cc +++ b/packager/media/formats/wvm/wvm_media_parser.cc @@ -488,25 +488,25 @@ bool WvmMediaParser::EmitLastSample(uint32_t stream_id, .append(base::UintToString(stream_id)); std::map::iterator it = program_demux_stream_map_.find(key); - if (it != program_demux_stream_map_.end()) { - EmitSample(stream_id, (*it).second, new_sample, true); - } else { + if (it == program_demux_stream_map_.end()) return false; - } - return true; + return EmitSample(stream_id, (*it).second, new_sample, true); } -void WvmMediaParser::EmitPendingSamples() { +bool WvmMediaParser::EmitPendingSamples() { // Emit queued samples which were built when not initialized. while (!media_sample_queue_.empty()) { DemuxStreamIdMediaSample& demux_stream_media_sample = media_sample_queue_.front(); - EmitSample( - demux_stream_media_sample.parsed_audio_or_video_stream_id, - demux_stream_media_sample.demux_stream_id, - demux_stream_media_sample.media_sample, false); + if (!EmitSample(demux_stream_media_sample.parsed_audio_or_video_stream_id, + demux_stream_media_sample.demux_stream_id, + demux_stream_media_sample.media_sample, + false)) { + return false; + } media_sample_queue_.pop_front(); } + return true; } void WvmMediaParser::Flush() { @@ -855,64 +855,76 @@ bool WvmMediaParser::Output() { } else { // flush the sample queue and emit all queued samples. while (!media_sample_queue_.empty()) { - EmitPendingSamples(); + if (!EmitPendingSamples()) + return false; } // Emit current sample. - EmitSample(prev_pes_stream_id_, (*it).second, media_sample_, false); + if (!EmitSample(prev_pes_stream_id_, (*it).second, media_sample_, false)) + return false; } return true; } -void WvmMediaParser::EmitSample(uint32_t parsed_audio_or_video_stream_id, +bool WvmMediaParser::EmitSample(uint32_t parsed_audio_or_video_stream_id, uint32_t stream_id, scoped_refptr& new_sample, bool isLastSample) { DCHECK(new_sample); if (isLastSample) { - if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask) - == kPesStreamIdVideo) { + if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask) == + kPesStreamIdVideo) { new_sample->set_duration(prev_media_sample_data_.video_sample_duration); - } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask) - == kPesStreamIdAudio) { + } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask) == + kPesStreamIdAudio) { new_sample->set_duration(prev_media_sample_data_.audio_sample_duration); } - new_sample_cb_.Run(stream_id, new_sample); - return; + if (!new_sample_cb_.Run(stream_id, new_sample)) { + LOG(ERROR) << "Failed to process the last sample."; + return false; + } + return true; } // Cannot emit current sample. Compute duration first and then, // emit previous sample. - if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask) - == kPesStreamIdVideo) { + if ((parsed_audio_or_video_stream_id & kPesStreamIdVideoMask) == + kPesStreamIdVideo) { if (prev_media_sample_data_.video_sample == NULL) { prev_media_sample_data_.video_sample = new_sample; prev_media_sample_data_.video_stream_id = stream_id; - return; + return true; } prev_media_sample_data_.video_sample->set_duration( new_sample->dts() - prev_media_sample_data_.video_sample->dts()); prev_media_sample_data_.video_sample_duration = prev_media_sample_data_.video_sample->duration(); - new_sample_cb_.Run(prev_media_sample_data_.video_stream_id, - prev_media_sample_data_.video_sample); + if (!new_sample_cb_.Run(prev_media_sample_data_.video_stream_id, + prev_media_sample_data_.video_sample)) { + LOG(ERROR) << "Failed to process the video sample."; + return false; + } prev_media_sample_data_.video_sample = new_sample; prev_media_sample_data_.video_stream_id = stream_id; - } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask) - == kPesStreamIdAudio) { + } else if ((parsed_audio_or_video_stream_id & kPesStreamIdAudioMask) == + kPesStreamIdAudio) { if (prev_media_sample_data_.audio_sample == NULL) { prev_media_sample_data_.audio_sample = new_sample; prev_media_sample_data_.audio_stream_id = stream_id; - return; + return true; } prev_media_sample_data_.audio_sample->set_duration( new_sample->dts() - prev_media_sample_data_.audio_sample->dts()); prev_media_sample_data_.audio_sample_duration = prev_media_sample_data_.audio_sample->duration(); - new_sample_cb_.Run(prev_media_sample_data_.audio_stream_id, - prev_media_sample_data_.audio_sample); + if (!new_sample_cb_.Run(prev_media_sample_data_.audio_stream_id, + prev_media_sample_data_.audio_sample)) { + LOG(ERROR) << "Failed to process the audio sample."; + return false; + } prev_media_sample_data_.audio_sample = new_sample; prev_media_sample_data_.audio_stream_id = stream_id; } + return true; } bool WvmMediaParser::GetAssetKey(const uint32_t asset_id, diff --git a/packager/media/formats/wvm/wvm_media_parser.h b/packager/media/formats/wvm/wvm_media_parser.h index 3ddb931f53..e2f940c73f 100644 --- a/packager/media/formats/wvm/wvm_media_parser.h +++ b/packager/media/formats/wvm/wvm_media_parser.h @@ -201,12 +201,12 @@ class WvmMediaParser : public MediaParser { // Callback invoked by the ES media parser // to emit a new audio/video access unit. - void EmitSample(uint32_t parsed_audio_or_video_stream_id, + bool EmitSample(uint32_t parsed_audio_or_video_stream_id, uint32_t stream_id, scoped_refptr& new_sample, bool isLastSample); - void EmitPendingSamples(); + bool EmitPendingSamples(); bool EmitLastSample(uint32_t stream_id, scoped_refptr& new_sample);