Include better OpenSSL error information in NetLog.

net/ssl/openssl_ssl_util.cc

Index: net/ssl/openssl_ssl_util.cc
diff --git a/net/ssl/openssl_ssl_util.cc b/net/ssl/openssl_ssl_util.cc
index 6b6771634968ba33eec64a14911c8f0ede790ca7..20b0153132a48eb4c6311371b33c231195be8708 100644
--- a/net/ssl/openssl_ssl_util.cc
+++ b/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)
@@ -156,6 +158,26 @@ int MapOpenSSLErrorSSL(unsigned long error_code) {
   }
 }
 
+base::Value* NetLogOpenSSLErrorCallback(int net_error,
+                                        int ssl_error,
+                                        uint32_t error_code,
+                                        const char* file,
+                                        int line,
+                                        NetLog::LogLevel /* log_level */) {
+  base::DictionaryValue* dict = new base::DictionaryValue();
+  dict->SetInteger("net_error", net_error);
+  dict->SetInteger("ssl_error", ssl_error);
+  if (error_code != 0) {
+    dict->SetInteger("error_lib", ERR_GET_LIB(error_code));
+    dict->SetInteger("error_reason", ERR_GET_REASON(error_code));

[Ryan Sleevi, 2014/08/25 06:24:35]

And we're guaranteed these will always be masks and never reach into something
like thread-local storage, right?

[davidben, 2014/08/26 22:13:51]

Yeah, that would be a pretty significant departure from how the API currently
works. I don't think you'd be able to make them not masks without serious hacks.
It needs to work with both ERR_get_error() and ERR_peek_error(), and the former
removes the error from the stack.

For more fun, there are fields that do come from per-thread storage, but you
extract them with a different API. Hence file/line being separate.

(I do actually want to eventually move ERR_GET_FUNC to per-thread storage
because the function codes are crazy. But I'm intentionally not calling that one
and the file/line information includes all that.)

+  }
+  if (file != NULL)
+    dict->SetString("file", file);
+  if (line != 0)
+    dict->SetInteger("line", line);
+  return dict;
+}
+
 }  // namespace
 
 void OpenSSLPutNetError(const tracked_objects::Location& location, int err) {
@@ -171,6 +193,21 @@ void OpenSSLPutNetError(const tracked_objects::Location& location, int err) {
 }
 
 int MapOpenSSLError(int err, const crypto::OpenSSLErrStackTracer& tracer) {
+  uint32_t error_code;
+  const char* file;
+  int line;
+  return MapOpenSSLErrorWithDetails(err, tracer, &error_code, &file, &line);
+}
+
+int MapOpenSSLErrorWithDetails(int err,
+                               const crypto::OpenSSLErrStackTracer& tracer,
+                               uint32_t* out_error_code,
+                               const char** out_file,
+                               int* out_line) {
+  *out_error_code = 0;
+  *out_file = NULL;
+  *out_line = 0;
+
   switch (err) {
     case SSL_ERROR_WANT_READ:
     case SSL_ERROR_WANT_WRITE:
@@ -182,12 +219,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_code = error_code;
+          *out_file = file;
+          *out_line = line;
           return MapOpenSSLErrorSSL(error_code);
         } else if (ERR_GET_LIB(error_code) == OpenSSLNetErrorLib()) {
+          *out_error_code = error_code;
+          *out_file = file;
+          *out_line = line;
           // Net error codes are negative but encoded in OpenSSL as positive
           // numbers.
           return -ERR_GET_REASON(error_code);
@@ -201,4 +246,13 @@ int MapOpenSSLError(int err, const crypto::OpenSSLErrStackTracer& tracer) {
   }
 }
 
+NetLog::ParametersCallback CreateNetLogOpenSSLErrorCallback(int net_error,
+                                                            int ssl_error,
+                                                            uint32_t error_code,
+                                                            const char* file,
+                                                            int line) {
+  return base::Bind(&NetLogOpenSSLErrorCallback,
+                    net_error, ssl_error, error_code, file, line);
+}
+
 }  // namespace net