Switch to BoringSSL.

Switch to BoringSSL.

(This is a much larger change than its diff suggests. If it breaks something, please revert first and ask questions later.)

R=davidben@chromium.org, eroman@chromium.org, rsleevi@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283542

[agl, 2014-07-15 18:18:57]

davidben: I can't test the keystore_openssl.cc code so there are surely bugs in
there.

[eroman, 2014-07-15 20:20:45]

https://codereview.chromium.org/345063006/diff/80001/content/child/webcrypto/...
File content/child/webcrypto/platform_crypto_openssl.cc (right):

https://codereview.chromium.org/345063006/diff/80001/content/child/webcrypto/...
content/child/webcrypto/platform_crypto_openssl.cc:56: // OpenSSL supports AES
CBC ciphers for only 2 key lengths: 128, 256 bits
LGTM for webcrypto changes!

For the record, removal of AES 192 support here does not impact anything, as
this code was not reachable.

(AES-192 key import and generation has already been turned off at a higher
layer:
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/webc...)

[davidben, 2014-07-15 21:53:35]

android_dbg_triggered_tests seems to be unhappy.

https://codereview.chromium.org/345063006/diff/80001/net/android/keystore_ope...
File net/android/keystore_openssl.cc (right):

https://codereview.chromium.org/345063006/diff/80001/net/android/keystore_ope...
net/android/keystore_openssl.cc:573: EVP_PKEY_assign_DSA(pkey, dsa.release());
This function will currently always fail, leaking a DSA and leaving the EVP_PKEY
uninitialized. Probably should either check the return value (maybe using
EVP_PKEY_set1_DSA(pkey, dsa.get()) to avoid the really awkward business where
EVP_PKEY_assign_* takes ownership of the parameter, but only if it succeeds) or
not call this code anywhere.

https://codereview.chromium.org/345063006/diff/80001/net/android/keystore_ope...
net/android/keystore_openssl.cc:595: return order.size();
The size hook is, as documented, supposed to return the maximum size of the
signature. I think this is why some of the ECDSA AndroidKeyStore tests are
failing. (Perhaps the hook should return this and BoringSSL internally convert
that to maximum signature size? Dunno. Otherwise we have to repeat DER size
calculations for each platform.)

https://codereview.chromium.org/345063006/diff/80001/net/http/des.cc
File net/http/des.cc (right):

