Include better OpenSSL error information in NetLog.

The 'ssl_lib_error' value reported is rather uninteresting. OpenSSL is has two
levels of errors. The one we were logging is almost always 1. Make the error
callback SSL-implementation-specific and include the library code, reason code,
file name, and line number, all of which OpenSSL already tracks.

BUG=286480

Review URL: https://codereview.chromium.org/494913002

Cr-Commit-Position: refs/heads/master@{#292173}

net/net.gypi

@@ -124,8 +124,6 @@
       'socket/ssl_client_socket_openssl.h',
       'socket/ssl_client_socket_pool.cc',
       'socket/ssl_client_socket_pool.h',
-      'socket/ssl_error_params.cc',
-      'socket/ssl_error_params.h',
       'socket/ssl_session_cache_openssl.cc',
       'socket/ssl_session_cache_openssl.h',
       'socket/ssl_socket.h',

net/socket/nss_ssl_util.cc

@@ -29,6 +29,8 @@
 #include "base/win/windows_version.h"
 #endif
 
+namespace net {
+
 namespace {
 
 // CiphersRemove takes a zero-terminated array of cipher suite ids in
@@ -77,9 +79,15 @@ size_t CiphersCopy(const uint16* in, uint16* out) {
   }
 }
 
-}  // anonymous namespace
-
-namespace net {
+base::Value* NetLogSSLErrorCallback(int net_error,
+                                    int ssl_lib_error,
+                                    NetLog::LogLevel /* log_level */) {
+  base::DictionaryValue* dict = new base::DictionaryValue();
+  dict->SetInteger("net_error", net_error);
+  if (ssl_lib_error)
+    dict->SetInteger("ssl_lib_error", ssl_lib_error);
+  return dict;
+}
 
 class NSSSSLInitSingleton {
  public:
@@ -201,9 +209,11 @@ class NSSSSLInitSingleton {
   PRFileDesc* model_fd_;
 };
 
-static base::LazyInstance<NSSSSLInitSingleton>::Leaky g_nss_ssl_init_singleton =
+base::LazyInstance<NSSSSLInitSingleton>::Leaky g_nss_ssl_init_singleton =
     LAZY_INSTANCE_INITIALIZER;
 
+}  // anonymous namespace
+
 // Initialize the NSS SSL library if it isn't already initialized.  This must
 // be called before any other NSS SSL functions.  This function is
 // thread-safe, and the NSS SSL library will only ever be initialized once.
@@ -399,4 +409,9 @@ void LogFailedNSSFunction(const BoundNetLog& net_log,
                  function, param, PR_GetError()));
 }
 
+NetLog::ParametersCallback CreateNetLogSSLErrorCallback(int net_error,
+                                                        int ssl_lib_error) {
+  return base::Bind(&NetLogSSLErrorCallback, net_error, ssl_lib_error);
+}
+
 }  // namespace net

net/socket/nss_ssl_util.h

@@ -12,6 +12,7 @@
 #include <prio.h>
 
 #include "net/base/net_export.h"
+#include "net/base/net_log.h"
 
 namespace net {
 
@@ -35,6 +36,11 @@ PRFileDesc* GetNSSModelSocket();
 // Map NSS error code to network error code.
 int MapNSSError(PRErrorCode err);
 
+// Creates a NetLog callback for an SSL error.
+NetLog::ParametersCallback CreateNetLogSSLErrorCallback(int net_error,
+                                                        int ssl_lib_error);
+
+
 }  // namespace net
 
 #endif  // NET_SOCKET_NSS_SSL_UTIL_H_

net/socket/ssl_client_socket_nss.cc

@@ -105,7 +105,6 @@
 #include "net/ocsp/nss_ocsp.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/nss_ssl_util.h"
-#include "net/socket/ssl_error_params.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_info.h"

net/socket/ssl_client_socket_openssl.cc

@@ -24,7 +24,6 @@
 #include "net/cert/single_request_cert_verifier.h"
 #include "net/cert/x509_certificate_net_log_param.h"
 #include "net/http/transport_security_state.h"
