Expose WebCrypto's algorithm normalization.

Expose WebCrypto's algorithm normalization.

This exposes the WebCore::parseAlgorithm function through  blink::WebScriptBindings.

This will allow reusing the normalization in the implementation of the Chrome extension API enterprise.platformKeys.

To keep the exposure of types minimal, the dependency of parseAlgorithm on CryptoResult was removed.
On the way, renames parseAlgorithm to normalizeAlgorithm in order to match the WebCrypto spec and the filename.

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

[pneubeck (no reviews), 2014-05-26 16:07:59]

Hi Eric,
could you take a quick look whether this approximately makes sense to you?
The counterpart would be a ObjectBackedNativeHandler, which could be called from
the enterprise.platformKeys' custom_binding.

Thanks!

[pneubeck (no reviews), 2014-05-26 16:12:39]

On 2014/05/26 16:07:59, pneubeck wrote:
> Hi Eric,
> could you take a quick look whether this approximately makes sense to you?
> The counterpart would be a ObjectBackedNativeHandler, which could be called
from
> the enterprise.platformKeys' custom_binding.
> 
> Thanks!

I updated the commit message to explain better what's going on.

[pneubeck (no reviews), 2014-05-26 16:50:48]

On 2014/05/26 16:12:39, pneubeck wrote:
> On 2014/05/26 16:07:59, pneubeck wrote:
> > Hi Eric,
> > could you take a quick look whether this approximately makes sense to you?
> > The counterpart would be a ObjectBackedNativeHandler, which could be called
> from
> > the enterprise.platformKeys' custom_binding.
> > 
> > Thanks!
> 
> I updated the commit message to explain better what's going on.

Is it expected that there will ever be an actual normalization? I.e. two
different algorithm identifiers (DOMString or Dictionary) that are mapped to the
same WebCryptoAlgorithm?
If not, I could just continue using the original Dictionary.
If yes, then sadly, it seems that I will have to add some serialization
(WebCryptoAlgorithm -> v8::Object or base::DictionaryValue) in chrome/ .

[pneubeck (no reviews), 2014-05-27 16:49:35]

On 2014/05/26 16:50:48, pneubeck wrote:
> On 2014/05/26 16:12:39, pneubeck wrote:
> > On 2014/05/26 16:07:59, pneubeck wrote:
> > > Hi Eric,
> > > could you take a quick look whether this approximately makes sense to you?
> > > The counterpart would be a ObjectBackedNativeHandler, which could be
called
> > from
> > > the enterprise.platformKeys' custom_binding.
> > > 
> > > Thanks!
> > 
> > I updated the commit message to explain better what's going on.
> 
> Is it expected that there will ever be an actual normalization? I.e. two
> different algorithm identifiers (DOMString or Dictionary) that are mapped to
the
> same WebCryptoAlgorithm?
> If not, I could just continue using the original Dictionary.
> If yes, then sadly, it seems that I will have to add some serialization
> (WebCryptoAlgorithm -> v8::Object or base::DictionaryValue) in chrome/ .

FYI: I've also uploaded the chrome change,
https://codereview.chromium.org/298073009/ .

[eroman, 2014-05-28 20:54:50]

general approach looks fine.

what algorithms/parmeters are going to be used? will it require putting any new
parsing code on the blink side?

[pneubeck (no reviews), 2014-05-29 12:43:34]

On 2014/05/28 20:54:50, eroman wrote:
> general approach looks fine.
> 
> what algorithms/parmeters are going to be used? will it require putting any
new
> parsing code on the blink side?

For now only RSASSA will be supported by enterprise.platformKeys; AFAIK our TPMs
don't support anything else.
So, no changes to the blink side should be required.

[pneubeck (no reviews), 2014-05-29 12:52:56]

https://codereview.chromium.org/295423004/diff/1/Source/modules/crypto/Crypto...
File Source/modules/crypto/CryptoResultImpl.h (right):

https://codereview.chromium.org/295423004/diff/1/Source/modules/crypto/Crypto...
Source/modules/crypto/CryptoResultImpl.h:43: ExceptionCode
toExceptionCode(blink::WebCryptoErrorType);
should likely be renamed to webCryptoErrorToExceptionCode

https://codereview.chromium.org/295423004/diff/1/Source/web/WebScriptBindings...
File Source/web/WebScriptBindings.cpp (right):

https://codereview.chromium.org/295423004/diff/1/Source/web/WebScriptBindings...
Source/web/WebScriptBindings.cpp:60: v8::TryCatch block;
ignore this line, I will remove it once I have access to my workstation

[pneubeck (no reviews), 2014-05-29 12:56:26]

+Adam, please review the other parts outside Source/modules/crypto
Thanks!

[pneubeck (no reviews), 2014-06-02 08:30:36]

@Jochen, do you have a chance to review the other changes outside
Source/modules/crypto ?

