Bug 123647 - Add WebCrypto AES-CBC
Summary: Add WebCrypto AES-CBC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks: 122679
  Show dependency treegraph
 
Reported: 2013-11-01 16:12 PDT by Alexey Proskuryakov
Modified: 2013-11-01 23:57 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch (85.80 KB, patch)
2013-11-01 16:29 PDT, Alexey Proskuryakov
andersca: review+
Details | Formatted Diff | Diff
patch for landing (86.27 KB, patch)
2013-11-01 16:57 PDT, Alexey Proskuryakov
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch for landing (86.28 KB, patch)
2013-11-01 17:03 PDT, Alexey Proskuryakov
commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch for landing (88.34 KB, patch)
2013-11-01 23:09 PDT, Alexey Proskuryakov
commit-queue: commit-queue-
Details | Formatted Diff | Diff
maybe now? (86.60 KB, patch)
2013-11-01 23:25 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2013-11-01 16:12:05 PDT
Implement some functions from AES-CBC.
Comment 1 Alexey Proskuryakov 2013-11-01 16:29:07 PDT
Created attachment 215776 [details]
proposed patch
Comment 2 WebKit Commit Bot 2013-11-01 16:31:45 PDT
Attachment 215776 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/crypto/subtle/aes-cbc-192-encrypt-decrypt-expected.txt', u'LayoutTests/crypto/subtle/aes-cbc-192-encrypt-decrypt.html', u'LayoutTests/crypto/subtle/aes-cbc-256-encrypt-decrypt-expected.txt', u'LayoutTests/crypto/subtle/aes-cbc-256-encrypt-decrypt.html', u'LayoutTests/crypto/subtle/aes-cbc-encrypt-decrypt-expected.txt', u'LayoutTests/crypto/subtle/aes-cbc-encrypt-decrypt-with-padding-expected.txt', u'LayoutTests/crypto/subtle/aes-cbc-encrypt-decrypt-with-padding.html', u'LayoutTests/crypto/subtle/aes-cbc-encrypt-decrypt.html', u'LayoutTests/crypto/subtle/aes-cbc-invalid-length-expected.txt', u'LayoutTests/crypto/subtle/aes-cbc-invalid-length.html', u'LayoutTests/crypto/subtle/aes-cbc-wrong-key-class-expected.txt', u'LayoutTests/crypto/subtle/aes-cbc-wrong-key-class.html', u'LayoutTests/crypto/subtle/hmac-sign-verify-expected.txt', u'LayoutTests/crypto/subtle/hmac-sign-verify.html', u'LayoutTests/crypto/subtle/resources/common.js', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Vector.h', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp', u'Source/WebCore/bindings/js/JSCryptoOperationData.cpp', u'Source/WebCore/bindings/js/JSCryptoOperationData.h', u'Source/WebCore/bindings/js/JSDOMPromise.h', u'Source/WebCore/bindings/js/JSSubtleCryptoCustom.cpp', u'Source/WebCore/crypto/CryptoAlgorithmAesCbcParams.h', u'Source/WebCore/crypto/CryptoKey.h', u'Source/WebCore/crypto/CryptoKeyAES.cpp', u'Source/WebCore/crypto/CryptoKeyAES.h', u'Source/WebCore/crypto/SubtleCrypto.idl', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp', u'Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.h', u'Source/WebCore/crypto/keys/CryptoKeyHMAC.h', u'Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp', u'Source/WebCore/crypto/mac/CryptoAlgorithmHMACMac.cpp', u'Source/WebCore/crypto/mac/CryptoAlgorithmRegistryMac.cpp']" exit_code: 1
Source/WebCore/bindings/js/JSCryptoOperationData.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:38:  CryptoAlgorithmAES_CBC::s_name is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:48:  CryptoAlgorithmAES_CBC::create is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:53:  CryptoAlgorithmAES_CBC::identifier is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:58:  CryptoAlgorithmAES_CBC::importKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/algorithms/CryptoAlgorithmAES_CBC.cpp:70:  CryptoAlgorithmAES_CBC::exportKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp:142:  Missing space after ,  [whitespace/comma] [3]
Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:39:  transformAES_CBC is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:87:  CryptoAlgorithmAES_CBC::encrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:100:  CryptoAlgorithmAES_CBC::decrypt is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/mac/CryptoAlgorithmAES_CBCMac.cpp:113:  CryptoAlgorithmAES_CBC::generateKey is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WebCore/crypto/CryptoKey.h:43:  This { should be at the end of the previous line  [whitespace/braces] [4]
Source/WebCore/crypto/CryptoKey.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/crypto/CryptoKey.h:45:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 14 in 36 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Anders Carlsson 2013-11-01 16:37:36 PDT
Comment on attachment 215776 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=215776&action=review

>> Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp:142
>> +    memcpy(result->iv,ivData.first, ivData.second);
> 
> Missing space after ,  [whitespace/comma] [3]

Missing space.

> Source/WebCore/bindings/js/JSCryptoAlgorithmDictionary.cpp:144
> +    return std::move(result);

Shouldn’t need to do std::move here.

> Source/WebCore/crypto/CryptoAlgorithmAesCbcParams.h:39
> +    char iv[16];

I’d just use a WTF FixedArray here.
Comment 4 Alexey Proskuryakov 2013-11-01 16:48:59 PDT
> Shouldn’t need to do std::move here.

It appears to be necessary when the type is not the same. Local variable is std::unique_ptr<CryptoAlgorithmAesCbcParams>, and the return type is std::unique_ptr<CryptoAlgorithmParameters>.

> I’d just use a WTF FixedArray here.

Neat, didn't know about this one.
Comment 5 Alexey Proskuryakov 2013-11-01 16:57:13 PDT
Created attachment 215779 [details]
patch for landing

Added a const version of FixedArray::data() with an rs=Anders on IRC.
Comment 6 WebKit Commit Bot 2013-11-01 16:58:17 PDT
Comment on attachment 215779 [details]
patch for landing

Rejecting attachment 215779 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 215779, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Anders Carlson found in /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.appspot.com/results/19558118
Comment 7 Alexey Proskuryakov 2013-11-01 17:03:33 PDT
Created attachment 215780 [details]
patch for landing
Comment 8 WebKit Commit Bot 2013-11-01 17:46:15 PDT
Comment on attachment 215780 [details]
patch for landing

Rejecting attachment 215780 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
eUnits.dia -c /Volumes/Data/EWS/WebKit/Source/WebCore/editing/VisibleUnits.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/VisibleUnits.o


** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/CryptoAlgorithmAES_CBCMac.o crypto/mac/CryptoAlgorithmAES_CBCMac.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: http://webkit-queues.appspot.com/results/19588153
Comment 9 Alexey Proskuryakov 2013-11-01 23:09:15 PDT
Created attachment 215796 [details]
patch for landing

With a Mountain Lion build fix, and an attribution for a minor part of layout tests.
Comment 10 WebKit Commit Bot 2013-11-01 23:10:20 PDT
Comment on attachment 215796 [details]
patch for landing

Rejecting attachment 215796 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 215796, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/19628186
Comment 11 Alexey Proskuryakov 2013-11-01 23:25:01 PDT
Created attachment 215798 [details]
maybe now?
Comment 12 WebKit Commit Bot 2013-11-01 23:57:51 PDT
Comment on attachment 215798 [details]
maybe now?

Clearing flags on attachment: 215798

Committed r158485: <http://trac.webkit.org/changeset/158485>
Comment 13 WebKit Commit Bot 2013-11-01 23:57:54 PDT
All reviewed patches have been landed.  Closing bug.