25.8 Backport of #91003: Prevent HTTP endpoints from exposing the exact server version to unauthenticated users

src/Common/Exception.cpp

@@ -515,12 +515,20 @@ std::string getExtraExceptionInfo(const std::exception & e)
     return msg;
 }
 
-std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded_stacktrace /*= false*/, bool with_extra_info /*= true*/)
+std::string getCurrentExceptionMessage(
+    bool with_stacktrace,
+    bool check_embedded_stacktrace /*= false*/,
+    bool with_extra_info /*= true*/,
+    bool with_version /*= true*/)
 {
-    return getCurrentExceptionMessageAndPattern(with_stacktrace, check_embedded_stacktrace, with_extra_info).text;
+    return getCurrentExceptionMessageAndPattern(with_stacktrace, check_embedded_stacktrace, with_extra_info, with_version).text;
 }
 
-PreformattedMessage getCurrentExceptionMessageAndPattern(bool with_stacktrace, bool check_embedded_stacktrace /*= false*/, bool with_extra_info /*= true*/)
+PreformattedMessage getCurrentExceptionMessageAndPattern(
+    bool with_stacktrace,
+    bool check_embedded_stacktrace /*= false*/,
+    bool with_extra_info /*= true*/,
+    bool with_version /*= true*/)
 {
     WriteBufferFromOwnString stream;
     std::string_view message_format_string;
@@ -533,8 +541,9 @@ PreformattedMessage getCurrentExceptionMessageAndPattern(bool with_stacktrace, b
     catch (const Exception & e)
     {
         stream << getExceptionMessage(e, with_stacktrace, check_embedded_stacktrace)
-               << (with_extra_info ? getExtraExceptionInfo(e) : "")
-               << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
+               << (with_extra_info ? getExtraExceptionInfo(e) : "");
+        if (with_version)
+            stream << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
         message_format_string = e.tryGetMessageFormatString();
         message_format_string_args = e.getMessageFormatStringArgs();
     }
@@ -545,8 +554,9 @@ PreformattedMessage getCurrentExceptionMessageAndPattern(bool with_stacktrace, b
             stream << "Poco::Exception. Code: " << ErrorCodes::POCO_EXCEPTION << ", e.code() = " << e.code()
                 << ", " << e.displayText()
                 << (with_stacktrace ? ", Stack trace (when copying this message, always include the lines below):\n\n" + getExceptionStackTraceString(e) : "")
-                << (with_extra_info ? getExtraExceptionInfo(e) : "")
-                << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
+                << (with_extra_info ? getExtraExceptionInfo(e) : "");
+            if (with_version)
+                stream << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
         }
         catch (...) {} // NOLINT(bugprone-empty-catch)
     }
@@ -562,8 +572,9 @@ PreformattedMessage getCurrentExceptionMessageAndPattern(bool with_stacktrace, b
 
             stream << "std::exception. Code: " << ErrorCodes::STD_EXCEPTION << ", type: " << name << ", e.what() = " << e.what()
                 << (with_stacktrace ? ", Stack trace (when copying this message, always include the lines below):\n\n" + getExceptionStackTraceString(e) : "")
-                << (with_extra_info ? getExtraExceptionInfo(e) : "")
-                << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
+                << (with_extra_info ? getExtraExceptionInfo(e) : "");
+            if (with_version)
+                stream << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
         }
         catch (...) {} // NOLINT(bugprone-empty-catch)
 
@@ -593,7 +604,9 @@ PreformattedMessage getCurrentExceptionMessageAndPattern(bool with_stacktrace, b
             if (status)
                 name += " (demangling status: " + toString(status) + ")";
 
-            stream << "Unknown exception. Code: " << ErrorCodes::UNKNOWN_EXCEPTION << ", type: " << name << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
+            stream << "Unknown exception. Code: " << ErrorCodes::UNKNOWN_EXCEPTION << ", type: " << name;
+            if (with_version)
+                stream << " (version " << VERSION_STRING << VERSION_OFFICIAL << ")";
         }
         catch (...) {} // NOLINT(bugprone-empty-catch)
     }
@@ -766,9 +779,9 @@ bool ExecutionStatus::tryDeserializeText(const std::string & data)
     return true;
 }
 
