Summary: | [WebCrypto] Support SPKI/PKCS8 for Elliptic Curve | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jiewen Tan <jiewen_tan> | ||||||
Component: | WebCore Misc. | Assignee: | Jiewen Tan <jiewen_tan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, jiewen_tan, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 166746 | ||||||||
Attachments: |
|
Description
Jiewen Tan
2017-03-07 16:04:24 PST
Created attachment 304603 [details]
Patch
Comment on attachment 304603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304603&action=review Looks good! I had a few minor comments before you land. R=me. > Source/WebCore/crypto/mac/CommonCryptoDERUtilities.h:43 > + if (octet < 128) I think these '128' magic number values should be represented as a constant. > Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:96 > + for (size_t i = 0; i < length; i++) { I'm surprised we don't already have an efficient compare you can use for this. Wouldn't memcmp do what you want here? > Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:272 > + Vector<uint8_t> result; Suggest you hint to Vector how large it will be to avoid reallocations. You seem to have the sizing information already here. (Vector::reserve) > Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:273 > + result.append(SequenceMark); I meant reserveCapacity > Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:377 > + Vector<uint8_t> result; ReserveCapacity > LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/import_export/ec_importKey.worker-expected.txt:56 > +PASS Good parameters: P-384 bits (pkcs8, buffer(185), {name: ECDH, namedCurve: P-384}, false, [deriveBits]) Hooray! So many passes! Comment on attachment 304603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304603&action=review Thanks Brent for r+ my patch. >> Source/WebCore/crypto/mac/CommonCryptoDERUtilities.h:43 >> + if (octet < 128) > > I think these '128' magic number values should be represented as a constant. Fixed. >> Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:96 >> + for (size_t i = 0; i < length; i++) { > > I'm surprised we don't already have an efficient compare you can use for this. Wouldn't memcmp do what you want here? Oops. Always forget we have memcmp. >> Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:272 >> + Vector<uint8_t> result; > > Suggest you hint to Vector how large it will be to avoid reallocations. You seem to have the sizing information already here. (Vector::reserve) Fixed. I pass the expected volume to the constructor directly. Comment on attachment 304603 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=304603&action=review >> Source/WebCore/crypto/mac/CryptoKeyECMac.cpp:273 >> + result.append(SequenceMark); > > I meant reserveCapacity Yup. Fixed it. Created attachment 304677 [details]
Patch for landing
Comment on attachment 304677 [details] Patch for landing Clearing flags on attachment: 304677 Committed r214074: <http://trac.webkit.org/changeset/214074> |