- From: Charles Engelke <w3c@engelke.com>
- Date: Fri, 13 May 2016 20:44:48 -0400
- To: Eric Roman <ericroman@google.com>
- Cc: Charles Engelke <w3c@engelke.com>, "public-webcrypto@w3.org" <public-webcrypto@w3.org>
I agree with you completely. It seems to me to make more sense to check for empty usages at the start, when you do the other usages check. That's what Chrome does now. I'd certainly support changing the specification if that's still an option, and then I'd fix the tests to match. Charlie On Fri, May 13, 2016 at 8:10 PM, Eric Roman <ericroman@google.com> wrote: > A similar issue will apply to unwrapKey() -- Chrome verifies creation > parameters before attempting to decrypt, in order to fail-fast if the import > parameters are bogus. > > (So depending on the outcome of this discussion, I may need to change that > behavior as well) > > On Fri, May 13, 2016 at 5:02 PM, Eric Roman <ericroman@google.com> wrote: >> >> >> >> On Fri, May 13, 2016 at 2:22 PM, Charles Engelke <w3c@engelke.com> wrote: >>> >>> Tests are now available for all supported algorithms for generateKey. >>> They run as individual pages and under the test >>> >>> hahttps://developer.microsoft.com/en-us/microsoft-edge/platform/issues/7444732/rness, >>> including in workers. >>> >>> Chrome passes all success tests except for those involving AES with >>> 192-bit keys (which it does not support). Chrome fails some failure >>> tests by reporting the wrong error in some cases where there are two >>> bad parameters. It seems it may be checking them in a different order >>> than in the spec. That has been reported as issue 611801, at >>> >>> https://bugs.chromium.org/p/chromium/issues/detail?id=611801&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified >> >> >> Thanks Charles. >> >> I had previously filed this as a question against the spec: >> https://www.w3.org/Bugs/Public/show_bug.cgi?id=27564 >> There was some commentary there, but no conclusive action (i.e. closing >> the bug). >> >> I still feel as I did then, that the order given by the spec is not a good >> choice of error order. >> >> The spec is asking us to actually generate the RSA key (a very expensive >> operation) before checking that the creation parameters (namely the key >> usages) are valid. >> >> This leads to awkward layering of the code. >> >> Conceptually you want to bind the key usages to the key at generation >> time. Your crypto library in fact may require this. Here we are being asked >> to proceed with generating the key (with undefined usages), only to reject >> it after successful generation. As you can tell from your tests, generating >> these keys is *slow*. >> >> So from a layering perspective, and from a performance perspective (error >> fast when you can), I feel SyntaxError is a better choice. >> >> (Admittedly though it does complicate the wording in the spec, since usage >> tests for secret keys cannot be conveniently written in one location) >> >>> >>> . >>> >>> Firefox passes all success tests. I used Firefox nightly because it >>> has RSA-PSS support, which will apparently be in the next regular >>> release. However, Firefox generation of RSA keys is several times >>> slower than in Chrome, so the tests take 10-30 minutes to run. >>> >>> Firefox fails most failure tests. In one case, Firefox seems to ignore >>> bad usage parameters instead of throwing an error (bug 1270634 at >>> https://bugzilla.mozilla.org/show_bug.cgi?id=1270634). In another case >>> it reports the wrong error when given certain bad parameters (bug >>> 1270599 at https://bugzilla.mozilla.org/show_bug.cgi?id=1270599). >>> There may be others; it takes a long time to run the tests and when >>> there are thousands of failures for just these two root causes it >>> takes a while to check to see if there are others. >>> >>> Edge is far from complete, though it does pass many of the success >>> tests as of now. I have only reported one bug to Microsoft: the >>> CryptoKey object's algorithm name is set to whatever parameter is >>> given to generateKey, instead of adjusted to match the official >>> capitalization. That's issue 7444732 at >>> >>> https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/7444732/ >>> . >>> >>> I haven't tested Safari because it uses a prefixed implementation. >>> >>> I've submitted the tests as a pull request and updated them based on >>> feedback from Jim. I'm not sure of the next steps for having them >>> accepted. >>> >>> Charlie >>> >> >
Received on Saturday, 14 May 2016 00:45:16 UTC