https://codereview.chromium.org/345063006/diff/80001/net/http/des.cc#newcode99
net/http/des.cc:99:
DES_ecb_encrypt(reinterpret_cast<DES_cblock*>(const_cast<uint8*>(src)),
const DES_cblock*?

[Ryan Sleevi, 2014-07-16 01:06:57]

All else LGTM, but I defer to David for the keystore bits, which I did not
review.

https://codereview.chromium.org/345063006/diff/140001/net/cert/x509_certifica...
File net/cert/x509_certificate_openssl.cc (right):

https://codereview.chromium.org/345063006/diff/140001/net/cert/x509_certifica...
net/cert/x509_certificate_openssl.cc:49: if (PKCS7_get_certificates(certs,
&der_data)) {
Sort of a weird API (output variable before input variable? Or at least before
input/output variable?), but I guess this is OpenSSL-world?

[agl, 2014-07-16 01:15:48]

https://codereview.chromium.org/345063006/diff/140001/net/cert/x509_certifica...
File net/cert/x509_certificate_openssl.cc (right):

https://codereview.chromium.org/345063006/diff/140001/net/cert/x509_certifica...
net/cert/x509_certificate_openssl.cc:49: if (PKCS7_get_certificates(certs,
&der_data)) {
On 2014/07/16 01:06:57, Ryan Sleevi wrote:
> Sort of a weird API (output variable before input variable? Or at least before
> input/output variable?), but I guess this is OpenSSL-world?

This is C world :) (See memcpy, strcpy etc. Destination args come first.)
BoringSSL tries to stick to context args followed by outputs followed by inputs.

[davidben, 2014-07-16 21:58:54]

LGTM

https://codereview.chromium.org/345063006/diff/180001/net/android/keystore_op...
File net/android/keystore_openssl.cc (right):

https://codereview.chromium.org/345063006/diff/180001/net/android/keystore_op...
net/android/keystore_openssl.cc:9: #include <openssl/bytestring.h>
Nit: Is this include being used?

https://codereview.chromium.org/345063006/diff/180001/net/http/des.cc
File net/http/des.cc (right):

https://codereview.chromium.org/345063006/diff/180001/net/http/des.cc#newcode99
net/http/des.cc:99:
DES_ecb_encrypt(reinterpret_cast<DES_cblock*>(const_cast<uint8*>(src)),
Why not
  reinterpret_cast<const DES_cblock*>(src)?

It looks like the first argument is const-correct anyway.

[agl, 2014-07-16 22:08:02]

https://codereview.chromium.org/345063006/diff/180001/net/android/keystore_op...
File net/android/keystore_openssl.cc (right):

https://codereview.chromium.org/345063006/diff/180001/net/android/keystore_op...
net/android/keystore_openssl.cc:9: #include <openssl/bytestring.h>
On 2014/07/16 21:58:54, David Benjamin wrote:
> Nit: Is this include being used?

Good point. Was included to build an ASN.1 signature structure, but then I
changed the callback instead. Removed.

https://codereview.chromium.org/345063006/diff/180001/net/http/des.cc
File net/http/des.cc (right):

https://codereview.chromium.org/345063006/diff/180001/net/http/des.cc#newcode99
net/http/des.cc:99:
DES_ecb_encrypt(reinterpret_cast<DES_cblock*>(const_cast<uint8*>(src)),
On 2014/07/16 21:58:54, David Benjamin wrote:
> Why not
>   reinterpret_cast<const DES_cblock*>(src)?
> 
> It looks like the first argument is const-correct anyway.

Thanks, done.

[agl, 2014-07-16 22:15:39]

Committed patchset #13 manually as r283542.

[kuan, 2014-07-17 00:07:07]

A revert of this CL has been created in
https://codereview.chromium.org/398233002/ by kuan@chromium.org.

The reason for reverting is: it broke "compile,steps" on "Android Webview AOSP
Builder":
http://build.chromium.org/p/chromium.linux/builders/Android%20Webview%20AOSP%....

[kuan, 2014-07-17 00:10:47]

A revert of this CL has been created in
https://codereview.chromium.org/395373002/ by kuan@chromium.org.

The reason for reverting is: it broke "compile,steps" on "Android Webview AOSP
Builder":
http://build.chromium.org/p/chromium.linux/builders/Android%20Webview%20AOSP%....

[sgurun-gerrit only, 2014-07-17 00:30:35]

On 2014/07/17 00:10:47, kuan wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/395373002/ by mailto:kuan@chromium.org.
> 
> The reason for reverting is: it broke "compile,steps" on "Android Webview AOSP
> Builder":
>
http://build.chromium.org/p/chromium.linux/builders/Android%20Webview%20AOSP%....

Since this is a new project that webview now depends on, I think we needed to do
sth in our side to sync the source code. The error also shows:
xport includes file: external/chromium_org/GypAndroid.linux-arm.mk --
out/target/product/generic/obj/STATIC_LIBRARIES/third_party_modp_b64_modp_b64_gyp_intermediates/export_includes
Import includes file:
out/target/product/generic/obj/STATIC_LIBRARIES/crypto_crypto_gyp_intermediates/import_includes
Export includes file: external/chromium_org/GypAndroid.linux-arm.mk --
out/target/product/generic/obj/STATIC_LIBRARIES/crypto_crypto_gyp_intermediates/export_includes
make: *** No rule to make target
`external/chromium_org/third_party/boringssl/src/crypto/chacha/chacha_vec_arm.S',
needed by
`out/target/product/generic/obj/STATIC_LIBRARIES/third_party_boringssl_boringssl_gyp_intermediates/third_party/boringssl/src/crypto/chacha/chacha_vec_arm.o'.
 Stop.
make: *** Waiting for unfinished jobs....
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
warning: [options] bootstrap class path not set in conjunction with -source 1.6
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
1 warning
make: INTERNAL: Exiting with 151 jobserver tokens available; should be 150!

/mnt/data/b/build/goma/goma_ctl.sh stat
request: total=2 success=1 failure=0


looking...

[agl, 2014-07-17 00:36:15]

On 2014/07/17 00:30:35, sgurun wrote:
> looking...

This has been reverted for now. This change has been suggested to fix it for the
next landing attempt: https://codereview.chromium.org/395353002/