[jochen (gone - plz use gerrit), 2014-06-02 09:01:45]

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
File public/platform/WebCryptoAlgorithmOperation.h (right):

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
public/platform/WebCryptoAlgorithmOperation.h:37: Encrypt,
public enums should start with Web, I'd put Crypto in the name, and the values
should start with the same name as the enum, e.g.
WebCryptoAlgorithmOperationEncrypt

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
public/platform/WebCryptoAlgorithmOperation.h:48: // <---- End of list (keep
this up-to-date)
that comment is unnecessary

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
public/platform/WebCryptoAlgorithmOperation.h:49: LastAlgorithmOperation =
UnwrapKey,
Should be WebCryptoAlgorithmOperationLast

https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
File public/web/WebScriptBindings.h (right):

https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
public/web/WebScriptBindings.h:67: BLINK_EXPORT static WebCryptoAlgorithm
normalizeCryptoAlgorithm(v8::Handle<v8::Object>, AlgorithmOperation, int*,
WebString*, v8::Isolate*);
this should probably be in a WebCrypto header

also, please add parameter names for not obvious parameters, like int*

[pneubeck (no reviews), 2014-06-02 10:12:16]

https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
File public/web/WebScriptBindings.h (right):

https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
public/web/WebScriptBindings.h:67: BLINK_EXPORT static WebCryptoAlgorithm
normalizeCryptoAlgorithm(v8::Handle<v8::Object>, AlgorithmOperation, int*,
WebString*, v8::Isolate*);
On 2014/06/02 09:01:45, jochen wrote:
> this should probably be in a WebCrypto header
> 
> also, please add parameter names for not obvious parameters, like int*

Instead of WebScriptBindings, should it be in a
 public/web/WebCryptoNormalizeAlgorithm.h
or
 public/platform/WebCryptoNormalizeAlgorithm.h
?

Platform.h does not depend on this one.

[jochen (gone - plz use gerrit), 2014-06-02 10:15:25]

On 2014/06/02 10:12:16, pneubeck wrote:
>
https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
> File public/web/WebScriptBindings.h (right):
> 
>
https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
> public/web/WebScriptBindings.h:67: BLINK_EXPORT static WebCryptoAlgorithm
> normalizeCryptoAlgorithm(v8::Handle<v8::Object>, AlgorithmOperation, int*,
> WebString*, v8::Isolate*);
> On 2014/06/02 09:01:45, jochen wrote:
> > this should probably be in a WebCrypto header
> > 
> > also, please add parameter names for not obvious parameters, like int*
> 
> Instead of WebScriptBindings, should it be in a
>  public/web/WebCryptoNormalizeAlgorithm.h
> or
>  public/platform/WebCryptoNormalizeAlgorithm.h
> ?
> 
> Platform.h does not depend on this one.

I'd put it all in WebCryptoAlgorithm.h (the new method and the new enum)

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

On 2014/06/02 10:15:25, jochen wrote:
> On 2014/06/02 10:12:16, pneubeck wrote:
> >
>
https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
> > File public/web/WebScriptBindings.h (right):
> > 
> >
>
https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
> > public/web/WebScriptBindings.h:67: BLINK_EXPORT static WebCryptoAlgorithm
> > normalizeCryptoAlgorithm(v8::Handle<v8::Object>, AlgorithmOperation, int*,
> > WebString*, v8::Isolate*);
> > On 2014/06/02 09:01:45, jochen wrote:
> > > this should probably be in a WebCrypto header
> > > 
> > > also, please add parameter names for not obvious parameters, like int*
> > 
> > Instead of WebScriptBindings, should it be in a
> >  public/web/WebCryptoNormalizeAlgorithm.h
> > or
> >  public/platform/WebCryptoNormalizeAlgorithm.h
> > ?
> > 
> > Platform.h does not depend on this one.
> 
> I'd put it all in WebCryptoAlgorithm.h (the new method and the new enum)

doing that and implementing it in the existing
Source/platform/exported/WebCryptoAlgorithm.cpp
doesn't work with the current DEPS (and linkage):

platform/DEPS has explicitly listed -core, -modules.

/ssd/cr/src/third_party/binutils/Linux_x64/Release/bin/ld: warning: hidden
symbol 'WebCore::Dictionary::isObject() const' in
obj/third_party/WebKit/Source/core/libwebcore_generated.a(obj/third_party/WebKit/Source/core/../bindings/v8/webcore_generated.Dictionary.o)
is referenced by DSO lib/libblink_platform.so

So, it seems it must be part of web/ instead of platform/ .

[pneubeck (no reviews), 2014-06-02 13:54:25]

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
File public/platform/WebCryptoAlgorithmOperation.h (right):

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
public/platform/WebCryptoAlgorithmOperation.h:37: Encrypt,
On 2014/06/02 09:01:45, jochen wrote:
> public enums should start with Web, I'd put Crypto in the name, and the values
> should start with the same name as the enum, e.g.
> WebCryptoAlgorithmOperationEncrypt

Done.

