Add support for the Pkcs11GetTpmTokenInfoForUser cryptohome call.

Add support for the Pkcs11GetTpmTokenInfoForUser cryptohome call.

This call already existed but had not been added to CryptohomeClient.

BUG=chromium:205206
TEST=unit, manual

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=228033

[Darren Krahn, 2013-10-08 00:20:06]

Note: depends on https://chromium-review.googlesource.com/172109

[satorux1, 2013-10-08 04:29:32]

https://codereview.chromium.org/26407002/diff/1/chromeos/dbus/cryptohome_clie...
File chromeos/dbus/cryptohome_client.cc (right):

https://codereview.chromium.org/26407002/diff/1/chromeos/dbus/cryptohome_clie...
chromeos/dbus/cryptohome_client.cc:820: !reader.PopInt32(&slot)) {
might want to add:

  LOG(ERROR) << "Invalid response: " << response->ToString();

I think it'd be nice to add this to other functions.

https://codereview.chromium.org/26407002/diff/1/chromeos/dbus/cryptohome_clie...
File chromeos/dbus/cryptohome_client.h (right):

https://codereview.chromium.org/26407002/diff/1/chromeos/dbus/cryptohome_clie...
chromeos/dbus/cryptohome_client.h:43: int slot)> Pkcs11GetTpmTokenInfoCallback;
What is slot? could you add some comments about label, user_bin, and slot while
you are at it?

https://codereview.chromium.org/26407002/diff/1/chromeos/dbus/cryptohome_clie...
chromeos/dbus/cryptohome_client.h:194: virtual void
Pkcs11GetTpmTokenInfoForUser(
Function comment is missing.  "Calls XXX method." is not particularly useful,
but might be better than nothing for consistency. :P 

I think it'd be nice to describe the difference between this function and
Pkcs11GetTpmTokenInfo, which looks very similar.

[Darren Krahn, 2013-10-08 17:06:41]

https://chromiumcodereview.appspot.com/26407002/diff/1/chromeos/dbus/cryptoho...
File chromeos/dbus/cryptohome_client.cc (right):

https://chromiumcodereview.appspot.com/26407002/diff/1/chromeos/dbus/cryptoho...
chromeos/dbus/cryptohome_client.cc:820: !reader.PopInt32(&slot)) {
On 2013/10/08 04:29:33, satorux1 wrote:
> might want to add:
> 
>   LOG(ERROR) << "Invalid response: " << response->ToString();
> 
> I think it'd be nice to add this to other functions.

Done.  Some responses may be sensitive but I added logging where I'm confident
it is appropriate.

https://chromiumcodereview.appspot.com/26407002/diff/1/chromeos/dbus/cryptoho...
File chromeos/dbus/cryptohome_client.h (right):

https://chromiumcodereview.appspot.com/26407002/diff/1/chromeos/dbus/cryptoho...
chromeos/dbus/cryptohome_client.h:43: int slot)> Pkcs11GetTpmTokenInfoCallback;
On 2013/10/08 04:29:33, satorux1 wrote:
> What is slot? could you add some comments about label, user_bin, and slot
while
> you are at it?

Done.

