Skip to content

Commit 807f69c

Browse files
rschu1zerschu1ze
authored
Merge pull request ClickHouse#76214 from ClickHouse/backport/24.3/75954
2 parents 5f90953 + 423cf55 commit 807f69c

File tree

12 files changed

+116
-25
lines changed

12 files changed

+116
-25
lines changed

‎programs/library-bridge/LibraryBridge.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ std::string LibraryBridge::bridgeName() const
2525

2626
LibraryBridge::HandlerFactoryPtr LibraryBridge::getHandlerFactoryPtr(ContextPtr context) const
2727
{
28-
return std::make_shared<LibraryBridgeHandlerFactory>("LibraryRequestHandlerFactory", keep_alive_timeout, context);
28+
return std::make_shared<LibraryBridgeHandlerFactory>("LibraryRequestHandlerFactory", keep_alive_timeout, context, libraries_paths);
2929
}
3030

3131
}

‎programs/library-bridge/LibraryBridgeHandlerFactory.cpp‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ namespace DB
1010
LibraryBridgeHandlerFactory::LibraryBridgeHandlerFactory(
1111
const std::string & name_,
1212
size_t keep_alive_timeout_,
13-
ContextPtr context_)
13+
ContextPtr context_,
14+
std::vector<std::string> libraries_paths_)
1415
: WithContext(context_)
1516
, log(getLogger(name_))
1617
, name(name_)
1718
, keep_alive_timeout(keep_alive_timeout_)
19+
, libraries_paths(std::move(libraries_paths_))
1820
{
1921
}
2022

@@ -34,9 +36,9 @@ std::unique_ptr<HTTPRequestHandler> LibraryBridgeHandlerFactory::createRequestHa
3436
if (request.getMethod() == Poco::Net::HTTPRequest::HTTP_POST)
3537
{
3638
if (uri.getPath() == "/extdict_request")
37-
return std::make_unique<ExternalDictionaryLibraryBridgeRequestHandler>(keep_alive_timeout, getContext());
39+
return std::make_unique<ExternalDictionaryLibraryBridgeRequestHandler>(keep_alive_timeout, getContext(), libraries_paths);
3840
else if (uri.getPath() == "/catboost_request")
39-
return std::make_unique<CatBoostLibraryBridgeRequestHandler>(keep_alive_timeout, getContext());
41+
return std::make_unique<CatBoostLibraryBridgeRequestHandler>(keep_alive_timeout, getContext(), libraries_paths);
4042
}
4143

4244
return nullptr;

‎programs/library-bridge/LibraryBridgeHandlerFactory.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,16 @@ class LibraryBridgeHandlerFactory : public HTTPRequestHandlerFactory, WithContex
1414
LibraryBridgeHandlerFactory(
1515
const std::string & name_,
1616
size_t keep_alive_timeout_,
17-
ContextPtr context_);
17+
ContextPtr context_,
18+
std::vector<std::string> libraries_paths_);
1819

1920
std::unique_ptr<HTTPRequestHandler> createRequestHandler(const HTTPServerRequest & request) override;
2021

2122
private:
2223
LoggerPtr log;
2324
const std::string name;
2425
const size_t keep_alive_timeout;
26+
const std::vector<std::string> libraries_paths;
2527
};
2628

2729
}

‎programs/library-bridge/LibraryBridgeHandlers.cpp‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
#include "CatBoostLibraryHandlerFactory.h"
44
#include "Common/ProfileEvents.h"
5-
#include "ExternalDictionaryLibraryHandler.h"
65
#include "ExternalDictionaryLibraryHandlerFactory.h"
76

87
#include <Formats/FormatFactory.h>
98
#include <IO/ReadBufferFromString.h>
109
#include <IO/ReadHelpers.h>
1110
#include <Common/BridgeProtocolVersion.h>
11+
#include <Common/filesystemHelpers.h>
1212
#include <IO/WriteHelpers.h>
1313
#include <Poco/Net/HTTPServerRequest.h>
1414
#include <Poco/Net/HTTPServerResponse.h>
@@ -24,7 +24,8 @@
2424
#include <Formats/NativeReader.h>
2525
#include <Formats/NativeWriter.h>
2626
#include <DataTypes/DataTypesNumber.h>
27-
#include <DataTypes/DataTypeString.h>
27+
#include <filesystem>
28+
#include <boost/algorithm/string/join.hpp>
2829

2930

3031
namespace DB
@@ -86,14 +87,26 @@ static void writeData(Block data, OutputFormatPtr format)
8687
}
8788

8889

