From e5e647f675cab69b0aaf2196c09b651ea25f71f0 Mon Sep 17 00:00:00 2001 From: exzile Date: Fri, 26 Jun 2026 21:52:52 -0400 Subject: [PATCH 1/2] Add native TLS for the gRPC endpoint (REST gated) Implements native TLS/HTTPS for the API endpoints (issue #2144) so traffic can be encrypted without relying on an external reverse proxy. gRPC TLS (functional): - New per-protocol CLI params (optional, empty = disabled): grpc_certificate_path, grpc_key_path, and grpc_ca_path. Setting cert+key enables server TLS; adding a CA enables mutual TLS (client certificates are required and verified). - grpcservermodule builds grpc::SslServerCredentials from the cert/key (and CA for mTLS via GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY). No insecure fallback: a missing/unreadable cert/key aborts startup rather than serving plaintext. REST TLS (gated, fail-closed): - The same rest_certificate_path/rest_key_path/rest_ca_path params and Drogon SSL wiring are in place, but the bundled Drogon is built without OpenSSL and cannot serve HTTPS. To avoid silently serving plaintext on a port the operator believes is encrypted, Config::validate() rejects any rest TLS parameter at startup. The wiring is retained so REST HTTPS activates once Drogon is built with OpenSSL. Also: - ServerSettings fields, Config accessors, and C-API setters (OVMS_ServerSettingsSet*) for all six params, declared in ovms.h. - Validation: cert/key must be set together, files must exist and be non-empty, CA requires cert+key. Runs on both CLI and C-API start paths. - Private key contents are never logged; only paths appear in logs/errors. Tested: config-validation death tests (cert/key/ca combinations, missing/empty files, REST-gated rejection), C-API setter coverage, and an end-to-end gRPC TLS handshake verified with openssl s_client (TLS 1.2, server cert presented). Reviewed for security (no plaintext fallback, mTLS enforced, no key leakage). Implements #2144 Co-Authored-By: Claude Opus 4.8 --- docs/parameters.md | 4 ++ docs/security_considerations.md | 4 +- src/capi_frontend/capi.cpp | 78 ++++++++++++++++++++++ src/capi_frontend/server_settings.hpp | 6 ++ src/cli_parser.cpp | 39 ++++++++++- src/config.cpp | 78 ++++++++++++++++++++++ src/config.hpp | 9 +++ src/drogon_http_server.cpp | 26 +++++++- src/drogon_http_server.hpp | 8 ++- src/grpcservermodule.cpp | 34 +++++++++- src/http_server.cpp | 2 +- src/ovms.h | 69 +++++++++++++++++++ src/test/c_api_tests.cpp | 48 ++++++++++++++ src/test/ovmsconfig_test.cpp | 95 +++++++++++++++++++++++++++ 14 files changed, 492 insertions(+), 8 deletions(-) diff --git a/docs/parameters.md b/docs/parameters.md index edfddafa7e..c0a3c6c3d8 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -37,6 +37,10 @@ Configuration options for the server are defined only via command-line options a | `rest_port` | `integer` | Number of the port used by HTTP server (if not provided or set to 0, HTTP server will not be launched). | | `grpc_bind_address` | `string` | Comma separated list of ipv4/ipv6 network interface addresses or hostnames, to which gRPC server will bind to. Default: all interfaces: 0.0.0.0 | | `rest_bind_address` | `string` | Comma separated list of ipv4/ipv6 network interface addresses or hostnames, to which REST server will bind to. Default: all interfaces: 0.0.0.0 | +| `grpc_certificate_path` | `string` | Path to a PEM server certificate to enable TLS on the gRPC endpoint. Must be set together with `grpc_key_path`. When unset, gRPC is served in plaintext. | +| `grpc_key_path` | `string` | Path to the PEM private key matching `grpc_certificate_path`. Required to enable gRPC TLS. | +| `grpc_ca_path` | `string` | Optional path to a PEM CA certificate. When set, enables mutual TLS (mTLS) on the gRPC endpoint — clients must present a certificate signed by this CA, which is required and verified. Requires `grpc_certificate_path` and `grpc_key_path`. | +| `rest_certificate_path` / `rest_key_path` / `rest_ca_path` | `string` | TLS for the REST endpoint. **Note:** native REST HTTPS is not enabled in the current build (the bundled Drogon is built without OpenSSL); setting these parameters is rejected at startup to avoid silently serving plaintext. Use gRPC TLS, or terminate REST TLS with a reverse proxy. See [issue #2144](https://github.com/openvinotoolkit/model_server/issues/2144). | | `grpc_workers` | `integer` | Number of the gRPC server instances (must be from 1 to CPU core count). Default value is 1 and it's optimal for most use cases. Consider setting higher value while expecting heavy load. | | `rest_workers` | `integer` | Number of HTTP server threads. Effective when `rest_port` > 0. Default value is set based on the number of CPUs. | | `file_system_poll_wait_seconds` | `integer` | Time interval between config and model versions changes detection in seconds. Default value is 1. Zero value disables changes monitoring. | diff --git a/docs/security_considerations.md b/docs/security_considerations.md index 56f33c2a87..73ae334d9e 100644 --- a/docs/security_considerations.md +++ b/docs/security_considerations.md @@ -11,7 +11,9 @@ docker run --rm -d --user $(id -u):$(id -g) --read-only --tmpfs /tmp -p 9000:900 ``` --- -OpenVINO Model Server currently does not provide access restrictions and traffic encryption on gRPC and REST API endpoints. The endpoints can be secured using network settings like docker network settings or network firewall on the host. The recommended configuration is to place OpenVINO Model Server behind any reverse proxy component or load balancer, which provides traffic encryption and user authorization. +OpenVINO Model Server does not provide user authorization on the gRPC and REST API endpoints. The endpoints can be secured using network settings like docker network settings or network firewall on the host. The recommended configuration is to place OpenVINO Model Server behind any reverse proxy component or load balancer, which provides traffic encryption and user authorization. + +The gRPC endpoint additionally supports native TLS traffic encryption. Provide a PEM server certificate and key via `--grpc_certificate_path` and `--grpc_key_path` to serve gRPC over TLS, and optionally `--grpc_ca_path` to require and verify client certificates (mutual TLS). See [parameters](parameters.md). Native REST HTTPS is not enabled in the current build (setting `--rest_certificate_path`/`--rest_key_path` is rejected at startup rather than silently serving plaintext); terminate REST TLS with a reverse proxy. When deploying in environments where only local access is required, administrators can configure the server to bind exclusively to localhost addresses. This can be achieved by setting the bind address to `127.0.0.1` for IPv4 or `::1` for IPv6, which restricts incoming connections to the local machine only. This configuration prevents external network access to the server endpoints, providing an additional layer of security for local development or testing environments. ``` diff --git a/src/capi_frontend/capi.cpp b/src/capi_frontend/capi.cpp index 48b7b4101a..c39a2a3f65 100644 --- a/src/capi_frontend/capi.cpp +++ b/src/capi_frontend/capi.cpp @@ -606,6 +606,84 @@ DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetAllowedMediaDomains(OVMS_ServerSet return nullptr; } +DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetGrpcCertPath(OVMS_ServerSettings* settings, + const char* grpc_cert_path) { + if (settings == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "server settings")); + } + if (grpc_cert_path == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "grpc certificate path")); + } + ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); + serverSettings->grpcCertPath.assign(grpc_cert_path); + return nullptr; +} + +DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetGrpcKeyPath(OVMS_ServerSettings* settings, + const char* grpc_key_path) { + if (settings == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "server settings")); + } + if (grpc_key_path == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "grpc key path")); + } + ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); + serverSettings->grpcKeyPath.assign(grpc_key_path); + return nullptr; +} + +DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetGrpcCaPath(OVMS_ServerSettings* settings, + const char* grpc_ca_path) { + if (settings == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "server settings")); + } + if (grpc_ca_path == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "grpc CA path")); + } + ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); + serverSettings->grpcCaPath.assign(grpc_ca_path); + return nullptr; +} + +DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetRestCertPath(OVMS_ServerSettings* settings, + const char* rest_cert_path) { + if (settings == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "server settings")); + } + if (rest_cert_path == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "rest certificate path")); + } + ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); + serverSettings->restCertPath.assign(rest_cert_path); + return nullptr; +} + +DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetRestKeyPath(OVMS_ServerSettings* settings, + const char* rest_key_path) { + if (settings == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "server settings")); + } + if (rest_key_path == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "rest key path")); + } + ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); + serverSettings->restKeyPath.assign(rest_key_path); + return nullptr; +} + +DLL_PUBLIC OVMS_Status* OVMS_ServerSettingsSetRestCaPath(OVMS_ServerSettings* settings, + const char* rest_ca_path) { + if (settings == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "server settings")); + } + if (rest_ca_path == nullptr) { + return reinterpret_cast(new Status(StatusCode::NONEXISTENT_PTR, "rest CA path")); + } + ovms::ServerSettingsImpl* serverSettings = reinterpret_cast(settings); + serverSettings->restCaPath.assign(rest_ca_path); + return nullptr; +} + DLL_PUBLIC OVMS_Status* OVMS_ModelsSettingsSetConfigPath(OVMS_ModelsSettings* settings, const char* config_path) { if (settings == nullptr) { diff --git a/src/capi_frontend/server_settings.hpp b/src/capi_frontend/server_settings.hpp index 2dda86a6f4..9e5d486e78 100644 --- a/src/capi_frontend/server_settings.hpp +++ b/src/capi_frontend/server_settings.hpp @@ -241,6 +241,12 @@ struct ServerSettingsImpl { uint32_t filesystemPollWaitMilliseconds = 1000; uint32_t resourcesCleanerPollWaitSeconds = 300; std::string cacheDir; + std::string grpcCertPath; + std::string grpcKeyPath; + std::string grpcCaPath; + std::string restCertPath; + std::string restKeyPath; + std::string restCaPath; bool withPython = false; bool startedWithCLI = false; ConfigExportType exportConfigType = UNKNOWN_MODEL; diff --git a/src/cli_parser.cpp b/src/cli_parser.cpp index e05109d905..dbf588ad64 100644 --- a/src/cli_parser.cpp +++ b/src/cli_parser.cpp @@ -175,7 +175,31 @@ std::variant> CLIParser::parse(int argc, char* ("api_key_file", "path to the text file containing API key for authentication for generative endpoints. If not set, authentication is disabled.", cxxopts::value()->default_value(""), - "API_KEY"); + "API_KEY") + ("grpc_certificate_path", + "Path to the PEM-encoded server certificate for gRPC TLS. Must be set together with grpc_key_path to enable TLS.", + cxxopts::value(), + "GRPC_CERTIFICATE_PATH") + ("grpc_key_path", + "Path to the PEM-encoded private key for gRPC TLS. Must be set together with grpc_certificate_path to enable TLS.", + cxxopts::value(), + "GRPC_KEY_PATH") + ("grpc_ca_path", + "Path to the PEM-encoded CA certificate for gRPC mutual TLS (mTLS). Requires grpc_certificate_path and grpc_key_path. When set, client certificates are required and verified.", + cxxopts::value(), + "GRPC_CA_PATH") + ("rest_certificate_path", + "Path to the PEM-encoded server certificate for REST TLS (HTTPS). Must be set together with rest_key_path to enable TLS.", + cxxopts::value(), + "REST_CERTIFICATE_PATH") + ("rest_key_path", + "Path to the PEM-encoded private key for REST TLS (HTTPS). Must be set together with rest_certificate_path to enable TLS.", + cxxopts::value(), + "REST_KEY_PATH") + ("rest_ca_path", + "Path to the PEM-encoded CA certificate for REST client-certificate verification. Requires rest_certificate_path and rest_key_path. Note: mTLS for REST requires Drogon TLS support.", + cxxopts::value(), + "REST_CA_PATH"); options->add_options("multi model") ("config_path", @@ -532,6 +556,19 @@ void CLIParser::prepareServer(ServerSettingsImpl& serverSettings) { if (result->count("rest_bind_address")) serverSettings.restBindAddress = result->operator[]("rest_bind_address").as(); + if (result->count("grpc_certificate_path")) + serverSettings.grpcCertPath = result->operator[]("grpc_certificate_path").as(); + if (result->count("grpc_key_path")) + serverSettings.grpcKeyPath = result->operator[]("grpc_key_path").as(); + if (result->count("grpc_ca_path")) + serverSettings.grpcCaPath = result->operator[]("grpc_ca_path").as(); + if (result->count("rest_certificate_path")) + serverSettings.restCertPath = result->operator[]("rest_certificate_path").as(); + if (result->count("rest_key_path")) + serverSettings.restKeyPath = result->operator[]("rest_key_path").as(); + if (result->count("rest_ca_path")) + serverSettings.restCaPath = result->operator[]("rest_ca_path").as(); + if (result->count("grpc_max_threads")) serverSettings.grpcMaxThreads = result->operator[]("grpc_max_threads").as(); diff --git a/src/config.cpp b/src/config.cpp index aa888aa4de..7ae77da506 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -352,6 +352,78 @@ bool Config::validate() { return false; } + // check TLS paths: + bool grpcHasCert = !this->serverSettings.grpcCertPath.empty(); + bool grpcHasKey = !this->serverSettings.grpcKeyPath.empty(); + bool grpcHasCa = !this->serverSettings.grpcCaPath.empty(); + bool restHasCert = !this->serverSettings.restCertPath.empty(); + bool restHasKey = !this->serverSettings.restKeyPath.empty(); + bool restHasCa = !this->serverSettings.restCaPath.empty(); + + // A TLS material file must exist and be non-empty (an empty/truncated cert or key + // would otherwise pass and later cause an opaque gRPC bind failure). + auto tlsFileUsable = [](const std::string& p) { + std::error_code ec; + return std::filesystem::exists(p) && std::filesystem::file_size(p, ec) > 0 && !ec; + }; + + if (grpcHasCert != grpcHasKey) { + std::cerr << "grpc_certificate_path and grpc_key_path must both be set to enable gRPC TLS" << std::endl; + return false; + } + if (grpcHasCert && !tlsFileUsable(this->serverSettings.grpcCertPath)) { + std::cerr << "File path provided as --grpc_certificate_path does not exist or is empty: " << this->serverSettings.grpcCertPath << std::endl; + return false; + } + if (grpcHasKey && !tlsFileUsable(this->serverSettings.grpcKeyPath)) { + std::cerr << "File path provided as --grpc_key_path does not exist or is empty: " << this->serverSettings.grpcKeyPath << std::endl; + return false; + } + if (grpcHasCa) { + if (!grpcHasCert) { + std::cerr << "grpc_ca_path requires grpc_certificate_path and grpc_key_path to be set" << std::endl; + return false; + } + if (!tlsFileUsable(this->serverSettings.grpcCaPath)) { + std::cerr << "File path provided as --grpc_ca_path does not exist or is empty: " << this->serverSettings.grpcCaPath << std::endl; + return false; + } + } + + // REST TLS is gated: the bundled Drogon is currently built without OpenSSL, so it + // cannot serve HTTPS (enabling SSL would silently fall back to plaintext). Fail + // closed rather than expose a plaintext endpoint a user believes is encrypted. + // The REST TLS wiring (addListener SSL) is in place and will activate once Drogon + // is built with OpenSSL; remove this guard at that point. Use gRPC TLS or a + // TLS-terminating proxy for REST in the meantime. See issue #2144. + if (restHasCert || restHasKey || restHasCa) { + std::cerr << "REST TLS (rest_certificate_path/rest_key_path/rest_ca_path) is not supported in this build because the bundled Drogon was built without OpenSSL. Use gRPC TLS (grpc_certificate_path/grpc_key_path) or terminate REST TLS with a proxy." << std::endl; + return false; + } + + if (restHasCert != restHasKey) { + std::cerr << "rest_certificate_path and rest_key_path must both be set to enable REST TLS" << std::endl; + return false; + } + if (restHasCert && !std::filesystem::exists(this->serverSettings.restCertPath)) { + std::cerr << "File path provided as --rest_certificate_path does not exist: " << this->serverSettings.restCertPath << std::endl; + return false; + } + if (restHasKey && !std::filesystem::exists(this->serverSettings.restKeyPath)) { + std::cerr << "File path provided as --rest_key_path does not exist: " << this->serverSettings.restKeyPath << std::endl; + return false; + } + if (restHasCa) { + if (!restHasCert) { + std::cerr << "rest_ca_path requires rest_certificate_path and rest_key_path to be set" << std::endl; + return false; + } + if (!std::filesystem::exists(this->serverSettings.restCaPath)) { + std::cerr << "File path provided as --rest_ca_path does not exist: " << this->serverSettings.restCaPath << std::endl; + return false; + } + } + // check bind addresses: if (!restBindAddress().empty() && check_hostname_or_ip(restBindAddress()) == false) { std::cerr << "rest_bind_address has invalid format: proper hostname or IP address expected." << std::endl; @@ -427,5 +499,11 @@ const std::string& Config::allowedMethods() const { return this->serverSettings. const std::string& Config::allowedHeaders() const { return this->serverSettings.allowedHeaders; } const std::string Config::cacheDir() const { return this->serverSettings.cacheDir; } const std::string& Config::apiKey() const { return this->serverSettings.apiKey; } +const std::string Config::grpcCertPath() const { return this->serverSettings.grpcCertPath; } +const std::string Config::grpcKeyPath() const { return this->serverSettings.grpcKeyPath; } +const std::string Config::grpcCaPath() const { return this->serverSettings.grpcCaPath; } +const std::string Config::restCertPath() const { return this->serverSettings.restCertPath; } +const std::string Config::restKeyPath() const { return this->serverSettings.restKeyPath; } +const std::string Config::restCaPath() const { return this->serverSettings.restCaPath; } } // namespace ovms diff --git a/src/config.hpp b/src/config.hpp index 881cccf7f3..34da977683 100644 --- a/src/config.hpp +++ b/src/config.hpp @@ -324,6 +324,15 @@ class Config { * @return const std::string& */ const std::string cacheDir() const; + + // TLS configuration accessors + const std::string grpcCertPath() const; + const std::string grpcKeyPath() const; + const std::string grpcCaPath() const; + const std::string restCertPath() const; + const std::string restKeyPath() const; + const std::string restCaPath() const; + bool startedFromCLI() { return serverSettings.startedWithCLI; } diff --git a/src/drogon_http_server.cpp b/src/drogon_http_server.cpp index c4c976b64a..9582fca411 100644 --- a/src/drogon_http_server.cpp +++ b/src/drogon_http_server.cpp @@ -32,12 +32,15 @@ namespace ovms { -DrogonHttpServer::DrogonHttpServer(size_t numWorkersForUnary, size_t numWorkersForStreaming, int port, const std::string& address) : +DrogonHttpServer::DrogonHttpServer(size_t numWorkersForUnary, size_t numWorkersForStreaming, int port, const std::string& address, const std::string& certPath, const std::string& keyPath, const std::string& caPath) : numWorkersForUnary(numWorkersForUnary), numWorkersForStreaming(numWorkersForStreaming), pool(std::make_unique("DrogonThreadPool", numWorkersForStreaming)), port(port), - address(address) { + address(address), + certPath(certPath), + keyPath(keyPath), + caPath(caPath) { SPDLOG_DEBUG("Starting http thread pool for streaming ({} threads)", numWorkersForStreaming); pool->StartWorkers(); // this tp is for streaming workload which cannot use drogon's internal listener threads SPDLOG_DEBUG("Thread pool started"); @@ -153,9 +156,26 @@ Status DrogonHttpServer::startAcceptingRequests() { }); auto ips = ovms::tokenize(this->address, ','); + const bool useTls = !this->certPath.empty() && !this->keyPath.empty(); + if (useTls) { + if (!this->caPath.empty()) { + SPDLOG_INFO("REST TLS enabled with client certificate verification (mTLS)"); + } else { + SPDLOG_INFO("REST TLS enabled (server-only TLS, no client certificate verification)"); + } + } for (const auto& ip : ips) { SPDLOG_INFO("Binding REST server to address: {}:{}", ip, this->port); - drogon::app().addListener(ip, this->port); + if (useTls) { + std::vector> sslConfCmds; + if (!this->caPath.empty()) { + sslConfCmds.push_back({"CAfile", this->caPath}); + sslConfCmds.push_back({"VerifyPeer", "1"}); + } + drogon::app().addListener(ip, this->port, /*useSSL=*/true, this->certPath, this->keyPath, /*useOldTLS=*/false, sslConfCmds); + } else { + drogon::app().addListener(ip, this->port); + } } drogon::app().run(); } catch (...) { diff --git a/src/drogon_http_server.hpp b/src/drogon_http_server.hpp index f6a6a34bee..83802b6b0f 100644 --- a/src/drogon_http_server.hpp +++ b/src/drogon_http_server.hpp @@ -35,6 +35,9 @@ class DrogonHttpServer { std::unique_ptr pool; int port; std::string address; + std::string certPath; + std::string keyPath; + std::string caPath; std::function)> @@ -47,7 +50,10 @@ class DrogonHttpServer { size_t numWorkersForUnary, size_t numWorkersForStreaming, int port, - const std::string& address); + const std::string& address, + const std::string& certPath = "", + const std::string& keyPath = "", + const std::string& caPath = ""); Status startAcceptingRequests(); void terminate(); diff --git a/src/grpcservermodule.cpp b/src/grpcservermodule.cpp index 4296e49489..96f1ef4513 100644 --- a/src/grpcservermodule.cpp +++ b/src/grpcservermodule.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -97,6 +98,14 @@ GRPCServerModule::GRPCServerModule(Server& server) : tfsModelService(this->server), kfsGrpcInferenceService(this->server) {} +static std::string read_file_to_string(const std::string& path) { + std::ifstream f(path, std::ios::in | std::ios::binary); + if (!f) { + throw std::runtime_error("Cannot open file: " + path); + } + return std::string(std::istreambuf_iterator(f), std::istreambuf_iterator()); +} + static std::string host_with_port(const std::string& host, int port) { if (Config::is_ipv6(host)) { return "[" + host + "]:" + std::to_string(port); @@ -128,6 +137,29 @@ Status GRPCServerModule::start(const ovms::Config& config) { return status; } + // Build gRPC server credentials (TLS or insecure) + std::shared_ptr serverCredentials; + const std::string certPath = config.grpcCertPath(); + const std::string keyPath = config.grpcKeyPath(); + const std::string caPath = config.grpcCaPath(); + if (!certPath.empty() && !keyPath.empty()) { + grpc::SslServerCredentialsOptions sslOpts; + grpc::SslServerCredentialsOptions::PemKeyCertPair keyCertPair; + keyCertPair.cert_chain = read_file_to_string(certPath); + keyCertPair.private_key = read_file_to_string(keyPath); + sslOpts.pem_key_cert_pairs.push_back(keyCertPair); + if (!caPath.empty()) { + sslOpts.pem_root_certs = read_file_to_string(caPath); + sslOpts.client_certificate_request = GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY; + SPDLOG_INFO("gRPC TLS enabled with mutual TLS (mTLS) — client certificates required"); + } else { + SPDLOG_INFO("gRPC TLS enabled (server-only TLS, no client certificate verification)"); + } + serverCredentials = grpc::SslServerCredentials(sslOpts); + } else { + serverCredentials = grpc::InsecureServerCredentials(); + } + ServerBuilder builder; builder.SetMaxReceiveMessageSize(GIGABYTE); builder.SetMaxSendMessageSize(GIGABYTE); @@ -135,7 +167,7 @@ Status GRPCServerModule::start(const ovms::Config& config) { for (const auto& ip : ips) { auto hostWithPort = host_with_port(ip, config.port()); SPDLOG_INFO("Binding gRPC server to address: {}", hostWithPort); - builder.AddListeningPort(hostWithPort, grpc::InsecureServerCredentials()); + builder.AddListeningPort(hostWithPort, serverCredentials); } builder.RegisterService(&tfsPredictService); builder.RegisterService(&tfsModelService); diff --git a/src/http_server.cpp b/src/http_server.cpp index 747187d873..82028d1b99 100644 --- a/src/http_server.cpp +++ b/src/http_server.cpp @@ -156,7 +156,7 @@ static const ovms::HTTPStatusCode http(const ovms::Status& status) { } std::unique_ptr createAndStartDrogonHttpServer(const std::string& address, int port, int num_threads, ovms::Server& ovmsServer, const ovms::Config& config, int timeout_in_ms) { - auto server = std::make_unique(num_threads, num_threads, port, address); + auto server = std::make_unique(num_threads, num_threads, port, address, config.restCertPath(), config.restKeyPath(), config.restCaPath()); auto handler = std::make_shared(ovmsServer, timeout_in_ms, config.apiKey()); auto& pool = server->getPool(); server->registerRequestDispatcher([handler, &pool](const drogon::HttpRequestPtr& req, std::function drogonResponseInitializeCallback) { diff --git a/src/ovms.h b/src/ovms.h index 839fad13ce..1c8e994fab 100644 --- a/src/ovms.h +++ b/src/ovms.h @@ -354,6 +354,75 @@ OVMS_Status* OVMS_ServerSettingsSetAllowedLocalMediaPath(OVMS_ServerSettings* se OVMS_Status* OVMS_ServerSettingsSetAllowedMediaDomains(OVMS_ServerSettings* settings, const char* allowed_media_domains); +// Set the path to the PEM-encoded server certificate for gRPC TLS. +// Equivalent of starting server with --grpc_certificate_path. +// Must be set together with OVMS_ServerSettingsSetGrpcKeyPath to enable TLS. +// +// \param settings The server settings object to be set +// \param grpc_cert_path Path to the PEM certificate file +// \return OVMS_Status object in case of failure +OVMS_Status* OVMS_ServerSettingsSetGrpcCertPath(OVMS_ServerSettings* settings, + const char* grpc_cert_path); + +// Set the path to the PEM-encoded private key for gRPC TLS. +// Equivalent of starting server with --grpc_key_path. +// Must be set together with OVMS_ServerSettingsSetGrpcCertPath to enable TLS. +// +// \param settings The server settings object to be set +// \param grpc_key_path Path to the PEM private key file +// \return OVMS_Status object in case of failure +OVMS_Status* OVMS_ServerSettingsSetGrpcKeyPath(OVMS_ServerSettings* settings, + const char* grpc_key_path); + +// Set the path to the PEM-encoded CA certificate for gRPC mutual TLS (mTLS). +// Equivalent of starting server with --grpc_ca_path. +// Requires OVMS_ServerSettingsSetGrpcCertPath and OVMS_ServerSettingsSetGrpcKeyPath. +// When set, client certificates are required and verified against this CA. +// +// \param settings The server settings object to be set +// \param grpc_ca_path Path to the PEM CA certificate file +// \return OVMS_Status object in case of failure +OVMS_Status* OVMS_ServerSettingsSetGrpcCaPath(OVMS_ServerSettings* settings, + const char* grpc_ca_path); + +// NOTE: REST TLS is currently gated. The bundled Drogon is built without OpenSSL, so +// native REST HTTPS cannot be served; setting any REST TLS path causes the server to +// fail startup (rather than silently serving plaintext). Use the gRPC TLS setters, or +// terminate REST TLS with a reverse proxy. The REST setters below are retained for +// forward compatibility (they will work once Drogon is built with OpenSSL). See #2144. +// +// Set the path to the PEM-encoded server certificate for REST TLS (HTTPS). +// Equivalent of starting server with --rest_certificate_path. +// Must be set together with OVMS_ServerSettingsSetRestKeyPath to enable HTTPS. +// +// \param settings The server settings object to be set +// \param rest_cert_path Path to the PEM certificate file +// \return OVMS_Status object in case of failure +OVMS_Status* OVMS_ServerSettingsSetRestCertPath(OVMS_ServerSettings* settings, + const char* rest_cert_path); + +// Set the path to the PEM-encoded private key for REST TLS (HTTPS). +// Equivalent of starting server with --rest_key_path. +// Must be set together with OVMS_ServerSettingsSetRestCertPath to enable HTTPS. +// +// \param settings The server settings object to be set +// \param rest_key_path Path to the PEM private key file +// \return OVMS_Status object in case of failure +OVMS_Status* OVMS_ServerSettingsSetRestKeyPath(OVMS_ServerSettings* settings, + const char* rest_key_path); + +// Set the path to the PEM-encoded CA certificate for REST client certificate verification. +// Equivalent of starting server with --rest_ca_path. +// Requires OVMS_ServerSettingsSetRestCertPath and OVMS_ServerSettingsSetRestKeyPath. +// When set, client certificates are required and verified against this CA via Drogon's +// SSL config commands (OpenSSL CAfile + VerifyPeer). +// +// \param settings The server settings object to be set +// \param rest_ca_path Path to the PEM CA certificate file +// \return OVMS_Status object in case of failure +OVMS_Status* OVMS_ServerSettingsSetRestCaPath(OVMS_ServerSettings* settings, + const char* rest_ca_path); + //// //// OVMS_ModelsSettings //// Options for starting multi model server controlled by config.json file diff --git a/src/test/c_api_tests.cpp b/src/test/c_api_tests.cpp index e59f27cbf4..d5b5555453 100644 --- a/src/test/c_api_tests.cpp +++ b/src/test/c_api_tests.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -107,6 +108,12 @@ TEST(CAPIConfigTest, MultiModelConfiguration) { EXPECT_EQ(serverSettings->filesystemPollWaitMilliseconds, 1000); EXPECT_EQ(serverSettings->resourcesCleanerPollWaitSeconds, 300); EXPECT_EQ(serverSettings->cacheDir, ""); + EXPECT_EQ(serverSettings->grpcCertPath, ""); + EXPECT_EQ(serverSettings->grpcKeyPath, ""); + EXPECT_EQ(serverSettings->grpcCaPath, ""); + EXPECT_EQ(serverSettings->restCertPath, ""); + EXPECT_EQ(serverSettings->restKeyPath, ""); + EXPECT_EQ(serverSettings->restCaPath, ""); testDefaultSingleModelOptions(modelsSettings); EXPECT_EQ(modelsSettings->configPath, ""); @@ -135,6 +142,12 @@ TEST(CAPIConfigTest, MultiModelConfiguration) { ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetLogPath(_serverSettings, getGenericFullPathForTmp("/tmp/logs").c_str())); ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetAllowedLocalMediaPath(_serverSettings, getGenericFullPathForTmp("/tmp/path").c_str())); ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetAllowedMediaDomains(_serverSettings, "raw.githubusercontent.com,githubusercontent.com,google.com")); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetGrpcCertPath(_serverSettings, getGenericFullPathForTmp("/tmp/grpc_cert.pem").c_str())); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetGrpcKeyPath(_serverSettings, getGenericFullPathForTmp("/tmp/grpc_key.pem").c_str())); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetGrpcCaPath(_serverSettings, getGenericFullPathForTmp("/tmp/grpc_ca.pem").c_str())); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetRestCertPath(_serverSettings, getGenericFullPathForTmp("/tmp/rest_cert.pem").c_str())); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetRestKeyPath(_serverSettings, getGenericFullPathForTmp("/tmp/rest_key.pem").c_str())); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetRestCaPath(_serverSettings, getGenericFullPathForTmp("/tmp/rest_ca.pem").c_str())); ASSERT_CAPI_STATUS_NULL(OVMS_ModelsSettingsSetConfigPath(_modelsSettings, getGenericFullPathForTmp("/tmp/config").c_str())); // check nullptr ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetGrpcPort(nullptr, 5555), StatusCode::NONEXISTENT_PTR); @@ -163,6 +176,18 @@ TEST(CAPIConfigTest, MultiModelConfiguration) { ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetAllowedLocalMediaPath(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetAllowedMediaDomains(nullptr, "raw.githubusercontent.com,githubusercontent.com,google.com"), StatusCode::NONEXISTENT_PTR); ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetAllowedMediaDomains(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetGrpcCertPath(nullptr, getGenericFullPathForTmp("/tmp/grpc_cert.pem").c_str()), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetGrpcCertPath(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetGrpcKeyPath(nullptr, getGenericFullPathForTmp("/tmp/grpc_key.pem").c_str()), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetGrpcKeyPath(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetGrpcCaPath(nullptr, getGenericFullPathForTmp("/tmp/grpc_ca.pem").c_str()), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetGrpcCaPath(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetRestCertPath(nullptr, getGenericFullPathForTmp("/tmp/rest_cert.pem").c_str()), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetRestCertPath(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetRestKeyPath(nullptr, getGenericFullPathForTmp("/tmp/rest_key.pem").c_str()), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetRestKeyPath(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetRestCaPath(nullptr, getGenericFullPathForTmp("/tmp/rest_ca.pem").c_str()), StatusCode::NONEXISTENT_PTR); + ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ServerSettingsSetRestCaPath(_serverSettings, nullptr), StatusCode::NONEXISTENT_PTR); ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ModelsSettingsSetConfigPath(nullptr, getGenericFullPathForTmp("/tmp/config").c_str()), StatusCode::NONEXISTENT_PTR); ASSERT_CAPI_STATUS_NOT_NULL_EXPECT_CODE(OVMS_ModelsSettingsSetConfigPath(_modelsSettings, nullptr), StatusCode::NONEXISTENT_PTR); @@ -192,10 +217,30 @@ TEST(CAPIConfigTest, MultiModelConfiguration) { EXPECT_EQ(serverSettings->filesystemPollWaitMilliseconds, 2000); EXPECT_EQ(serverSettings->resourcesCleanerPollWaitSeconds, 4); EXPECT_EQ(serverSettings->cacheDir, getGenericFullPathForTmp("/tmp/cache")); + EXPECT_EQ(serverSettings->grpcCertPath, getGenericFullPathForTmp("/tmp/grpc_cert.pem")); + EXPECT_EQ(serverSettings->grpcKeyPath, getGenericFullPathForTmp("/tmp/grpc_key.pem")); + EXPECT_EQ(serverSettings->grpcCaPath, getGenericFullPathForTmp("/tmp/grpc_ca.pem")); + EXPECT_EQ(serverSettings->restCertPath, getGenericFullPathForTmp("/tmp/rest_cert.pem")); + EXPECT_EQ(serverSettings->restKeyPath, getGenericFullPathForTmp("/tmp/rest_key.pem")); + EXPECT_EQ(serverSettings->restCaPath, getGenericFullPathForTmp("/tmp/rest_ca.pem")); testDefaultSingleModelOptions(modelsSettings); EXPECT_EQ(modelsSettings->configPath, getGenericFullPathForTmp("/tmp/config")); + // The setters above are verified by the read-back asserts. For the parse() below: + // gRPC TLS validation checks the cert/key/ca files exist, so create them. REST TLS + // is gated (fail-closed) in this build, so clear the rest_* paths before parsing — + // otherwise validation would reject them. (The rest setters are already verified above.) + std::vector tlsTempFiles = { + getGenericFullPathForTmp("/tmp/grpc_cert.pem"), getGenericFullPathForTmp("/tmp/grpc_key.pem"), + getGenericFullPathForTmp("/tmp/grpc_ca.pem")}; + for (const auto& f : tlsTempFiles) { + std::ofstream(f) << "placeholder"; + } + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetRestCertPath(_serverSettings, "")); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetRestKeyPath(_serverSettings, "")); + ASSERT_CAPI_STATUS_NULL(OVMS_ServerSettingsSetRestCaPath(_serverSettings, "")); + // Test config parser ConstructorEnabledConfig cfg; #ifdef __linux__ @@ -211,6 +256,9 @@ TEST(CAPIConfigTest, MultiModelConfiguration) { EXPECT_EQ(cfg.grpcBindAddress(), "2.2.2.2"); EXPECT_EQ(cfg.restWorkers(), 31); EXPECT_EQ(cfg.restBindAddress(), "3.3.3.3"); + for (const auto& f : tlsTempFiles) { + std::filesystem::remove(f); + } // EXPECT_EQ(serverSettings->metricsEnabled, false); // EXPECT_EQ(serverSettings->metricsList, ""); EXPECT_EQ(cfg.cpuExtensionLibraryPath(), getGenericFullPathForSrcTest("/ovms/src/test")); diff --git a/src/test/ovmsconfig_test.cpp b/src/test/ovmsconfig_test.cpp index a4bcad1224..399b483af0 100644 --- a/src/test/ovmsconfig_test.cpp +++ b/src/test/ovmsconfig_test.cpp @@ -270,6 +270,48 @@ TEST_F(OvmsConfigDeathTest, invalidGrpcBindAddress) { EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "grpc_bind_address has invalid format"); } +TEST_F(OvmsConfigDeathTest, grpcCertWithoutKey) { + char* n_argv[] = {"ovms", "--config_path", "/path1", "--port", "8080", "--grpc_certificate_path", "/some/cert.pem"}; + int arg_count = 7; + EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "grpc_certificate_path and grpc_key_path must both be set"); +} + +TEST_F(OvmsConfigDeathTest, grpcKeyWithoutCert) { + char* n_argv[] = {"ovms", "--config_path", "/path1", "--port", "8080", "--grpc_key_path", "/some/key.pem"}; + int arg_count = 7; + EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "grpc_certificate_path and grpc_key_path must both be set"); +} + +TEST_F(OvmsConfigDeathTest, restCertWithoutKey) { + char* n_argv[] = {"ovms", "--config_path", "/path1", "--rest_port", "8081", "--port", "8080", "--rest_certificate_path", "/some/cert.pem"}; + int arg_count = 9; + EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "REST TLS .* is not supported in this build"); +} + +TEST_F(OvmsConfigDeathTest, restKeyWithoutCert) { + char* n_argv[] = {"ovms", "--config_path", "/path1", "--rest_port", "8081", "--port", "8080", "--rest_key_path", "/some/key.pem"}; + int arg_count = 9; + EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "REST TLS .* is not supported in this build"); +} + +TEST_F(OvmsConfigDeathTest, grpcCertNonExistentFile) { + char* n_argv[] = {"ovms", "--config_path", "/path1", "--port", "8080", "--grpc_certificate_path", "/nonexistent/cert.pem", "--grpc_key_path", "/nonexistent/key.pem"}; + int arg_count = 9; + EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "File path provided as --grpc_certificate_path does not exist"); +} + +TEST_F(OvmsConfigDeathTest, grpcCaWithoutCertKey) { + char* n_argv[] = {"ovms", "--config_path", "/path1", "--port", "8080", "--grpc_ca_path", "/some/ca.pem"}; + int arg_count = 7; + EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "grpc_ca_path requires grpc_certificate_path and grpc_key_path"); +} + +TEST_F(OvmsConfigDeathTest, restCaWithoutCertKey) { + char* n_argv[] = {"ovms", "--config_path", "/path1", "--rest_port", "8081", "--port", "8080", "--rest_ca_path", "/some/ca.pem"}; + int arg_count = 9; + EXPECT_EXIT(ovms::Config::instance().parse(arg_count, n_argv), ::testing::ExitedWithCode(OVMS_EX_USAGE), "REST TLS .* is not supported in this build"); +} + TEST_F(OvmsConfigDeathTest, negativeMultiParams) { char* n_argv[] = {"ovms", "--config_path", "/path1", "--batch_size", "10"}; int arg_count = 5; @@ -2351,6 +2393,59 @@ TEST(OvmsAPIKeyConfig, positiveAPIKeyFile) { std::remove("api_key.txt"); } +TEST(OvmsTLSConfig, positiveGrpcTLSWithExistingFiles) { + // Config validation only checks file existence, not certificate validity, + // so empty placeholder files are sufficient here. + std::ofstream certFileTmp("tls_cert.pem"); + certFileTmp << "cert"; + certFileTmp.close(); + std::ofstream keyFileTmp("tls_key.pem"); + keyFileTmp << "key"; + keyFileTmp.close(); + std::ofstream caFileTmp("tls_ca.pem"); + caFileTmp << "ca"; + caFileTmp.close(); + + std::string modelName = "test_name"; + std::string modelPath = "model_path"; + std::string certPath = "tls_cert.pem"; + std::string keyPath = "tls_key.pem"; + std::string caPath = "tls_ca.pem"; + std::string rest_port = "8080"; + std::string grpc_port = "8081"; + char* n_argv[] = { + (char*)"ovms", + (char*)"--model_path", + (char*)modelPath.c_str(), + (char*)"--model_name", + (char*)modelName.c_str(), + (char*)"--port", + (char*)grpc_port.c_str(), + (char*)"--rest_port", + (char*)rest_port.c_str(), + (char*)"--grpc_certificate_path", + (char*)certPath.c_str(), + (char*)"--grpc_key_path", + (char*)keyPath.c_str(), + (char*)"--grpc_ca_path", + (char*)caPath.c_str(), + }; + + int arg_count = 15; + ConstructorEnabledConfig config; + // parse() calls exit() on validation failure; reaching the asserts below means validation passed. + // Note: REST TLS is intentionally NOT exercised here — it is gated (fail-closed) in this + // build because Drogon lacks OpenSSL; see OvmsConfigDeathTest.restCertWithoutKey etc. + config.parse(arg_count, n_argv); + EXPECT_EQ(config.grpcCertPath(), certPath); + EXPECT_EQ(config.grpcKeyPath(), keyPath); + EXPECT_EQ(config.grpcCaPath(), caPath); + + std::remove("tls_cert.pem"); + std::remove("tls_key.pem"); + std::remove("tls_ca.pem"); +} + TEST(OvmsAPIKeyConfig, positiveAPIKeyEnv) { EnvGuard envGuard; envGuard.set("API_KEY", "ABCD"); From 19eb7d324db3d5f1db3a166120e2b390a6f6c9af Mon Sep 17 00:00:00 2001 From: exzile Date: Fri, 26 Jun 2026 22:06:29 -0400 Subject: [PATCH 2/2] Refactor: extract shared TLS path validation helper Deduplicate the gRPC and REST TLS validation in Config::validate() into a single validateTlsMaterial() helper (cert/key set together, files exist and are non-empty, CA requires cert+key). Removes ~40 lines of duplication and the previously-unreachable REST validation block that sat after the fail-closed REST guard. The REST guard remains (Drogon has no OpenSSL in this build); a comment documents the one-line change to reactivate REST validation once it does. No behavior change: gRPC validation is identical, REST stays fail-closed. All TLS config/C-API unit tests still pass. Co-Authored-By: Claude Opus 4.8 --- src/config.cpp | 108 +++++++++++++++++++++---------------------------- 1 file changed, 46 insertions(+), 62 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 7ae77da506..d935012136 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -164,6 +164,44 @@ bool Config::validateUserSettingsInConfigAddRemoveModel(const ModelsSettingsImpl return true; } +// Validates a set of TLS material paths (certificate, private key, optional CA) for one +// endpoint. cert and key must be set together; each provided file must exist and be +// non-empty (an empty/truncated cert or key would otherwise pass and later cause an +// opaque bind/handshake failure); a CA requires cert+key. flagPrefix is the user-facing +// option prefix, e.g. "grpc" or "rest", used only in error messages. +static bool validateTlsMaterial(const std::string& certPath, const std::string& keyPath, const std::string& caPath, const std::string& flagPrefix) { + auto fileUsable = [](const std::string& p) { + std::error_code ec; + return std::filesystem::exists(p) && std::filesystem::file_size(p, ec) > 0 && !ec; + }; + const bool hasCert = !certPath.empty(); + const bool hasKey = !keyPath.empty(); + const bool hasCa = !caPath.empty(); + if (hasCert != hasKey) { + std::cerr << flagPrefix << "_certificate_path and " << flagPrefix << "_key_path must both be set to enable TLS" << std::endl; + return false; + } + if (hasCert && !fileUsable(certPath)) { + std::cerr << "File path provided as --" << flagPrefix << "_certificate_path does not exist or is empty: " << certPath << std::endl; + return false; + } + if (hasKey && !fileUsable(keyPath)) { + std::cerr << "File path provided as --" << flagPrefix << "_key_path does not exist or is empty: " << keyPath << std::endl; + return false; + } + if (hasCa) { + if (!hasCert) { + std::cerr << flagPrefix << "_ca_path requires " << flagPrefix << "_certificate_path and " << flagPrefix << "_key_path to be set" << std::endl; + return false; + } + if (!fileUsable(caPath)) { + std::cerr << "File path provided as --" << flagPrefix << "_ca_path does not exist or is empty: " << caPath << std::endl; + return false; + } + } + return true; +} + bool Config::validate() { if (!this->serverSettings.hfSettings.sourceModel.empty() && this->serverSettings.hfSettings.task == UNKNOWN_GRAPH) { std::cerr << "--source_model should be used combined with --task" << std::endl; @@ -352,78 +390,24 @@ bool Config::validate() { return false; } - // check TLS paths: - bool grpcHasCert = !this->serverSettings.grpcCertPath.empty(); - bool grpcHasKey = !this->serverSettings.grpcKeyPath.empty(); - bool grpcHasCa = !this->serverSettings.grpcCaPath.empty(); - bool restHasCert = !this->serverSettings.restCertPath.empty(); - bool restHasKey = !this->serverSettings.restKeyPath.empty(); - bool restHasCa = !this->serverSettings.restCaPath.empty(); - - // A TLS material file must exist and be non-empty (an empty/truncated cert or key - // would otherwise pass and later cause an opaque gRPC bind failure). - auto tlsFileUsable = [](const std::string& p) { - std::error_code ec; - return std::filesystem::exists(p) && std::filesystem::file_size(p, ec) > 0 && !ec; - }; - - if (grpcHasCert != grpcHasKey) { - std::cerr << "grpc_certificate_path and grpc_key_path must both be set to enable gRPC TLS" << std::endl; + // gRPC TLS: validate cert/key/CA paths. + if (!validateTlsMaterial(this->serverSettings.grpcCertPath, this->serverSettings.grpcKeyPath, this->serverSettings.grpcCaPath, "grpc")) { return false; } - if (grpcHasCert && !tlsFileUsable(this->serverSettings.grpcCertPath)) { - std::cerr << "File path provided as --grpc_certificate_path does not exist or is empty: " << this->serverSettings.grpcCertPath << std::endl; - return false; - } - if (grpcHasKey && !tlsFileUsable(this->serverSettings.grpcKeyPath)) { - std::cerr << "File path provided as --grpc_key_path does not exist or is empty: " << this->serverSettings.grpcKeyPath << std::endl; - return false; - } - if (grpcHasCa) { - if (!grpcHasCert) { - std::cerr << "grpc_ca_path requires grpc_certificate_path and grpc_key_path to be set" << std::endl; - return false; - } - if (!tlsFileUsable(this->serverSettings.grpcCaPath)) { - std::cerr << "File path provided as --grpc_ca_path does not exist or is empty: " << this->serverSettings.grpcCaPath << std::endl; - return false; - } - } // REST TLS is gated: the bundled Drogon is currently built without OpenSSL, so it // cannot serve HTTPS (enabling SSL would silently fall back to plaintext). Fail // closed rather than expose a plaintext endpoint a user believes is encrypted. - // The REST TLS wiring (addListener SSL) is in place and will activate once Drogon - // is built with OpenSSL; remove this guard at that point. Use gRPC TLS or a - // TLS-terminating proxy for REST in the meantime. See issue #2144. - if (restHasCert || restHasKey || restHasCa) { + // The REST TLS wiring (addListener SSL, see drogon_http_server.cpp) is in place and + // will activate once Drogon is built with OpenSSL. To enable REST TLS at that point, + // replace this guard with: + // if (!validateTlsMaterial(restCertPath, restKeyPath, restCaPath, "rest")) return false; + // Use gRPC TLS or a TLS-terminating proxy for REST in the meantime. See issue #2144. + if (!this->serverSettings.restCertPath.empty() || !this->serverSettings.restKeyPath.empty() || !this->serverSettings.restCaPath.empty()) { std::cerr << "REST TLS (rest_certificate_path/rest_key_path/rest_ca_path) is not supported in this build because the bundled Drogon was built without OpenSSL. Use gRPC TLS (grpc_certificate_path/grpc_key_path) or terminate REST TLS with a proxy." << std::endl; return false; } - if (restHasCert != restHasKey) { - std::cerr << "rest_certificate_path and rest_key_path must both be set to enable REST TLS" << std::endl; - return false; - } - if (restHasCert && !std::filesystem::exists(this->serverSettings.restCertPath)) { - std::cerr << "File path provided as --rest_certificate_path does not exist: " << this->serverSettings.restCertPath << std::endl; - return false; - } - if (restHasKey && !std::filesystem::exists(this->serverSettings.restKeyPath)) { - std::cerr << "File path provided as --rest_key_path does not exist: " << this->serverSettings.restKeyPath << std::endl; - return false; - } - if (restHasCa) { - if (!restHasCert) { - std::cerr << "rest_ca_path requires rest_certificate_path and rest_key_path to be set" << std::endl; - return false; - } - if (!std::filesystem::exists(this->serverSettings.restCaPath)) { - std::cerr << "File path provided as --rest_ca_path does not exist: " << this->serverSettings.restCaPath << std::endl; - return false; - } - } - // check bind addresses: if (!restBindAddress().empty() && check_hostname_or_ip(restBindAddress()) == false) { std::cerr << "rest_bind_address has invalid format: proper hostname or IP address expected." << std::endl;