https://chromiumcodereview.appspot.com/26407002/diff/1/chromeos/dbus/cryptoho...
chromeos/dbus/cryptohome_client.h:194: virtual void
Pkcs11GetTpmTokenInfoForUser(
On 2013/10/08 04:29:33, satorux1 wrote:
> Function comment is missing.  "Calls XXX method." is not particularly useful,
> but might be better than nothing for consistency. :P 
> 
> I think it'd be nice to describe the difference between this function and
> Pkcs11GetTpmTokenInfo, which looks very similar.

Done.

[mattm, 2013-10-08 23:10:25]

https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
File chromeos/dbus/cryptohome_client.h (right):

https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
chromeos/dbus/cryptohome_client.h:208: const std::string& username,
Is username the "u-<long hex string>" value like in the home dir path?

[Darren Krahn, 2013-10-08 23:29:37]

https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
File chromeos/dbus/cryptohome_client.h (right):

https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
chromeos/dbus/cryptohome_client.h:208: const std::string& username,
On 2013/10/08 23:10:25, mattm wrote:
> Is username the "u-<long hex string>" value like in the home dir path?

No, it's a canonical email address as returned by chromeos::User::email().  At
least, that is my intention.  Let me know if this is inconvenient for any
reason.

[mattm, 2013-10-08 23:38:08]

On 2013/10/08 23:29:37, Darren Krahn wrote:
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> File chromeos/dbus/cryptohome_client.h (right):
> 
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> chromeos/dbus/cryptohome_client.h:208: const std::string& username,
> On 2013/10/08 23:10:25, mattm wrote:
> > Is username the "u-<long hex string>" value like in the home dir path?
> 
> No, it's a canonical email address as returned by chromeos::User::email().  At
> least, that is my intention.  Let me know if this is inconvenient for any
> reason.

Probably fine, do you know what the function is to get the chromeos::User given
a Profile or BrowserContext?

[satorux1, 2013-10-08 23:46:31]

LGTM with a request:

https://codereview.chromium.org/26407002/diff/6001/chromeos/dbus/cryptohome_c...
File chromeos/dbus/cryptohome_client.h (right):

https://codereview.chromium.org/26407002/diff/6001/chromeos/dbus/cryptohome_c...
chromeos/dbus/cryptohome_client.h:45: // duration of the signed-in session.
thank you for adding this! we need more of these comments!

https://codereview.chromium.org/26407002/diff/6001/chromeos/dbus/cryptohome_c...
chromeos/dbus/cryptohome_client.h:208: const std::string& username,
On 2013/10/08 23:29:38, Darren Krahn wrote:
> On 2013/10/08 23:10:25, mattm wrote:
> > Is username the "u-<long hex string>" value like in the home dir path?
> 
> No, it's a canonical email address as returned by chromeos::User::email().  At
> least, that is my intention.  Let me know if this is inconvenient for any
> reason.

I think it's a good explanation about |username| parameter. Please add it as a
comment.

[Darren Krahn, 2013-10-09 01:09:25]

On 2013/10/08 23:38:08, mattm wrote:
> On 2013/10/08 23:29:37, Darren Krahn wrote:
> >
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> > File chromeos/dbus/cryptohome_client.h (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> > chromeos/dbus/cryptohome_client.h:208: const std::string& username,
> > On 2013/10/08 23:10:25, mattm wrote:
> > > Is username the "u-<long hex string>" value like in the home dir path?
> > 
> > No, it's a canonical email address as returned by chromeos::User::email(). 
At
> > least, that is my intention.  Let me know if this is inconvenient for any
> > reason.
> 
> Probably fine, do you know what the function is to get the chromeos::User
given
> a Profile or BrowserContext?

Not offhand.  You may be able to do some kind of reverse lookup using
UserManager::GetUserProfileDir.  It seems there should be a better way, however.

[mattm, 2013-10-09 01:16:33]

On 2013/10/09 01:09:25, Darren Krahn wrote:
> On 2013/10/08 23:38:08, mattm wrote:
> > On 2013/10/08 23:29:37, Darren Krahn wrote:
> > >
> >
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> > > File chromeos/dbus/cryptohome_client.h (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> > > chromeos/dbus/cryptohome_client.h:208: const std::string& username,
> > > On 2013/10/08 23:10:25, mattm wrote:
> > > > Is username the "u-<long hex string>" value like in the home dir path?
> > > 
> > > No, it's a canonical email address as returned by chromeos::User::email().

> At
> > > least, that is my intention.  Let me know if this is inconvenient for any
> > > reason.
> > 
> > Probably fine, do you know what the function is to get the chromeos::User
> given
> > a Profile or BrowserContext?
> 
> Not offhand.  You may be able to do some kind of reverse lookup using
> UserManager::GetUserProfileDir.  It seems there should be a better way,
however.

I just found UserManagerImpl::GetUserByProfile, though it's private there
doesn't seem to be any reason it couldn't be made public.

[mattm, 2013-10-09 01:38:57]

On 2013/10/09 01:16:33, mattm wrote:
> On 2013/10/09 01:09:25, Darren Krahn wrote:
> > On 2013/10/08 23:38:08, mattm wrote:
> > > On 2013/10/08 23:29:37, Darren Krahn wrote:
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> > > > File chromeos/dbus/cryptohome_client.h (right):
> > > > 
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/26407002/diff/6001/chromeos/dbus/crypt...
> > > > chromeos/dbus/cryptohome_client.h:208: const std::string& username,
> > > > On 2013/10/08 23:10:25, mattm wrote:
> > > > > Is username the "u-<long hex string>" value like in the home dir path?
> > > > 
> > > > No, it's a canonical email address as returned by
chromeos::User::email().
> 
> > At
> > > > least, that is my intention.  Let me know if this is inconvenient for
any
> > > > reason.
> > > 
> > > Probably fine, do you know what the function is to get the chromeos::User
> > given
> > > a Profile or BrowserContext?
> > 
> > Not offhand.  You may be able to do some kind of reverse lookup using
> > UserManager::GetUserProfileDir.  It seems there should be a better way,
> however.
> 
> I just found UserManagerImpl::GetUserByProfile, though it's private there
> doesn't seem to be any reason it couldn't be made public.

My quick test of exposing that works, so I'll try making a CL to do that.

This CL lgtm.

[commit-bot: I haz the power, 2013-10-10 18:59:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dkrahn@chromium.org/26407002/20001

[commit-bot: I haz the power, 2013-10-10 22:41:54]

Change committed as 228033