Native TLS for the gRPC endpoint (REST gated) (#2144)#4334
Open
exzile wants to merge 2 commits into
Open
Conversation
Implements native TLS/HTTPS for the API endpoints (issue openvinotoolkit#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 openvinotoolkit#2144 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements native TLS/HTTPS for the API endpoints (#2144), so traffic can be encrypted without relying on an external reverse proxy (the NGINX sidecar leaves the NGINX↔OVMS hop in plaintext, which is the gap this issue raised).
gRPC TLS — functional
--grpc_certificate_path,--grpc_key_path,--grpc_ca_path.--grpc_ca_path→ mutual TLS: client certificates are required and verified (GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_AND_VERIFY).grpcservermodulebuildsgrpc::SslServerCredentialsfrom the cert/key/CA. No insecure fallback — a missing/unreadable/empty cert or key aborts startup rather than serving plaintext.REST TLS — gated (fail-closed) in this build
The same
--rest_certificate_path/--rest_key_path/--rest_ca_pathparams and the DrogonaddListenerSSL wiring are implemented, but the bundled Drogon is built without OpenSSL and cannot serve HTTPS. To avoid silently serving plaintext on a port an operator believes is encrypted,Config::validate()rejects any REST TLS parameter at startup with a clear error. The wiring is retained so REST HTTPS activates once Drogon is built with OpenSSL (a build-system follow-up). Use gRPC TLS or terminate REST TLS with a proxy in the meantime.Also
ServerSettingsfields,Configaccessors, and C-API setters (OVMS_ServerSettingsSet{Grpc,Rest}{Cert,Key,Ca}Path) inovms.h.parameters.md(new params) andsecurity_considerations.md(gRPC TLS now native; REST via proxy).Testing
Unit: config-validation death tests for every cert/key/ca combination, missing/empty files, and the REST-gated rejection; C-API setter coverage; a positive gRPC-TLS config test.
Server-only TLS handshake (CPU): started a server with gRPC TLS and verified a real handshake with
openssl s_client— TLS 1.2, server certificate presented.Live mTLS + real inference over TLS (CPU): with an mTLS-configured server (
src/test/dummymodel), generated a CA, a CA-signed server cert (CN=localhost+ SAN), a CA-signed client cert, and an untrusted client cert (different CA), then ran real gRPC/KFS inference for three cases:Server-side handshake logs confirm the rejection reasons, i.e. that
REQUIRE_AND_VERIFYenforces both the require and the verify:This proves mTLS genuinely enforces at runtime and that a real inference request completes end-to-end over the TLS channel — not just the handshake.
Security review: no plaintext fallback when TLS is requested; mTLS genuinely enforces; no key material in logs;
validate()not bypassable.REST TLS is intentionally untested beyond the startup-rejection unit test, since it is gated/fail-closed in this build pending a Drogon-with-OpenSSL build.
Notes