-ExecutionStatus ExecutionStatus::fromCurrentException(const std::string & start_of_message, bool with_stacktrace)
+ExecutionStatus ExecutionStatus::fromCurrentException(const std::string & start_of_message, bool with_stacktrace, bool with_version)
 {
-    String msg = (start_of_message.empty() ? "" : (start_of_message + ": ")) + getCurrentExceptionMessage(with_stacktrace, true);
+    String msg = (start_of_message.empty() ? "" : (start_of_message + ": ")) + getCurrentExceptionMessage(with_stacktrace, /*check_embedded_stacktrace=*/true, /*with_extra_info=*/true, with_version);
     return ExecutionStatus(getCurrentExceptionCode(), msg);
 }
 

src/Common/Exception.h

@@ -313,10 +313,16 @@ void tryLogCurrentException(const AtomicLogger & logger, const std::string & sta
   *  only this stack trace will be printed.
   * with_extra_info - add information about the filesystem in case of "No space left on device" and similar.
   */
-std::string getCurrentExceptionMessage(bool with_stacktrace, bool check_embedded_stacktrace = false,
-                                       bool with_extra_info = true);
-PreformattedMessage getCurrentExceptionMessageAndPattern(bool with_stacktrace, bool check_embedded_stacktrace = false,
-                                       bool with_extra_info = true);
+std::string getCurrentExceptionMessage(
+    bool with_stacktrace,
+    bool check_embedded_stacktrace = false,
+    bool with_extra_info = true,
+    bool with_version = true);
+PreformattedMessage getCurrentExceptionMessageAndPattern(
+    bool with_stacktrace,
+    bool check_embedded_stacktrace = false,
+    bool with_extra_info = true,
+    bool with_version = true);
 
 /// Returns error code from ErrorCodes
 int getCurrentExceptionCode();
@@ -336,7 +342,7 @@ struct ExecutionStatus
     explicit ExecutionStatus(int return_code, const std::string & exception_message = "")
     : code(return_code), message(exception_message) {}
 
-    static ExecutionStatus fromCurrentException(const std::string & start_of_message = "", bool with_stacktrace = false);
+    static ExecutionStatus fromCurrentException(const std::string & start_of_message = "", bool with_stacktrace = false, bool with_version = true);
 
     static ExecutionStatus fromText(const std::string & data);
 

src/Server/HTTPHandler.cpp

@@ -754,7 +754,8 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse
         /** If exception is received from remote server, then stack trace is embedded in message.
           * If exception is thrown on local server, then stack trace is in separate field.
           */
-        ExecutionStatus status = ExecutionStatus::fromCurrentException("", with_stacktrace);
+        const bool include_version = session && session->sessionContext();
+        ExecutionStatus status = ExecutionStatus::fromCurrentException("", with_stacktrace, include_version);
         auto error_sent = trySendExceptionToClient(status.code, status.message, request, response, used_output);
 
         used_output.cancel();

tests/queries/0_stateless/03708_http_unauth_hide_version.reference

@@ -0,0 +1,4 @@
+HTTP/1.1 401 Unauthorized
+no_version
+HTTP/1.1 404 Not Found
+version_present_after_auth

tests/queries/0_stateless/03708_http_unauth_hide_version.sh

@@ -0,0 +1,30 @@
+#!/usr/bin/env bash
+
+CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+# shellcheck source=../shell_config.sh
+. "$CUR_DIR"/../shell_config.sh
+
+TMP_DIR=$(mktemp -d "$CUR_DIR"/03708_XXXXXX)
+trap 'rm -rf "$TMP_DIR"' EXIT
+
+URL="${CLICKHOUSE_PORT_HTTP_PROTO}://default:invalid_password@${CLICKHOUSE_HOST}:${CLICKHOUSE_PORT_HTTP}/"
+
+${CLICKHOUSE_CURL} -s -D "$TMP_DIR/headers" -o "$TMP_DIR/body" "$URL" --data-binary "SELECT 1" || true
+
+head -n 1 "$TMP_DIR/headers" | sed 's/\r$//'
+if grep -q "(version " "$TMP_DIR/body"; then
+    echo "version_leaked"
+else
+    echo "no_version"
+fi
+
+# Now authenticate successfully and trigger an exception to ensure version is present.
+GOOD_URL="${CLICKHOUSE_PORT_HTTP_PROTO}://default:@${CLICKHOUSE_HOST}:${CLICKHOUSE_PORT_HTTP}/"
+${CLICKHOUSE_CURL} -s -D "$TMP_DIR/headers2" -o "$TMP_DIR/body2" "$GOOD_URL" --data-binary "SELECT * FROM no_such_table" || true
+
+head -n 1 "$TMP_DIR/headers2" | sed 's/\r$//'
+if grep -q "(version " "$TMP_DIR/body2"; then
+    echo "version_present_after_auth"
+else
+    echo "version_missing_after_auth"
+fi