Commit e545f646 authored by Greg Thompson's avatar Greg Thompson Committed by Commit Bot

Revert "Disable the buggy RSA parser by default."

This reverts commit 2878e36f.

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/545715Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481877}
parent 20ac2167
...@@ -9,13 +9,10 @@ ...@@ -9,13 +9,10 @@
#include <string> #include <string>
#include "base/feature_list.h"
#include "base/lazy_instance.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/strings/string_piece.h" #include "base/strings/string_piece.h"
#include "third_party/boringssl/src/include/openssl/crypto.h" #include "third_party/boringssl/src/include/openssl/crypto.h"
#include "third_party/boringssl/src/include/openssl/err.h" #include "third_party/boringssl/src/include/openssl/err.h"
#include "third_party/boringssl/src/include/openssl/evp.h"
namespace crypto { namespace crypto {
...@@ -35,30 +32,11 @@ int OpenSSLErrorCallback(const char* str, size_t len, void* context) { ...@@ -35,30 +32,11 @@ int OpenSSLErrorCallback(const char* str, size_t len, void* context) {
return 1; return 1;
} }
// TODO(davidben): Remove this after Chrome 61 is released to
// stable. https://crbug.com/735616.
const base::Feature kBuggyRSAParser{
"BuggyRSAParser", base::FEATURE_DISABLED_BY_DEFAULT,
};
class BuggyRSAParser {
public:
BuggyRSAParser() {
EVP_set_buggy_rsa_parser(base::FeatureList::IsEnabled(kBuggyRSAParser));
}
};
base::LazyInstance<BuggyRSAParser>::Leaky g_buggy_rsa_parser =
LAZY_INSTANCE_INITIALIZER;
} // namespace } // namespace
void EnsureOpenSSLInit() { void EnsureOpenSSLInit() {
// CRYPTO_library_init may be safely called concurrently. // CRYPTO_library_init may be safely called concurrently.
CRYPTO_library_init(); CRYPTO_library_init();
// Configure the RSA parser.
g_buggy_rsa_parser.Get();
} }
void ClearOpenSSLERRStack(const tracked_objects::Location& location) { void ClearOpenSSLERRStack(const tracked_objects::Location& location) {
......
...@@ -173,26 +173,26 @@ int MapOpenSSLErrorWithDetails(int err, ...@@ -173,26 +173,26 @@ int MapOpenSSLErrorWithDetails(int err,
return ERR_FAILED; return ERR_FAILED;
case SSL_ERROR_SSL: case SSL_ERROR_SSL:
// Walk down the error stack to find an SSL or net error. // Walk down the error stack to find an SSL or net error.
while (true) { uint32_t error_code;
OpenSSLErrorInfo error_info; const char* file;
error_info.error_code = int line;
ERR_get_error_line(&error_info.file, &error_info.line); do {
if (error_info.error_code == 0) { error_code = ERR_get_error_line(&file, &line);
// Map errors to ERR_SSL_PROTOCOL_ERROR by default, reporting the most if (ERR_GET_LIB(error_code) == ERR_LIB_SSL) {
// recent error in |*out_error_info|. out_error_info->error_code = error_code;
return ERR_SSL_PROTOCOL_ERROR; out_error_info->file = file;
} out_error_info->line = line;
return MapOpenSSLErrorSSL(error_code);
*out_error_info = error_info; } else if (ERR_GET_LIB(error_code) == OpenSSLNetErrorLib()) {
if (ERR_GET_LIB(error_info.error_code) == ERR_LIB_SSL) { out_error_info->error_code = error_code;
return MapOpenSSLErrorSSL(error_info.error_code); out_error_info->file = file;
} out_error_info->line = line;
if (ERR_GET_LIB(error_info.error_code) == OpenSSLNetErrorLib()) {
// Net error codes are negative but encoded in OpenSSL as positive // Net error codes are negative but encoded in OpenSSL as positive
// numbers. // numbers.
return -ERR_GET_REASON(error_info.error_code); return -ERR_GET_REASON(error_code);
} }
} } while (error_code != 0);
return ERR_FAILED;
default: default:
// TODO(joth): Implement full mapping. // TODO(joth): Implement full mapping.
LOG(WARNING) << "Unknown OpenSSL error " << err; LOG(WARNING) << "Unknown OpenSSL error " << err;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment