Add CryptAuth managers to EasyUnlockServiceRegular

Add CryptAuth managers to EasyUnlockServiceRegular

To replace CryptAuth features in the Smart Lock Chrome app, these managers perform the same functionality natively.

These managers are gated behind the kEnableBluetoothLowEnergyDiscovery flag, and are only initialized on ChromeOS.

BUG=512230
Committed: https://crrev.com/fd6f252f8c10f3e30a9f7997783a30397bdf039d
Cr-Commit-Position: refs/heads/master@{#346703}

[Tim Song, 2015-08-25 20:01:15]

Patchset #1 (id:1) has been deleted

[Tim Song, 2015-08-25 20:01:19]

Patchset #1 (id:20001) has been deleted

[Tim Song, 2015-08-25 20:01:23]

Patchset #1 (id:40001) has been deleted

[Tim Song, 2015-08-25 22:30:54]

Patchset #1 (id:60001) has been deleted

[Tim Song, 2015-08-25 22:33:22]

Patchset #1 (id:80001) has been deleted

[Tim Song, 2015-08-25 22:46:52]

tengs@chromium.org changed reviewers:
+ sacomoto@chromium.org

[Tim Song, 2015-08-25 22:50:40]

Patchset #1 (id:100001) has been deleted

[Tim Song, 2015-08-26 02:43:32]

tengs@chromium.org changed reviewers:
+ msarda@chromium.org

[sacomoto, 2015-08-27 15:38:05]

Hi Tim,

I have a few question, about the high-level structure of EasyUnlockService.

What is the difference between EasyUnlockServiceRegular and
EasyUnlockServiceSignin? 

What's the difference between a signin profile and a regular one? 

When each of them is used?

Gustavo

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
File chrome/browser/signin/easy_unlock_service_regular.cc (right):

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
chrome/browser/signin/easy_unlock_service_regular.cc:496: height_in_inches *
height_in_inches);
Why is all this necessary? What is CryptAuth going to do with it?

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
File chrome/browser/signin/easy_unlock_service_regular.h (right):

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
chrome/browser/signin/easy_unlock_service_regular.h:61: private:
Why everything is private? How is this class being used?

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
File components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc (right):

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc:21:
base::MD5Final(&digest, &context);
Why don't you use |base::MD5Sum()|?

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc:23: // Fold
the digest into an int64 value.
Please add a comment saying that |digest.a| has 128 bits, and you are splitting
it in two 64bits parts and summing them up.

[Tim Song, 2015-08-27 18:12:27]

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
File chrome/browser/signin/easy_unlock_service_regular.cc (right):

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
chrome/browser/signin/easy_unlock_service_regular.cc:496: height_in_inches *
height_in_inches);
On 2015/08/27 15:38:05, sacomoto wrote:
> Why is all this necessary? What is CryptAuth going to do with it? 

This is used to distinguish between form factors (e.g. phones/tablets or
laptops/desktops).

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
File chrome/browser/signin/easy_unlock_service_regular.h (right):

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
chrome/browser/signin/easy_unlock_service_regular.h:61: private:
On 2015/08/27 15:38:05, sacomoto wrote:
> Why everything is private? How is this class being used? 

These functions are publicly exposed in the base EasyUnlockService, and are
accessed virtually. I suppose these functions could be public without any loss,
though.

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
File components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc (right):

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc:21:
base::MD5Final(&digest, &context);
On 2015/08/27 15:38:05, sacomoto wrote:
> Why don't you use |base::MD5Sum()|?

I feel it's a bit safer to use the MD5Context interface, since we don't have to
pass the raw c_str().

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc:23: // Fold
the digest into an int64 value.
On 2015/08/27 15:38:05, sacomoto wrote:
> Please add a comment saying that |digest.a| has 128 bits, and you are
splitting
> it in two 64bits parts and summing them up.

Done.

[Tim Song, 2015-08-27 18:20:37]

To give a brief summary, on ChromeOS, there is a difference between "profiles"
and "users".

A profile is like a profile for Chrome on Windows or Mac. It is the context that
user preferences, cookies, history, etc. are saved. There is a user-data-dir
that backs each profile.

A user, on the other hand, has an associated GAIA account. Each user has a
separate profile that is encrypted. When Chrome is in a user session after
signing in, this profile is decrypted and used. The EasyUnlockServiceRegular
instance runs with the user profile.

However, Chrome has a profile even if there are no users signed in. The
EasyUnlockServiceSignInChromeOS runs with the login profile.

There are functions specific to the sign-in screen or the lock screen, so
different EasyUnlockService subclasses are instantiated.

In our case, there is no GAIA user for the login profile, so it doesn't make
sense to do CryptAuth related things.

[sacomoto, 2015-08-28 14:47:05]

On 2015/08/27 18:20:37, Tim Song wrote:
> To give a brief summary, on ChromeOS, there is a difference between "profiles"
> and "users".
> 
> A profile is like a profile for Chrome on Windows or Mac. It is the context
that
> user preferences, cookies, history, etc. are saved. There is a user-data-dir
> that backs each profile.
> 
> A user, on the other hand, has an associated GAIA account. Each user has a
> separate profile that is encrypted. When Chrome is in a user session after
> signing in, this profile is decrypted and used. The EasyUnlockServiceRegular
> instance runs with the user profile.
> 
> However, Chrome has a profile even if there are no users signed in. The
> EasyUnlockServiceSignInChromeOS runs with the login profile.
> 
> There are functions specific to the sign-in screen or the lock screen, so
> different EasyUnlockService subclasses are instantiated.
> 
> In our case, there is no GAIA user for the login profile, so it doesn't make
> sense to do CryptAuth related things.

It's clearer now, thanks.

[sacomoto, 2015-08-28 14:47:17]

LGTM.

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
File chrome/browser/signin/easy_unlock_service_regular.h (right):

https://codereview.chromium.org/1308393007/diff/140001/chrome/browser/signin/...
chrome/browser/signin/easy_unlock_service_regular.h:61: private:
On 2015/08/27 18:12:27, Tim Song wrote:
> On 2015/08/27 15:38:05, sacomoto wrote:
> > Why everything is private? How is this class being used? 
> 
> These functions are publicly exposed in the base EasyUnlockService, and are
> accessed virtually. I suppose these functions could be public without any
loss,
> though.

Acknowledged.

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
File components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc (right):

https://codereview.chromium.org/1308393007/diff/140001/components/proximity_a...
components/proximity_auth/cryptauth/cryptauth_enrollment_utils.cc:21:
base::MD5Final(&digest, &context);
On 2015/08/27 18:12:27, Tim Song wrote:
> On 2015/08/27 15:38:05, sacomoto wrote:
> > Why don't you use |base::MD5Sum()|?
> 
> I feel it's a bit safer to use the MD5Context interface, since we don't have
to
> pass the raw c_str().

Ok. It makes sense.

[Tim Song, 2015-09-01 18:41:16]

The CQ bit was checked by tengs@chromium.org

[Tim Song, 2015-09-01 18:41:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
sacomoto@chromium.org
Link to the patchset: https://codereview.chromium.org/1308393007/#ps180001
(title: "fix windows compile")

[commit-bot: I haz the power, 2015-09-01 18:42:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1308393007/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1308393007/180001

[commit-bot: I haz the power, 2015-09-01 19:18:08]

Committed patchset #4 (id:180001)

[commit-bot: I haz the power, 2015-09-01 19:19:04]

Patchset 4 (id:??) landed as
https://crrev.com/fd6f252f8c10f3e30a9f7997783a30397bdf039d
Cr-Commit-Position: refs/heads/master@{#346703}