Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2226,3 +2226,10 @@ if(EXISTS "${_VLLM_CDNA_TEST_SRC}")
target_link_libraries(test_vllm_cdna_gating PRIVATE lemonade-server-core)
add_test(NAME VllmCdnaGatingTest COMMAND test_vllm_cdna_gating)
endif()

set(_HTTP_CLIENT_SECURITY_TEST_SRC "${CMAKE_CURRENT_SOURCE_DIR}/test/cpp/test_http_client_security.cpp")
if(EXISTS "${_HTTP_CLIENT_SECURITY_TEST_SRC}")
add_executable(test_http_client_security test/cpp/test_http_client_security.cpp)
target_link_libraries(test_http_client_security PRIVATE lemonade-server-core)
add_test(NAME HttpClientSecurityTest COMMAND test_http_client_security)
endif()
3 changes: 2 additions & 1 deletion src/cpp/include/lemon/backends/cloud/cloud_server.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ class CloudServer : public WrappedServer {
/// can continue with other providers.
static std::vector<ModelInfo> discover_models(const std::string& provider,
const std::string& api_key,
const std::string& base_url);
const std::string& base_url,
bool allow_insecure_http = false);

private:
struct ResolvedCreds {
Expand Down
26 changes: 23 additions & 3 deletions src/cpp/include/lemon/utils/http_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,23 @@ struct DownloadResult {
using ProgressCallback = std::function<bool(size_t downloaded, size_t total)>;
using StreamCallback = std::function<bool(const char* data, size_t length)>;

// Trust boundary for HTTP requests that may follow redirects. Selects which URL
// schemes are allowed for the initial request and for redirect targets, and
// whether redirects are followed at all.
enum class HttpSecurityPolicy {
// External hosts (Hugging Face, GitHub, release CDNs). Require https for the
// initial request and every redirect hop; bound the redirect chain. This is
// the default so untrusted destinations are HTTPS-only unless a caller opts
// out for a specific trust boundary.
ExternalHttpsOnly,
// Lemonade-managed backends reached over loopback http://127.0.0.1. Require
// http and never follow redirects.
TrustedLoopback,
// User-configured plaintext endpoints that explicitly opted in via
// allow_insecure_http. Permit http and https; bound the redirect chain.
AllowInsecureHttp,
};

// Download configuration options
struct DownloadOptions {
int max_retries = 5; // Maximum retry attempts
Expand Down Expand Up @@ -86,7 +103,8 @@ class HttpClient {
// Simple GET request. timeout_seconds=0 (default) uses default_timeout_seconds_.
static HttpResponse get(const std::string& url,
const std::map<std::string, std::string>& headers = {},
long timeout_seconds = 0);
long timeout_seconds = 0,
HttpSecurityPolicy policy = HttpSecurityPolicy::ExternalHttpsOnly);

// Simple POST request
static HttpResponse post(const std::string& url,
Expand Down Expand Up @@ -114,7 +132,8 @@ class HttpClient {
const std::string& output_path,
ProgressCallback callback = nullptr,
const std::map<std::string, std::string>& headers = {},
const DownloadOptions& options = DownloadOptions());
const DownloadOptions& options = DownloadOptions(),
HttpSecurityPolicy policy = HttpSecurityPolicy::ExternalHttpsOnly);

// Check if URL is reachable
static bool is_reachable(const std::string& url, int timeout_seconds = 5);
Expand All @@ -129,7 +148,8 @@ class HttpClient {
ProgressCallback callback,
const std::map<std::string, std::string>& headers,
const DownloadOptions& options,
bool initial_range_request);
bool initial_range_request,
HttpSecurityPolicy policy);
};

// Creates a throttled progress callback that prints at most once per second.
Expand Down
6 changes: 4 additions & 2 deletions src/cpp/server/backends/acestep/acestep_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ bool AceStepServer::run_job(const std::string& path, const std::string& body,
const std::string job_url = base + "/job?id=" + job_id;
std::string status;
for (int i = 0; i < 1200; ++i) {
auto poll = utils::HttpClient::get(job_url, {}, 30);
auto poll = utils::HttpClient::get(job_url, {}, 30,
utils::HttpSecurityPolicy::TrustedLoopback);
if (poll.status_code == 200) {
try { status = json::parse(poll.body).value("status", std::string()); }
catch (...) { status.clear(); }
Expand All @@ -220,7 +221,8 @@ bool AceStepServer::run_job(const std::string& path, const std::string& body,
return false;
}

auto fetched = utils::HttpClient::get(job_url + "&result=1", {}, 120);
auto fetched = utils::HttpClient::get(job_url + "&result=1", {}, 120,
utils::HttpSecurityPolicy::TrustedLoopback);
if (fetched.status_code != 200) {
LOG(ERROR, "acestep-server") << "fetching " << stage << " job result failed (HTTP "
<< fetched.status_code << ")" << std::endl;
Expand Down
16 changes: 13 additions & 3 deletions src/cpp/server/backends/cloud/cloud_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,8 @@ void CloudServer::forward_streaming_request(const std::string& endpoint,

std::vector<ModelInfo> CloudServer::discover_models(const std::string& provider,
const std::string& api_key,
const std::string& base_url) {
const std::string& base_url,
bool allow_insecure_http) {
std::vector<ModelInfo> models;
if (api_key.empty()) {
return models;
Expand All @@ -759,13 +760,21 @@ std::vector<ModelInfo> CloudServer::discover_models(const std::string& provider,
{"Authorization", "Bearer " + api_key}
};

// HTTPS providers and remote endpoints use the default external
// HTTPS-only policy. Plaintext http:// providers that explicitly set
// allow_insecure_http use the AllowInsecureHttp opt-in so http and https
// are accepted but redirect chains remain bounded.

utils::HttpResponse response;
try {
// Short timeout: this runs synchronously inside cache build, once per
// configured provider. The 300 s default would block model listing
// for minutes if a provider's API is unreachable. 15 s is plenty for
// a /v1/models response under normal conditions.
response = utils::HttpClient::get(url, headers, /*timeout_seconds=*/15);
response = utils::HttpClient::get(url, headers, /*timeout_seconds=*/15,
allow_insecure_http
? utils::HttpSecurityPolicy::AllowInsecureHttp
: utils::HttpSecurityPolicy::ExternalHttpsOnly);
} catch (const std::exception& e) {
LOG(WARNING, "Cloud") << "Model discovery failed for provider '" << provider
<< "': " << e.what() << std::endl;
Expand Down Expand Up @@ -901,7 +910,8 @@ class CloudOps : public BackendOps {
continue;
}
try {
for (auto& m : CloudServer::discover_models(rec.name, api_key, rec.base_url)) {
for (auto& m : CloudServer::discover_models(rec.name, api_key, rec.base_url,
rec.allow_insecure_http)) {
if (m.recipe == "cloud" && !m.model_name.empty()) {
out.push_back(std::move(m));
}
Expand Down
3 changes: 2 additions & 1 deletion src/cpp/server/backends/vllm/vllm_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,8 @@ std::map<std::string, nlohmann::json> VLLMServer::get_additional_telemetry() {

std::string url = "http://127.0.0.1:" + std::to_string(get_backend_port()) + "/metrics";
try {
auto response = utils::HttpClient::get(url, {}, 1);
auto response = utils::HttpClient::get(url, {}, 1,
utils::HttpSecurityPolicy::TrustedLoopback);
if (response.status_code == 200) {
return parse_vllm_metrics_text(response.body);
}
Expand Down
4 changes: 3 additions & 1 deletion src/cpp/server/model_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2334,7 +2334,9 @@ size_t ModelManager::refresh_cloud_models(const std::string& provider) {
// provider and we don't want to block /models on it.
std::vector<ModelInfo> models;
try {
models = backends::CloudServer::discover_models(provider, api_key, base_url);
models = backends::CloudServer::discover_models(
provider, api_key, base_url,
cloud_registry_->allow_insecure_http_for(provider));
} catch (const std::exception& e) {
LOG(WARNING, "ModelManager") << "Cloud discovery threw for provider '"
<< provider << "': " << e.what() << std::endl;
Expand Down
3 changes: 2 additions & 1 deletion src/cpp/server/telemetry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,8 @@ class MetricsWorker {

if (!task.metrics_url.empty() && task.parser) {
try {
auto response = utils::HttpClient::get(task.metrics_url, {}, 1);
auto response = utils::HttpClient::get(
task.metrics_url, {}, 1, utils::HttpSecurityPolicy::TrustedLoopback);
if (response.status_code == 200) {
auto extra = task.parser(response.body);
for (const auto& [k, v] : extra) {
Expand Down
57 changes: 50 additions & 7 deletions src/cpp/server/utils/http_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,9 +358,37 @@ static int progress_callback(void* clientp, curl_off_t dltotal, curl_off_t dlnow
return 0;
}

namespace {
// Applies the scheme and redirect restrictions for a trust boundary. Returns
// false if any security-relevant option fails to apply so callers can fail
// closed instead of issuing an unrestricted request.
bool apply_http_security_policy(CURL* curl, HttpSecurityPolicy policy) {
auto set = [curl](CURLoption opt, auto value) {
return curl_easy_setopt(curl, opt, value) == CURLE_OK;
};
switch (policy) {
case HttpSecurityPolicy::TrustedLoopback:
return set(CURLOPT_FOLLOWLOCATION, 0L) &&
set(CURLOPT_PROTOCOLS_STR, "http");
case HttpSecurityPolicy::AllowInsecureHttp:
return set(CURLOPT_FOLLOWLOCATION, 1L) &&
set(CURLOPT_MAXREDIRS, 5L) &&
set(CURLOPT_PROTOCOLS_STR, "http,https") &&
set(CURLOPT_REDIR_PROTOCOLS_STR, "http,https");
case HttpSecurityPolicy::ExternalHttpsOnly:
default:
return set(CURLOPT_FOLLOWLOCATION, 1L) &&
set(CURLOPT_MAXREDIRS, 5L) &&
set(CURLOPT_PROTOCOLS_STR, "https") &&
set(CURLOPT_REDIR_PROTOCOLS_STR, "https");
}
}
} // namespace

HttpResponse HttpClient::get(const std::string& url,
const std::map<std::string, std::string>& headers,
long timeout_seconds) {
long timeout_seconds,
HttpSecurityPolicy policy) {
CURL* curl = curl_easy_init();
if (!curl) {
throw std::runtime_error("Failed to initialize CURL");
Expand All @@ -372,7 +400,10 @@ HttpResponse HttpClient::get(const std::string& url,
curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_callback);
curl_easy_setopt(curl, CURLOPT_WRITEDATA, &response_body);
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
if (!apply_http_security_policy(curl, policy)) {
curl_easy_cleanup(curl);
throw std::runtime_error("Failed to apply HTTP security policy");
}
curl_easy_setopt(curl, CURLOPT_TIMEOUT,
timeout_seconds > 0 ? timeout_seconds : default_timeout_seconds_.load());
curl_easy_setopt(curl, CURLOPT_USERAGENT, "lemon.cpp/1.0");
Expand Down Expand Up @@ -651,7 +682,8 @@ DownloadResult HttpClient::download_attempt(const std::string& url,
ProgressCallback callback,
const std::map<std::string, std::string>& headers,
const DownloadOptions& options,
bool initial_range_request) {
bool initial_range_request,
HttpSecurityPolicy policy) {
DownloadResult result;

CURL* curl = curl_easy_init();
Expand All @@ -677,7 +709,12 @@ DownloadResult HttpClient::download_attempt(const std::string& url,
curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_file_callback);
curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp);
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
if (!apply_http_security_policy(curl, policy)) {
result.error_message = "Failed to apply HTTP security policy";
fclose(fp);
curl_easy_cleanup(curl);
return result;
}
curl_easy_setopt(curl, CURLOPT_TIMEOUT, 0L);
curl_easy_setopt(curl, CURLOPT_USERAGENT, "lemon.cpp/1.0");
curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, static_cast<long>(options.connect_timeout));
Expand Down Expand Up @@ -833,7 +870,12 @@ DownloadResult HttpClient::download_attempt(const std::string& url,
if (head_curl) {
curl_easy_setopt(head_curl, CURLOPT_URL, url.c_str());
curl_easy_setopt(head_curl, CURLOPT_NOBODY, 1L); // HEAD request
curl_easy_setopt(head_curl, CURLOPT_FOLLOWLOCATION, 1L);
if (!apply_http_security_policy(head_curl, policy)) {
curl_easy_cleanup(head_curl);
result.error_message = "Resume failed (HTTP 416) - could not apply security policy";
result.can_resume = false;
return result;
}
curl_easy_setopt(head_curl, CURLOPT_TIMEOUT, 30L);

// Add headers
Expand Down Expand Up @@ -899,7 +941,8 @@ DownloadResult HttpClient::download_file(const std::string& url,
const std::string& output_path,
ProgressCallback callback,
const std::map<std::string, std::string>& headers,
const DownloadOptions& options) {
const DownloadOptions& options,
HttpSecurityPolicy policy) {
DownloadResult final_result;
int retry_delay_ms = options.initial_retry_delay_ms;
const ExpectedHash expected_hash = parse_expected_hash(options);
Expand Down Expand Up @@ -1022,7 +1065,7 @@ DownloadResult HttpClient::download_file(const std::string& url,

final_result = download_attempt(url, partial_path, resume_offset,
adjusted_callback, headers, options,
initial_range_request);
initial_range_request, policy);

// If cancelled by user, return immediately without retrying
if (final_result.cancelled) {
Expand Down
3 changes: 2 additions & 1 deletion src/cpp/server/wrapped_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,8 @@ json WrappedServer::forward_get_request(const std::string& endpoint, long timeou
std::map<std::string, std::string> headers;

try {
auto response = utils::HttpClient::get(url, headers);
auto response = utils::HttpClient::get(url, headers, 0,
utils::HttpSecurityPolicy::TrustedLoopback);
note_backend_activity();

if (response.status_code == 200) {
Expand Down
Loading
Loading