8000 Improve endpoint handling. (#11236) · arangodb/arangodb@fbd3151 · GitHub
[go: up one dir, main page]
Skip to content

Commit fbd3151

Browse files
ObiWahnObiWahn
authored
Improve endpoint handling. (#11236)
1 parent 00baa8f commit fbd3151

File tree

4 files changed

+143
-113
lines changed

4 files changed

+143
-113
lines changed

‎arangosh/Shell/V8ClientConnection.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,10 @@ static void ClientConnection_reconnect(v8::FunctionCallbackInfo<v8::Value> const
510510
}
511511

512512
V8SecurityFeature& v8security = v8connection->server().getFeature<V8SecurityFeature>();
513-
if (!v8security.isAllowedToConnectToEndpoint(isolate, endpoint)) {
513+
if (!v8security.isAllowedToConnectToEndpoint(isolate, endpoint, endpoint)) {
514514
TRI_V8_THROW_EXCEPTION_MESSAGE(TRI_ERROR_FORBIDDEN,
515-
"not allowed to connect to this endpoint");
515+
std::string("not allowed to connect to this endpoint") +
516+
endpoint);
516517
}
517518

518519
client->setEndpoint(endpoint);
@@ -1933,7 +1934,7 @@ v8::Local<v8::Value> V8ClientConnection::requestDataRaw(
19331934
TRI_V8_ASCII_STRING(isolate, "body"),
19341935
bufObj).FromMaybe(false);
19351936
}
1936-
1937+
19371938
for (auto const& it : response->header.meta()) {
19381939
headers->Set(context,
19391940
TRI_V8_STD_STRING(isolate, it.first),
@@ -1947,7 +1948,7 @@ v8::Local<v8::Value> V8ClientConnection::requestDataRaw(
19471948
headers->Set(context,
19481949
TRI_V8_STD_STRING(isolate, StaticStrings::ContentLength),
19491950
TRI_V8_STD_STRING(isolate, std::to_string(responseBody.size()))).FromMaybe(false);
1950-
1951+
19511952
}
19521953

19531954
result->Set(context,

‎lib/ApplicationFeatures/V8SecurityFeature.cpp‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -121,21 +121,29 @@ void convertToSingleExpression(std::unordered_set<std::string> const& values,
121121
targetRegex = ss.str();
122122
}
123123

124-
bool checkBlackAndWhitelist(std::string const& value, bool hasWhitelist,
124+
struct checkBlackWhiteResult {
125+
bool result;
126+
bool white;
127+
bool black;
128+
};
129+
130+
checkBlackWhiteResult checkBlackAndWhitelist(std::string const& value, bool hasWhitelist,
125131
std::regex const& whitelist, bool hasBlacklist,
126132
std::regex const& blacklist) {
127133
if (!hasWhitelist && !hasBlacklist) {
128-
return true;
134+
return {true, false, false};
129135
}
130136

131137
if (!hasBlacklist) {
132138
// only have a whitelist
133-
return std::regex_search(value, whitelist);
139+
bool white = std::regex_search(value, whitelist);
140+
return {white, white, false};
134141
}
135142

136143
if (!hasWhitelist) {
137144
// only have a blacklist
138-
return !std::regex_search(value, blacklist);
145+
bool black = std::regex_search(value, blacklist);
146+
return {!black, false, black};
139147
}
140148

141149
std::smatch white_result{};
@@ -145,17 +153,18 @@ bool checkBlackAndWhitelist(std::string const& value, bool hasWhitelist,
145153

146154
if (white && !black) {
147155
// we only have a whitelist hit => allow
148-
return true;
156+
return {true, white, black};
149157
} else if (!white && black) {
150158
// we only have a blacklist hit => deny
151-
return false;
159+
return {false, white, black};
152160
} else if (!white && !black) {
153161
// we have neither a whitelist nor a blacklist hit => deny
154-
return false;
162+
return {false, white, black};
155163
}
156164

157165
// longer match or blacklist wins
158-
return white_result[0].length() > black_result[0].length();
166+
bool white_longer_black = white_result[0].length() > black_result[0].length();
167+
return {white_longer_black, white_longer_black, !white_longer_black};
159168
}
160169
} // namespace
161170

@@ -410,19 +419,20 @@ bool V8SecurityFeature::shouldExposeStartupOption(v8::Isolate* isolate,
410419
return checkBlackAndWhitelist(name, !_startupOptionsWhitelist.empty(),
411420
_startupOptionsWhitelistRegex,
412421
!_startupOptionsBlacklist.empty(),
413-
_startupOptionsBlacklistRegex);
422+
_startupOptionsBlacklistRegex).result;
414423
}
415424

416425
bool V8SecurityFeature::shouldExposeEnvironmentVariable(v8::Isolate* isolate,
417426
std::string const& name) const {
418427
return checkBlackAndWhitelist(name, !_environmentVariablesWhitelist.empty(),
419428
_environmentVariablesWhitelistRegex,
420429
!_environmentVariablesBlacklist.empty(),
421-
_environmentVariablesBlacklistRegex);
430+
_environmentVariablesBlacklistRegex).result;
422431
}
423432

424433
bool V8SecurityFeature::isAllowedToConnectToEndpoint(v8::Isolate* isolate,
425-
std::string const& name) const {
434+
std::string const& endpoint,
435+
std::string const& url) const {
426436
TRI_GET_GLOBALS();
427437
TRI_ASSERT(v8g != nullptr);
428438
if (v8g->_securityContext.isInternal()) {
@@ -431,8 +441,13 @@ bool V8SecurityFeature::isAllowedToConnectToEndpoint(v8::Isolate* isolate,
431441
return true;
432442
}
433443

434-
return checkBlackAndWhitelist(name, !_endpointsWhitelist.empty(), _endpointsWhitelistRegex,
444+
auto endpointResult = checkBlackAndWhitelist(endpoint, !_endpointsWhitelist.empty(), _endpointsWhitelistRegex,
435445
!_endpointsBlacklist.empty(), _endpointsBlacklistRegex);
446+
447+
auto urlResult = checkBlackAndWhitelist(url, !_endpointsWhitelist.empty(), _endpointsWhitelistRegex,
448+
!_endpointsBlacklist.empty(), _endpointsBlacklistRegex);
449+
450+
return endpointResult.result || ( urlResult.result && !endpointResult.black);
436451
}
437452

438453
bool V8SecurityFeature::isAllowedToAccessPath(v8::Isolate* isolate, std::string const& path,
@@ -478,5 +493,5 @@ bool V8SecurityFeature::isAllowedToAccessPath(v8::Isolate* isolate, char const*
478493
}
479494

480495
return checkBlackAndWhitelist(path, !_filesWhitelist.empty(), _filesWhitelistRegex,
481-
false, _filesWhitelistRegex /*passed to match the signature but not used*/);
496+
false, _filesWhitelistRegex /*passed to match the signature but not used*/).result;
482497
}

‎lib/ApplicationFeatures/V8SecurityFeature.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class V8SecurityFeature final : public application_features::ApplicationFeature
8484
/// accessible via the JS_Download (internal.download) function in JavaScript
8585
/// actions the endpoint is passed in via protocol (e.g. tcp://, ssl://,
8686
/// unix://) and port number (if applicable)
87-
bool isAllowedToConnectToEndpoint(v8::Isolate* isolate, std::string const& endpoint) const;
87+
bool isAllowedToConnectToEndpoint(v8::Isolate* isolate, std::string const& endpoint, std::string const& url) const;
8888

8989
/// @brief tests if the path (or path component) shall be accessible for the
9090
/// calling JavaScript code

‎lib/V8/v8-utils.cpp‎

Lines changed: 109 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,105 @@ static std::string GetEndpointFromUrl(std::string const& url) {
661661
/// `process-utils.js` depends on simple http client error messages.
662662
/// this needs to be adjusted if this is ever 1E79 changed!
663663
////////////////////////////////////////////////////////////////////////////////
664+
namespace {
665+
666+
auto getEndpoint(v8::Isolate* isolate, std::vector<std::string> const& endpoints,
667+
std::string& url, std::string& lastEndpoint)
668+
-> std::tuple<std::string, std::string, std::string> {
669+
// returns endpoint, relative, error
670+
std::string relative;
671+
std::string endpoint;
672+
if (url.substr(0, 7) == "http://") {
673+
endpoint = GetEndpointFromUrl(url).substr(7);
674+
relative = url.substr(7 + endpoint.length());
675+
676+
if (relative.empty() || relative[0] != '/') {
677+
relative = "/" + relative;
678+
}
679+
if (endpoint.find(':') == std::string::npos) {
680+
endpoint.append(":80");
681+
}
682+
endpoint = "tcp://" + endpoint;
683+
} else if (url.substr(0, 8) == "https://") {
684+
endpoint = GetEndpointFromUrl(url).substr(8);
685+
relative = url.substr(8 + endpoint.length());
686+
687+
if (relative.empty() || relative[0] != '/') {
688+
relative = "/" + relative;
689+
}
690+
if (endpoint.find(':') == std::string::npos) {
691+
endpoint.append(":443");
692+
}
693+
endpoint = "ssl://" + endpoint;
694+
} else if (url.substr(0, 5) == "h2://") {
695+
endpoint = GetEndpointFromUrl(url).substr(5);
696+
relative = url.substr(5 + endpoint.length());
697+
698+
if (relative.empty() || relative[0] != '/') {
699+
relative = "/" + relative;
700+
}
701+
if (endpoint.find(':') == std::string::npos) {
702+
endpoint.append(":80");
703+
}
704+
endpoint = "tcp://" + endpoint;
705+
} else if (url.substr(0, 6) == "h2s://") {
706+
endpoint = GetEndpointFromUrl(url).substr(6);
707+
relative = url.substr(6 + endpoint.length());
708+
709+
if (relative.empty() || relative[0] != '/') {
710+
relative = "/" + relative;
711+
}
712+
if (endpoint.find(':') == std::string::npos) {
713+ 474D
endpoint.append(":443");
714+
}
715+
endpoint = "ssl://" + endpoint;
716+
} else if (url.substr(0, 6) == "srv://") {
717+
size_t found = url.find('/', 6);
718+
719+
relative = "/";
720+
if (found != std::string::npos) {
721+
relative.append(url.substr(found + 1));
722+
endpoint = url.substr(6, found - 6);
723+
} else {
724+
endpoint = url.substr(6);
725+
}
726+
endpoint = "srv://" + endpoint;
727+
} else if (url.substr(0, 7) == "unix://") {
728+
// Can only have arrived here if endpoints is non empty
729+
if (endpoints.empty()) {
730+
return {"", "", std::move("unsupported URL specified")};
731+
}
732+
endpoint = endpoints.front();
733+
relative = url.substr(endpoint.size());
734+
} else if (!url.empty() && url[0] == '/') {
735+
size_t found;
736+
// relative URL. prefix it with last endpoint
737+
relative = url;
738+
url = lastEndpoint + url;
739+
endpoint = lastEndpoint;
740+
if (endpoint.substr(0, 5) == "http:") {
741+
endpoint = endpoint.substr(5);
742+
found = endpoint.find(":");
743+
if (found == std::string::npos) {
744+
endpoint = endpoint + ":80";
745+
}
746+
endpoint = "tcp:" + endpoint;
747+
} else if (endpoint.substr(0, 6) == "https:") {
748+
endpoint = endpoint.substr(6);
749+
found = endpoint.find(":");
750+
if (found == std::string::npos) {
751+
endpoint = endpoint + ":443";
752+
}
753+
endpoint = "ssl:" + endpoint;
754+
}
755+
} else {
756+
std::string msg("unsupported URL specified: '");
757+
msg.append(url).append("'");
758+
return {"", "", std::move(msg)};
759< 474D /td>+
}
760+
return {std::move(endpoint), std::move(relative), ""};
761+
}
762+
} // namespace
664763

665764
void JS_Download(v8::FunctionCallbackInfo<v8::Value> const& args) {
666765
TRI_V8_TRY_CATCH_BEGIN(isolate);
@@ -679,7 +778,8 @@ void JS_Download(v8::FunctionCallbackInfo<v8::Value> const& args) {
679778
TRI_V8_THROW_EXCEPTION_USAGE(signature);
680779
}
681780

682-
std::string url = TRI_ObjectToString(isolate, args[0]);
781+
std::string const inputUrl = TRI_ObjectToString(isolate, args[0]);
782+
std::string url = inputUrl;
683783
std::vector<std::string> endpoints;
684784

685785
bool isLocalUrl = false;
@@ -892,104 +992,18 @@ void JS_Download(v8::FunctionCallbackInfo<v8::Value> const& args) {
892992
int numRedirects = 0;
893993

894994
while (numRedirects < maxRedirects) {
895-
std::string endpoint;
896-
std::string relative;
897-
898-
if (url.substr(0, 7) == "http://") {
899-
endpoint = GetEndpointFromUrl(url).substr(7);
900-
relative = url.substr(7 + endpoint.length());
901-
902-
if (relative.empty() || relative[0] != '/') {
903-
relative = "/" + relative;
904-
}
905-
if (endpoint.find(':') == std::string::npos) {
906-
endpoint.append(":80");
907-
}
908-
endpoint = "tcp://" + endpoint;
909-
} else if (url.substr(0, 8) == "https://") {
910-
endpoint = GetEndpointFromUrl(url).substr(8);
911-
relative = url.substr(8 + endpoint.length());
912-
913-
if (relative.empty() || relative[0] != '/') {
914-
relative = "/" + relative;
915-
}
916-
if (endpoint.find(':') == std::string::npos) {
917-
endpoint.append(":443");
918-
}
919-
endpoint = "ssl://" + endpoint;
920-
} < 474D span class="pl-k">else if (url.substr(0, 5) == "h2://") {
921-
endpoint = GetEndpointFromUrl(url).substr(5);
922-
relative = url.substr(5 + endpoint.length());
923995

924-
if (relative.empty() || relative[0] != '/') {
925-
relative = "/" + relative;
926-
}
927-
if (endpoint.find(':') == std::string::npos) {
928-
endpoint.append(":80");
929-
}
930-
endpoint = "tcp://" + endpoint;
931-
} else if (url.substr(0, 6) == "h2s://") {
932-
endpoint = GetEndpointFromUrl(url).substr(6);
933-
relative = url.substr(6 + endpoint.length());
934-
935-
if (relative.empty() || relative[0] != '/') {
936-
relative = "/" + relative;
937-
}
938-
if (endpoint.find(':') == std::string::npos) {
939-
endpoint.append(":80");
940-
}
941-
endpoint = "tcp://" + endpoint;
942-
} else if (url.substr(0, 6) == "srv://") {
943-
size_t found = url.find('/', 6);
944-
945-
relative = "/";
946-
if (found != std::string::npos) {
947-
relative.append(url.substr(found + 1));
948-
endpoint = url.substr(6, found - 6);
949-
} else {
950-
endpoint = url.substr(6);
951-
}
952-
endpoint = "srv://" + endpoint;
953-
} else if (url.substr(0, 7) == "unix://") {
954-
// Can only have arrived here if endpoints is non empty
955-
if (endpoints.empty()) {
956-
TRI_V8_THROW_SYNTAX_ERROR("unsupported URL specified");
957-
}
958-
endpoint = endpoints.front();
959-
relative = url.substr(endpoint.size());
960-
} else if (!url.empty() && url[0] == '/') {
961-
size_t found;
962-
// relative URL. prefix it with last endpoint
963-
relative = url;
964-
url = lastEndpoint + url;
965-
endpoint = lastEndpoint;
966-
if (endpoint.substr(0, 5) == "http:") {
967-
endpoint = endpoint.substr(5);
968-
found = endpoint.find(":");
969-
if (found == std::string::npos) {
970-
endpoint = endpoint + ":80";
971-
}
972-
endpoint = "tcp:" + endpoint;
973-
} else if (endpoint.substr(0, 6) == "https:") {
974-
endpoint = endpoint.substr(6);
975-
found = endpoint.find(":");
976-
if (found == std::string::npos) {
977-
endpoint = endpoint + ":443";
978-
}
979-
endpoint = "ssl:" + endpoint;
980-
}
981-
} else {
982-
std::string msg("unsupported URL specified: '");
983-
msg.append(url).append("'");
984-
TRI_V8_THROW_ERROR(msg.c_str());
996+
auto [endpoint, relative, error] = getEndpoint(isolate, endpoints, url, lastEndpoint);
997+
if(!error.empty()) {
998+
TRI_V8_THROW_SYNTAX_ERROR(error.c_str());
985999
}
9861000

9871001
LOG_TOPIC("d6bdb", TRACE, arangodb::Logger::FIXME)
988-
<< "downloading file. endpoint: " << endpoint << ", relative URL: " << url;
1002+
<< "downloading file. endpoint: " << endpoint << ", relative URL: " << url;
9891003

990-
if (!isLocalUrl && !v8security.isAllowedToConnectToEndpoint(isolate, endpoint)) {
1004+
if (!isLocalUrl && !v8security.isAllowedToConnectToEndpoint(isolate, endpoint, inputUrl)) {
9911005
TRI_V8_THROW_EXCEPTION_MESSAGE(TRI_ERROR_FORBIDDEN,
992-
"not allowed to connect to this endpoint");
1006+
"not allowed to connect to this URL: " + inputUrl);
9931007
}
9941008

9951009
std::unique_ptr<Endpoint> ep(Endpoint::clientFactory(endpoint));
@@ -1000,8 +1014,8 @@ void JS_Download(v8::FunctionCallbackInfo<v8::Value> const& args) {
10001014
}
10011015

10021016
std::unique_ptr<GeneralClientConnection> connection(
1003-
GeneralClientConnection::factory(v8g->_server, ep.get(), timeout,
1004-
timeout, 3, sslProtocol));
1017+
GeneralClientConnection::factory(v8g->_server, ep.get(), timeout,
1018+
timeout, 3, sslProtocol));
10051019

10061020
if (connection == nullptr) {
10071021
TRI_V8_THROW_EXCEPTION_M 38DD EMORY();

0 commit comments

Comments
 (0)
0