Add support for AES-CTR as requested by Pandora Media: https://www.w3.org/TR/WebCryptoAPI/#aes-ctr
<rdar://problem/31331321>
Created attachment 306032 [details] Patch
Created attachment 306137 [details] Patch
Created attachment 306139 [details] Patch
(In reply to Jiewen Tan from comment #4) > Created attachment 306139 [details] > Patch This patch is large because of rebaselines of w3c tests.
Attachment 306139 [details] did not pass style-queue: ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:43: usagesAreInvalidForCryptoAlgorithmAES_CTR is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:57: CryptoAlgorithmAES_CTR::create is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:62: CryptoAlgorithmAES_CTR::identifier is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:67: CryptoAlgorithmAES_CTR::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:78: CryptoAlgorithmAES_CTR::decrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:89: CryptoAlgorithmAES_CTR::generateKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:107: CryptoAlgorithmAES_CTR::importKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:147: CryptoAlgorithmAES_CTR::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:187: CryptoAlgorithmAES_CTR::getKeyLength is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gcrypt/CryptoAlgorithmAES_CTRGCrypt.cpp:36: CryptoAlgorithmAES_CTR::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gcrypt/CryptoAlgorithmAES_CTRGCrypt.cpp:41: CryptoAlgorithmAES_CTR::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:50: transformAES_CTR is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:144: CryptoAlgorithmAES_CTR::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:168: CryptoAlgorithmAES_CTR::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 14 in 80 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 306148 [details] Patch
Attachment 306148 [details] did not pass style-queue: ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:43: usagesAreInvalidForCryptoAlgorithmAES_CTR is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:57: CryptoAlgorithmAES_CTR::create is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:62: CryptoAlgorithmAES_CTR::identifier is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:67: CryptoAlgorithmAES_CTR::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:78: CryptoAlgorithmAES_CTR::decrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:89: CryptoAlgorithmAES_CTR::generateKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:107: CryptoAlgorithmAES_CTR::importKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:147: CryptoAlgorithmAES_CTR::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:187: CryptoAlgorithmAES_CTR::getKeyLength is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gcrypt/CryptoAlgorithmAES_CTRGCrypt.cpp:36: CryptoAlgorithmAES_CTR::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gcrypt/CryptoAlgorithmAES_CTRGCrypt.cpp:41: CryptoAlgorithmAES_CTR::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:50: transformAES_CTR is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:144: CryptoAlgorithmAES_CTR::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:168: CryptoAlgorithmAES_CTR::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 14 in 80 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 306148 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306148&action=review I think this looks good, but I'd like you to clean up a few things before we land. > Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:48 > +static bool parametersAreValid(CryptoAlgorithmAesCtrParams& paramters) paramters -> parameters (missing an 'e'). > Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:40 > +static size_t bigIntegerToSize(const Vector<uint8_t>& bigInteger) I think you should call this bigIntegerToSizeT. Also, do none of the "clampTo" > Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:55 > + // by the counterLength. It then increments the nonce which should stay same for the whole operatoin. To remedy this issue, operatoin -> operation. > Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:66 > + // Set Nonce to 1s This comment makes it sound like you are setting the Nonce to some set of 1s: "111111111111". But it's not clear what you're doing here. > Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:107 > + Vector<uint8_t> resetedCounter(counter.size()); reseted is a weird word. I suggest "remainingCounter" or something like that. > Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:109 > + size_t nonceOffset = counter.size() - counterLength / 8 - !!counterOffset; !!counterOffset is overly clever. Is that to give you either 0 or 1? > Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:135 > + ASSERT(p <= tail.end()); ASSERT_WITH_SECURITY_IMPLICATION > LayoutTests/imported/w3c/web-platform-tests/WebCryptoAPI/derive_bits_keys/pbkdf2.worker-expected.txt:15 > +PASS Derived key of type name: AES-CTR length: 128 using short password, short salt, SHA-384, with 1 iterations Yay! More passes!
Comment on attachment 306148 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306148&action=review Thanks Brent for reviewing my patch. >> Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:48 >> +static bool parametersAreValid(CryptoAlgorithmAesCtrParams& paramters) > > paramters -> parameters (missing an 'e'). Fixed. >> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:40 >> +static size_t bigIntegerToSize(const Vector<uint8_t>& bigInteger) > > I think you should call this bigIntegerToSizeT. > > Also, do none of the "clampTo" Fixed. >> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:55 >> + // by the counterLength. It then increments the nonce which should stay same for the whole operatoin. To remedy this issue, > > operatoin -> operation. Fixed. > Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:63 > + size_t rightMost = bigIntegerToSize(counter); Comment added: // convert right most __WORDSIZE bits into a size_t value, which is the longest counter we could possibly use. >> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:66 >> + // Set Nonce to 1s > > This comment makes it sound like you are setting the Nonce to some set of 1s: "111111111111". But it's not clear what you're doing here. The comment is updated to: // Used to set nonce in rightMost(nonce + counter) to 1s >> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:107 >> + Vector<uint8_t> resetedCounter(counter.size()); > > reseted is a weird word. I suggest "remainingCounter" or something like that. Fixed. >> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:109 >> + size_t nonceOffset = counter.size() - counterLength / 8 - !!counterOffset; > > !!counterOffset is overly clever. Is that to give you either 0 or 1? Yes, that's my purpose. Any other suggestions? >> Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:135 >> + ASSERT(p <= tail.end()); > > ASSERT_WITH_SECURITY_IMPLICATION Fixed.
Created attachment 306349 [details] Patch
Attachment 306349 [details] did not pass style-queue: ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:43: usagesAreInvalidForCryptoAlgorithmAES_CTR is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:57: CryptoAlgorithmAES_CTR::create is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:62: CryptoAlgorithmAES_CTR::identifier is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:67: CryptoAlgorithmAES_CTR::encrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:78: CryptoAlgorithmAES_CTR::decrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:89: CryptoAlgorithmAES_CTR::generateKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:107: CryptoAlgorithmAES_CTR::importKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:147: CryptoAlgorithmAES_CTR::exportKey is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CTR.cpp:187: CryptoAlgorithmAES_CTR::getKeyLength is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gcrypt/CryptoAlgorithmAES_CTRGCrypt.cpp:36: CryptoAlgorithmAES_CTR::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/gcrypt/CryptoAlgorithmAES_CTRGCrypt.cpp:41: CryptoAlgorithmAES_CTR::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:50: transformAES_CTR is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:143: CryptoAlgorithmAES_CTR::platformEncrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] ERROR: Source/WebCore/crypto/mac/CryptoAlgorithmAES_CTRMac.cpp:167: CryptoAlgorithmAES_CTR::platformDecrypt is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 14 in 80 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 306349 [details] Patch Looks good! R=me
Comment on attachment 306349 [details] Patch Thanks Brent for r+ my patch.
Comment on attachment 306349 [details] Patch Rejecting attachment 306349 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 306349, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: .webkit.org/git/WebKit 42dd4f7..c5190fd master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 215043 = 42dd4f7308880730ce7f117bd47c606298384771 r215044 = c5190fdc1fead1177c5b2e15c9e53254eeb0ff22 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/3485692
Committed r215051: <http://trac.webkit.org/changeset/215051>
(In reply to Jiewen Tan from comment #16) > Committed r215051: <http://trac.webkit.org/changeset/215051> And the Apple Mac cmake buildfix landed in https://trac.webkit.org/changeset/215142/webkit