-#include "net/socket/ssl_error_params.h"
 #include "net/socket/ssl_session_cache_openssl.h"
 #include "net/ssl/openssl_ssl_util.h"
 #include "net/ssl/ssl_cert_request_info.h"
@@ -922,7 +921,8 @@ int SSLClientSocketOpenSSL::DoHandshake() {
       return OK;
     }
 
-    net_error = MapOpenSSLError(ssl_error, err_tracer);
+    OpenSSLErrorInfo error_info;
+    net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer, &error_info);
 
     // If not done, stay in this state
     if (net_error == ERR_IO_PENDING) {
@@ -933,7 +933,7 @@ int SSLClientSocketOpenSSL::DoHandshake() {
                  << ", net_error " << net_error;
       net_log_.AddEvent(
           NetLog::TYPE_SSL_HANDSHAKE_ERROR,
-          CreateNetLogSSLErrorCallback(net_error, ssl_error));
+          CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
     }
   }
   return net_error;

net/socket/ssl_error_params.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "net/socket/ssl_error_params.h"
-
-#include "base/bind.h"
-#include "base/values.h"
-
-namespace net {
-
-namespace {
-
-base::Value* NetLogSSLErrorCallback(int net_error,
-                                    int ssl_lib_error,
-                                    NetLog::LogLevel /* log_level */) {
-  base::DictionaryValue* dict = new base::DictionaryValue();
-  dict->SetInteger("net_error", net_error);
-  if (ssl_lib_error)
-    dict->SetInteger("ssl_lib_error", ssl_lib_error);
-  return dict;
-}
-
-}  // namespace
-
-NetLog::ParametersCallback CreateNetLogSSLErrorCallback(int net_error,
-                                                        int ssl_lib_error) {
-  return base::Bind(&NetLogSSLErrorCallback, net_error, ssl_lib_error);
-}
-
-}  // namespace net

net/socket/ssl_error_params.h

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef NET_SOCKET_SSL_ERROR_PARAMS_H_
-#define NET_SOCKET_SSL_ERROR_PARAMS_H_
-
-#include "net/base/net_log.h"
-
-namespace net {
-
-// Creates NetLog callback for when we receive an SSL error.
-NetLog::ParametersCallback CreateNetLogSSLErrorCallback(int net_error,
-                                                        int ssl_lib_error);
-
-}  // namespace net
-
-#endif  // NET_SOCKET_SSL_ERROR_PARAMS_H_

net/socket/ssl_server_socket_nss.cc

@@ -38,7 +38,6 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/socket/nss_ssl_util.h"
-#include "net/socket/ssl_error_params.h"
 
 // SSL plaintext fragments are shorter than 16KB. Although the record layer
 // overhead is allowed to be 2K + 5 bytes, in practice the overhead is much

net/socket/ssl_server_socket_openssl.cc

@@ -13,7 +13,6 @@
 #include "crypto/rsa_private_key.h"
 #include "crypto/scoped_openssl_types.h"
 #include "net/base/net_errors.h"
-#include "net/socket/ssl_error_params.h"
 #include "net/ssl/openssl_ssl_util.h"
 
 #define GotoState(s) next_handshake_state_ = s
@@ -457,10 +456,13 @@ int SSLServerSocketOpenSSL::DoPayloadRead() {
   if (rv >= 0)
     return rv;
   int ssl_error = SSL_get_error(ssl_, rv);
-  int net_error = MapOpenSSLError(ssl_error, err_tracer);
+  OpenSSLErrorInfo error_info;
+  int net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer,
+                                             &error_info);
   if (net_error != ERR_IO_PENDING) {
-    net_log_.AddEvent(NetLog::TYPE_SSL_READ_ERROR,
-                      CreateNetLogSSLErrorCallback(net_error, ssl_error));
+    net_log_.AddEvent(
+        NetLog::TYPE_SSL_READ_ERROR,
+        CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
   }
   return net_error;
 }
