Don't use nss_util in CertificateManagerModel::IsHardwareBacked.

Don't use nss_util in CertificateManagerModel::IsHardwareBacked.

Avoids threading issues.

IsHardwareBacked works on non-ChromeOS platforms now too.

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

[mattm, 2013-11-19 22:15:36]

little update: the IsHardwareBacked test isn't chromeos specific anymore, so I
removed the chromeos ifdefs from the rest of that file.

[wtc, 2013-11-19 22:30:53]

Patch set 2 LGTM.

https://codereview.chromium.org/76653003/diff/70001/chrome/browser/certificat...
File chrome/browser/certificate_manager_model.cc (left):

https://codereview.chromium.org/76653003/diff/70001/chrome/browser/certificat...
chrome/browser/certificate_manager_model.cc:163:
cert_db_->GetPrivateModule()->os_module_handle();

Please doublecheck this is OK.

The original code returns true only if the certificate is in the ChromeOS TPM.

The new code returns true if the certificate is in any hardware slot. So
CertificateManagerModel::IsHardwareBacked has a different meaning for ChromeOS
now.

Based on the usage of IsHardwareBacked in this file, I think the new meaning is
correct.

https://codereview.chromium.org/76653003/diff/70001/chrome/browser/certificat...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/76653003/diff/70001/chrome/browser/certificat...
chrome/browser/certificate_manager_model.cc:150: bool
CertificateManagerModel::IsHardwareBacked(

1. Please edit chrome/browser/certificate_manager_model.h and remove this
comment for IsHardwareBacked:
  // This function is only implemented for Chrome OS and always returns false
  // for other platforms.

2. Optional: in chrome/app/generated_resources.grd,
IDS_CERT_MANAGER_HARDWARE_BACKED has this description:
"A device name to be used for ChromeOS device backed keys."

We should change "ChromeOS device backed keys" to "hardware backed keys".

[mattm, 2013-11-19 22:47:03]

+xiyuan to double-check the IsHardwareBacked change is ok from chromeos side.
Also to double-check the
chrome/browser/ui/webui/options/certificate_manager_handler.cc change. Those
strings didn't seem to be used anywhere.

[mattm, 2013-11-19 22:47:26]

https://codereview.chromium.org/76653003/diff/70001/chrome/browser/certificat...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/76653003/diff/70001/chrome/browser/certificat...
chrome/browser/certificate_manager_model.cc:150: bool
CertificateManagerModel::IsHardwareBacked(
On 2013/11/19 22:30:53, wtc wrote:
> 
> 1. Please edit chrome/browser/certificate_manager_model.h and remove this
> comment for IsHardwareBacked:
>   // This function is only implemented for Chrome OS and always returns false
>   // for other platforms.
> 
> 2. Optional: in chrome/app/generated_resources.grd,
> IDS_CERT_MANAGER_HARDWARE_BACKED has this description:
> "A device name to be used for ChromeOS device backed keys."
> 
> We should change "ChromeOS device backed keys" to "hardware backed keys".
> 

Done.

[wtc, 2013-11-19 23:07:43]

Patch set 3 LGTM.

https://codereview.chromium.org/76653003/diff/70002/chrome/app/generated_reso...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/76653003/diff/70002/chrome/app/generated_reso...
chrome/app/generated_resources.grd:3258: +      <message
name="IDS_CERT_MANAGER_HARDWARE_BACKED" desc="A device name to be used for
hardware backed keys.">

Nit: hyphenate "hardware backed". Sorry I didn't notice this earlier.

[xiyuan, 2013-11-19 23:13:34]

LGTM

[mattm, 2013-11-19 23:24:20]

+jhawkins for chrome/browser OWNERS

[James Hawkins, 2013-11-19 23:25:49]

lgtm

[commit-bot: I haz the power, 2013-11-19 23:48:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/76653003/270001

[commit-bot: I haz the power, 2013-11-20 05:07:06]

Change committed as 236160