Switch away from the buggy RSA parser. |
|||
Issue descriptionBoringSSL uses a buggy RSA parser by default, originally due to issue #532048 . This is getting removed in BoringSSL. There is a temporary EVP_set_buggy_rsa_parser function we can use to toggle the parser which we should connect to base::FeatureList.
,
Jun 23 2017
Please check https://bugs.chromium.org/p/chromium/issues/detail?id=736281 . I think this broke Chrome OS.
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e545f6464160070b1b14d61d2ed3e2ef52aff6ed commit e545f6464160070b1b14d61d2ed3e2ef52aff6ed Author: Greg Thompson <grt@chromium.org> Date: Fri Jun 23 14:26:56 2017 Revert "Disable the buggy RSA parser by default." This reverts commit 2878e36f5fea9a7404ffd0ab7b4be83d035d199a. Reason for revert: This is checking for a feature before FeatureList::InitializeInstance is called, causing a crash at startup; see https://crbug.com/736251. Original change's description: > Disable the buggy RSA parser by default. > > In doing so, fix the error mapping in openssl_ssl_util.cc. An SSL > connection may fail due to errors in other modules as well (notably the > RSA parser lives in libcrypto). Map any unknown error to > ERR_SSL_PROTOCOL_ERROR, rather than ERR_FAILED and continue to > report the error info. > > Bug: 735616 > Change-Id: Icb587e66987ddd9d5445d30d456de1c029cda21a > Reviewed-on: https://chromium-review.googlesource.com/540536 > Commit-Queue: Steven Valdez <svaldez@chromium.org> > Reviewed-by: Steven Valdez <svaldez@chromium.org> > Cr-Commit-Position: refs/heads/master@{#481640} TBR=davidben@chromium.org,svaldez@chromium.org Change-Id: If7f24fa8a99bfd7a8daa2efad926b06350d5d9a6 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 735616 Reviewed-on: https://chromium-review.googlesource.com/545715 Reviewed-by: Greg Thompson <grt@chromium.org> Commit-Queue: Greg Thompson <grt@chromium.org> Cr-Commit-Position: refs/heads/master@{#481877} [modify] https://crrev.com/e545f6464160070b1b14d61d2ed3e2ef52aff6ed/crypto/openssl_util.cc [modify] https://crrev.com/e545f6464160070b1b14d61d2ed3e2ef52aff6ed/net/ssl/openssl_ssl_util.cc
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b576541bed6393420b4d2c2919ff0d673efa4622 commit b576541bed6393420b4d2c2919ff0d673efa4622 Author: Greg Thompson <grt@chromium.org> Date: Fri Jun 23 14:38:50 2017 Revert "Disable the buggy RSA parser by default." This reverts commit 2878e36f5fea9a7404ffd0ab7b4be83d035d199a. Reason for revert: This is checking for a feature before FeatureList::InitializeInstance is called, causing a crash at startup; see https://crbug.com/736251. Original change's description: > Disable the buggy RSA parser by default. > > In doing so, fix the error mapping in openssl_ssl_util.cc. An SSL > connection may fail due to errors in other modules as well (notably the > RSA parser lives in libcrypto). Map any unknown error to > ERR_SSL_PROTOCOL_ERROR, rather than ERR_FAILED and continue to > report the error info. > > Bug: 735616 > Change-Id: Icb587e66987ddd9d5445d30d456de1c029cda21a > Reviewed-on: https://chromium-review.googlesource.com/540536 > Commit-Queue: Steven Valdez <svaldez@chromium.org> > Reviewed-by: Steven Valdez <svaldez@chromium.org> > Cr-Commit-Position: refs/heads/master@{#481640} TBR=davidben@chromium.org,svaldez@chromium.org Change-Id: If7f24fa8a99bfd7a8daa2efad926b06350d5d9a6 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 735616 Reviewed-on: https://chromium-review.googlesource.com/545715 Reviewed-by: Greg Thompson <grt@chromium.org> Commit-Queue: Greg Thompson <grt@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#481877} Review-Url: https://codereview.chromium.org/2958563002 . Cr-Commit-Position: refs/branch-heads/3139@{#1} Cr-Branched-From: 2dd0ea84298d45e0962c595c591529bc84dd0ebd-refs/heads/master@{#481757} [modify] https://crrev.com/b576541bed6393420b4d2c2919ff0d673efa4622/crypto/openssl_util.cc [modify] https://crrev.com/b576541bed6393420b4d2c2919ff0d673efa4622/net/ssl/openssl_ssl_util.cc
,
Jun 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b946811caf3fc4bb8f19374c4f0fd395e88147d7 commit b946811caf3fc4bb8f19374c4f0fd395e88147d7 Author: David Benjamin <davidben@chromium.org> Date: Tue Jun 27 16:23:49 2017 Disable the buggy RSA parser by default. In doing so, fix the error mapping in openssl_ssl_util.cc. An SSL connection may fail due to errors in other modules as well (notably the RSA parser lives in libcrypto). Map any unknown error to ERR_SSL_PROTOCOL_ERROR, rather than ERR_FAILED and continue to report the error info. This is a reland of https://chromium-review.googlesource.com/540536, but with different base::FeatureList hooks. Although base::FeatureList is in //base, it is not safe to call in low-level modules because there is no layering guarantee that //crypto isn't called into before startup. (And indeed it is. See https://crbug.com/736251.) Since //content contains the code which initializes base::FeatureList, the most straightforward sequence is to keep base::FeatureList uses in //content. The downside is //content does not have a clear place to initialize features shared by all processes, so we'll pick up just the browser (net stack) and renderer (WebCrypto) processes. Elsewhere and before this point, we'll just pick up the BoringSSL default which is currently to use the buggy parser, but later will switch to the spec-compliant parser. Once this change survives on stable channel, the base::FeatureList toggle is intended to be merely a short-lived escape hatch, in case there are unforeseen problems. See also https://groups.google.com/a/chromium.org/d/msg/security-dev/H10jNknb5Ro/hkh15XHXAQAJ Bug: 735616 Change-Id: I3c53e2028f3ea6f478b1cdff5688428f017b2e52 Reviewed-on: https://chromium-review.googlesource.com/549495 Reviewed-by: Steven Valdez <svaldez@chromium.org> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: David Benjamin <davidben@chromium.org> Cr-Commit-Position: refs/heads/master@{#482645} [modify] https://crrev.com/b946811caf3fc4bb8f19374c4f0fd395e88147d7/content/browser/browser_main_loop.cc [modify] https://crrev.com/b946811caf3fc4bb8f19374c4f0fd395e88147d7/content/public/common/content_features.cc [modify] https://crrev.com/b946811caf3fc4bb8f19374c4f0fd395e88147d7/content/public/common/content_features.h [modify] https://crrev.com/b946811caf3fc4bb8f19374c4f0fd395e88147d7/content/renderer/render_thread_impl.cc [modify] https://crrev.com/b946811caf3fc4bb8f19374c4f0fd395e88147d7/net/ssl/openssl_ssl_util.cc
,
Sep 18 2017
The following revision refers to this bug: https://boringssl.googlesource.com/boringssl/+/246e27d807ed67fd577bf9c24ab337bb831e89f1 commit 246e27d807ed67fd577bf9c24ab337bb831e89f1 Author: David Benjamin <davidben@google.com> Date: Mon Sep 18 19:42:51 2017 Switch the buggy RSA parser off by default. I'll fully remove this once Chrome 62 hits stable, in case any bug reports come in for Chrome 61. Meanwhile switch the default to off so that other consumers pick up the behavior. (Should have done this sooner and forgot.) Bug: chromium:735616 Change-Id: Ib27c4072f228cd3b5cce283accd22732eeef46b2 Reviewed-on: https://boringssl-review.googlesource.com/20484 Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> [modify] https://crrev.com/246e27d807ed67fd577bf9c24ab337bb831e89f1/crypto/evp/evp_tests.txt [modify] https://crrev.com/246e27d807ed67fd577bf9c24ab337bb831e89f1/include/openssl/evp.h [modify] https://crrev.com/246e27d807ed67fd577bf9c24ab337bb831e89f1/crypto/evp/p_rsa_asn1.c
,
Oct 24 2017
The following revision refers to this bug: https://boringssl.googlesource.com/boringssl/+/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1 commit a37f286f4efb3a5fb7d6b008e6e98257c4f072e1 Author: David Benjamin <davidben@google.com> Date: Tue Oct 24 17:39:46 2017 Remove the buggy RSA parser. I've left EVP_set_buggy_rsa_parser as a no-op stub for now, but it shouldn't need to last very long. (Just waiting for a CL to land in a consumer.) Bug: chromium:735616 Change-Id: I6426588f84dd0803661a79c6636a0414f4e98855 Reviewed-on: https://boringssl-review.googlesource.com/22124 Reviewed-by: Steven Valdez <svaldez@google.com> Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/crypto/fipsmodule/bn/bn_test.cc [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/crypto/bn_extra/bn_asn1.c [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/include/openssl/rsa.h [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/include/openssl/bn.h [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/crypto/rsa_extra/rsa_asn1.c [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/include/openssl/evp.h [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/crypto/rsa_extra/rsa_test.cc [modify] https://crrev.com/a37f286f4efb3a5fb7d6b008e6e98257c4f072e1/crypto/evp/p_rsa_asn1.c
,
Oct 24 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by bugdroid1@chromium.org
, Jun 22 2017