|
|
DescriptionChange ClearKey session IDs to be random strings
Currently AesDecryptor session IDs are numbered sequentially (e.g. "1",
"2", etc.). To support persistent sessions using AesDecryptor we need random
session IDs so that there is less chance that a saved ID matches an existing
session ID. The new session IDs are a 8 digit random hexadecimal number
followed by an 8 digit sequence number to ensure they are unique in the
process. Session IDs are a printable string to make logging them easier.
As AesDecryptor may be run in a sandbox (External Clear Key, used for
testing), we are limited in what functions can be called. In particular,
the functions in base/rand_util.h and crypto::GenerateRandomKey() can't
be used. So use a trivial rand function for the random number.
BUG=478960
TEST=media unit tests pass
Review-Url: https://codereview.chromium.org/2873373003
Cr-Commit-Position: refs/heads/master@{#476857}
Committed: https://chromium.googlesource.com/chromium/src/+/1862926fcdeb5bd96594c55e674947b9e04e111e
Patch Set 1 #
Total comments: 5
Patch Set 2 : GenerateRandomKey #
Total comments: 2
Patch Set 3 : add count (+rebase) #
Total comments: 8
Patch Set 4 : changes #
Total comments: 8
Patch Set 5 : changes #Patch Set 6 : add length test #Patch Set 7 : No GenerateRandomKey #
Total comments: 2
Patch Set 8 : comments #
Messages
Total messages: 52 (30 generated)
jrummell@chromium.org changed reviewers: + xhwang@chromium.org
PTAL
The CQ bit was checked by jrummell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
xhwang@chromium.org changed reviewers: + hubbe@chromium.org
+hubbe, who added test_random.h. Please see my question in the comment. https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc#... media/cdm/aes_decryptor.cc:70: seed = Rand(n4); The ID returned by this function has two requirements: 1. The ID MUST be unique within the lifetime of an AesDecryptor instance. Otherwise, we could overwrite a previously created temporary session. This is used in production and we should make sure it never happens. 2. The ID SHOULD be unique over multiple AesDecryptor instances, including over different browser sessions (e.g in a new process). This is only used for testing, so a super small chance of failure is okay. Looking at this code, is it ever possible that the |seed| on line 70 is the same as a previous |seed|? It's hard to tell, and the likelihood is probably super small, but I can't see why it's impossible. +hubbe: are you aware of any quantitive analysis on this? One way to get around this is to keep the original incremented session ID as the first 32bit of the 128bit session ID, so that we make sure we never generate the same ID in the same process, hence in the lifetime of an AesDecryptor instance. The rest 96 bits are randomized, to give us some guarantee of randomness for our test.
https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc#... media/cdm/aes_decryptor.cc:62: for (int i = seed % 10; i > 0; --i) This doesn't work. The sequence is still just as predictable. https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc#... media/cdm/aes_decryptor.cc:70: seed = Rand(n4); On 2017/05/10 22:25:36, xhwang wrote: > The ID returned by this function has two requirements: > 1. The ID MUST be unique within the lifetime of an AesDecryptor instance. > Otherwise, we could overwrite a previously created temporary session. This is > used in production and we should make sure it never happens. > 2. The ID SHOULD be unique over multiple AesDecryptor instances, including over > different browser sessions (e.g in a new process). This is only used for > testing, so a super small chance of failure is okay. > > Looking at this code, is it ever possible that the |seed| on line 70 is the same > as a previous |seed|? It's hard to tell, and the likelihood is probably super > small, but I can't see why it's impossible. > > +hubbe: are you aware of any quantitive analysis on this? > > One way to get around this is to keep the original incremented session ID as the > first 32bit of the 128bit session ID, so that we make sure we never generate the > same ID in the same process, hence in the lifetime of an AesDecryptor instance. > The rest 96 bits are randomized, to give us some guarantee of randomness for our > test. I don't see the point of calling random 4 times and concatenating the results when the seed only has 32 bits of state. Why not just hash time+pid with AES instead?
dalecurtis@chromium.org changed reviewers: + dalecurtis@chromium.org
Isn't this what base::UnguessableToken is for?
On 2017/05/15 19:33:05, DaleCurtis_OOO_May_5_To_May23 wrote: > Isn't this what base::UnguessableToken is for? base::UnguessableToken calls base::randbytes(), so it's unusable in the sandbox (for ExternalClearKey). This code is trying to do something similar.
Is the sandbox configured correctly? I.e., are we sure we can't enable randbytes in this sandbox? I thought that runs everywhere.
Looks like crypto::SymmetricKey::GenerateRandomKey() works in the sandbox, so using it to create a random session ID. https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc#... media/cdm/aes_decryptor.cc:62: for (int i = seed % 10; i > 0; --i) On 2017/05/10 22:40:00, hubbe wrote: > This doesn't work. > The sequence is still just as predictable. Acknowledged. https://codereview.chromium.org/2873373003/diff/1/media/cdm/aes_decryptor.cc#... media/cdm/aes_decryptor.cc:70: seed = Rand(n4); On 2017/05/10 22:40:00, hubbe wrote: > On 2017/05/10 22:25:36, xhwang wrote: > > The ID returned by this function has two requirements: > > 1. The ID MUST be unique within the lifetime of an AesDecryptor instance. > > Otherwise, we could overwrite a previously created temporary session. This is > > used in production and we should make sure it never happens. > > 2. The ID SHOULD be unique over multiple AesDecryptor instances, including > over > > different browser sessions (e.g in a new process). This is only used for > > testing, so a super small chance of failure is okay. > > > > Looking at this code, is it ever possible that the |seed| on line 70 is the > same > > as a previous |seed|? It's hard to tell, and the likelihood is probably super > > small, but I can't see why it's impossible. > > > > +hubbe: are you aware of any quantitive analysis on this? > > > > One way to get around this is to keep the original incremented session ID as > the > > first 32bit of the 128bit session ID, so that we make sure we never generate > the > > same ID in the same process, hence in the lifetime of an AesDecryptor > instance. > > The rest 96 bits are randomized, to give us some guarantee of randomness for > our > > test. > > I don't see the point of calling random 4 times and concatenating the results > when the seed only has 32 bits of state. > > Why not just hash time+pid with AES instead? Updated to let crypto::SymmetricKey generate a random ID.
Thanks! Please update the CL description. Also, I have a question on using crypto::SymmetricKey::GenerateRandomKey(). https://codereview.chromium.org/2873373003/diff/20001/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/20001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:35: std::string GenerateSessionId() { Does this guarantee unique results on repeating calls withing the same process?
Description was changed from ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are basically a 32 digit hexadecimal number. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h can't be used. So it uses a trivial random number generator that generates pseudo-random numbers. BUG=478960 TEST=media unit tests pass ========== to ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are basically a 32 digit hexadecimal number. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h can't be used. However, crypto::SymmetricKey::GenerateRandomKey() works and is used to generate a random ID. BUG=478960 TEST=media unit tests pass ==========
Updated description, and comment below. https://codereview.chromium.org/2873373003/diff/20001/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/20001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:35: std::string GenerateSessionId() { On 2017/05/15 23:12:40, xhwang wrote: > Does this guarantee unique results on repeating calls withing the same process? I'm not sure how to guarantee it. I added a test that generates 25 sessions and checks that they're unique. I thought of adding a check when adding the new session into the list (maybe do { session_id = ... } until open_sessions_.insert(session_id).second(); ), but this could get into a potentially infinite loop if the session_id's were all the same. Guess I could add a loop limit. WDYT?
On 2017/05/16 21:18:20, jrummell wrote: > Updated description, and comment below. > > https://codereview.chromium.org/2873373003/diff/20001/media/cdm/aes_decryptor.cc > File media/cdm/aes_decryptor.cc (right): > > https://codereview.chromium.org/2873373003/diff/20001/media/cdm/aes_decryptor... > media/cdm/aes_decryptor.cc:35: std::string GenerateSessionId() { > On 2017/05/15 23:12:40, xhwang wrote: > > Does this guarantee unique results on repeating calls withing the same > process? > > I'm not sure how to guarantee it. I added a test that generates 25 sessions and > checks that they're unique. I thought of adding a check when adding the new > session into the list (maybe do { session_id = ... } until > open_sessions_.insert(session_id).second(); ), but this could get into a > potentially infinite loop if the session_id's were all the same. Guess I could > add a loop limit. WDYT? As discussed offline, it's probably easier to just keep the incrementing number to prepend/append it to a ramdom string, so that we can make sure there's no collision for temp sessions. Persistent sessions is only used for testing, and we are fine if there's a small chance of collision.
Updated to add |next_session_id++| to the ID.
https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:37: // Create a random session ID. The generated 128 bit key produces a 32 Move general comments above the function declaration and keep the implementation detail comments here. https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:43: static unsigned next_session_id = 1; use precise-width integer types (e.g. uint32_t) per style guide: https://uma.googleplex.com/p/chrome/timeline_v2/?sid=d8c39a1363fcaaab7c8f1563... https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:43: static unsigned next_session_id = 1; naming nit: This is only part of the session id (suffix), maybe next_session_id_suffix? https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:48: base::UintToString(next_session_id++); This makes the session ID variable-length which could be annoying. Also, we are mixing HEX numbers and decimal numbers. Does it make sense to HexEncode the next_session_id as well so that our session ID will be fixed-length, and everything is in HEX?
Updated. https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:37: // Create a random session ID. The generated 128 bit key produces a 32 On 2017/05/24 18:53:26, xhwang wrote: > Move general comments above the function declaration and keep the implementation > detail comments here. Done. https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:43: static unsigned next_session_id = 1; On 2017/05/24 18:53:26, xhwang wrote: > naming nit: This is only part of the session id (suffix), maybe > next_session_id_suffix? Done. https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:43: static unsigned next_session_id = 1; On 2017/05/24 18:53:26, xhwang wrote: > use precise-width integer types (e.g. uint32_t) per style guide: > https://uma.googleplex.com/p/chrome/timeline_v2/?sid=d8c39a1363fcaaab7c8f1563... > I just copied the type used by UintToString(). Done. https://codereview.chromium.org/2873373003/diff/40001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:48: base::UintToString(next_session_id++); On 2017/05/24 18:53:26, xhwang wrote: > This makes the session ID variable-length which could be annoying. Also, we are > mixing HEX numbers and decimal numbers. > > Does it make sense to HexEncode the next_session_id as well so that our session > ID will be fixed-length, and everything is in HEX? Done.
The CQ bit was checked by jrummell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Please update the CL description, which is outdated now. https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:41: // value to ensure that the session ID is unique in this instance. s/instance/process Probably worth mentioning that there's a slight chance of collision over different processes, but that is only used for testing so we are okay. https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:46: std::string data = key->key(); nit: seems no need to make a copy here? https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... File media/cdm/aes_decryptor_unittest.cc (right): https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor_unittest.cc:1059: EXPECT_EQ(kNumIterations, seen_sessions.size()); Also check the size of the session id?
Description was changed from ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are basically a 32 digit hexadecimal number. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h can't be used. However, crypto::SymmetricKey::GenerateRandomKey() works and is used to generate a random ID. BUG=478960 TEST=media unit tests pass ========== to ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are a 32 digit random hexadecimal number followed by an 8 digit sequence number to ensure they are unique in the process. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h can't be used. However, crypto::SymmetricKey::GenerateRandomKey() works and is used to generate the random part of the session ID. BUG=478960 TEST=media unit tests pass ==========
Updated. https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor.cc File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:41: // value to ensure that the session ID is unique in this instance. On 2017/05/26 23:25:48, xhwang wrote: > s/instance/process > > Probably worth mentioning that there's a slight chance of collision over > different processes, but that is only used for testing so we are okay. Done. https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor.cc:46: std::string data = key->key(); On 2017/05/26 23:25:48, xhwang wrote: > nit: seems no need to make a copy here? Done. https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... File media/cdm/aes_decryptor_unittest.cc (right): https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor_unittest.cc:1059: EXPECT_EQ(kNumIterations, seen_sessions.size()); On 2017/05/26 23:25:48, xhwang wrote: > Also check the size of the session id? I don't think there should be a requirement that the session IDs are a certain length. They just need to be unique.
The CQ bit was checked by jrummell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm % nit, thanks! https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... File media/cdm/aes_decryptor_unittest.cc (right): https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor_unittest.cc:1059: EXPECT_EQ(kNumIterations, seen_sessions.size()); On 2017/05/27 00:20:33, jrummell wrote: > On 2017/05/26 23:25:48, xhwang wrote: > > Also check the size of the session id? > > I don't think there should be a requirement that the session IDs are a certain > length. They just need to be unique. In CL description you mentioned that "the new session IDs are a 32 digit random hexadecimal number followed by an 8 digit sequence number to ensure they are unique in the process". How do we make sure that's true?
The CQ bit was checked by jrummell@chromium.org to run a CQ dry run
Updated test. Thanks. https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... File media/cdm/aes_decryptor_unittest.cc (right): https://codereview.chromium.org/2873373003/diff/60001/media/cdm/aes_decryptor... media/cdm/aes_decryptor_unittest.cc:1059: EXPECT_EQ(kNumIterations, seen_sessions.size()); On 2017/05/27 00:30:05, xhwang wrote: > On 2017/05/27 00:20:33, jrummell wrote: > > On 2017/05/26 23:25:48, xhwang wrote: > > > Also check the size of the session id? > > > > I don't think there should be a requirement that the session IDs are a certain > > length. They just need to be unique. > > In CL description you mentioned that "the new session IDs are a 32 digit random > hexadecimal number followed by an 8 digit sequence number to ensure they are > unique in the process". How do we make sure that's true? Added length test.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by jrummell@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Looks like GenerateRandomKey() doesn't work in the linux sandbox (no problems locally, but the trybots for PS#5 failed). So I've converted this back to use the trivial rand() function + id, and the session IDs are now 16 characters. PTAL. (I'll update the description if people are happy with this.)
Thanks! Yes, please update the CL description :) LGTM % tiny nit https://codereview.chromium.org/2873373003/diff/120001/media/cdm/aes_decrypto... File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/120001/media/cdm/aes_decrypto... media/cdm/aes_decryptor.cc:52: // ever saved by External Clear Key, which is test only. It probably makes sense to split this comment and spread them in the code.
Description was changed from ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are a 32 digit random hexadecimal number followed by an 8 digit sequence number to ensure they are unique in the process. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h can't be used. However, crypto::SymmetricKey::GenerateRandomKey() works and is used to generate the random part of the session ID. BUG=478960 TEST=media unit tests pass ========== to ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are a 8 digit random hexadecimal number followed by an 8 digit sequence number to ensure they are unique in the process. Session IDs are a printable string to make logging them easier. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h and crypto::GenerateRandomKey() can't be used. So use a trivial rand function for the random number. BUG=478960 TEST=media unit tests pass ==========
Thanks for the reviews. https://codereview.chromium.org/2873373003/diff/120001/media/cdm/aes_decrypto... File media/cdm/aes_decryptor.cc (right): https://codereview.chromium.org/2873373003/diff/120001/media/cdm/aes_decrypto... media/cdm/aes_decryptor.cc:52: // ever saved by External Clear Key, which is test only. On 2017/06/02 16:54:08, xhwang wrote: > It probably makes sense to split this comment and spread them in the code. Done.
The CQ bit was checked by jrummell@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org Link to the patchset: https://codereview.chromium.org/2873373003/#ps140001 (title: "comments")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 140001, "attempt_start_ts": 1496445899597830, "parent_rev": "3cac35f9fdcde39a17ac119d9190139b90f0b75a", "commit_rev": "1862926fcdeb5bd96594c55e674947b9e04e111e"}
Message was sent while issue was closed.
Description was changed from ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are a 8 digit random hexadecimal number followed by an 8 digit sequence number to ensure they are unique in the process. Session IDs are a printable string to make logging them easier. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h and crypto::GenerateRandomKey() can't be used. So use a trivial rand function for the random number. BUG=478960 TEST=media unit tests pass ========== to ========== Change ClearKey session IDs to be random strings Currently AesDecryptor session IDs are numbered sequentially (e.g. "1", "2", etc.). To support persistent sessions using AesDecryptor we need random session IDs so that there is less chance that a saved ID matches an existing session ID. The new session IDs are a 8 digit random hexadecimal number followed by an 8 digit sequence number to ensure they are unique in the process. Session IDs are a printable string to make logging them easier. As AesDecryptor may be run in a sandbox (External Clear Key, used for testing), we are limited in what functions can be called. In particular, the functions in base/rand_util.h and crypto::GenerateRandomKey() can't be used. So use a trivial rand function for the random number. BUG=478960 TEST=media unit tests pass Review-Url: https://codereview.chromium.org/2873373003 Cr-Commit-Position: refs/heads/master@{#476857} Committed: https://chromium.googlesource.com/chromium/src/+/1862926fcdeb5bd96594c55e6749... ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://chromium.googlesource.com/chromium/src/+/1862926fcdeb5bd96594c55e6749... |