enterprise.platformKeys: Support the publicExponent parameter.

enterprise.platformKeys: Support the publicExponent parameter.

Depends on
https://codereview.chromium.org/298073009/

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

[pneubeck (no reviews), 2014-06-03 13:54:47]

@Kalman, you will be interested in the json schema changes to support
Uint8Array. Need your lgtm for:
  c/c/e/api/enterprise_platform_keys_internal.idl
  c/r/e/enterprise_platform_keys_natives.cc
  c/r/resources/extensions/json_schema.js

@Ryan, ptal at the rest. In particular the RSAPrivateKey changes.

[not at google - send to devlin, 2014-06-03 17:53:32]

https://codereview.chromium.org/306433003/diff/100001/chrome/common/extension...
File chrome/common/extensions/api/enterprise_platform_keys_internal.idl (right):

https://codereview.chromium.org/306433003/diff/100001/chrome/common/extension...
chrome/common/extensions/api/enterprise_platform_keys_internal.idl:30: //
|modulusLength| The length, in bits, of the RSA modulus.
|publicExponent| ...

https://codereview.chromium.org/306433003/diff/100001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/json_schema.js (right):

https://codereview.chromium.org/306433003/diff/100001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/json_schema.js:140: return "binary";
+asargent here who I think implemented binary? 2 issues:
- why the typeof(ArrayBuffer|Uint8Array) check?
- this isn't necessarily safe cross-context (because each context gets its own
constructor instance and == doesn't work), but it could be with the check
value.constructor.name == 'Uint8Array'

[Ryan Sleevi, 2014-06-03 19:35:29]

It's perfectly fine - and arguably, desirable, from a security perspective - to
reject all parameters that are not equal to N_4 (aka 65537), you just have to
expect the parameter in the WebCrypto signature.

That WebCrypto allows it to be parameterized is complex, but I would instead
recommend in your .js helper you .reject() anything where it's not 65537, and
don't update the signature for anything below that (eg: your IPCs, or the
//crypto impl)

https://codereview.chromium.org/306433003/diff/100001/chrome/browser/extensio...
File
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_api.cc
(right):

https://codereview.chromium.org/306433003/diff/100001/chrome/browser/extensio...
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_api.cc:46:
return false;
https://code.google.com/p/chromium/issues/detail?id=373552

https://codereview.chromium.org/306433003/diff/100001/crypto/rsa_private_key_...
File crypto/rsa_private_key_nss.cc (right):

https://codereview.chromium.org/306433003/diff/100001/crypto/rsa_private_key_...
crypto/rsa_private_key_nss.cc:109: uint64 RSAPrivateKey::kDefaultPublicExponent
= 65537L;
1) Wrong suffix
2) Wrong format for declaring constants

uint64 RSAPrivateKey::kDefaultPublicExponent = UINT64_C(65537);

[asargent_no_longer_on_chrome, 2014-06-03 20:52:50]

https://codereview.chromium.org/306433003/diff/100001/chrome/renderer/resourc...
File chrome/renderer/resources/extensions/json_schema.js (right):

https://codereview.chromium.org/306433003/diff/100001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/json_schema.js:140: return "binary";
On 2014/06/03 17:53:33, kalman wrote:
> +asargent here who I think implemented binary? 2 issues:
> - why the typeof(ArrayBuffer|Uint8Array) check?
> - this isn't necessarily safe cross-context (because each context gets its own
> constructor instance and == doesn't work), but it could be with the check
> value.constructor.name == 'Uint8Array'

I was neither author nor reviewer - looks like this was added in
https://chromiumcodereview.appspot.com/10560027 for crbug.com/133178

[pneubeck (no reviews), 2014-06-04 12:39:16]

On 2014/06/03 19:35:29, Ryan Sleevi wrote:
> It's perfectly fine - and arguably, desirable, from a security perspective -
to
> reject all parameters that are not equal to N_4 (aka 65537), you just have to
> expect the parameter in the WebCrypto signature.
> 
> That WebCrypto allows it to be parameterized is complex, but I would instead
> recommend in your .js helper you .reject() anything where it's not 65537, and
> don't update the signature for anything below that (eg: your IPCs, or the
> //crypto impl)
It would safe me a lot of time if we could figure out such
requirements/assumptions in advance in the future. :-/

> 
>
https://codereview.chromium.org/306433003/diff/100001/chrome/browser/extensio...
> File
>
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_api.cc
> (right):
> 
>
https://codereview.chromium.org/306433003/diff/100001/chrome/browser/extensio...
>
chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_api.cc:46:
> return false;
> https://code.google.com/p/chromium/issues/detail?id=373552
> 
>
https://codereview.chromium.org/306433003/diff/100001/crypto/rsa_private_key_...
> File crypto/rsa_private_key_nss.cc (right):
> 
>
https://codereview.chromium.org/306433003/diff/100001/crypto/rsa_private_key_...
> crypto/rsa_private_key_nss.cc:109: uint64
RSAPrivateKey::kDefaultPublicExponent
> = 65537L;
> 1) Wrong suffix
> 2) Wrong format for declaring constants
> 
> uint64 RSAPrivateKey::kDefaultPublicExponent = UINT64_C(65537);

[pneubeck (no reviews), 2014-06-04 12:40:02]

Dropped the json schema changes as they're not required for this CL anymore.

[pneubeck (no reviews), 2014-06-04 12:42:16]

-Antony

@Benjamin, you only have to rubberstamp
chrome/renderer/extensions/enterprise_platform_keys_natives.cc
(I might have to pick up the Uint8Array or in general ArrayBufferView in a later
CL though)

Thanks!

[not at google - send to devlin, 2014-06-04 15:57:48]

lgtm

[pneubeck (no reviews), 2014-06-06 12:46:28]

-Ryan

@Eric, 
following the 'share WebCrypto's normalize function', this is now adding
publicExponent support.
ptal

[eroman, 2014-06-09 22:49:02]

lgtm

https://codereview.chromium.org/306433003/diff/160001/chrome/renderer/resourc...
File
chrome/renderer/resources/extensions/enterprise_platform_keys/subtle_crypto.js
(right):

https://codereview.chromium.org/306433003/diff/160001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/enterprise_platform_keys/subtle_crypto.js:55:
// exponent 65537.
The description could be clearer, indicating that it is allowing leading zeros.

[pneubeck (no reviews), 2014-06-16 17:54:51]

https://codereview.chromium.org/306433003/diff/160001/chrome/renderer/resourc...
File
chrome/renderer/resources/extensions/enterprise_platform_keys/subtle_crypto.js
(right):

https://codereview.chromium.org/306433003/diff/160001/chrome/renderer/resourc...
chrome/renderer/resources/extensions/enterprise_platform_keys/subtle_crypto.js:55:
// exponent 65537.
On 2014/06/09 22:49:01, eroman wrote:
> The description could be clearer, indicating that it is allowing leading
zeros.

Done.

[pneubeck (no reviews), 2014-06-17 07:53:14]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-06-17 07:55:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/pneubeck@chromium.org/306433003/220001

[commit-bot: I haz the power, 2014-06-17 11:50:46]

Change committed as 277730