aboutsummaryrefslogtreecommitdiff
path: root/src/zenserver/storage/admin/admin.cpp
diff options
context:
space:
mode:
authorStefan Boberg <[email protected]>2026-05-04 16:46:03 +0200
committerGitHub Enterprise <[email protected]>2026-05-04 16:46:03 +0200
commit10d2a61fe1c848f44033e8450ff3a5ffa7f4322a (patch)
treeaa66c6a068b50d2390bdae5f857c7151f15e5a86 /src/zenserver/storage/admin/admin.cpp
parentTui picker fixes (#1027) (diff)
downloadarchived-zen-10d2a61fe1c848f44033e8450ff3a5ffa7f4322a.tar.xz
archived-zen-10d2a61fe1c848f44033e8450ff3a5ffa7f4322a.zip
zenhttp improvements (robustness / correctness) (#968)
A collection of security, correctness, and robustness fixes in `zenhttp` and `zencore` surfaced by security review. Most items are small, independent commits grouped here because they all tighten trust boundaries or fix UB along the same code paths. ## WebSocket protocol hardening (RFC 6455) - **Enforce the client-side mask bit**. Server-side frame loops now reject unmasked frames with close code 1002 per §5.1. Prevents HTTP intermediary smuggling. - **Validate control frames and RSV bits**. Fragmented control frames, oversized (>125 B) control payloads, and any non-zero RSV bit now fail the connection before allocation. - **Lower per-frame payload cap** from 256 MB → 4 MB. Bounds per-connection accumulator memory. - **Implement message fragmentation**. Continuation frames are coalesced and delivered as a single message; interleaved non-control frames close with 1002; assembled messages are capped at 4 MB (1009 on overflow). Previously partial fragments were delivered to handlers, bypassing payload validation. - **Parse the 101 handshake response properly** in `HttpWsClient`. Status-line, `Upgrade`, `Connection`, and `Sec-WebSocket-Accept` are now matched exactly rather than via substring searches against the full body. ## Auth / OIDC hardening - **Constant-time password compare** in `PasswordSecurity::IsAllowed` (closes a remote length/content timing oracle). Adds a shared `ConstantTimeEquals` helper. - **Harden Basic-auth header parsing**: trim trailing LWS, reject control bytes and DEL in the credential. - **OIDC discovery pinning**: require HTTPS (loopback exempt), verify `issuer` matches `BaseUrl`, require `token_endpoint` / `userinfo_endpoint` / `jwks_uri` to share origin with `BaseUrl`, reject empty `token_endpoint`. - **Restrict `POST /auth/oidc/refreshtoken`** to local-machine requests. Previously unauthenticated in default deployments — remote callers could evict or replace cached tokens. - **Stop logging OIDC provider response bodies** on refresh failure (IdPs echo `refresh_token` back in error bodies). - **Drop the unused `IdentityToken` field** from `OidcClient` / `OpenIdToken` so nothing in the tree accidentally trusts an unverified JWT. ## Auth state encryption migration - Add `AesGcm` AEAD primitive (BCrypt / OpenSSL backends, mbedTLS stubbed) and `CryptoRandom::Fill` CSPRNG helper in `zencore/crypto.h`. - Migrate authstate file from AES-256-CBC with a fixed IV to AES-GCM with a fresh 12-byte random nonce per write and the 4-byte `ZEN1` magic bound as AAD. Legacy-CBC files are transparently read once and rewritten in the new format. ## Filesystem / IO robustness - `IoBufferExtendedCore::Materialize` now checks `MAP_FAILED` on POSIX (was comparing to `nullptr`, which let the failure sentinel propagate into later reads and `munmap(MAP_FAILED, ...)`). - `IoBufferBuilder::MakeFromFile / MakeFromTemporaryFile`: close the FD/HANDLE on exception via a dismissable `ScopeGuard`; actually check the `fstat()` return value (previously used an uninitialized `FileSize`). - `ReadFromFileMaybe`: loop short reads, retry `EINTR`, chunk Windows `ReadFile` at `0xFFFFFFFF` bytes (fixes silent truncation of multi-GiB reads). - `WipeDirectory`: compare `FindFirstFileW` handle against `INVALID_HANDLE_VALUE` rather than `nullptr`. - `RemoveFileNative` (Linux/macOS): report non-`ENOENT` stat failures via the `std::error_code` out-param and stop reading `st_mode` after a failed stat. ## Buffer / compression correctness - Avoid per-copy `IoBufferCore` heap allocations in `CompositeBuffer::CopyTo / ViewOrCopyRange` iterators; add fast path for `BufferHeader::Read` when the 64-byte header fits in the first plain-memory segment. - `BufferHeader`: add `IsHeaderValid()` gate covering `BlockSizeExponent` range, `BlockCount * BlockSize` overflow, and `TotalRawSize` bounds before any arithmetic uses them. Defends against attacker-controlled headers that can pass the CRC and trigger OOB writes in `DecompressBlock`.
Diffstat (limited to 'src/zenserver/storage/admin/admin.cpp')
-rw-r--r--src/zenserver/storage/admin/admin.cpp106
1 files changed, 86 insertions, 20 deletions
diff --git a/src/zenserver/storage/admin/admin.cpp b/src/zenserver/storage/admin/admin.cpp
index 34d9e570e..1de5f74fe 100644
--- a/src/zenserver/storage/admin/admin.cpp
+++ b/src/zenserver/storage/admin/admin.cpp
@@ -26,6 +26,60 @@
namespace zen {
+#if ZEN_WITH_TRACE
+namespace {
+ // Accept only loopback destinations for admin-triggered trace streams. Handles
+ // "localhost", "127.0.0.1", "::1", and bracketed IPv6 ("[::1]"), each optionally
+ // followed by ":<digits>". Rejects any control characters so the value is also
+ // safe to log.
+ bool IsLoopbackTraceHost(std::string_view Host)
+ {
+ if (Host.empty())
+ {
+ return false;
+ }
+ for (char C : Host)
+ {
+ if (static_cast<unsigned char>(C) < 0x20 || C == 0x7F)
+ {
+ return false;
+ }
+ }
+
+ std::string_view HostOnly = Host;
+ if (HostOnly.front() == '[')
+ {
+ const size_t Close = HostOnly.find(']');
+ if (Close == std::string_view::npos)
+ {
+ return false;
+ }
+ const std::string_view Tail = HostOnly.substr(Close + 1);
+ if (!Tail.empty())
+ {
+ if (Tail.front() != ':' || Tail.size() < 2 || Tail.find_first_not_of("0123456789", 1) != std::string_view::npos)
+ {
+ return false;
+ }
+ }
+ HostOnly = HostOnly.substr(1, Close - 1);
+ }
+ else if (const size_t Colon = HostOnly.find(':');
+ Colon != std::string_view::npos && HostOnly.find(':', Colon + 1) == std::string_view::npos)
+ {
+ const std::string_view Port = HostOnly.substr(Colon + 1);
+ if (Port.empty() || Port.find_first_not_of("0123456789") != std::string_view::npos)
+ {
+ return false;
+ }
+ HostOnly = HostOnly.substr(0, Colon);
+ }
+
+ return HostOnly == "localhost" || HostOnly == "127.0.0.1" || HostOnly == "::1";
+ }
+} // namespace
+#endif // ZEN_WITH_TRACE
+
struct DirStats
{
uint64_t FileCount = 0;
@@ -149,17 +203,13 @@ HttpAdminService::HttpAdminService(GcScheduler& Scheduler,
[&](HttpRouterRequest& Req) {
const auto& JobIdString = Req.GetCapture(1);
std::optional<uint64_t> JobIdArg = ParseInt<uint64_t>(JobIdString);
- if (!JobIdArg)
- {
- Req.ServerRequest().WriteResponse(HttpResponseCode::BadRequest);
- }
- JobId Id{.Id = JobIdArg.value_or(0)};
- if (Id.Id == 0)
+ if (!JobIdArg || JobIdArg.value() == 0)
{
return Req.ServerRequest().WriteResponse(HttpResponseCode::BadRequest,
ZenContentType::kText,
- fmt::format("Invalid Job Id: {}", Id.Id));
+ fmt::format("Invalid Job Id: '{}'", JobIdString));
}
+ const JobId Id{.Id = JobIdArg.value()};
std::optional<JobQueue::JobDetails> CurrentState = m_BackgroundJobQueue.Get(Id);
if (!CurrentState)
@@ -271,11 +321,13 @@ HttpAdminService::HttpAdminService(GcScheduler& Scheduler,
[&](HttpRouterRequest& Req) {
const auto& JobIdString = Req.GetCapture(1);
std::optional<uint64_t> JobIdArg = ParseInt<uint64_t>(JobIdString);
- if (!JobIdArg)
+ if (!JobIdArg || JobIdArg.value() == 0)
{
- Req.ServerRequest().WriteResponse(HttpResponseCode::BadRequest);
+ return Req.ServerRequest().WriteResponse(HttpResponseCode::BadRequest,
+ ZenContentType::kText,
+ fmt::format("Invalid Job Id: '{}'", JobIdString));
}
- JobId Id{.Id = JobIdArg.value_or(0)};
+ const JobId Id{.Id = JobIdArg.value()};
if (m_BackgroundJobQueue.CancelJob(Id))
{
Req.ServerRequest().WriteResponse(HttpResponseCode::OK);
@@ -610,11 +662,6 @@ HttpAdminService::HttpAdminService(GcScheduler& Scheduler,
const HttpServerRequest::QueryParams Params = HttpReq.GetQueryParams();
TraceOptions TraceOptions;
- if (!IsTracing())
- {
- TraceInit("zenserver");
- }
-
if (auto Channels = Params.GetValue("channels"); Channels.empty() == false)
{
TraceOptions.Channels = Channels;
@@ -622,22 +669,41 @@ HttpAdminService::HttpAdminService(GcScheduler& Scheduler,
if (auto File = Params.GetValue("file"); File.empty() == false)
{
- TraceOptions.File = File;
+ const std::filesystem::path TracesRoot = m_ServerOptions.DataDir / "traces";
+ std::optional<std::filesystem::path> Resolved = ResolveSafeRelativePath(TracesRoot, File);
+ if (!Resolved)
+ {
+ ZEN_WARN("admin trace/start rejected unsafe 'file' parameter '{}'", File);
+ return HttpReq.WriteResponse(HttpResponseCode::BadRequest, HttpContentType::kText, "Invalid 'file' parameter"sv);
+ }
+ TraceOptions.File = Resolved->string();
}
else if (auto Host = Params.GetValue("host"); Host.empty() == false)
{
+ if (!IsLoopbackTraceHost(Host))
+ {
+ ZEN_WARN("admin trace/start rejected non-loopback 'host' parameter '{}'", Host);
+ return HttpReq.WriteResponse(HttpResponseCode::BadRequest,
+ HttpContentType::kText,
+ "Invalid 'host' parameter (must be a loopback address)"sv);
+ }
TraceOptions.Host = Host;
}
else
{
- return Req.ServerRequest().WriteResponse(HttpResponseCode::BadRequest,
- HttpContentType::kText,
- "Invalid trace type, use `file` or `host`"sv);
+ return HttpReq.WriteResponse(HttpResponseCode::BadRequest,
+ HttpContentType::kText,
+ "Invalid trace type, use `file` or `host`"sv);
+ }
+
+ if (!IsTracing())
+ {
+ TraceInit("zenserver");
}
TraceConfigure(TraceOptions);
- return Req.ServerRequest().WriteResponse(HttpResponseCode::OK, HttpContentType::kText, "Tracing started");
+ return HttpReq.WriteResponse(HttpResponseCode::OK, HttpContentType::kText, "Tracing started");
},
HttpVerb::kPost);