Revert "Fix bssl client/server's error-handling."
This reverts commit e7ca8a5d78396388570df4d91058f6e170e8647f.
Change-Id: Ib2f923760dc54400f45e9327b3a45466be1dd6d1
Reviewed-on: https://boringssl-review.googlesource.com/28184
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/crypto/err/err_test.cc b/crypto/err/err_test.cc
index d975721..489d248 100644
--- a/crypto/err/err_test.cc
+++ b/crypto/err/err_test.cc
@@ -23,14 +23,6 @@
#include "./internal.h"
-#if defined(OPENSSL_WINDOWS)
-OPENSSL_MSVC_PRAGMA(warning(push, 3))
-#include <windows.h>
-OPENSSL_MSVC_PRAGMA(warning(pop))
-#else
-#include <errno.h>
-#endif
-
TEST(ErrTest, Overflow) {
for (unsigned i = 0; i < ERR_NUM_ERRORS*2; i++) {
@@ -220,18 +212,3 @@
EXPECT_EQ(0u, ERR_get_error());
}
}
-
-// Querying the error queue should not affect the OS error.
-#if defined(OPENSSL_WINDOWS)
-TEST(ErrTest, PreservesLastError) {
- SetLastError(ERROR_INVALID_FUNCTION);
- ERR_get_error();
- EXPECT_EQ(ERROR_INVALID_FUNCTION, GetLastError());
-}
-#else
-TEST(ErrTest, PreservesErrno) {
- errno = EINVAL;
- ERR_get_error();
- EXPECT_EQ(EINVAL, errno);
-}
-#endif
diff --git a/crypto/thread_win.c b/crypto/thread_win.c
index 248870a..d6fa548 100644
--- a/crypto/thread_win.c
+++ b/crypto/thread_win.c
@@ -190,31 +190,13 @@
#endif // _WIN64
-static void **get_thread_locals(void) {
- // |TlsGetValue| clears the last error even on success, so that callers may
- // distinguish it successfully returning NULL or failing. It is documented to
- // never fail if the argument is a valid index from |TlsAlloc|, so we do not
- // need to handle this.
- //
- // However, this error-mangling behavior interferes with the caller's use of
- // |GetLastError|. In particular |SSL_get_error| queries the error queue to
- // determine whether the caller should look at the OS's errors. To avoid
- // destroying state, save and restore the Windows error.
- //
- // https://msdn.microsoft.com/en-us/library/windows/desktop/ms686812(v=vs.85).aspx
- DWORD last_error = GetLastError();
- void **ret = TlsGetValue(g_thread_local_key);
- SetLastError(last_error);
- return ret;
-}
-
void *CRYPTO_get_thread_local(thread_local_data_t index) {
CRYPTO_once(&g_thread_local_init_once, thread_local_init);
if (g_thread_local_failed) {
return NULL;
}
- void **pointers = get_thread_locals();
+ void **pointers = TlsGetValue(g_thread_local_key);
if (pointers == NULL) {
return NULL;
}
@@ -229,7 +211,7 @@
return 0;
}
- void **pointers = get_thread_locals();
+ void **pointers = TlsGetValue(g_thread_local_key);
if (pointers == NULL) {
pointers = OPENSSL_malloc(sizeof(void *) * NUM_OPENSSL_THREAD_LOCALS);
if (pointers == NULL) {
diff --git a/tool/client.cc b/tool/client.cc
index 037e10c..bdb5de7 100644
--- a/tool/client.cc
+++ b/tool/client.cc
@@ -181,7 +181,7 @@
if (!PEM_write_bio_SSL_SESSION(session_out.get(), session) ||
BIO_flush(session_out.get()) <= 0) {
fprintf(stderr, "Error while saving session:\n");
- ERR_print_errors_fp(stderr);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return 0;
}
}
@@ -221,7 +221,8 @@
if (ssl_err == SSL_ERROR_WANT_READ) {
continue;
}
- PrintSSLError(stderr, "Error while reading", ssl_err, ssl_ret);
+ fprintf(stderr, "Error while reading: %d\n", ssl_err);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
}
}
@@ -266,14 +267,14 @@
"rb"));
if (!in) {
fprintf(stderr, "Error reading session\n");
- ERR_print_errors_fp(stderr);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
}
bssl::UniquePtr<SSL_SESSION> session(PEM_read_bio_SSL_SESSION(in.get(),
nullptr, nullptr, nullptr));
if (!session) {
fprintf(stderr, "Error reading session\n");
- ERR_print_errors_fp(stderr);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
}
SSL_set_session(ssl.get(), session.get());
@@ -293,7 +294,8 @@
int ret = SSL_connect(ssl.get());
if (ret != 1) {
int ssl_err = SSL_get_error(ssl.get(), ret);
- PrintSSLError(stderr, "Error while connecting", ssl_err, ret);
+ fprintf(stderr, "Error while connecting: %d\n", ssl_err);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
}
@@ -313,7 +315,8 @@
int ssl_ret = SSL_write(ssl.get(), early_data.data(), ed_size);
if (ssl_ret <= 0) {
int ssl_err = SSL_get_error(ssl.get(), ssl_ret);
- PrintSSLError(stderr, "Error while writing", ssl_err, ssl_ret);
+ fprintf(stderr, "Error while writing: %d\n", ssl_err);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
} else if (ssl_ret != ed_size) {
fprintf(stderr, "Short write from SSL_write.\n");
@@ -497,7 +500,7 @@
if (!session_out) {
fprintf(stderr, "Error while opening %s:\n",
args_map["-session-out"].c_str());
- ERR_print_errors_fp(stderr);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
}
}
@@ -510,7 +513,7 @@
if (!SSL_CTX_load_verify_locations(
ctx.get(), args_map["-root-certs"].c_str(), nullptr)) {
fprintf(stderr, "Failed to load root certificates.\n");
- ERR_print_errors_fp(stderr);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
}
SSL_CTX_set_verify(ctx.get(), SSL_VERIFY_PEER, nullptr);
diff --git a/tool/server.cc b/tool/server.cc
index 7a4e53b..23a47e9 100644
--- a/tool/server.cc
+++ b/tool/server.cc
@@ -185,7 +185,8 @@
SSL_read(ssl, request + request_len, sizeof(request) - request_len);
if (ssl_ret <= 0) {
int ssl_err = SSL_get_error(ssl, ssl_ret);
- PrintSSLError(stderr, "Error while reading", ssl_err, ssl_ret);
+ fprintf(stderr, "Error while reading: %d\n", ssl_err);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
}
request_len += static_cast<size_t>(ssl_ret);
@@ -341,7 +342,8 @@
int ret = SSL_accept(ssl.get());
if (ret != 1) {
int ssl_err = SSL_get_error(ssl.get(), ret);
- PrintSSLError(stderr, "Error while connecting", ssl_err, ret);
+ fprintf(stderr, "Error while connecting: %d\n", ssl_err);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
result = false;
continue;
}
diff --git a/tool/transport_common.cc b/tool/transport_common.cc
index dcb8e0d..55f2059 100644
--- a/tool/transport_common.cc
+++ b/tool/transport_common.cc
@@ -91,33 +91,6 @@
}
}
-static std::string GetLastSocketErrorString() {
-#if defined(OPENSSL_WINDOWS)
- int error = WSAGetLastError();
- char *buffer;
- DWORD len = FormatMessageA(
- FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER, 0, error, 0,
- reinterpret_cast<char *>(&buffer), 0, nullptr);
- if (len == 0) {
- char buf[256];
- snprintf(buf, sizeof(buf), "unknown error (0x%x)", error);
- return buf;
- }
- std::string ret(buffer, len);
- LocalFree(buffer);
- return ret;
-#else
- return strerror(errno);
-#endif
-}
-
-static void PrintSocketError(const char *function) {
- // On Windows, |perror| and |errno| are part of the C runtime, while sockets
- // are separate, so we must print errors manually.
- std::string error = GetLastSocketErrorString();
- fprintf(stderr, "%s: %s\n", function, error.c_str());
-}
-
// Connect sets |*out_sock| to be a socket connected to the destination given
// in |hostname_and_port|, which should be of the form "www.example.com:123".
// It returns true on success and false otherwise.
@@ -148,7 +121,7 @@
*out_sock =
socket(result->ai_family, result->ai_socktype, result->ai_protocol);
if (*out_sock < 0) {
- PrintSocketError("socket");
+ perror("socket");
goto out;
}
@@ -172,7 +145,7 @@
}
if (connect(*out_sock, result->ai_addr, result->ai_addrlen) != 0) {
- PrintSocketError("connect");
+ perror("connect");
goto out;
}
ok = true;
@@ -215,18 +188,18 @@
server_sock_ = socket(addr.sin6_family, SOCK_STREAM, 0);
if (server_sock_ < 0) {
- PrintSocketError("socket");
+ perror("socket");
return false;
}
if (setsockopt(server_sock_, SOL_SOCKET, SO_REUSEADDR, (const char *)&enable,
sizeof(enable)) < 0) {
- PrintSocketError("setsockopt");
+ perror("setsockopt");
return false;
}
if (bind(server_sock_, (struct sockaddr *)&addr, sizeof(addr)) != 0) {
- PrintSocketError("connect");
+ perror("connect");
return false;
}
@@ -377,7 +350,7 @@
#else
WSAEVENT socket_handle = WSACreateEvent();
if (socket_handle == WSA_INVALID_EVENT ||
- WSAEventSelect(sock, socket_handle, FD_READ | FD_CLOSE) != 0) {
+ WSAEventSelect(sock, socket_handle, FD_READ) != 0) {
WSACloseEvent(socket_handle);
return false;
}
@@ -406,26 +379,11 @@
#endif
}
-void PrintSSLError(FILE *file, const char *msg, int ssl_err, int ret) {
- switch (ssl_err) {
- case SSL_ERROR_SSL:
- fprintf(file, "%s: %s\n", msg, ERR_reason_error_string(ERR_peek_error()));
- break;
- case SSL_ERROR_SYSCALL:
- if (ret == 0) {
- fprintf(file, "%s: peer closed connection\n", msg);
- } else {
- std::string error = GetLastSocketErrorString();
- fprintf(file, "%s: %s\n", msg, error.c_str());
- }
- break;
- case SSL_ERROR_ZERO_RETURN:
- fprintf(file, "%s: received close_notify\n", msg);
- break;
- default:
- fprintf(file, "%s: unknown error type (%d)\n", msg, ssl_err);
- }
- ERR_print_errors_fp(file);
+// PrintErrorCallback is a callback function from OpenSSL's
+// |ERR_print_errors_cb| that writes errors to a given |FILE*|.
+int PrintErrorCallback(const char *str, size_t len, void *ctx) {
+ fwrite(str, len, 1, reinterpret_cast<FILE*>(ctx));
+ return 1;
}
bool TransferData(SSL *ssl, int sock) {
@@ -469,18 +427,17 @@
}
#endif
int ssl_ret = SSL_write(ssl, buffer, n);
- if (ssl_ret <= 0) {
- int ssl_err = SSL_get_error(ssl, ssl_ret);
- PrintSSLError(stderr, "Error while writing", ssl_err, ssl_ret);
- return false;
- } else if (ssl_ret != n) {
- fprintf(stderr, "Short write from SSL_write.\n");
+ if (!SocketSetNonBlocking(sock, true)) {
return false;
}
- // Note we handle errors before restoring the non-blocking state. On
- // Windows, |SocketSetNonBlocking| internally clears the last error.
- if (!SocketSetNonBlocking(sock, true)) {
+ if (ssl_ret <= 0) {
+ int ssl_err = SSL_get_error(ssl, ssl_ret);
+ fprintf(stderr, "Error while writing: %d\n", ssl_err);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
+ return false;
+ } else if (ssl_ret != n) {
+ fprintf(stderr, "Short write from SSL_write.\n");
return false;
}
}
@@ -494,7 +451,8 @@
if (ssl_err == SSL_ERROR_WANT_READ) {
continue;
}
- PrintSSLError(stderr, "Error while reading", ssl_err, ssl_ret);
+ fprintf(stderr, "Error while reading: %d\n", ssl_err);
+ ERR_print_errors_cb(PrintErrorCallback, stderr);
return false;
} else if (ssl_ret == 0) {
return true;
diff --git a/tool/transport_common.h b/tool/transport_common.h
index 7d45d1c..492416a 100644
--- a/tool/transport_common.h
+++ b/tool/transport_common.h
@@ -53,10 +53,7 @@
bool SocketSetNonBlocking(int sock, bool is_non_blocking);
-// PrintSSLError prints information about the most recent SSL error to stderr.
-// |ssl_err| must be the output of |SSL_get_error| and the |SSL| object must be
-// connected to socket from |Connect|.
-void PrintSSLError(FILE *file, const char *msg, int ssl_err, int ret);
+int PrintErrorCallback(const char *str, size_t len, void *ctx);
bool TransferData(SSL *ssl, int sock);
