[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #797: Add TLS support for kvrocks connections

[GitBox <gi...@apache.org>]

git-hulk commented on code in PR #797:
URL: https://github.com/apache/incubator-kvrocks/pull/797#discussion_r961308941


##########
.github/workflows/kvrocks.yaml:
##########
@@ -126,14 +130,15 @@ jobs:
       - name: Setup macOS
         if: ${{ startsWith(matrix.os, 'macos') }}
         run: |
-          brew install cmake gcc autoconf automake libtool
+          brew install cmake gcc autoconf automake libtool openssl
           echo "NPROC=$(sysctl -n hw.ncpu)" >> $GITHUB_ENV
+          echo "CMAKE_EXTRA_DEFS=-DOPENSSL_ROOT_DIR=/usr/local/opt/openssl" >> $GITHUB_ENV

Review Comment:
   Can also use `brew link --force openssl`



##########
README.md:
##########
@@ -69,14 +69,14 @@ Kvrocks has the following key features:
 ```shell
 # CentOS / RedHat
 sudo yum install -y epel-release
-sudo yum install -y git gcc gcc-c++ make cmake autoconf automake libtool libstdc++-static python3
+sudo yum install -y git gcc gcc-c++ make cmake autoconf automake libtool libstdc++-static python3 which openssl-devel
 
 # Ubuntu / Debian
 sudo apt update
-sudo apt install -y git gcc g++ make cmake autoconf automake libtool python3
+sudo apt install -y git gcc g++ make cmake autoconf automake libtool python3 libssl-dev
 
 # macOS
-brew install autoconf automake libtool cmake
+brew install autoconf automake libtool cmake openssl

Review Comment:
   ```suggestion
   brew install autoconf automake libtool cmake openssl
   # Please force linking the openssl if still can't find it after installing openssl
   brew link --force openssl
   ```



##########
README.md:
##########
@@ -69,14 +69,14 @@ Kvrocks has the following key features:
 ```shell
 # CentOS / RedHat
 sudo yum install -y epel-release
-sudo yum install -y git gcc gcc-c++ make cmake autoconf automake libtool libstdc++-static python3
+sudo yum install -y git gcc gcc-c++ make cmake autoconf automake libtool libstdc++-static python3 which openssl-devel
 
 # Ubuntu / Debian
 sudo apt update
-sudo apt install -y git gcc g++ make cmake autoconf automake libtool python3
+sudo apt install -y git gcc g++ make cmake autoconf automake libtool python3 libssl-dev
 
 # macOS
-brew install autoconf automake libtool cmake
+brew install autoconf automake libtool cmake openssl

Review Comment:
   ```suggestion
   brew install autoconf automake libtool cmake openssl
   
   # Please force linking the openssl if still can't find after installing openssl
   brew link --force openssl
   ```



##########
src/worker.cc:
##########
@@ -105,9 +115,38 @@ void Worker::newTCPConnection(evconnlistener *listener, evutil_socket_t fd,
   }
   event_base *base = evconnlistener_get_base(listener);
   auto evThreadSafeFlags = BEV_OPT_THREADSAFE | BEV_OPT_DEFER_CALLBACKS | BEV_OPT_UNLOCK_CALLBACKS;
-  bufferevent *bev = bufferevent_socket_new(base,
-                                            fd,
-                                            evThreadSafeFlags);
+
+  bufferevent *bev;
+#ifdef ENABLE_OPENSSL
+  if (local_port == worker->svr_->GetConfig()->tls_port) {
+    auto ssl = SSL_new(worker->svr_->ssl_ctx_.get());
+    if (!ssl) {
+      LOG(ERROR) << "Failed to construct SSL structure for new connection: " << SSLErrors{};
+      evutil_closesocket(fd);
+      return;
+    }
+    bev = bufferevent_openssl_socket_new(base, fd, ssl, BUFFEREVENT_SSL_ACCEPTING, evThreadSafeFlags);
+  } else {
+    bev = bufferevent_socket_new(base, fd, evThreadSafeFlags);
+  }
+#else
+  bev = bufferevent_socket_new(base, fd, evThreadSafeFlags);
+#endif
+  if (!bev) {
+    auto socket_err = evutil_socket_error_to_string(EVUTIL_SOCKET_ERROR());
+#ifdef ENABLE_OPENSSL
+    LOG(ERROR) << "Failed to construct socket for new connection: " << socket_err << ", SSL error: " << SSLErrors{};
+#else
+    LOG(ERROR) << "Failed to construct socket for new connection: " << socket_err;
+#endif
+    evutil_closesocket(fd);

Review Comment:
   ```suggestion
       SSL_free(ssl);
       evutil_closesocket(fd);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org