Repository: kudu Updated Branches: refs/heads/master abbde75e1 -> 1f380f279
KUDU-2351 Add IP/port for Recv() failure Error messages like "Recv() got EOF from remote", "recv error:" and "BlockingRecv error: failed to read from TLS socket" don't contain the remote address, making it hard to locate the server the connection failed to. This change introduces adds the peer's IP address and port number to these error messages. Examples: Recv() got EOF from remote 127.0.0.1:7050 recv error from 127.0.0.1:7050: Resource temporarily unavailable BlockingRecv error: failed to read from TLS socket (remote: \ 127.0.0.1:7050): Can't send after socket shutdown Change-Id: I22436b13bb351b132e1c0b7159294dd0c980c2b3 Reviewed-on: http://gerrit.cloudera.org:8080/9818 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert <danburk...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/1f380f27 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/1f380f27 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/1f380f27 Branch: refs/heads/master Commit: 1f380f27960021b267766fcefb6764e95e1d98dc Parents: abbde75 Author: Attila Bukor <abu...@cloudera.com> Authored: Thu Apr 19 21:18:45 2018 +0200 Committer: Dan Burkert <danburk...@apache.org> Committed: Thu Apr 19 20:07:16 2018 +0000 ---------------------------------------------------------------------- src/kudu/security/tls_socket-test.cc | 30 +++++++++++ src/kudu/security/tls_socket.cc | 11 +++- src/kudu/util/CMakeLists.txt | 1 + src/kudu/util/net/socket-test.cc | 89 +++++++++++++++++++++++++++++++ src/kudu/util/net/socket.cc | 12 +++-- 5 files changed, 138 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/1f380f27/src/kudu/security/tls_socket-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/tls_socket-test.cc b/src/kudu/security/tls_socket-test.cc index db4ff9b..001a206 100644 --- a/src/kudu/security/tls_socket-test.cc +++ b/src/kudu/security/tls_socket-test.cc @@ -45,6 +45,7 @@ #include "kudu/util/random.h" #include "kudu/util/random_util.h" #include "kudu/util/scoped_cleanup.h" +#include "kudu/util/slice.h" #include "kudu/util/status.h" #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" @@ -171,6 +172,8 @@ class EchoServer { SleepFor(MonoDelta::FromMilliseconds(10)); } } + + CHECK_OK(listener_.Close()); }); } @@ -212,6 +215,33 @@ class EchoServer { void handler(int /* signal */) {} +TEST_F(TlsSocketTest, TestRecvFailure) { + EchoServer server; + server.Start(); + unique_ptr<Socket> client_sock; + NO_FATALS(ConnectClient(server.listen_addr(), &client_sock)); + unique_ptr<uint8_t[]> buf(new uint8_t[kEchoChunkSize]); + + SleepFor(MonoDelta::FromMilliseconds(100)); + server.Stop(); + + size_t nwritten; + ASSERT_OK(client_sock->BlockingWrite(buf.get(), kEchoChunkSize, &nwritten, + MonoTime::Now() + kTimeout)); + size_t nread; + + ASSERT_OK(client_sock->BlockingRecv(buf.get(), kEchoChunkSize, &nread, + MonoTime::Now() + kTimeout)); + + Status s = client_sock->BlockingRecv(buf.get(), kEchoChunkSize, &nread, + MonoTime::Now() + kTimeout); + + ASSERT_TRUE(!s.ok()); + ASSERT_TRUE(s.IsNetworkError()); + ASSERT_STR_MATCHES(s.message().ToString(), "BlockingRecv error: failed to read from " + "TLS socket \\(remote: 127.0.0.1:[0-9]+\\): "); +} + // Test for failures to handle EINTR during TLS connection // negotiation and data send/receive. TEST_F(TlsSocketTest, TestTlsSocketInterrupted) { http://git-wip-us.apache.org/repos/asf/kudu/blob/1f380f27/src/kudu/security/tls_socket.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/tls_socket.cc b/src/kudu/security/tls_socket.cc index 4fce907..355f04b 100644 --- a/src/kudu/security/tls_socket.cc +++ b/src/kudu/security/tls_socket.cc @@ -20,14 +20,17 @@ #include <sys/uio.h> #include <cerrno> +#include <string> #include <utility> #include <glog/logging.h> #include <openssl/err.h> #include "kudu/gutil/basictypes.h" +#include "kudu/gutil/strings/substitute.h" #include "kudu/security/openssl_util.h" #include "kudu/util/errno.h" +#include "kudu/util/net/sockaddr.h" #include "kudu/util/net/socket.h" namespace kudu { @@ -104,20 +107,24 @@ Status TlsSocket::Writev(const struct ::iovec *iov, int iov_len, int64_t *nwritt Status TlsSocket::Recv(uint8_t *buf, int32_t amt, int32_t *nread) { SCOPED_OPENSSL_NO_PENDING_ERRORS; - const char* kErrString = "failed to read from TLS socket"; CHECK(ssl_); errno = 0; int32_t bytes_read = SSL_read(ssl_.get(), buf, amt); int save_errno = errno; if (bytes_read <= 0) { + Sockaddr remote; + Socket::GetPeerAddress(&remote); + std::string kErrString = strings::Substitute("failed to read from TLS socket (remote: $0)", + remote.ToString()); + if (bytes_read == 0 && SSL_get_shutdown(ssl_.get()) == SSL_RECEIVED_SHUTDOWN) { return Status::NetworkError(kErrString, ErrnoToString(ESHUTDOWN), ESHUTDOWN); } auto error_code = SSL_get_error(ssl_.get(), bytes_read); if (error_code == SSL_ERROR_WANT_READ) { if (save_errno != 0) { - return Status::NetworkError("SSL_read error", + return Status::NetworkError("SSL_read error from " + remote.ToString(), ErrnoToString(save_errno), save_errno); } // Nothing available to read yet. http://git-wip-us.apache.org/repos/asf/kudu/blob/1f380f27/src/kudu/util/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt index c91a780..1302d30 100644 --- a/src/kudu/util/CMakeLists.txt +++ b/src/kudu/util/CMakeLists.txt @@ -372,6 +372,7 @@ ADD_KUDU_TEST(mt-metrics-test RUN_SERIAL true) ADD_KUDU_TEST(mt-threadlocal-test RUN_SERIAL true) ADD_KUDU_TEST(net/dns_resolver-test) ADD_KUDU_TEST(net/net_util-test) +ADD_KUDU_TEST(net/socket-test) ADD_KUDU_TEST(object_pool-test) ADD_KUDU_TEST(oid_generator-test) ADD_KUDU_TEST(once-test) http://git-wip-us.apache.org/repos/asf/kudu/blob/1f380f27/src/kudu/util/net/socket-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/net/socket-test.cc b/src/kudu/util/net/socket-test.cc new file mode 100644 index 0000000..b69879f --- /dev/null +++ b/src/kudu/util/net/socket-test.cc @@ -0,0 +1,89 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "kudu/util/net/socket.h" + +#include <thread> + +#include <cstdint> +#include <glog/logging.h> +#include <gtest/gtest.h> +#include <memory> +#include <stddef.h> +#include <string> + +#include "kudu/util/monotime.h" +#include "kudu/util/net/sockaddr.h" +#include "kudu/util/slice.h" +#include "kudu/util/status.h" +#include "kudu/util/test_macros.h" +#include "kudu/util/test_util.h" + + +namespace kudu { + +constexpr size_t kEchoChunkSize = 32 * 1024 * 1024; + +class SocketTest : public KuduTest { + protected: + void DoTest(bool accept, const std::string &message) { + Sockaddr address; + address.ParseString("0.0.0.0", 0); + Socket listener_; + + CHECK_OK(listener_.Init(0)); + CHECK_OK(listener_.BindAndListen(address, 0)); + Sockaddr listen_address; + CHECK_OK(listener_.GetSocketAddress(&listen_address)); + + std::thread t([&]{ + if (accept) { + Sockaddr new_addr; + Socket sock; + CHECK_OK(listener_.Accept(&sock, &new_addr, 0)); + CHECK_OK(sock.Close()); + } else { + SleepFor(MonoDelta::FromMilliseconds(200)); + CHECK_OK(listener_.Close()); + } + }); + + Socket client; + ASSERT_OK(client.Init(0)); + ASSERT_OK(client.Connect(listen_address)); + CHECK_OK(client.SetRecvTimeout(MonoDelta::FromMilliseconds(100))); + + int n; + std::unique_ptr<uint8_t[]> buf(new uint8_t[kEchoChunkSize]); + Status s = client.Recv(buf.get(), kEchoChunkSize, &n); + + ASSERT_TRUE(!s.ok()); + ASSERT_TRUE(s.IsNetworkError()); + ASSERT_STR_MATCHES(s.message().ToString(), message); + + t.join(); + } +}; + +TEST_F(SocketTest, TestRecvReset) { + DoTest(false, "recv error from 127.0.0.1:[0-9]+: Resource temporarily unavailable"); +} + +TEST_F(SocketTest, TestRecvEOF) { + DoTest(true, "Recv\\(\\) got EOF from remote 127.0.0.1:[0-9]+"); +} +} // namespace kudu http://git-wip-us.apache.org/repos/asf/kudu/blob/1f380f27/src/kudu/util/net/socket.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/net/socket.cc b/src/kudu/util/net/socket.cc index cd37782..dd420b2 100644 --- a/src/kudu/util/net/socket.cc +++ b/src/kudu/util/net/socket.cc @@ -521,12 +521,18 @@ Status Socket::Recv(uint8_t *buf, int32_t amt, int32_t *nread) { int res; RETRY_ON_EINTR(res, recv(fd_, buf, amt, 0)); if (res <= 0) { + Sockaddr remote; + GetPeerAddress(&remote); if (res == 0) { - return Status::NetworkError("Recv() got EOF from remote", Slice(), ESHUTDOWN); + std::string error_message = strings::Substitute("Recv() got EOF from remote $0", + remote.ToString()); + return Status::NetworkError(error_message, Slice(), ESHUTDOWN); } int err = errno; - return Status::NetworkError(std::string("recv error: ") + - ErrnoToString(err), Slice(), err); + std::string error_message = strings::Substitute("recv error from $0: $1", + remote.ToString(), + ErrnoToString(err)); + return Status::NetworkError(error_message, Slice(), err); } *nread = res; return Status::OK();
