Add support for v0.3 of the encrypted WebM specification.

Add support for v0.3 of the encrypted WebM specification.

- Added code to handle the signal_byte contained within WebM
  encrypted Blocks.
- Added a unittest to aes_decryptor to hanlde an encrypted WebM
  Block with an unencrypted frame.

BUG=139876
TEST=Run media_unittests  --gtest_filter=AesDecryptor* and all
     tests must pass.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149449

[xhwang, 2012-08-01 00:45:41]

Looks pretty good. Just some naming/style related comments.

http://codereview.chromium.org/10823110/diff/2003/media/base/decrypt_config.cc
File media/base/decrypt_config.cc (right):

http://codereview.chromium.org/10823110/diff/2003/media/base/decrypt_config.c...
media/base/decrypt_config.cc:24: iv.empty(), true);
CHECK(iv.empty() || iv.size() == static_cast<size_t>(kDecryptionKeySize));

http://codereview.chromium.org/10823110/diff/2003/media/crypto/aes_decryptor.cc
File media/crypto/aes_decryptor.cc (right):

http://codereview.chromium.org/10823110/diff/2003/media/crypto/aes_decryptor....
media/crypto/aes_decryptor.cc:295: decrypted = DecoderBuffer::CopyFrom(frame,
frame_size);
decrypted = DecoderBuffer::CopyFrom(encrypted->GetData() + data_offset,
encrypted->GetDataSize() - data_offset);

http://codereview.chromium.org/10823110/diff/2003/media/crypto/aes_decryptor_...
File media/crypto/aes_decryptor_unittest.cc (right):

http://codereview.chromium.org/10823110/diff/2003/media/crypto/aes_decryptor_...
media/crypto/aes_decryptor_unittest.cc:229: // an HMAC and signal byte prepended
to a frame. If the frame is encrypted
s/signal/a signal/

http://codereview.chromium.org/10823110/diff/2003/media/crypto/aes_decryptor_...
media/crypto/aes_decryptor_unittest.cc:230: // than an IV is prepended to the
Block. Current encrypted WebM request for
s/than/then

http://codereview.chromium.org/10823110/diff/2003/media/crypto/aes_decryptor_...
media/crypto/aes_decryptor_unittest.cc:250: memcpy(&network_iv, data +
kWebMHmacSize + 1, sizeof(network_iv));
s/1/data_offset/

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_cluster_par...
File media/webm/webm_cluster_parser.cc (right):

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_cluster_par...
media/webm/webm_cluster_parser.cc:18: static scoped_array<uint8>
GenerateCounterBlock(uint64 iv) {
CounterBlock is only 16bytes, let's just make it a std::string instead of a
scoped_array.

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_cluster_par...
media/webm/webm_cluster_parser.cc:223: bool encrypted_stream = track_num ==
video_.track_num() &&
"encrypted_stream" is a noun. How about something like "is_stream_encrypted".

Also, in WebM, we use "track" instead of "stream", correct? If so,
"is_track_encrypted"?

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_cluster_par...
media/webm/webm_cluster_parser.cc:240: uint8 signal_byte = data[kWebMHmacSize];
How about moving "int data_offset = 1" after this line and change it to:
int data_offset = sizeof(signal_byte);

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_cluster_par...
media/webm/webm_cluster_parser.cc:249: memcpy(&network_iv, data + kWebMHmacSize
+ 1, sizeof(network_iv));
s/1/data_offset/

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_cluster_par...
media/webm/webm_cluster_parser.cc:255: DecryptConfig::kDecryptionKeySize);
This code will be much easier if GenerateCounterBlock() returns std::string. See
comment above.

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_constants.h
File media/webm/webm_constants.h (right):

http://codereview.chromium.org/10823110/diff/2003/media/webm/webm_constants.h...
media/webm/webm_constants.h:206: const int kWebMEncryptedFrame = 1;
This sounds like a "frame" not a flag. Since we have kWebMFlagKeyframe above,
how about something like kWebMFlagEncryptedFrame? Also sort these constants in
alphabetic order.

