You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kvrocks.apache.org by tw...@apache.org on 2025/02/04 11:43:04 UTC

(kvrocks) 02/04: fix(conn): detect and drop HTTP requests in connection (#2765)

This is an automated email from the ASF dual-hosted git repository.

twice pushed a commit to branch 2.11
in repository https://gitbox.apache.org/repos/asf/kvrocks.git

commit efc67150a2866f32f64f28babf6e6c3612c0fc7e
Author: Twice <tw...@gmail.com>
AuthorDate: Tue Feb 4 11:37:29 2025 +0800

    fix(conn): detect and drop HTTP requests in connection (#2765)
    
    Co-authored-by: hulk <hu...@gmail.com>
---
 src/server/redis_connection.cc                  |  9 ++++-
 tests/gocase/unit/connection/connection_test.go | 46 +++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc
index 092df73d..d3de392a 100644
--- a/src/server/redis_connection.cc
+++ b/src/server/redis_connection.cc
@@ -385,10 +385,17 @@ void Connection::ExecuteCommands(std::deque<CommandTokens> *to_process_cmds) {
 
     auto cmd_s = Server::LookupAndCreateCommand(cmd_tokens.front());
     if (!cmd_s.IsOK()) {
+      auto cmd_name = cmd_tokens.front();
+      if (util::EqualICase(cmd_name, "host:") || util::EqualICase(cmd_name, "post")) {
+        LOG(WARNING) << "A likely HTTP request is detected in the RESP connection, indicating a potential "
+                        "Cross-Protocol Scripting attack. Connection aborted.";
+        EnableFlag(kCloseAsync);
+        return;
+      }
       if (is_multi_exec) multi_error_ = true;
       Reply(redis::Error(
           {Status::NotOK,
-           fmt::format("unknown command `{}`, with args beginning with: {}", cmd_tokens.front(),
+           fmt::format("unknown command `{}`, with args beginning with: {}", cmd_name,
                        util::StringJoin(nonstd::span(cmd_tokens.begin() + 1, cmd_tokens.end()),
                                         [](const auto &v) -> decltype(auto) { return fmt::format("`{}`", v); }))}));
       continue;
diff --git a/tests/gocase/unit/connection/connection_test.go b/tests/gocase/unit/connection/connection_test.go
new file mode 100644
index 00000000..267d23da
--- /dev/null
+++ b/tests/gocase/unit/connection/connection_test.go
@@ -0,0 +1,46 @@
+/*
+ * 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.
+ */
+
+package connection
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	"github.com/apache/kvrocks/tests/gocase/util"
+	"github.com/stretchr/testify/require"
+)
+
+func TestConnection(t *testing.T) {
+	srv := util.StartServer(t, map[string]string{})
+	defer srv.Close()
+
+	t.Run("HTTP requests will be dropped", func(t *testing.T) {
+		_, err := http.Get(fmt.Sprintf("http://%s", srv.HostPort())) //nolint:bodyclose
+		require.Error(t, err)
+		require.True(t, srv.LogFileMatches(t, "HTTP request.*Connection aborted"), "should contain HTTP drop log")
+
+		c := srv.NewTCPClient()
+		err = c.Write("GET / HTTP/1.1\r\nHOST: example.com\r\n")
+		require.NoError(t, err)
+		_, err = c.ReadLine()
+		require.Error(t, err)
+	})
+}