Unship legacy OpenSSL ASN.1 and X.509 (saves about 270k) |
||||
Issue descriptionThis should decrease the binary size significantly. This will likely help Cronet long before it helps the rest of Chrome as Chrome will need it excised from WebRTC. One thing at a time. Filing this so we have some place to staple CLs that don't fit under one of the two dependent CLs. Issue #671420 is the big piece of this, and boringssl:54 covers work we need to tease apart code on the BoringSSL side. (I believe BoringSSL work is now all done except for PKCS7_get_certificates. That and we should reshuffle some things between files in ssl/tls_method.c to make the static linker a little happier.)
,
Mar 29 2017
(Assigning to myself so the bug isn't just sitting unassigned, but the bulk is in the bug it's blocked on. I'll gradually chew away at the other pieces when I'm bored.)
,
Mar 29 2017
Unshipping ASN.1 will have a nice binary saving for Cronet. Cronet's continuous buildbots report binary size to the perf dashboard. We should know how much this saves across different architectures once the CL lands. Thanks, David!
,
Mar 29 2017
> the CL Many many CLs. :-) Mostly the other bug. We did a quick test earlier (got a build of BoringSSL's command-line tool without the library linked in) and it looked like something like 200k of binary dropped? We'll see what it actually ends up at.
,
Mar 29 2017
(Making a note before I forget: ev_root_cert_metadata.cc also needs a OBJ_txt2obj replacement on macOS to convert from "1.2.3.4.5.6" to serialized OID.)
,
Apr 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8adc35a73c0c9981d9d26e34de07e0d0600dcbd commit b8adc35a73c0c9981d9d26e34de07e0d0600dcbd Author: davidben <davidben@chromium.org> Date: Mon Apr 03 21:10:32 2017 Use new APIs for parsing encrypted ECPrivateKeys. These APIs don't depend on the giant OID table and are much easier to use. BUG=706445 Review-Url: https://codereview.chromium.org/2781993006 Cr-Commit-Position: refs/heads/master@{#461539} [modify] https://crrev.com/b8adc35a73c0c9981d9d26e34de07e0d0600dcbd/crypto/ec_private_key.cc
,
Apr 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdf74a5e0076644d318f7cac67bf0c6c03c89718 commit bdf74a5e0076644d318f7cac67bf0c6c03c89718 Author: davidben <davidben@chromium.org> Date: Mon Apr 17 17:21:26 2017 Trim some dependencies on crypto/x509 headers. This makes it a bit easier to keep track of what's left. BUG=706445 Review-Url: https://codereview.chromium.org/2824713002 Cr-Commit-Position: refs/heads/master@{#464939} [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/chrome/browser/extensions/api/networking_private/networking_private_crypto.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/cert/cert_verify_proc_android.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/cert/internal/signature_policy.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/cert/internal/verify_signed_data_unittest.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/cert/x509_certificate_ios.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/quic/test_tools/crypto_test_utils.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/socket/ssl_client_socket_impl.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/ssl/openssl_ssl_util.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/ssl/openssl_ssl_util.h [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/ssl/ssl_client_session_cache.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/ssl/ssl_platform_key_win.cc [modify] https://crrev.com/bdf74a5e0076644d318f7cac67bf0c6c03c89718/net/tools/quic/test_tools/quic_test_client.cc
,
Apr 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cf9295d43d7aee7b837e17b185e9cf80958f81e commit 9cf9295d43d7aee7b837e17b185e9cf80958f81e Author: davidben <davidben@chromium.org> Date: Wed Apr 19 21:34:01 2017 Use PKCS7_get_raw_certificates in x509_certificate_bytes.cc. This removes a dependency on the legacy X.509 stack. BUG=706445 Review-Url: https://codereview.chromium.org/2831743002 Cr-Commit-Position: refs/heads/master@{#465760} [modify] https://crrev.com/9cf9295d43d7aee7b837e17b185e9cf80958f81e/net/cert/x509_certificate_bytes.cc
,
Apr 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9587eaf061cc8429228e1e1c5e494e8bf983fc9f commit 9587eaf061cc8429228e1e1c5e494e8bf983fc9f Author: davidben <davidben@chromium.org> Date: Mon Apr 24 20:26:05 2017 Decouple ct_objects_extractor.cc without legacy ASN.1 code BUG=706445 Review-Url: https://codereview.chromium.org/2816363002 Cr-Commit-Position: refs/heads/master@{#466739} [modify] https://crrev.com/9587eaf061cc8429228e1e1c5e494e8bf983fc9f/net/cert/ct_objects_extractor.cc [modify] https://crrev.com/9587eaf061cc8429228e1e1c5e494e8bf983fc9f/net/cert/ct_objects_extractor_unittest.cc
,
May 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/965721b2c4c2701a64740b0187824ab09b11954e commit 965721b2c4c2701a64740b0187824ab09b11954e Author: davidben <davidben@chromium.org> Date: Wed May 03 19:40:31 2017 Remove CertVerifyProcOpenSSL. This has never been shipped in anything to my knowledge, is unmaintained, unsupported, and I'm not sure it's even still compiled. Remove it before someone tries to use it. (The only non-Android use_openssl_certs port is the NaCl one which does not use these files.) BUG=706445 Review-Url: https://codereview.chromium.org/2862543003 Cr-Commit-Position: refs/heads/master@{#469080} [modify] https://crrev.com/965721b2c4c2701a64740b0187824ab09b11954e/net/BUILD.gn [modify] https://crrev.com/965721b2c4c2701a64740b0187824ab09b11954e/net/cert/cert_verify_proc.cc [delete] https://crrev.com/a86f3e33aeeb1e7d0d98319c8000ebd0a20bcabf/net/cert/cert_verify_proc_openssl.cc [delete] https://crrev.com/a86f3e33aeeb1e7d0d98319c8000ebd0a20bcabf/net/cert/cert_verify_proc_openssl.h [modify] https://crrev.com/965721b2c4c2701a64740b0187824ab09b11954e/net/cert/cert_verify_proc_unittest.cc [modify] https://crrev.com/965721b2c4c2701a64740b0187824ab09b11954e/net/cert/test_root_certs.h [delete] https://crrev.com/a86f3e33aeeb1e7d0d98319c8000ebd0a20bcabf/net/cert/test_root_certs_openssl.cc [modify] https://crrev.com/965721b2c4c2701a64740b0187824ab09b11954e/net/cert/test_root_certs_unittest.cc [modify] https://crrev.com/965721b2c4c2701a64740b0187824ab09b11954e/net/url_request/url_request_unittest.cc
,
May 18 2017
Tagging with Performance-Size. This saved us about 270k on Cronet. (Chrome proper will take a lot more work though, due to WebRTC.)
,
Jul 6 2017
(Looks like the Performance-Size bugs like to have little size summaries in their titles.)
,
Aug 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8ab3858206a75fc26df5a76b05239b8c5db4d31 commit b8ab3858206a75fc26df5a76b05239b8c5db4d31 Author: David Benjamin <davidben@chromium.org> Date: Fri Aug 04 00:17:32 2017 Switch SSLServerSocket to CRYPTO_BUFFER. This also allows us to support asynchronous client certificate verification, but this CL leaves it as a TODO for now. Bug: 706445 Change-Id: I792eb91a854bb15a67317d7ea4d04a80ba5ca4da Reviewed-on: https://chromium-review.googlesource.com/586431 Reviewed-by: Steven Valdez <svaldez@chromium.org> Reviewed-by: Matt Mueller <mattm@chromium.org> Commit-Queue: David Benjamin <davidben@chromium.org> Cr-Commit-Position: refs/heads/master@{#491886} [modify] https://crrev.com/b8ab3858206a75fc26df5a76b05239b8c5db4d31/net/cert/x509_util.h [modify] https://crrev.com/b8ab3858206a75fc26df5a76b05239b8c5db4d31/net/cert/x509_util_openssl.cc [modify] https://crrev.com/b8ab3858206a75fc26df5a76b05239b8c5db4d31/net/socket/ssl_client_socket_impl.cc [modify] https://crrev.com/b8ab3858206a75fc26df5a76b05239b8c5db4d31/net/socket/ssl_server_socket_impl.cc [modify] https://crrev.com/b8ab3858206a75fc26df5a76b05239b8c5db4d31/net/socket/ssl_server_socket_impl.h [modify] https://crrev.com/b8ab3858206a75fc26df5a76b05239b8c5db4d31/net/ssl/openssl_ssl_util.cc [modify] https://crrev.com/b8ab3858206a75fc26df5a76b05239b8c5db4d31/net/ssl/openssl_ssl_util.h
,
Aug 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f349dc4bf8d1c4f56a54722ceb116103f28350c3 commit f349dc4bf8d1c4f56a54722ceb116103f28350c3 Author: David Benjamin <davidben@chromium.org> Date: Fri Aug 04 19:17:00 2017 Partially rearrange x509_util* organization. x509_util_openssl.cc was historically both implementations of things in x509_util_openssl.h and things in x509_util.h that depended on OpenSSL. The latter distinction is no longer useful. This moves some things over to x509_util.cc. CreateSelfSignedCert is still in the old file because we'd otherwise need to duplicate or export DerEncodeCert. Later work to decouple it from X509* will allow us to complete the move, and then x509_util_openssl.h will be one of the files removed when use_byte_certs is everywhere. Bug: 706445 Change-Id: I1ea1e3d80fdde397932aa9065901aabe23ad977a Reviewed-on: https://chromium-review.googlesource.com/601093 Reviewed-by: Matt Mueller <mattm@chromium.org> Commit-Queue: David Benjamin <davidben@chromium.org> Cr-Commit-Position: refs/heads/master@{#492096} [modify] https://crrev.com/f349dc4bf8d1c4f56a54722ceb116103f28350c3/net/cert/x509_util.cc [modify] https://crrev.com/f349dc4bf8d1c4f56a54722ceb116103f28350c3/net/cert/x509_util_openssl.cc
,
Aug 7 2017
The following revision refers to this bug: https://boringssl.googlesource.com/boringssl/+/ba2d3df75981449c56e8cc276b2a56319483fc4a commit ba2d3df75981449c56e8cc276b2a56319483fc4a Author: David Benjamin <davidben@google.com> Date: Mon Aug 07 21:01:25 2017 Add DTLS_with_buffers_method. WebRTC will need this (probably among other things) to lose crypto/x509 at some point. Bug: chromium:706445 Change-Id: I988e7300c4d913986b6ebbd1fa4130548dde76a4 Reviewed-on: https://boringssl-review.googlesource.com/18904 Reviewed-by: David Benjamin <davidben@google.com> [modify] https://crrev.com/ba2d3df75981449c56e8cc276b2a56319483fc4a/ssl/dtls_method.cc [modify] https://crrev.com/ba2d3df75981449c56e8cc276b2a56319483fc4a/ssl/tls_method.cc [modify] https://crrev.com/ba2d3df75981449c56e8cc276b2a56319483fc4a/ssl/internal.h [modify] https://crrev.com/ba2d3df75981449c56e8cc276b2a56319483fc4a/include/openssl/ssl.h
,
Aug 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bdc94d7d65cd331d75e1a9483af56aba1142431d commit bdc94d7d65cd331d75e1a9483af56aba1142431d Author: David Benjamin <davidben@chromium.org> Date: Mon Aug 07 23:25:21 2017 Implement CreateSelfSignedCert without crypto/x509. This is a fair amount of code that will probably be replaced by shared utilities in BoringSSL (perhaps after the PKI library is moved there) but write it out for now. With this, the only dependency on crypto/x509 in the Chrome binary (there are still dependencies in tests and build tools that only matter if we ever exclude the files together) is WebRTC. Alas, WebRTC is going to be difficult. Bug: 706445 Change-Id: Ic1053abdd5589f2a490684ee26351e635ed59603 Reviewed-on: https://chromium-review.googlesource.com/602772 Reviewed-by: Matt Mueller <mattm@chromium.org> Commit-Queue: David Benjamin <davidben@chromium.org> Cr-Commit-Position: refs/heads/master@{#492440} [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/cert/x509_util.cc [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/cert/x509_util_openssl.cc [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/der/encode_values.cc [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/der/encode_values.h [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/der/encode_values_unittest.cc [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/der/parse_values.cc [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/der/parse_values.h [modify] https://crrev.com/bdc94d7d65cd331d75e1a9483af56aba1142431d/net/der/parse_values_unittest.cc
,
Sep 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/15004212f4cda388c8acf1be5c4b5e3b6715ec42 commit 15004212f4cda388c8acf1be5c4b5e3b6715ec42 Author: Matt Mueller <mattm@chromium.org> Date: Fri Sep 08 04:44:13 2017 Remove x509_certificate_openssl and use_openssl_certs build flag Bug: 671420 , 706445 Change-Id: Ieca79e475c0d4ea025405275d2164451b3f124dd Reviewed-on: https://chromium-review.googlesource.com/656519 Commit-Queue: Matt Mueller <mattm@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: David Benjamin <davidben@chromium.org> Cr-Commit-Position: refs/heads/master@{#500508} [modify] https://crrev.com/15004212f4cda388c8acf1be5c4b5e3b6715ec42/build/build_config.h [modify] https://crrev.com/15004212f4cda388c8acf1be5c4b5e3b6715ec42/build/config/BUILD.gn [modify] https://crrev.com/15004212f4cda388c8acf1be5c4b5e3b6715ec42/build/config/crypto.gni [modify] https://crrev.com/15004212f4cda388c8acf1be5c4b5e3b6715ec42/net/BUILD.gn [rename] https://crrev.com/15004212f4cda388c8acf1be5c4b5e3b6715ec42/net/cert/cert_database_stub.cc [modify] https://crrev.com/15004212f4cda388c8acf1be5c4b5e3b6715ec42/net/cert/x509_certificate.h [delete] https://crrev.com/e976a3897d7a8abbbacce4c2622cc5ecf5a7b067/net/cert/x509_certificate_openssl.cc [delete] https://crrev.com/e976a3897d7a8abbbacce4c2622cc5ecf5a7b067/net/cert/x509_util_openssl.cc [delete] https://crrev.com/e976a3897d7a8abbbacce4c2622cc5ecf5a7b067/net/cert/x509_util_openssl.h [modify] https://crrev.com/15004212f4cda388c8acf1be5c4b5e3b6715ec42/net/socket/ssl_server_socket_impl.cc
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/07f660cc2af71bb22723f971c8f4775f76d2326b commit 07f660cc2af71bb22723f971c8f4775f76d2326b Author: David Benjamin <davidben@chromium.org> Date: Thu Sep 28 18:50:08 2017 Remove unused include. The Fuchsia code is not using the legacy X.509 stack. Bug: 706445 Change-Id: I6c2cc42bc302c01d38067657317aa1b958442270 Reviewed-on: https://chromium-review.googlesource.com/690376 Commit-Queue: David Benjamin <davidben@chromium.org> Commit-Queue: Matt Mueller <mattm@chromium.org> Reviewed-by: Matt Mueller <mattm@chromium.org> Cr-Commit-Position: refs/heads/master@{#505102} [modify] https://crrev.com/07f660cc2af71bb22723f971c8f4775f76d2326b/net/cert/internal/system_trust_store.cc
,
Sep 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/99dada2121f657bf1846130f1006beadc91d51bc commit 99dada2121f657bf1846130f1006beadc91d51bc Author: David Benjamin <davidben@chromium.org> Date: Thu Sep 28 20:04:00 2017 Don't depend on crypto/x509 in ssl_server_socket_unittest. This doesn't particularly matter, but if we ever split the libraries up for enforcement, we'll want the dependencies out. The name of the issuing cert isn't going to change, so just hard-code it in the test. The ClientCertStore tests already do this and I'm not sure this field is used at all anyway. Bug: 706445 Change-Id: I780002ae90c9668507fba64a5c3acf0139c7bfe0 Reviewed-on: https://chromium-review.googlesource.com/690159 Reviewed-by: Matt Mueller <mattm@chromium.org> Commit-Queue: David Benjamin <davidben@chromium.org> Cr-Commit-Position: refs/heads/master@{#505125} [modify] https://crrev.com/99dada2121f657bf1846130f1006beadc91d51bc/net/socket/ssl_server_socket_unittest.cc
,
Nov 22 2017
https://crbug.com/526260#c21 will probably need to be resolved for us to be able to do this.
,
Nov 27 2017
The following revision refers to this bug: https://boringssl.googlesource.com/boringssl/+/47b8f00fdc62372caef30b2f38f6242435a638ee commit 47b8f00fdc62372caef30b2f38f6242435a638ee Author: David Benjamin <davidben@google.com> Date: Mon Nov 27 21:29:00 2017 Reimplement OBJ_txt2obj and add a lower-level function. OBJ_txt2obj is currently implemented using BIGNUMs which is absurd. It also depends on the giant OID table, which is undesirable. Write a new one and expose the low-level function so Chromium can use it without the OID table. Bug: chromium:706445 Change-Id: I61ff750a914194f8776cb8d81ba5d3eb5eaa3c3d Reviewed-on: https://boringssl-review.googlesource.com/23364 Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Reviewed-by: Steven Valdez <svaldez@google.com> [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/include/openssl/obj.h [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/crypto/bytestring/cbb.c [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/include/openssl/asn1.h [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/crypto/obj/obj.c [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/crypto/err/obj.errordata [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/include/openssl/bytestring.h [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/crypto/asn1/a_object.c [modify] https://crrev.com/47b8f00fdc62372caef30b2f38f6242435a638ee/crypto/bytestring/bytestring_test.cc
,
Nov 30 2017
The following revision refers to this bug: https://boringssl.googlesource.com/boringssl/+/095b6c9baa93fecf5e0d8591f3586a666b60e00a commit 095b6c9baa93fecf5e0d8591f3586a666b60e00a Author: David Benjamin <davidben@google.com> Date: Thu Nov 30 18:21:48 2017 Also add a decoupled OBJ_obj2txt. We need it in both directions. Also I missed that in OBJ_obj2txt we allowed uint64_t components, but in my new OBJ_txt2obj we only allowed uint32_t. For consistency, upgrade that to uint64_t. Bug: chromium:706445 Change-Id: I38cfeea8ff64b9acf7998e552727c6c3b2cc600f Reviewed-on: https://boringssl-review.googlesource.com/23544 Commit-Queue: Steven Valdez <svaldez@google.com> Reviewed-by: Steven Valdez <svaldez@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> [modify] https://crrev.com/095b6c9baa93fecf5e0d8591f3586a666b60e00a/crypto/bytestring/bytestring_test.cc [modify] https://crrev.com/095b6c9baa93fecf5e0d8591f3586a666b60e00a/crypto/bytestring/cbb.c [modify] https://crrev.com/095b6c9baa93fecf5e0d8591f3586a666b60e00a/crypto/obj/obj.c [modify] https://crrev.com/095b6c9baa93fecf5e0d8591f3586a666b60e00a/crypto/bytestring/cbs.c [modify] https://crrev.com/095b6c9baa93fecf5e0d8591f3586a666b60e00a/include/openssl/bytestring.h
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eece13443023fc3331e855289edfd134bb114f9c commit eece13443023fc3331e855289edfd134bb114f9c Author: David Benjamin <davidben@chromium.org> Date: Fri Dec 01 23:49:34 2017 Use new BoringSSL functions for handling text representations of OIDs. The new ones don't pull in a dependency on the giant OID table. This should be the last link in all of Chromium aside from WebRTC. (Also that random build tool in //net, but that's not shipped.) Bug: 706445 Change-Id: I06d0c953b829a592afeeb8399e788e8ea8e53af6 Reviewed-on: https://chromium-review.googlesource.com/804119 Commit-Queue: David Benjamin <davidben@chromium.org> Reviewed-by: Matt Mueller <mattm@chromium.org> Cr-Commit-Position: refs/heads/master@{#521128} [modify] https://crrev.com/eece13443023fc3331e855289edfd134bb114f9c/net/cert/ev_root_ca_metadata.cc
,
Jan 21
(2 days ago)
David, we came across this bug when triaging Performance-Size issues. Is there still a point in keeping this open given its age and status? If not, we'd like to archive it, or possibly remove the Perf-Size label. Thoughts?
,
Yesterday
(33 hours ago)
No, it's still worth keeping open. It's blocked on WebRTC work that has yet to be a priority bug should still be done. When done, it should indeed be a size (and security) win. |
||||
►
Sign in to add a comment |
||||
Comment 1 by davidben@chromium.org
, Mar 29 2017Status: Assigned (was: Untriaged)