@@ -472,10 +474,13 @@ int SSLServerSocketOpenSSL::DoPayloadWrite() {
   if (rv >= 0)
     return rv;
   int ssl_error = SSL_get_error(ssl_, rv);
-  int net_error = MapOpenSSLError(ssl_error, err_tracer);
+  OpenSSLErrorInfo error_info;
+  int net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer,
+                                             &error_info);
   if (net_error != ERR_IO_PENDING) {
-    net_log_.AddEvent(NetLog::TYPE_SSL_WRITE_ERROR,
-                      CreateNetLogSSLErrorCallback(net_error, ssl_error));
+    net_log_.AddEvent(
+        NetLog::TYPE_SSL_WRITE_ERROR,
+        CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
   }
   return net_error;
 }
@@ -554,7 +559,8 @@ int SSLServerSocketOpenSSL::DoHandshake() {
     completed_handshake_ = true;
   } else {
     int ssl_error = SSL_get_error(ssl_, rv);
-    net_error = MapOpenSSLError(ssl_error, err_tracer);
+    OpenSSLErrorInfo error_info;
+    net_error = MapOpenSSLErrorWithDetails(ssl_error, err_tracer, &error_info);
 
     // If not done, stay in this state
     if (net_error == ERR_IO_PENDING) {
@@ -563,8 +569,9 @@ int SSLServerSocketOpenSSL::DoHandshake() {
       LOG(ERROR) << "handshake failed; returned " << rv
                  << ", SSL error code " << ssl_error
                  << ", net_error " << net_error;
-      net_log_.AddEvent(NetLog::TYPE_SSL_HANDSHAKE_ERROR,
-                        CreateNetLogSSLErrorCallback(net_error, ssl_error));
+      net_log_.AddEvent(
+          NetLog::TYPE_SSL_HANDSHAKE_ERROR,
+          CreateNetLogOpenSSLErrorCallback(net_error, ssl_error, error_info));
     }
   }
   return net_error;

net/ssl/openssl_ssl_util.cc

@@ -9,9 +9,11 @@
 #include <openssl/err.h>
 #include <openssl/ssl.h>
 
+#include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/values.h"
 #include "crypto/openssl_util.h"
 #include "net/base/net_errors.h"
 
@@ -54,7 +56,7 @@ unsigned OpenSSLNetErrorLib() {
   return g_openssl_net_error_lib.Get().net_error_lib();
 }
 
-int MapOpenSSLErrorSSL(unsigned long error_code) {
+int MapOpenSSLErrorSSL(uint32_t error_code) {
   DCHECK_EQ(ERR_LIB_SSL, ERR_GET_LIB(error_code));
 
   DVLOG(1) << "OpenSSL SSL error, reason: " << ERR_GET_REASON(error_code)
@@ -145,6 +147,24 @@ int MapOpenSSLErrorSSL(unsigned long error_code) {
   }
 }
 
+base::Value* NetLogOpenSSLErrorCallback(int net_error,
+                                        int ssl_error,
+                                        const OpenSSLErrorInfo& error_info,
+                                        NetLog::LogLevel /* log_level */) {
+  base::DictionaryValue* dict = new base::DictionaryValue();
+  dict->SetInteger("net_error", net_error);
+  dict->SetInteger("ssl_error", ssl_error);
+  if (error_info.error_code != 0) {
+    dict->SetInteger("error_lib", ERR_GET_LIB(error_info.error_code));
+    dict->SetInteger("error_reason", ERR_GET_REASON(error_info.error_code));
+  }
+  if (error_info.file != NULL)
+    dict->SetString("file", error_info.file);
+  if (error_info.line != 0)
+    dict->SetInteger("line", error_info.line);
+  return dict;
+}
+
 }  // namespace
 
 void OpenSSLPutNetError(const tracked_objects::Location& location, int err) {
@@ -160,6 +180,15 @@ void OpenSSLPutNetError(const tracked_objects::Location& location, int err) {
 }
 
 int MapOpenSSLError(int err, const crypto::OpenSSLErrStackTracer& tracer) {
+  OpenSSLErrorInfo error_info;
+  return MapOpenSSLErrorWithDetails(err, tracer, &error_info);
+}
+
+int MapOpenSSLErrorWithDetails(int err,
+                               const crypto::OpenSSLErrStackTracer& tracer,
+                               OpenSSLErrorInfo* out_error_info) {
+  *out_error_info = OpenSSLErrorInfo();
+
   switch (err) {
     case SSL_ERROR_WANT_READ:
     case SSL_ERROR_WANT_WRITE:
@@ -171,12 +200,20 @@ int MapOpenSSLError(int err, const crypto::OpenSSLErrStackTracer& tracer) {
       return ERR_SSL_PROTOCOL_ERROR;
     case SSL_ERROR_SSL:
       // Walk down the error stack to find an SSL or net error.
-      unsigned long error_code;
+      uint32_t error_code;
+      const char* file;
+      int line;
       do {
-        error_code = ERR_get_error();
+        error_code = ERR_get_error_line(&file, &line);
         if (ERR_GET_LIB(error_code) == ERR_LIB_SSL) {
+          out_error_info->error_code = error_code;
+          out_error_info->file = file;
+          out_error_info->line = line;
           return MapOpenSSLErrorSSL(error_code);
         } else if (ERR_GET_LIB(error_code) == OpenSSLNetErrorLib()) {
+          out_error_info->error_code = error_code;
+          out_error_info->file = file;
+          out_error_info->line = line;
           // Net error codes are negative but encoded in OpenSSL as positive
           // numbers.
           return -ERR_GET_REASON(error_code);
@@ -190,4 +227,12 @@ int MapOpenSSLError(int err, const crypto::OpenSSLErrStackTracer& tracer) {
   }
 }
 
+NetLog::ParametersCallback CreateNetLogOpenSSLErrorCallback(
+    int net_error,
+    int ssl_error,
+    const OpenSSLErrorInfo& error_info) {
+  return base::Bind(&NetLogOpenSSLErrorCallback,
+                    net_error, ssl_error, error_info);
+}
+
 }  // namespace net

net/ssl/openssl_ssl_util.h

@@ -5,6 +5,8 @@
 #ifndef NET_SSL_OPENSSL_SSL_UTIL_H_
 #define NET_SSL_OPENSSL_SSL_UTIL_H_
 
+#include "net/base/net_log.h"
+
 namespace crypto {
 class OpenSSLErrStackTracer;
 }
@@ -30,11 +32,40 @@ struct SslSetClearMask {
 };
 
 // Converts an OpenSSL error code into a net error code, walking the OpenSSL
-// error stack if needed. Note that |tracer| is not currently used in the
-// implementation, but is passed in anyway as this ensures the caller will clear
-// any residual codes left on the error stack.
+// error stack if needed.
+//
+// Note that |tracer| is not currently used in the implementation, but is passed
+// in anyway as this ensures the caller will clear any residual codes left on
+// the error stack.
 int MapOpenSSLError(int err, const crypto::OpenSSLErrStackTracer& tracer);
 
+// Helper struct to store information about an OpenSSL error stack entry.
+struct OpenSSLErrorInfo {
+  OpenSSLErrorInfo() : error_code(0), file(NULL), line(0) {}
+
+  uint32_t error_code;
+  const char* file;
+  int line;
+};
+
+// Converts an OpenSSL error code into a net error code, walking the OpenSSL
+// error stack if needed. If a value on the stack is used, the error code and
+// associated information are returned in |*out_error_info|. Otherwise its
+// fields are set to 0 and NULL.
+//
+// Note that |tracer| is not currently used in the implementation, but is passed
+// in anyway as this ensures the caller will clear any residual codes left on
+// the error stack.
+int MapOpenSSLErrorWithDetails(int err,
+                               const crypto::OpenSSLErrStackTracer& tracer,
+                               OpenSSLErrorInfo* out_error_info);
+
+// Creates NetLog callback for an OpenSSL error.
+NetLog::ParametersCallback CreateNetLogOpenSSLErrorCallback(
+    int net_error,
+    int ssl_error,
+    const OpenSSLErrorInfo& error_info);
+
 }  // namespace net
 
 #endif  // NET_SSL_OPENSSL_SSL_UTIL_H_