89-
ExternalDictionaryLibraryBridgeRequestHandler::ExternalDictionaryLibraryBridgeRequestHandler(size_t keep_alive_timeout_, ContextPtr context_)
90+
ExternalDictionaryLibraryBridgeRequestHandler::ExternalDictionaryLibraryBridgeRequestHandler(size_t keep_alive_timeout_, ContextPtr context_, std::vector<std::string> libraries_paths_)
9091
: WithContext(context_)
9192
, keep_alive_timeout(keep_alive_timeout_)
9293
, log(getLogger("ExternalDictionaryLibraryBridgeRequestHandler"))
94+
, libraries_paths(std::move(libraries_paths_))
9395
{
9496
}
9597

9698

99+
static bool checkLibraryPath(const std::string & path, const std::vector<std::string> & allowed_prefixes, HTTPServerResponse & response)
100+
{
101+
for (const auto & prefix : allowed_prefixes)
102+
if (fileOrSymlinkPathStartsWith(path, prefix))
103+
return true;
104+
processError(response, fmt::format("The provided library path {} is not inside any of the allowed prefixes {} ('libraries-path') from the configuration.",
105+
path, boost::join(allowed_prefixes, ", ")));
106+
return false;
107+
}
108+
109+
97110
void ExternalDictionaryLibraryBridgeRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & /*write_event*/)
98111
{
99112
LOG_TRACE(log, "Request URI: {}", request.getURI());
@@ -172,12 +185,15 @@ void ExternalDictionaryLibraryBridgeRequestHandler::handleRequest(HTTPServerRequ
172185

173186
if (!params.has("library_path"))
174187
{
175-
processError(response, "No 'library_path' in request URL");
188+
processError(response, "No 'library_path' in the request URL");
176189
return;
177190
}
178191

179192
const String & library_path = params.get("library_path");
180193

194+
if (!checkLibraryPath(library_path, libraries_paths, response))
195+
return;
196+
181197
if (!params.has("library_settings"))
182198
{
183199
processError(response, "No 'library_settings' in request URL");
@@ -413,10 +429,11 @@ void ExternalDictionaryLibraryBridgeExistsHandler::handleRequest(HTTPServerReque
413429

414430

415431
CatBoostLibraryBridgeRequestHandler::CatBoostLibraryBridgeRequestHandler(
416-
size_t keep_alive_timeout_, ContextPtr context_)
432+
size_t keep_alive_timeout_, ContextPtr context_, std::vector<std::string> libraries_paths_)
417433
: WithContext(context_)
418434
, keep_alive_timeout(keep_alive_timeout_)
419435
, log(getLogger("CatBoostLibraryBridgeRequestHandler"))
436+
, libraries_paths(std::move(libraries_paths_))
420437
{
421438
}
422439

@@ -522,6 +539,9 @@ void CatBoostLibraryBridgeRequestHandler::handleRequest(HTTPServerRequest & requ
522539

523540
const String & library_path = params.get("library_path");
524541

542+
if (!checkLibraryPath(library_path, libraries_paths, response))
543+
return;
544+
525545
if (!params.has("model_path"))
526546
{
527547
processError(response, "No 'model_path' in request URL");

‎programs/library-bridge/LibraryBridgeHandlers.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ namespace DB
1818
class ExternalDictionaryLibraryBridgeRequestHandler : public HTTPRequestHandler, WithContext
1919
{
2020
public:
21-
ExternalDictionaryLibraryBridgeRequestHandler(size_t keep_alive_timeout_, ContextPtr context_);
21+
ExternalDictionaryLibraryBridgeRequestHandler(size_t keep_alive_timeout_, ContextPtr context_, std::vector<std::string> libraries_paths_);
2222

2323
void handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & write_event) override;
2424

@@ -27,6 +27,7 @@ class ExternalDictionaryLibraryBridgeRequestHandler : public HTTPRequestHandler,
2727

2828
const size_t keep_alive_timeout;
2929
LoggerPtr log;
30+
std::vector<std::string> libraries_paths;
3031
};
3132

3233

@@ -63,13 +64,14 @@ class ExternalDictionaryLibraryBridgeExistsHandler : public HTTPRequestHandler,
6364
class CatBoostLibraryBridgeRequestHandler : public HTTPRequestHandler, WithContext
6465
{
6566
public:
66-
CatBoostLibraryBridgeRequestHandler(size_t keep_alive_timeout_, ContextPtr context_);
67+
CatBoostLibraryBridgeRequestHandler(size_t keep_alive_timeout_, ContextPtr context_, std::vector<std::string> libraries_paths_);
6768

6869
void handleRequest(HTTPServerRequest & request, HTTPServerResponse & response, const ProfileEvents::Event & write_event) override;
6970

7071
private:
7172
const size_t keep_alive_timeout;
7273
LoggerPtr log;
74+
std::vector<std::string> libraries_paths;
7375
};
7476

7577

‎programs/server/Server.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,13 +1157,11 @@ try
11571157
{
11581158
std::string dictionaries_lib_path = config().getString("dictionaries_lib_path", path / "dictionaries_lib/");
11591159
global_context->setDictionariesLibPath(dictionaries_lib_path);
1160-
fs::create_directories(dictionaries_lib_path);
11611160
}
11621161

11631162
{
11641163
std::string user_scripts_path = config().getString("user_scripts_path", path / "user_scripts/");
11651164
global_context->setUserScriptsPath(user_scripts_path);
1166-
fs::create_directories(user_scripts_path);
11671165
}
11681166

11691167
/// top_level_domains_lists

‎src/Bridge/IBridge.cpp‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@
1717
#include <base/range.h>
1818
#include <base/scope_guard.h>
1919

20-
#include <sys/time.h>
20+
#include <iostream>
21+
#include <thread>
22+
#include <filesystem>
2123
#include <sys/resource.h>
24+
#include <sys/time.h>
25+
#include <boost/algorithm/string.hpp>
2226

2327
#include "config.h"
2428

29+
2530
namespace DB
2631
{
2732

@@ -113,6 +118,9 @@ void IBridge::defineOptions(Poco::Util::OptionSet & options)
113118
options.addOption(
114119
Poco::Util::Option("stderr-path", "", "stderr log path, default console").argument("stderr-path").binding("logger.stderr"));
115120

121+
options.addOption(
122+
Poco::Util::Option("libraries-path", "", "A colon-separated (:) lists of paths from where we allow to load libraries. Subdirectories are also included. To prevent security risks, ensure this path is strictly read-only and not writable by user-controlled applications.").argument("libraries-path").binding("libraries-path"));
123+
116124
using Me = std::decay_t<decltype(*this)>;
117125

118126
options.addOption(
@@ -168,6 +176,10 @@ void IBridge::initialize(Application & self)
168176
keep_alive_timeout = config().getUInt64("keep-alive-timeout", DEFAULT_HTTP_KEEP_ALIVE_TIMEOUT);
169177
http_max_field_value_size = config().getUInt64("http-max-field-value-size", 128 * 1024);
170178

179+
boost::split(libraries_paths, config().getString("libraries-path", "/usr/lib/"), [](char c){ return c == ':'; });
180+
for (auto & path : libraries_paths)
181+
path = std::filesystem::canonical(path);
182+
171183
struct rlimit limit;
172184
const UInt64 gb = 1024 * 1024 * 1024;
173185

‎src/Bridge/IBridge.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class IBridge : public BaseDaemon
3535
virtual HandlerFactoryPtr getHandlerFactoryPtr(ContextPtr context) const = 0;
3636

3737
size_t keep_alive_timeout;
38+
std::vector<std::string> libraries_paths;
3839

3940
private:
4041
void handleHelp(const std::string &, const std::string &);

‎src/BridgeHelper/IBridgeHelper.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ namespace DB
1414
namespace ErrorCodes
1515
{
1616
extern const int EXTERNAL_SERVER_IS_NOT_RESPONDING;
17+
extern const int BAD_ARGUMENTS;
1718
}
1819

1920

@@ -95,6 +96,27 @@ std::unique_ptr<ShellCommand> IBridgeHelper::startBridgeCommand()
9596
cmd_args.push_back(config.getString("logger." + configPrefix() + "_level"));
9697
}
9798

99+
std::string allowed_paths;
100+
for (const auto * allowed_path_config : {"dictionaries_lib_path", "catboost_lib_path"})
101+
{
102+
if (config.has(allowed_path_config))
103+
{
104+
std::string allowed_path = config.getString(allowed_path_config);
105+
if (allowed_path.contains(':'))
106+
throw Exception(ErrorCodes::BAD_ARGUMENTS, "`{}` cannot contain the colon (:) symbol: {}", allowed_path_config, allowed_path);
107+
108+
if (!allowed_paths.empty())
109+
allowed_paths += ":";
110+
allowed_paths += allowed_path;
111+
}
112+
}
113+
114+
if (!allowed_paths.empty())
115+
{
116+
cmd_args.push_back("--libraries-path");
117+
cmd_args.push_back(allowed_paths);
118+
}
119+
98120
LOG_TRACE(getLog(), "Starting {}", serviceAlias());
99121

100122
/// We will terminate it with the KILL signal instead of the TERM signal,

‎src/Planner/ActionsChain.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@
44
#include <boost/algorithm/string/join.hpp>
55

66
#include <IO/WriteBuffer.h>
7-
#include <IO/WriteHelpers.h>
87
#include <IO/Operators.h>
9-
#include <IO/WriteBufferFromString.h>
8+
109

1110
namespace DB
1211
{

0 commit comments

Comments
 (0)