If easy sign-in is set up, allow TPMTokenLoader to start before login

If easy sign-in is set up, allow TPMTokenLoader to start before login

As part of easy sign-in protocol, a nonce has to be signed by TPM
private key. To be able to do this, we need to allow system token to be
loaded before a user is logged in. This cl starts system token
initialization (which is done by TPMTokenLoader) before LoginState is
LOGGED_IN if there is a user with easy sign-in enabled.

BUG=409027
Committed: https://crrev.com/8666f16f8a150275fab3f7529ede7da11983d06b
Cr-Commit-Position: refs/heads/master@{#308321}

[tbarzic, 2014-12-02 07:30:34]

tbarzic@chromium.org changed reviewers:
+ pneubeck@chromium.org, rsleevi@chromium.org, xiyuan@chromium.org

[Ryan Sleevi, 2014-12-02 11:24:48]

Note: I am not comfortable with this for M40. Nor do I think I can reason about
the 'simplicitly' or 'complexity' of the change to //crypto, which, while one
line, is as far as trivial as they can be.

This is going to take time and care to review.

[pneubeck (no reviews), 2014-12-02 11:56:47]

https://codereview.chromium.org/741593002/diff/60001/chrome/browser/profiles/...
File chrome/browser/profiles/profile_io_data.cc (right):

https://codereview.chromium.org/741593002/diff/60001/chrome/browser/profiles/...
chrome/browser/profiles/profile_io_data.cc:435: params->use_system_key_slot =
use_system_key_slot=true has two effects:
 - trigger initialization of the system token
(EnableNSSSystemKeySlotForResourceContext)
 - use the system token's certs in the ClientCertStoreChromeOS

I'm rather sure that you want only the first and not the second behavior.
You might have to split use_system_key_slot into two more fine-grained flags.

https://codereview.chromium.org/741593002/diff/60001/crypto/nss_util.cc
File crypto/nss_util.cc (right):

https://codereview.chromium.org/741593002/diff/60001/crypto/nss_util.cc#newco...
crypto/nss_util.cc:539: return ScopedPK11Slot(PK11_GetInternalKeySlot());
the fact that you have to change this to make the CHECK in NSSCertDatabase
happy, indicates that we have still/again a conceptual problem here:

In my latest changes I tried to ensure clearer/stronger guarantees like
 1) username(_hash) always be non-empty
 2) NSSCertDatabase to guarantee slots to exist (so that consumers don't have to
clutter slot accesses if conditionals)

I still think that 1) is right and important. Just overloading an empty string
with some special semantics is error-prone. Wherever we need a 'signin-profile'
mode, we should make it explicit.
2) seems more arguable to me, and maybe you should better remove that CHECK
instead of modifying nss_util.cc. Did you explore that route?

[tbarzic, 2014-12-02 22:57:39]

Ok, I updated the other cl so data signin does not depend on
chromeos::platform_keys and thus cert database (it just uses
crypto::GetSystemNSSKeySlot to get the system slot). With that, the changes in
nss_util and profile_io_data are not really needed..

[pneubeck (no reviews), 2014-12-04 16:21:49]

lgtm with a minor comment

https://codereview.chromium.org/741593002/diff/80001/chromeos/tpm_token_loader.h
File chromeos/tpm_token_loader.h (right):

https://codereview.chromium.org/741593002/diff/80001/chromeos/tpm_token_loade...
chromeos/tpm_token_loader.h:126: // This will be true iff |Start| was called.
Reference to |Start| is wrong

[tbarzic, 2014-12-04 16:46:50]

https://codereview.chromium.org/741593002/diff/80001/chromeos/tpm_token_loader.h
File chromeos/tpm_token_loader.h (right):

https://codereview.chromium.org/741593002/diff/80001/chromeos/tpm_token_loade...
chromeos/tpm_token_loader.h:126: // This will be true iff |Start| was called.
On 2014/12/04 16:21:49, pneubeck wrote:
> Reference to |Start| is wrong

Done.

[xiyuan, 2014-12-05 04:16:14]

easy_unlock_service_signin_chromeos.cc LGTM

[tbarzic, 2014-12-08 18:37:07]

tbarzic@chromium.org changed reviewers:
+ stevenjb@chromium.org

[tbarzic, 2014-12-08 18:37:07]

+stevenjb: owner for chromeos/

[stevenjb, 2014-12-08 18:43:43]

owner lgtm

[tbarzic, 2014-12-15 07:01:36]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-12-15 07:01:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/741593002/120001

[commit-bot: I haz the power, 2014-12-15 07:08:04]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2014-12-15 07:08:54]

Patchset 7 (id:??) landed as
https://crrev.com/8666f16f8a150275fab3f7529ede7da11983d06b
Cr-Commit-Position: refs/heads/master@{#308321}