Moved to WebCryptoAlgorithm

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
public/platform/WebCryptoAlgorithmOperation.h:48: // <---- End of list (keep
this up-to-date)
On 2014/06/02 09:01:45, jochen wrote:
> that comment is unnecessary

Done.

https://codereview.chromium.org/295423004/diff/20001/public/platform/WebCrypt...
public/platform/WebCryptoAlgorithmOperation.h:49: LastAlgorithmOperation =
UnwrapKey,
On 2014/06/02 09:01:45, jochen wrote:
> Should be WebCryptoAlgorithmOperationLast

Done.

https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
File public/web/WebScriptBindings.h (right):

https://codereview.chromium.org/295423004/diff/20001/public/web/WebScriptBind...
public/web/WebScriptBindings.h:67: BLINK_EXPORT static WebCryptoAlgorithm
normalizeCryptoAlgorithm(v8::Handle<v8::Object>, AlgorithmOperation, int*,
WebString*, v8::Isolate*);
On 2014/06/02 10:12:16, pneubeck wrote:
> On 2014/06/02 09:01:45, jochen wrote:
> > this should probably be in a WebCrypto header
> > 
> > also, please add parameter names for not obvious parameters, like int*
> 
> Instead of WebScriptBindings, should it be in a
>  public/web/WebCryptoNormalizeAlgorithm.h
> or
>  public/platform/WebCryptoNormalizeAlgorithm.h
> ?
> 
> Platform.h does not depend on this one.

Done.
Moved to public/web/WebCryptoNormalize.h

[jochen (gone - plz use gerrit), 2014-06-04 14:41:03]

Adam/Eric

I find it kinda odd to have half of the WebCrypto stuff in platform and the
other half in web.

Since for bindings stuff, we need to be in web/ what about moving everything
there?

[eroman, 2014-06-05 23:12:06]

LGTM.

Regarding web vs platform, I am not familiar enough to comment on where that
should go.

This changelist is just adding a single method into web so I think the
abstraction is OK.

I believe the rest of the webcrypto implementation does need to be in platform/
since it requires the implementation to be fulfilled by the chromium side.

[jochen (gone - plz use gerrit), 2014-06-06 06:58:23]

ok

lgtm with nit addressed

https://codereview.chromium.org/295423004/diff/70001/public/web/WebCryptoNorm...
File public/web/WebCryptoNormalize.h (right):

https://codereview.chromium.org/295423004/diff/70001/public/web/WebCryptoNorm...
public/web/WebCryptoNormalize.h:36: #include
"public/platform/WebCryptoAlgorithm.h"
../platform

https://codereview.chromium.org/295423004/diff/70001/public/web/WebCryptoNorm...
public/web/WebCryptoNormalize.h:56: BLINK_PLATFORM_EXPORT WebCryptoAlgorithm
normalizeCryptoAlgorithm(v8::Handle<v8::Object>, WebCryptoOperation, int*
exceptionCode, WebString* errorDetails, v8::Isolate*);
should be BLINK_EXPORT

[pneubeck (no reviews), 2014-06-06 09:17:09]

https://codereview.chromium.org/295423004/diff/70001/public/web/WebCryptoNorm...
File public/web/WebCryptoNormalize.h (right):

https://codereview.chromium.org/295423004/diff/70001/public/web/WebCryptoNorm...
public/web/WebCryptoNormalize.h:36: #include
"public/platform/WebCryptoAlgorithm.h"
On 2014/06/06 06:58:22, jochen traveling until Jun 23 wrote:
> ../platform

Done.

https://codereview.chromium.org/295423004/diff/70001/public/web/WebCryptoNorm...
public/web/WebCryptoNormalize.h:56: BLINK_PLATFORM_EXPORT WebCryptoAlgorithm
normalizeCryptoAlgorithm(v8::Handle<v8::Object>, WebCryptoOperation, int*
exceptionCode, WebString* errorDetails, v8::Isolate*);
On 2014/06/06 06:58:23, jochen traveling until Jun 23 wrote:
> should be BLINK_EXPORT

Done.

[pneubeck (no reviews), 2014-06-06 09:17:37]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-06-06 09:17:47]

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

[commit-bot: I haz the power, 2014-06-06 10:22:29]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10959)

[commit-bot: I haz the power, 2014-06-06 11:10:04]

Change committed as 175657

[abarth-chromium, 2014-06-06 17:12:48]

> This exposes the WebCore::parseAlgorithm function through
blink::WebScriptBindings

^^^ This part of the description doesn't match the final version of the CL that
landed.

https://codereview.chromium.org/295423004/diff/90001/public/web/WebCryptoNorm...
File public/web/WebCryptoNormalize.h (right):

https://codereview.chromium.org/295423004/diff/90001/public/web/WebCryptoNorm...
public/web/WebCryptoNormalize.h:48: // Converts a javascript Dictionary to a
WebCryptoAlgorithm object.
javascript -> JavaScript