Make CryptohomeAuthenticator's Login*() methods work with pre-hashed keys

Make CryptohomeAuthenticator's Login*() methods work with pre-hashed keys

When a user's cryptohome directory is created with a pre-hashed key,
the credentials provided need to be hashed in the same way whenever the
crypthome is to be unlocked/mounted. This CL updates the Login*() methods
in CryptohomeAuthenticator to retrieve key metadata and apply the correct
hashing algorithm to the given credentials. Follow-up CLs will update
the other CryptohomeAuthenticator methods to also work with pre-hashed
keys.

BUG=367847
TEST=Extended unit tests

Committed: https://crrev.com/37e866f487685f176c1861ecc507a2f25e2c1a33
Cr-Commit-Position: refs/heads/master@{#293227}

[bartfab (slow), 2014-08-28 16:44:04]

bartfab@chromium.org changed reviewers:
+ antrim@chromium.org, dkrahn@chromium.org

[bartfab (slow), 2014-08-28 16:44:04]

Hi Denis,

Could you review please?

Hi Darren,

Could you do an owner review of mock_homedir_methods.* please?

[Darren Krahn, 2014-08-28 17:52:52]

mock_homedir_methods lgtm

https://codereview.chromium.org/517653002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc
(right):

https://codereview.chromium.org/517653002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc:285:
.Times(1)
nit: This can be removed. WillOnce() implies Times(1)

[bartfab (slow), 2014-08-29 08:30:02]

https://codereview.chromium.org/517653002/diff/1/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc
(right):

https://codereview.chromium.org/517653002/diff/1/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc:285:
.Times(1)
On 2014/08/28 17:52:52, Darren Krahn wrote:
> nit: This can be removed. WillOnce() implies Times(1)

Done.

[bartfab (slow), 2014-09-01 17:04:08]

Hi Denis,

Friendly review ping.

[bartfab (slow), 2014-09-02 12:30:10]

bartfab@chromium.org changed reviewers:
+ dzhioev@chromium.org
- antrim@chromium.org

[bartfab (slow), 2014-09-02 12:30:10]

Hi Pavel,

It looks like Denis may be out. Could you take a look?

[dzhioev (left Google), 2014-09-02 15:56:27]

https://codereview.chromium.org/517653002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc
(right):

https://codereview.chromium.org/517653002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc:823:
TEST_F(CryptohomeAuthenticatorTest, FailLoginWithMissingSalt) {
What about missing/wrong "type" test?

https://codereview.chromium.org/517653002/diff/20001/chromeos/login/auth/cryp...
File chromeos/login/auth/cryptohome_authenticator.cc (right):

https://codereview.chromium.org/517653002/diff/20001/chromeos/login/auth/cryp...
chromeos/login/auth/cryptohome_authenticator.cc:41: 
Where is the code, that saves this data into cryptohome?

https://codereview.chromium.org/517653002/diff/20001/chromeos/login/auth/cryp...
chromeos/login/auth/cryptohome_authenticator.cc:185: if (success &&
key_data.size() == 1) {
When |key_data| with a size greater than 1 is returned?

[bartfab (slow), 2014-09-02 17:24:58]

https://codereview.chromium.org/517653002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc
(right):

https://codereview.chromium.org/517653002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/auth/cryptohome_authenticator_unittest.cc:823:
TEST_F(CryptohomeAuthenticatorTest, FailLoginWithMissingSalt) {
On 2014/09/02 15:56:26, dzhioev wrote:
> What about missing/wrong "type" test?

All existing cryptohomes have no type - this is the legacy code path in which we
use the default hashing algorithm and system salt.

https://codereview.chromium.org/517653002/diff/20001/chromeos/login/auth/cryp...
File chromeos/login/auth/cryptohome_authenticator.cc (right):

https://codereview.chromium.org/517653002/diff/20001/chromeos/login/auth/cryp...
chromeos/login/auth/cryptohome_authenticator.cc:41: 
On 2014/09/02 15:56:26, dzhioev wrote:
> Where is the code, that saves this data into cryptohome?

There is none yet. This CL adds the code path that reads metadata and has tests
that simulate metadata presence. A follow-up CL will wire up the write path.

https://codereview.chromium.org/517653002/diff/20001/chromeos/login/auth/cryp...
chromeos/login/auth/cryptohome_authenticator.cc:185: if (success &&
key_data.size() == 1) {
On 2014/09/02 15:56:26, dzhioev wrote:
> When |key_data| with a size greater than 1 is returned?

That would be an error. We ask for metadata for exactly one key, so we should
get back exactly one result. I added LOG(ERROR) statement when size() != 1.

[dzhioev (left Google), 2014-09-02 17:46:20]

LTGM

[dzhioev (left Google), 2014-09-03 16:16:20]

On 2014/09/02 17:46:20, dzhioev wrote:
> LTGM

Sorry, I meant LGTM =)

[bartfab (slow), 2014-09-03 20:26:25]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-09-03 20:30:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/517653002/40001

[commit-bot: I haz the power, 2014-09-04 00:03:13]

Committed patchset #3 (id:40001) as edd968e3405fb7c2915c8ab13192e43cdd56257a

[commit-bot: I haz the power, 2014-09-10 03:28:45]

Patchset 3 (id:??) landed as
https://crrev.com/37e866f487685f176c1861ecc507a2f25e2c1a33
Cr-Commit-Position: refs/heads/master@{#293227}