From 8021aed89b4135df0da7537ca7b8754fc91474a1 Mon Sep 17 00:00:00 2001 From: Konstantin Bogdanov Date: Mon, 15 Dec 2025 10:42:33 +0000 Subject: [PATCH] Merge pull request #91003 from filimonov/hide_version_non_authenticated_users Prevent HTTP endpoints from exposing the exact server version to unauthenticated users. --- src/Common/Exception.cpp | 37 +++++++++++++------ src/Common/Exception.h | 16 +++++--- src/Server/HTTPHandler.cpp | 3 +- .../03708_http_unauth_hide_version.reference | 4 ++ .../03708_http_unauth_hide_version.sh | 30 +++++++++++++++ 5 files changed, 72 insertions(+), 18 deletions(-) create mode 100644 tests/queries/0_stateless/03708_http_unauth_hide_version.reference create mode 100755 tests/queries/0_stateless/03708_http_unauth_hide_version.sh diff --git a/src/Common/Exception.cpp b/src/Common/Exception.cpp index 43f769c78d52..27bebfd30408 100644 --- a/src/Common/Exception.cpp +++ b/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); } diff --git a/src/Common/Exception.h b/src/Common/Exception.h index f21eb3084c2a..6d32186c58b1 100644 --- a/src/Common/Exception.h +++ b/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); diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index acb46c38a2d5..9328d557d61b 100644 --- a/src/Server/HTTPHandler.cpp +++ b/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(); diff --git a/tests/queries/0_stateless/03708_http_unauth_hide_version.reference b/tests/queries/0_stateless/03708_http_unauth_hide_version.reference new file mode 100644 index 000000000000..d768c8566c77 --- /dev/null +++ b/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 diff --git a/tests/queries/0_stateless/03708_http_unauth_hide_version.sh b/tests/queries/0_stateless/03708_http_unauth_hide_version.sh new file mode 100755 index 000000000000..081baf7908b1 --- /dev/null +++ b/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