[fgalligan1, 2012-08-01 01:36:10]

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/base/decrypt_...
File media/base/decrypt_config.cc (right):

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/base/decrypt_...
media/base/decrypt_config.cc:24: iv.empty(), true);
On 2012/08/01 00:45:41, xhwang wrote:
> CHECK(iv.empty() || iv.size() == static_cast<size_t>(kDecryptionKeySize));

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/crypto/aes_de...
File media/crypto/aes_decryptor.cc (right):

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/crypto/aes_de...
media/crypto/aes_decryptor.cc:295: decrypted = DecoderBuffer::CopyFrom(frame,
frame_size);
On 2012/08/01 00:45:41, xhwang wrote:
> decrypted = DecoderBuffer::CopyFrom(encrypted->GetData() + data_offset,
> encrypted->GetDataSize() - data_offset);

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/crypto/aes_de...
File media/crypto/aes_decryptor_unittest.cc (right):

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/crypto/aes_de...
media/crypto/aes_decryptor_unittest.cc:229: // an HMAC and signal byte prepended
to a frame. If the frame is encrypted
On 2012/08/01 00:45:41, xhwang wrote:
> s/signal/a signal/

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/crypto/aes_de...
media/crypto/aes_decryptor_unittest.cc:230: // than an IV is prepended to the
Block. Current encrypted WebM request for
On 2012/08/01 00:45:41, xhwang wrote:
> s/than/then

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/crypto/aes_de...
media/crypto/aes_decryptor_unittest.cc:250: memcpy(&network_iv, data +
kWebMHmacSize + 1, sizeof(network_iv));
On 2012/08/01 00:45:41, xhwang wrote:
> s/1/data_offset/

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_clu...
File media/webm/webm_cluster_parser.cc (right):

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_clu...
media/webm/webm_cluster_parser.cc:18: static scoped_array<uint8>
GenerateCounterBlock(uint64 iv) {
On 2012/08/01 00:45:41, xhwang wrote:
> CounterBlock is only 16bytes, let's just make it a std::string instead of a
> scoped_array.

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_clu...
media/webm/webm_cluster_parser.cc:223: bool encrypted_stream = track_num ==
video_.track_num() &&
On 2012/08/01 00:45:41, xhwang wrote:
> "encrypted_stream" is a noun. How about something like "is_stream_encrypted".
> 
> Also, in WebM, we use "track" instead of "stream", correct? If so,
> "is_track_encrypted"?

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_clu...
media/webm/webm_cluster_parser.cc:240: uint8 signal_byte = data[kWebMHmacSize];
On 2012/08/01 00:45:41, xhwang wrote:
> How about moving "int data_offset = 1" after this line and change it to:
> int data_offset = sizeof(signal_byte);

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_clu...
media/webm/webm_cluster_parser.cc:249: memcpy(&network_iv, data + kWebMHmacSize
+ 1, sizeof(network_iv));
On 2012/08/01 00:45:41, xhwang wrote:
> s/1/data_offset/

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_clu...
media/webm/webm_cluster_parser.cc:255: DecryptConfig::kDecryptionKeySize);
On 2012/08/01 00:45:41, xhwang wrote:
> This code will be much easier if GenerateCounterBlock() returns std::string.
See
> comment above.

Done.

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_con...
File media/webm/webm_constants.h (right):

https://chromiumcodereview.appspot.com/10823110/diff/2003/media/webm/webm_con...
media/webm/webm_constants.h:206: const int kWebMEncryptedFrame = 1;
On 2012/08/01 00:45:41, xhwang wrote:
> This sounds like a "frame" not a flag. Since we have kWebMFlagKeyframe above,
> how about something like kWebMFlagEncryptedFrame? Also sort these constants in
> alphabetic order.

Done.

[xhwang, 2012-08-01 04:38:23]

Thanks! LGTM w/ nits.

http://codereview.chromium.org/10823110/diff/5003/media/crypto/aes_decryptor_...
File media/crypto/aes_decryptor_unittest.cc (right):

http://codereview.chromium.org/10823110/diff/5003/media/crypto/aes_decryptor_...
media/crypto/aes_decryptor_unittest.cc:241: uint8 signal_byte =
data[kWebMHmacSize];
Move definition of "data_offset" here: int data_offset = sizeof(signal_byte);

http://codereview.chromium.org/10823110/diff/5003/media/webm/webm_cluster_par...
File media/webm/webm_cluster_parser.cc (right):

http://codereview.chromium.org/10823110/diff/5003/media/webm/webm_cluster_par...
media/webm/webm_cluster_parser.cc:28: return std::string(counter_block_data,
DecryptConfig::kDecryptionKeySize);
Can we do something like this:
std::string counter_block(DecryptConfig::kDecryptionKeySize, 0);
counter_block.replace(0, sizeof(iv), reinterpret_cast<char*>(&iv), sizeof(iv));
return counter_block;

[fgalligan1, 2012-08-01 15:07:38]

https://chromiumcodereview.appspot.com/10823110/diff/5003/media/crypto/aes_de...
File media/crypto/aes_decryptor_unittest.cc (right):

https://chromiumcodereview.appspot.com/10823110/diff/5003/media/crypto/aes_de...
media/crypto/aes_decryptor_unittest.cc:241: uint8 signal_byte =
data[kWebMHmacSize];
On 2012/08/01 04:38:23, xhwang wrote:
> Move definition of "data_offset" here: int data_offset = sizeof(signal_byte);

Done.

https://chromiumcodereview.appspot.com/10823110/diff/5003/media/webm/webm_clu...
File media/webm/webm_cluster_parser.cc (right):

https://chromiumcodereview.appspot.com/10823110/diff/5003/media/webm/webm_clu...
media/webm/webm_cluster_parser.cc:28: return std::string(counter_block_data,
DecryptConfig::kDecryptionKeySize);
On 2012/08/01 04:38:23, xhwang wrote:
> Can we do something like this:
> std::string counter_block(DecryptConfig::kDecryptionKeySize, 0);
> counter_block.replace(0, sizeof(iv), reinterpret_cast<char*>(&iv),
sizeof(iv));
> return counter_block;

I changed to this:
std::string counter_block(reinterpret_cast<char*>(&iv), sizeof(iv));
  counter_block.append(DecryptConfig::kDecryptionKeySize - sizeof(iv), 0);
  return counter_block;

[xhwang, 2012-08-01 15:46:40]

lgtm

[commit-bot: I haz the power, 2012-08-01 16:50:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fgalligan@chromium.org/10823110/3006

[commit-bot: I haz the power, 2012-08-01 18:29:41]

Change committed as 149449