diff options
| author | Stefan Boberg <[email protected]> | 2026-04-21 13:48:41 +0200 |
|---|---|---|
| committer | GitHub Enterprise <[email protected]> | 2026-04-21 13:48:41 +0200 |
| commit | 245d2e562165d048e5ee2ab97f1260975a8142d3 (patch) | |
| tree | c01a36119ccacf830c8486634d78d0af137ec47f | |
| parent | async consul register/deregister (#992) (diff) | |
| download | archived-zen-245d2e562165d048e5ee2ab97f1260975a8142d3.tar.xz archived-zen-245d2e562165d048e5ee2ab97f1260975a8142d3.zip | |
zen CLI security review fixes (#974)
Security review follow-ups to the `zen` CLI. Each fix stands on its own commit. Grouped by category below.
## Credentials and secrets
- **Per-install random auth encryption key instead of a hardcoded literal.** The default AES key and IV used to encrypt persisted OIDC refresh tokens / OAuth client secrets were ASCII literals compiled into the public source. Replaced with 32+16 random bytes persisted to `<system-root>/auth/machinekey.dat`. `SecureRandomBytes` added in zencore/crypto wrapping BCryptGenRandom / OpenSSL / mbedTLS CTR_DRBG. Partial override (only one of `--encryption-aes-key`/`--encryption-aes-iv`) is now rejected instead of silently using the hardcoded half.
- **Wrap the machine key with OS-protected storage.** `machinekey.dat` is now a tagged format (4-byte magic + flags + wrapped-or-raw payload). Windows wraps via DPAPI (`CryptProtectData` at per-user scope) so a stolen disk copy cannot decrypt without the OS master key. macOS uses Keychain Services (GenericPassword under `org.unrealengine.zen.auth`, `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`). Linux uses libsecret (opt-in via `--zenlibsecret=yes`, off by default because headless servers typically have no Secret Service daemon). All platforms fall back to raw persistence with `0600` perms on POSIX when wrapping is unavailable. Legacy files from the prior commit are detected by size and still read.
> Note: argv-redaction before Sentry on crash was previously part of this PR but was superseded by `ScrubSensitiveValues()` from #989; this PR now just calls that helper instead of walking argv itself.
## Path traversal
- **Reject unsafe filenames from the remote oplog in `oplog-mirror`.** The filename from each oplog entry was joined to the mirror root without normalisation; a compromised remote could use drive letters, UNC shares, device path prefixes, absolute paths, or `..` components to write anywhere the zen user could write. An `UnsafeFileNameReason` check runs immediately after extraction, logs the offending filename, and aborts the mirror.
- **Use the resolved absolute download-spec path in `builds download`.** `--download-spec-path` was computed into a sanitised absolute path, then the original unresolved path was passed to `ParseBuildManifest`, bypassing the `MakeSafeAbsolutePath` mitigations and reading from the process cwd rather than `--local-path`.
## Input validation
- **Stop asserting on malformed `--build-id` / `--build-part-id`.** `Oid::FromHexString` asserts on bad input and `ZEN_ASSERT` is active in release, so a too-short or non-hex user value aborted the process instead of surfacing an `OptionParseException`. Routed all callers through `TryFromHexString`. Also fixes `ParseBuildPartId` reporting errors under the wrong option name.
- **Check the JSON parse error in `oplog-export --builds-metadata-path`.** The single-arg `LoadCompactBinaryFromJson` overload discarded the parser error; malformed JSON shipped a truncated compact-binary `metadata` field to the server with no indication. Switched to the two-arg overload and throws a descriptive error naming the file and reason.
- **Format the actual value in the malformed `--url` error.** The message was constructed with a literal `{}` placeholder and no `fmt::format` call, so users saw the placeholder instead of the offending URL.
- **Require `--output-path` in `cache get` unless `--as-text` is set.** Previously an empty path auto-filled from the value key / attachment hash and wrote into the process cwd; the `--as-text && empty path` stdout branch was unreachable because the auto-fill ran first.
- **Clear the cxxopts `allow_unrecognised_options` flag after permissive parse.** `ParseOptionsPermissive` set the flag on the Options it received and never cleared it, priming that Options for silent typo acceptance on any later reuse. Added `disallow_unrecognised_options()` to the vendored cxxopts (local patch — flagged at the declaration) and wrapped the toggle in RAII.
## Resource lifecycle
- **Restore signal handlers via RAII.** `wipe`, `builds`, and `oplog-mirror` installed SIGINT/SIGBREAK handlers with raw `signal()` and never restored them; an option-parse throw left the handler targeting an abort flag nothing reads. Added `zen::ScopedSignalHandler` in zen.h and applied at all three sites (builds uses `std::optional` members so the guards survive past `OnParentOptionsParsed` into the subcommand's Run).
- **Route SIGINT in `oplog-mirror` to the worker-pool abort flag.** The command declared a local `std::atomic<bool> AbortFlag` but no handler targeted it — Ctrl-C killed the process instead of cleanly aborting. Added a `MirrorAbortFlag` / `MirrorSignalCallbackHandler` pair in projectstore_impl and bound the local as a reference; existing `.store`/`.load`/capture sites unchanged.
- **Clean up the `cache get` temp download on every exit path.** `Http.Download` parks the payload in the system temp dir; a failed `MoveToFile` (cross-volume, denied target) or an exception could leave the temp file behind. The downloaded buffer is already flagged delete-on-close by `HttpClient`, so the fix is just to clear that flag after a successful `MoveToFile` so the renamed-out file isn't reaped.
## Other
- **Fix wrong URL fields in `oplog-export` / `oplog-import` builds-branch descriptions.** Two operator-facing "[builds] URL/namespace/bucket/buildsid" messages formatted `m_CloudUrl` instead of `m_BuildsUrl` / `m_BuildsHost` (copy-paste from neighbouring `[cloud]` branches), shown as empty or stale at the start of an export/import.
- **Fix "Can't find oplog in project '{}'" formatting and a "Failed top mirror" typo in projectstore_cmd.**
- **Fix a misleading `oplog-export` comment on the `--zen` scheme default** ("Assume https" vs. the `http://` the code writes).
- **Fail `ScrambleDir` when `RemoveFile` doesn't delete.** The `zen builds test` scramble phase used `(void)RemoveFile(FilePath)`, discarding both the bool return and the error. A quiet delete failure let verification run against stale state; switched to the two-arg overload and throw on false return or non-empty `error_code`.
| -rw-r--r-- | CHANGELOG.md | 9 | ||||
| -rw-r--r-- | docs/dev/Testing.md | 53 | ||||
| -rw-r--r-- | src/zen/authutils.cpp | 237 | ||||
| -rw-r--r-- | src/zen/authutils.h | 9 | ||||
| -rw-r--r-- | src/zen/cmds/builds_cmd.cpp | 61 | ||||
| -rw-r--r-- | src/zen/cmds/builds_cmd.h | 7 | ||||
| -rw-r--r-- | src/zen/cmds/cache_cmd.cpp | 26 | ||||
| -rw-r--r-- | src/zen/cmds/projectstore_cmd.cpp | 91 | ||||
| -rw-r--r-- | src/zen/cmds/wipe_cmd.cpp | 6 | ||||
| -rw-r--r-- | src/zen/zen.cpp | 7 | ||||
| -rw-r--r-- | src/zen/zen.h | 29 | ||||
| -rw-r--r-- | src/zencore/crypto.cpp | 389 | ||||
| -rw-r--r-- | src/zencore/include/zencore/crypto.h | 16 | ||||
| -rw-r--r-- | src/zencore/xmake.lua | 13 | ||||
| -rw-r--r-- | src/zenremotestore/builds/buildupdatefolder.cpp | 2 | ||||
| -rw-r--r-- | thirdparty/cxxopts/include/cxxopts.hpp | 10 | ||||
| -rw-r--r-- | xmake.lua | 7 |
17 files changed, 868 insertions, 104 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index bb097b684..184b6b740 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,15 @@ - `zen history --print` prints the reconstructed command line of the selected row instead of launching it - `--enable-execution-history` global option on both binaries (default `true`) to opt out per invocation - The history file is attached to Sentry crash reports (alongside the existing zenserver log) +- Bugfix: `zen builds download --download-spec-path <file>` now resolves the manifest path relative to `--local-path` when relative; previously the computed absolute path was discarded and the file was looked up relative to the current working directory +- Bugfix: `zen project oplog-export --builds-metadata-path <file>` now fails with a descriptive error when the metadata JSON is malformed; previously a JSON parse failure was silently ignored and a truncated compact-binary payload was shipped to the server +- Bugfix: `zen project oplog-mirror` now rejects filenames from the remote oplog that could escape the mirror root (absolute paths, drive letters, UNC / device path prefixes, or `..` components); previously a malicious or compromised remote oplog could cause writes to arbitrary filesystem locations +- Bugfix: `zen project oplog-mirror` now handles Ctrl-C cleanly by signalling the worker pool to abort; previously SIGINT killed the process mid-transfer +- Bugfix: Malformed `--build-id` / `--build-part-id` values are now rejected with an `OptionParseException`; previously the `Oid` parser aborted the process on non-hex or wrong-length input +- Bugfix: `zen cache get` now requires `--output-path` unless `--as-text` is set; previously an empty path silently wrote a file named after the value key / attachment hash into the process's current working directory +- Bugfix: `zen project oplog-export` and `oplog-import` "[builds] ..." description messages now print the actual builds URL; previously they showed the (often empty) cloud URL due to a copy-paste error +- Bugfix: `zen` auth storage now uses a machine-specific random AES key/IV persisted to `<system-root>/auth/machinekey.dat` when `--encryption-aes-key`/`--encryption-aes-iv` are not supplied; previously a hardcoded constant visible in the source was used, letting anyone with source access decrypt persisted OIDC refresh tokens. Supplying only one of the two flags is now rejected rather than silently substituting the hardcoded default for the missing half. Existing auth state files will fail to decrypt once after upgrade and re-auth automatically. +- Improvement: `zen` auth machine key is now wrapped with OS-protected storage so the on-disk blob cannot be unwrapped off-machine or from a backup without also stealing the user's OS master key: Windows DPAPI at per-user scope, macOS Keychain Services (GenericPassword, `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`), and Linux libsecret (opt-in via `--zenlibsecret=yes`, default off because headless servers typically have no Secret Service daemon). On platforms/configurations without wrapping available, the file falls back to raw bytes with `0600` permissions. - Improvement: `tourist` trace-analysis library and `raw_pdb` vendored under `thirdparty/`; tourist made cross-platform (Windows/Linux/macOS) - Improvement: Trace viewer HTML/JS frontend bundled via `zen` frontend zip build rule; `ZipFs` moved from `zenserver` into `zenhttp` for reuse - Improvement: Replaced `OperationLogOutput` with `ProgressBase` in `zenutil`; logging and progress reporting are now separate concerns. Operation classes receive a `LoggerRef` for logging and a `ProgressBase&` for progress bars diff --git a/docs/dev/Testing.md b/docs/dev/Testing.md new file mode 100644 index 000000000..40930f5d6 --- /dev/null +++ b/docs/dev/Testing.md @@ -0,0 +1,53 @@ +# Testing + +## Testing a new `zen` binary via ushell + +Both ushell commands that shell out to the `zen` CLI honor two environment variables. They let you drop a freshly built `zen` executable in front of ushell without touching the UE tree. + +### Affected commands + +- **`.zen`** — `status`, `start`, `stop`, `version`, `import-snapshot`, `create-workspace`, `create-share`, and any other `ZenUtilityBaseCmd`. Implemented in `channels/unreal/core/pylib/zen/cmd.py` (`ZenUtilityBaseCmd.get_zen_utility_command`, ~line 657). + - Note: `.zen start` and `.zen dashboard` launch `ZenLaunch` / `ZenDashboard` UE program targets from the engine tree — **those are not affected** by these variables. Only invocations of the `zen` utility itself are. +- **`.getbuild`** — `channels/unreal/core/cmds/getbuild.py` (`GetBuild.run_zen`, ~line 365). Used for both the `builds list` query and the `builds download` step. + +### `UE_ZEN_BINARY_PATH` + +Absolute path to the `zen` executable to invoke. + +- **Default:** `<EngineDir>/Binaries/<HostPlatform>/zen[.exe]` from the active UE tree. +- **When set:** the value is used verbatim — no platform folder, no `.exe` suffix added. Point it straight at your build output, e.g. `.../Zen/Source/Programs/Zen/out/build/x64-Debug/zen.exe`. +- Both commands log `Using zen binary from UE_ZEN_BINARY_PATH: <path>` on each invocation, and raise `FileNotFoundError` if the file doesn't exist. + +### `UE_ZEN_ARGS` + +Extra global arguments prepended to every `zen` invocation. + +- Shell-split on whitespace (`str.split()`), then inserted **before** the subcommand args and before any user passthrough (`zenargs` / `runargs`). +- Useful for things like `--log-level=debug`, custom config overrides, or experimental flags your new build understands. +- Logged as `Using extra zen args from UE_ZEN_ARGS: <args>` when non-empty. + +### Typical workflow + +```bash +# bash +export UE_ZEN_BINARY_PATH=/path/to/new/zen.exe +export UE_ZEN_ARGS="--log-level=debug" + +# PowerShell +$env:UE_ZEN_BINARY_PATH = "C:\path\to\new\zen.exe" +$env:UE_ZEN_ARGS = "--log-level=debug" + +# cmd +set UE_ZEN_BINARY_PATH=C:\path\to\new\zen.exe +set UE_ZEN_ARGS=--log-level=debug +``` + +Then inside ushell: + +``` +.zen version # confirms which binary is being invoked +.zen status +.getbuild win64 staged --match <cl> +``` + +Unset the variables (or open a fresh shell) to fall back to the in-tree `zen` shipped with the current UE sync. diff --git a/src/zen/authutils.cpp b/src/zen/authutils.cpp index 2696cdcc3..a2af2b63e 100644 --- a/src/zen/authutils.cpp +++ b/src/zen/authutils.cpp @@ -2,8 +2,10 @@ #include "authutils.h" +#include <zencore/crypto.h> #include <zencore/filesystem.h> #include <zencore/fmtutils.h> +#include <zencore/iobuffer.h> #include <zencore/logging.h> #include <zenhttp/auth/authmgr.h> @@ -113,77 +115,204 @@ AuthCommandLineOptions::AddOptions(cxxopts::Options& Ops) ""); }; +// Load or generate a per-install machine AES key+IV under AuthDir/machinekey.dat +// so the auth-state file is encrypted with bytes unique to this machine rather +// than a hardcoded constant. +// +// When per-user OS-protected storage is available (DPAPI on Windows) the key +// material is wrapped before it lands on disk, so a copy of the file off-machine +// or out of a backup cannot be unwrapped without also stealing the user's OS +// master key. On platforms without OS-level wrapping we fall back to persisting +// the raw bytes with restrictive file permissions (0600 on POSIX; user-only on +// Windows via inheritance from the profile dir). +// +// File format: +// [4-byte magic 'Z','E','N','\x01'] [1-byte flags] [payload] +// flags bit 0 set -> payload is OS-protected (DPAPI blob) +// flags bit 0 clear -> payload is raw KeyBytes+IvBytes bytes +// Legacy files without the magic are interpreted as raw bytes. void -AuthCommandLineOptions::ParseOptions(cxxopts::Options& Ops, - const std::filesystem::path& SystemRootDir, - HttpClientSettings& ClientSettings, - std::string_view HostUrl, - std::unique_ptr<AuthMgr>& Auth, - bool Quiet, - bool Hidden, - bool Verbose) +AuthCommandLineOptions::LoadOrCreateMachineKey(const std::filesystem::path& AuthDir, bool Quiet) { - auto CreateAuthMgr = [&]() { - ZEN_ASSERT(!SystemRootDir.empty()); - if (!Auth) + constexpr size_t KeyBytes = AesKey256Bit::ByteCount; + constexpr size_t IvBytes = AesIV128Bit::ByteCount; + static constexpr std::array<uint8_t, 4> FileMagic = {'Z', 'E', 'N', 0x01}; + static constexpr uint8_t FlagProtected = 0x01; + const std::filesystem::path KeyFile = AuthDir / "machinekey.dat"; + std::array<uint8_t, KeyBytes + IvBytes> KeyMaterial{}; + bool Loaded = false; + + auto ParseFile = [&](MemoryView FileBytes) -> bool { + // Legacy: raw KeyBytes+IvBytes payload. + if (FileBytes.GetSize() == KeyMaterial.size()) + { + memcpy(KeyMaterial.data(), FileBytes.GetData(), KeyMaterial.size()); + return true; + } + if (FileBytes.GetSize() < FileMagic.size() + 1) + { + return false; + } + if (memcmp(FileBytes.GetData(), FileMagic.data(), FileMagic.size()) != 0) + { + return false; + } + const uint8_t Flags = static_cast<const uint8_t*>(FileBytes.GetData())[FileMagic.size()]; + const MemoryView Payload = FileBytes.Mid(FileMagic.size() + 1); + if (Flags & FlagProtected) { - static const std::string_view DefaultEncryptionKey("abcdefghijklmnopqrstuvxyz0123456"); - static const std::string_view DefaultEncryptionIV("0123456789abcdef"); - if (m_EncryptionKey.empty() && m_EncryptionIV.empty()) + std::vector<uint8_t> Plaintext; + if (!TryUnprotectData(Payload, Plaintext)) { - m_EncryptionKey = DefaultEncryptionKey; - m_EncryptionIV = DefaultEncryptionIV; if (!Quiet) { - ZEN_CONSOLE_WARN("Auth: Using default encryption key and initialization vector for auth storage"); + ZEN_CONSOLE_WARN("Auth: failed to unwrap OS-protected machine key at '{}', regenerating", KeyFile); } + return false; } - else + if (Plaintext.size() != KeyMaterial.size()) { - if (m_EncryptionKey.empty()) - { - m_EncryptionKey = DefaultEncryptionKey; - if (!Quiet) - { - ZEN_CONSOLE_WARN("Auth: Using default encryption key for auth storage"); - } - } - if (m_EncryptionIV.empty()) - { - m_EncryptionIV = DefaultEncryptionIV; - if (!Quiet) - { - ZEN_CONSOLE_WARN("Auth: Using default encryption initialization vector for auth storage"); - } - } + return false; } + memcpy(KeyMaterial.data(), Plaintext.data(), KeyMaterial.size()); + return true; + } + if (Payload.GetSize() != KeyMaterial.size()) + { + return false; + } + memcpy(KeyMaterial.data(), Payload.GetData(), KeyMaterial.size()); + return true; + }; - AuthConfig AuthMgrConfig = {.RootDirectory = SystemRootDir / "auth", - .EncryptionKey = AesKey256Bit::FromString(m_EncryptionKey), - .EncryptionIV = AesIV128Bit::FromString(m_EncryptionIV)}; - if (!AuthMgrConfig.EncryptionKey.IsValid()) - { - throw OptionParseException(fmt::format("'--encryption-aes-key' ('{}') is malformed", m_EncryptionKey), Ops.help()); - } - if (!AuthMgrConfig.EncryptionIV.IsValid()) + std::error_code Ec; + if (std::filesystem::exists(KeyFile, Ec)) + { + IoBuffer Data = ReadFile(KeyFile).Flatten(); + if (ParseFile(Data.GetView())) + { + Loaded = true; + } + else if (!Quiet) + { + ZEN_CONSOLE_WARN("Auth: machine key file '{}' is unreadable (size {}), regenerating", KeyFile, Data.GetSize()); + } + } + + if (!Loaded) + { + CreateDirectories(AuthDir); + if (!SecureRandomBytes(MutableMemoryView(KeyMaterial.data(), KeyMaterial.size()))) + { + throw std::runtime_error("failed to obtain secure random bytes for auth machine key"); + } + + std::vector<uint8_t> FileBytes; + FileBytes.reserve(FileMagic.size() + 1 + KeyMaterial.size()); + FileBytes.insert(FileBytes.end(), FileMagic.begin(), FileMagic.end()); + + std::vector<uint8_t> Wrapped; + if (TryProtectData(MemoryView(KeyMaterial.data(), KeyMaterial.size()), Wrapped)) + { + FileBytes.push_back(FlagProtected); + FileBytes.insert(FileBytes.end(), Wrapped.begin(), Wrapped.end()); + if (!Quiet) { - throw OptionParseException(fmt::format("'--encryption-aes-iv' ('{}') is malformed", m_EncryptionIV), Ops.help()); + ZEN_CONSOLE_WARN("Auth: generated OS-protected machine-specific auth encryption key at '{}'", KeyFile); } - if (Verbose) + } + else + { + FileBytes.push_back(0); + FileBytes.insert(FileBytes.end(), KeyMaterial.begin(), KeyMaterial.end()); + if (!Quiet) { - ExtendableStringBuilder<128> SB; - SB << "\n RootDirectory: " << AuthMgrConfig.RootDirectory.string(); - SB << "\n EncryptionKey: " << HideSensitiveString(m_EncryptionKey); - SB << "\n EncryptionIV: " << HideSensitiveString(m_EncryptionIV); - ZEN_CONSOLE("Auth: Creating auth manager with:{}", SB.ToString()); + ZEN_CONSOLE_WARN("Auth: generated machine-specific auth encryption key at '{}' (no OS wrapping available)", KeyFile); } - Auth = AuthMgr::Create(AuthMgrConfig); } - }; + WriteFile(KeyFile, IoBufferBuilder::MakeCloneFromMemory(FileBytes.data(), FileBytes.size())); + + // Belt and suspenders: restrict access on POSIX. On Windows the + // default DACL inherited from a per-user profile dir is already + // user-only in the common case; an explicit tighten there would + // require touching the DACL which is more code than it's worth + // while DPAPI wrapping is the primary defense. +#if !ZEN_PLATFORM_WINDOWS + std::error_code PermEc; + std::filesystem::permissions(KeyFile, + std::filesystem::perms::owner_read | std::filesystem::perms::owner_write, + std::filesystem::perm_options::replace, + PermEc); +#endif + } + + m_EncryptionKey.assign(reinterpret_cast<const char*>(KeyMaterial.data()), KeyBytes); + m_EncryptionIV.assign(reinterpret_cast<const char*>(KeyMaterial.data() + KeyBytes), IvBytes); +} + +void +AuthCommandLineOptions::CreateAuthMgr(cxxopts::Options& Ops, + const std::filesystem::path& SystemRootDir, + std::unique_ptr<AuthMgr>& InOutAuth, + bool Quiet, + bool Verbose) +{ + ZEN_ASSERT(!SystemRootDir.empty()); + if (InOutAuth) + { + return; + } + + const std::filesystem::path AuthDir = SystemRootDir / "auth"; + + if (m_EncryptionKey.empty() != m_EncryptionIV.empty()) + { + throw OptionParseException( + std::string("'--encryption-aes-key' and '--encryption-aes-iv' must be supplied together or both omitted"), + Ops.help()); + } + + if (m_EncryptionKey.empty() && m_EncryptionIV.empty()) + { + LoadOrCreateMachineKey(AuthDir, Quiet); + } + + AuthConfig AuthMgrConfig = {.RootDirectory = AuthDir, + .EncryptionKey = AesKey256Bit::FromString(m_EncryptionKey), + .EncryptionIV = AesIV128Bit::FromString(m_EncryptionIV)}; + if (!AuthMgrConfig.EncryptionKey.IsValid()) + { + throw OptionParseException(fmt::format("'--encryption-aes-key' ('{}') is malformed", m_EncryptionKey), Ops.help()); + } + if (!AuthMgrConfig.EncryptionIV.IsValid()) + { + throw OptionParseException(fmt::format("'--encryption-aes-iv' ('{}') is malformed", m_EncryptionIV), Ops.help()); + } + if (Verbose) + { + ExtendableStringBuilder<128> SB; + SB << "\n RootDirectory: " << AuthMgrConfig.RootDirectory.string(); + SB << "\n EncryptionKey: " << HideSensitiveString(m_EncryptionKey); + SB << "\n EncryptionIV: " << HideSensitiveString(m_EncryptionIV); + ZEN_CONSOLE("Auth: Creating auth manager with:{}", SB.ToString()); + } + InOutAuth = AuthMgr::Create(AuthMgrConfig); +} + +void +AuthCommandLineOptions::ParseOptions(cxxopts::Options& Ops, + const std::filesystem::path& SystemRootDir, + HttpClientSettings& ClientSettings, + std::string_view HostUrl, + std::unique_ptr<AuthMgr>& Auth, + bool Quiet, + bool Hidden, + bool Verbose) +{ if (!m_OpenIdProviderUrl.empty() && !m_OpenIdClientId.empty()) { - CreateAuthMgr(); + CreateAuthMgr(Ops, SystemRootDir, Auth, Quiet, Verbose); std::string ProviderName = m_OpenIdProviderName.empty() ? "Default" : m_OpenIdProviderName; if (Verbose) { @@ -250,7 +379,7 @@ AuthCommandLineOptions::ParseOptions(cxxopts::Options& Ops, } else if (!m_OpenIdProviderName.empty()) { - CreateAuthMgr(); + CreateAuthMgr(Ops, SystemRootDir, Auth, Quiet, Verbose); if (!Quiet) { ZEN_CONSOLE("Auth: Using OpenId provider: {}", m_OpenIdProviderName); @@ -283,7 +412,7 @@ AuthCommandLineOptions::ParseOptions(cxxopts::Options& Ops, if (!ClientSettings.AccessTokenProvider) { - CreateAuthMgr(); + CreateAuthMgr(Ops, SystemRootDir, Auth, Quiet, Verbose); if (!Quiet) { ZEN_CONSOLE("Auth: Using default Open ID provider"); diff --git a/src/zen/authutils.h b/src/zen/authutils.h index bd31d2daa..fa353ab04 100644 --- a/src/zen/authutils.h +++ b/src/zen/authutils.h @@ -44,6 +44,15 @@ struct AuthCommandLineOptions bool Quiet, bool Hidden, bool Verbose); + +private: + void CreateAuthMgr(cxxopts::Options& Ops, + const std::filesystem::path& SystemRootDir, + std::unique_ptr<AuthMgr>& InOutAuth, + bool Quiet, + bool Verbose); + + void LoadOrCreateMachineKey(const std::filesystem::path& AuthDir, bool Quiet); }; std::string ReadAccessTokenFromJsonFile(const std::filesystem::path& Path); diff --git a/src/zen/cmds/builds_cmd.cpp b/src/zen/cmds/builds_cmd.cpp index 820ca9c45..a98bec250 100644 --- a/src/zen/cmds/builds_cmd.cpp +++ b/src/zen/cmds/builds_cmd.cpp @@ -4,6 +4,7 @@ #include <zencore/basicfile.h> #include <zencore/compactbinarybuilder.h> +#include <zencore/except_fmt.h> #include <zencore/filesystem.h> #include <zencore/fmtutils.h> #include <zencore/logging.h> @@ -467,10 +468,12 @@ BuildsCommand::OnParentOptionsParsed(const ZenCliOptions& /*GlobalOptions*/) { using namespace builds_impl; - signal(SIGINT, SignalCallbackHandler); + // Held in member variables so the handlers stay installed through the + // subcommand's Run() and are restored when BuildsCommand is destroyed. + m_SigIntGuard.emplace(SIGINT, SignalCallbackHandler); #if ZEN_PLATFORM_WINDOWS - signal(SIGBREAK, SignalCallbackHandler); -#endif // ZEN_PLATFORM_WINDOWS + m_SigBreakGuard.emplace(SIGBREAK, SignalCallbackHandler); +#endif // Validate output options if (m_Configuration.Verbose && m_Configuration.Quiet) @@ -601,7 +604,9 @@ BuildsSubCmdBase::ParseStorageOptions(std::string& BuildId, } if (!ParseBuildStorageUrl(m_Config.Url, Resolved.Host, Resolved.Namespace, Resolved.Bucket, BuildId)) { - throw OptionParseException("'--url' ('{}') is malformed, it does not match the Cloud Artifact URL format", SubOpts.help()); + throw OptionParseException( + fmt::format("'--url' ('{}') is malformed, it does not match the Cloud Artifact URL format", m_Config.Url), + SubOpts.help()); } } @@ -826,39 +831,27 @@ BuildsSubCmdBase::CreateBuildStorage(const std::filesystem::path& ZenFolderDef Oid BuildsSubCmdBase::ParseBuildId(const std::string& BuildIdStr, cxxopts::Options& SubOpts) { - if (BuildIdStr.length() != Oid::StringLength) + const Oid BuildId = Oid::TryFromHexString(BuildIdStr); + if (BuildId == Oid::Zero) { throw OptionParseException( - fmt::format("'--build-id' ('{}') is malformed, it must be {} characters long", BuildIdStr, Oid::StringLength), + fmt::format("'--build-id' ('{}') is malformed, expected {} hex characters", BuildIdStr, Oid::StringLength), SubOpts.help()); } - else if (Oid BuildId = Oid::FromHexString(BuildIdStr); BuildId == Oid::Zero) - { - throw OptionParseException(fmt::format("'--build-id' ('{}') is invalid", BuildIdStr), SubOpts.help()); - } - else - { - return BuildId; - } + return BuildId; } Oid BuildsSubCmdBase::ParseBuildPartId(const std::string& BuildPartIdStr, cxxopts::Options& SubOpts) { - if (BuildPartIdStr.length() != Oid::StringLength) + const Oid BuildPartId = Oid::TryFromHexString(BuildPartIdStr); + if (BuildPartId == Oid::Zero) { throw OptionParseException( - fmt::format("'--build-id' ('{}') is malformed, it must be {} characters long", BuildPartIdStr, Oid::StringLength), + fmt::format("'--build-part-id' ('{}') is malformed, expected {} hex characters", BuildPartIdStr, Oid::StringLength), SubOpts.help()); } - else if (Oid BuildPartId = Oid::FromHexString(BuildPartIdStr); BuildPartId == Oid::Zero) - { - throw OptionParseException(fmt::format("'--build-id' ('{}') is malformed", BuildPartIdStr), SubOpts.help()); - } - else - { - return BuildPartId; - } + return BuildPartId; } std::vector<Oid> @@ -2217,12 +2210,11 @@ BuildsTestSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) const ResolvedStorage Resolved = ParseStorageOptions(TestBuildId, {}, Opts, m_TestSystemRootDir, m_TestStoragePath); StorageInstance Storage = CreateBuildStorage(m_Path / ZenFolderName, {}, Opts, StorageStats, StorageCacheStats, Auth, Resolved); - m_BuildId = Oid::NewOid().ToString(); - m_BuildPartId = Oid::NewOid().ToString(); - m_CreateBuild = true; - - const Oid BuildId = Oid::FromHexString(m_BuildId); - const Oid BuildPartId = Oid::FromHexString(m_BuildPartId); + const Oid BuildId = Oid::NewOid(); + const Oid BuildPartId = Oid::NewOid(); + m_BuildId = BuildId.ToString(); + m_BuildPartId = BuildPartId.ToString(); + m_CreateBuild = true; auto MakeMetaData = [](const Oid& BuildId) -> CbObject { CbObjectWriter BuildMetaDataWriter; @@ -2577,7 +2569,14 @@ BuildsTestSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) case 1: { (void)SetFileReadOnly(FilePath, false); - (void)RemoveFile(FilePath); + std::error_code Ec; + const bool Removed = RemoveFile(FilePath, Ec); + if (!Removed || Ec) + { + throw zen::runtime_error("ScrambleDir: failed to delete '{}'. Reason: '{}'", + FilePath, + Ec ? Ec.message() : "file no longer present"); + } } break; default: diff --git a/src/zen/cmds/builds_cmd.h b/src/zen/cmds/builds_cmd.h index 78d9fac33..c05ac03d0 100644 --- a/src/zen/cmds/builds_cmd.h +++ b/src/zen/cmds/builds_cmd.h @@ -11,6 +11,8 @@ #include "authutils.h" #include "consoleprogress.h" +#include <optional> + namespace zen { class ProgressBase; @@ -427,6 +429,11 @@ private: BuildsTestSubCmd m_TestSubCmd; BuildsMultiTestDownloadSubCmd m_MultiTestDownloadSubCmd; + std::optional<ScopedSignalHandler> m_SigIntGuard; +#if ZEN_PLATFORM_WINDOWS + std::optional<ScopedSignalHandler> m_SigBreakGuard; +#endif + bool OnParentOptionsParsed(const ZenCliOptions& GlobalOptions) override; }; diff --git a/src/zen/cmds/cache_cmd.cpp b/src/zen/cmds/cache_cmd.cpp index b6f272ee7..c03284462 100644 --- a/src/zen/cmds/cache_cmd.cpp +++ b/src/zen/cmds/cache_cmd.cpp @@ -704,6 +704,11 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } } + if (m_OutputPath.empty() && !m_AsText) + { + throw OptionParseException("'--output-path' is required (or pass '--as-text' to print to stdout)", m_SubOptions.help()); + } + if (!m_OutputPath.empty()) { if (IsDir(m_OutputPath)) @@ -715,10 +720,6 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) CreateDirectories(m_OutputPath.parent_path()); } } - if (m_OutputPath.empty()) - { - m_OutputPath = (m_AttachmentHash.empty() ? m_ValueKey : m_AttachmentHash); - } std::string Url = fmt::format("/z$/{}/{}/{}", m_Namespace, m_Bucket, ValueId); if (AttachmentHash != IoHash::Zero) @@ -727,6 +728,11 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } if (HttpClient::Response Result = Http.Download(Url, std::filesystem::temp_directory_path()); Result) { + // `Http.Download` parks the payload in the system temp dir and returns + // a buffer that already has delete-on-close set, so every exit path + // (exception, fallback WriteFile, `--as-text` console print) reaps it. + // A successful MoveToFile below clears the flag so the payload's + // handle-close doesn't delete the caller's output afterwards. auto TryDecompress = [](const IoBuffer& Buffer) -> IoBuffer { IoHash RawHash; uint64_t RawSize; @@ -753,7 +759,17 @@ CacheGetSubCmd::Run(const ZenCliOptions& /*GlobalOptions*/) } else { - if (!MoveToFile(m_OutputPath, ChunkData)) + if (MoveToFile(m_OutputPath, ChunkData)) + { + // The file was renamed into place; clearing DeleteOnClose prevents + // the move'd-out file at m_OutputPath from being deleted when the + // payload's handle closes. When m_Decompress is false ChunkData + // shares a core with ResponsePayload so either clear suffices; + // when decompressed ChunkData is in-memory and MoveToFile would + // have failed, so we don't reach this branch. + Result.ResponsePayload.SetDeleteOnClose(false); + } + else { WriteFile(m_OutputPath, ChunkData); } diff --git a/src/zen/cmds/projectstore_cmd.cpp b/src/zen/cmds/projectstore_cmd.cpp index 2bd4803f6..d9ce1cfa7 100644 --- a/src/zen/cmds/projectstore_cmd.cpp +++ b/src/zen/cmds/projectstore_cmd.cpp @@ -8,6 +8,7 @@ #include <zencore/compactbinarybuilder.h> #include <zencore/compactbinaryutil.h> #include <zencore/compress.h> +#include <zencore/except_fmt.h> #include <zencore/filesystem.h> #include <zencore/fmtutils.h> #include <zencore/logging.h> @@ -120,6 +121,26 @@ namespace projectstore_impl { } } + // `OplogMirrorCommand::Run` uses a latching boolean flag rather than the + // SignalCounter above, because it drives a worker pool that aborts on any + // interrupt. Kept separate from SignalCallbackHandler so neither interferes + // with the other when both are installed in the same process. + static std::atomic<bool> MirrorAbortFlag{false}; + + static void MirrorSignalCallbackHandler(int SigNum) + { + if (SigNum == SIGINT) + { + MirrorAbortFlag.store(true); + } +#if ZEN_PLATFORM_WINDOWS + if (SigNum == SIGBREAK) + { + MirrorAbortFlag.store(true); + } +#endif + } + void ExecuteAsyncOperation(HttpClient& Http, std::string_view Url, IoBuffer&& Payload, bool PlainProgress) { signal(SIGINT, SignalCallbackHandler); @@ -577,7 +598,7 @@ DropProjectCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg m_OplogName = ResolveOplog(Http, m_ProjectName, m_OplogName); if (m_OplogName.empty()) { - throw std::runtime_error(fmt::format("Can't find oplog in project '{}'", m_OplogName, m_ProjectName)); + throw zen::runtime_error("Can't find oplog '{}' in project '{}'", m_OplogName, m_ProjectName); } if (m_DryRun) { @@ -1098,7 +1119,7 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg std::string TargetUrlBase = m_ZenUrl; if (TargetUrlBase.find("://") == std::string::npos) { - // Assume https URL + // Assume http URL TargetUrlBase = fmt::format("http://{}", TargetUrlBase); } @@ -1287,7 +1308,14 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg std::filesystem::path MetadataPath(m_BuildsMetadataPath); IoBuffer MetaDataJson = ReadFile(MetadataPath).Flatten(); std::string_view Json(reinterpret_cast<const char*>(MetaDataJson.GetData()), MetaDataJson.GetSize()); - CbFieldIterator MetaData = LoadCompactBinaryFromJson(Json); + std::string JsonError; + CbFieldIterator MetaData = LoadCompactBinaryFromJson(Json, JsonError); + if (!JsonError.empty()) + { + throw zen::runtime_error("builds metadata file '{}' is malformed. Reason: '{}'", + MetadataPath.string(), + JsonError); + } Writer.AddBinary("metadata"sv, MetaData.GetBuffer()); } if (!m_BuildsMetadata.empty()) @@ -1297,7 +1325,7 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg size_t SplitPos = Pair.find('='); if (SplitPos == std::string::npos || SplitPos == 0) { - throw std::runtime_error(fmt::format("builds metadata key-value pair '{}' is malformed", Pair)); + throw zen::runtime_error("builds metadata key-value pair '{}' is malformed", Pair); } MetaDataWriter.AddString(Pair.substr(0, SplitPos), Pair.substr(SplitPos + 1)); return true; @@ -1307,7 +1335,7 @@ ExportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg } } Writer.EndObject(); // "builds" - TargetDescription = fmt::format("[builds] {}/{}/{}/{}", m_CloudUrl, m_JupiterNamespace, m_JupiterBucket, m_BuildsId); + TargetDescription = fmt::format("[builds] {}/{}/{}/{}", m_BuildsUrl, m_JupiterNamespace, m_JupiterBucket, m_BuildsId); } if (!m_ZenUrl.empty()) { @@ -1716,7 +1744,11 @@ ImportOplogCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg } } Writer.EndObject(); // "builds" - SourceDescription = fmt::format("[builds] {}/{}/{}/{}", m_CloudUrl, m_JupiterNamespace, m_JupiterBucket, m_BuildsId); + SourceDescription = fmt::format("[builds] {}/{}/{}/{}", + m_BuildsHost.empty() ? m_BuildsOverrideHost : m_BuildsHost, + m_JupiterNamespace, + m_JupiterBucket, + m_BuildsId); } if (!m_ZenUrl.empty()) { @@ -2062,16 +2094,59 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg std::unordered_set<std::u8string> FileNames; std::atomic<uint64_t> WrittenByteCount = 0; - std::atomic<bool> AbortFlag(false); + // Install Ctrl-C handler so SIGINT aborts the worker pool rather than killing + // the process. Without this the local AbortFlag would shadow whatever global + // handler is installed elsewhere and interrupts would be dropped. RAII so + // the previous handler is restored when the function returns or throws. + MirrorAbortFlag.store(false); + ScopedSignalHandler SigIntGuard(SIGINT, MirrorSignalCallbackHandler); +#if ZEN_PLATFORM_WINDOWS + ScopedSignalHandler SigBreakGuard(SIGBREAK, MirrorSignalCallbackHandler); +#endif + std::atomic<bool>& AbortFlag = MirrorAbortFlag; Stopwatch WriteStopWatch; + // Filenames come from the remote oplog, which may be compromised or untrusted. + // Reject anything that could escape the mirror root via an absolute path, drive + // letter / UNC / device path prefix, or '..' component before it is joined to + // RootPath. Returns nullptr when the filename is safe. + auto UnsafeFileNameReason = [](const std::filesystem::path& FileName) -> const char* { + if (FileName.empty()) + { + return "filename is empty"; + } + if (FileName.has_root_name()) + { + return "filename has a root name (drive letter, UNC share, or device path)"; + } + if (FileName.has_root_directory()) + { + return "filename is absolute"; + } + for (const std::filesystem::path& Component : FileName) + { + const std::u8string C = Component.u8string(); + if (C.empty() || C == u8"..") + { + return "filename contains a '..' or empty component"; + } + } + return nullptr; + }; + auto EmitFilesForDataArray = [&](CbArrayView DataArray) { for (auto DataIter : DataArray) { if (CbObjectView Data = DataIter.AsObjectView()) { std::filesystem::path FileName(Data["filename"sv].AsU8String()); + if (const char* Reason = UnsafeFileNameReason(FileName)) + { + ZEN_CONSOLE_ERROR("Rejecting unsafe filename '{}' from remote oplog: {}", FileName.string(), Reason); + AbortFlag.store(true); + break; + } if (!m_FilenameFilter.empty()) { std::string FileNameLowerCase = ToLower(FileName.string()); @@ -2217,7 +2292,7 @@ OplogMirrorCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** arg if (AbortFlag) { - throw std::runtime_error("Failed top mirror oplog"); + throw std::runtime_error("Failed to mirror oplog"); } } else diff --git a/src/zen/cmds/wipe_cmd.cpp b/src/zen/cmds/wipe_cmd.cpp index 713c5e386..d5344fb01 100644 --- a/src/zen/cmds/wipe_cmd.cpp +++ b/src/zen/cmds/wipe_cmd.cpp @@ -538,10 +538,10 @@ WipeCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv) using namespace wipe_impl; ZEN_UNUSED(GlobalOptions); - signal(SIGINT, SignalCallbackHandler); + ScopedSignalHandler SigIntGuard(SIGINT, SignalCallbackHandler); #if ZEN_PLATFORM_WINDOWS - signal(SIGBREAK, SignalCallbackHandler); -#endif // ZEN_PLATFORM_WINDOWS + ScopedSignalHandler SigBreakGuard(SIGBREAK, SignalCallbackHandler); +#endif if (!ParseOptions(argc, argv)) { diff --git a/src/zen/zen.cpp b/src/zen/zen.cpp index 9e39d2d67..048cbe920 100644 --- a/src/zen/zen.cpp +++ b/src/zen/zen.cpp @@ -42,6 +42,7 @@ #include <zencore/trace.h> #include <zencore/windows.h> #include <zenhttp/httpcommon.h> +#include <zenutil/config/commandlineoptions.h> #include <zenutil/config/environmentoptions.h> #include <zenutil/consoletui.h> #include <zenutil/invocationhistory.h> @@ -188,6 +189,9 @@ ZenCmdBase::ParseOptionsPermissive(cxxopts::Options& CmdOptions, int argc, char* { CmdOptions.set_width(TuiConsoleColumns(80)); CmdOptions.allow_unrecognised_options(); + // Revert the flag on scope exit so re-parsing the same Options later is strict. + // cxxopts has no getter for the previous state, so we unconditionally clear it. + auto _ = MakeGuard([&]() { CmdOptions.disallow_unrecognised_options(); }); cxxopts::ParseResult Result; @@ -964,8 +968,7 @@ main(int argc, char** argv) { SB.Append(' '); } - - SB.Append(argv[i]); + SB.Append(std::string_view(argv[i])); } SentryConfig.DatabasePath = SentryDatabasePath; diff --git a/src/zen/zen.h b/src/zen/zen.h index 1f8fd78eb..f62573958 100644 --- a/src/zen/zen.h +++ b/src/zen/zen.h @@ -9,6 +9,8 @@ #include <zenutil/config/commandlineoptions.h> #include <zenutil/config/loggingconfig.h> +#include <csignal> + namespace zen { struct ZenCliOptions @@ -38,6 +40,33 @@ extern ZenCmdCategory g_ProjectStoreCategory; extern ZenCmdCategory g_CacheStoreCategory; extern ZenCmdCategory g_StorageCategory; +// RAII wrapper around `signal(2)` that restores the previous handler on scope +// exit. Use in command Run() methods so an exception during option parsing or +// execution doesn't leave a handler installed whose flag is scoped to a +// now-dead lifetime. +class ScopedSignalHandler +{ +public: + using Handler = void (*)(int); + + ScopedSignalHandler(int SigNum, Handler NewHandler) : m_SigNum(SigNum), m_PrevHandler(std::signal(SigNum, NewHandler)) {} + + ~ScopedSignalHandler() + { + if (m_PrevHandler != SIG_ERR) + { + std::signal(m_SigNum, m_PrevHandler); + } + } + + ScopedSignalHandler(const ScopedSignalHandler&) = delete; + ScopedSignalHandler& operator=(const ScopedSignalHandler&) = delete; + +private: + int m_SigNum; + Handler m_PrevHandler; +}; + class ErrorWithReturnCode : public std::runtime_error { public: diff --git a/src/zencore/crypto.cpp b/src/zencore/crypto.cpp index 049854b42..9984f35ac 100644 --- a/src/zencore/crypto.cpp +++ b/src/zencore/crypto.cpp @@ -6,6 +6,7 @@ #include <zencore/scopeguard.h> #include <zencore/testing.h> +#include <array> #include <string> #include <string_view> @@ -47,14 +48,27 @@ ZEN_THIRD_PARTY_INCLUDES_START # include <openssl/conf.h> # include <openssl/err.h> # include <openssl/evp.h> +# include <openssl/rand.h> #elif ZEN_USE_MBEDTLS # include <mbedtls/cipher.h> +# include <mbedtls/ctr_drbg.h> +# include <mbedtls/entropy.h> #else # include <zencore/windows.h> # include <bcrypt.h> # define NT_SUCCESS(Status) (((NTSTATUS)(Status)) >= 0) # define STATUS_UNSUCCESSFUL ((NTSTATUS)0xC0000001L) #endif + +#if ZEN_PLATFORM_WINDOWS +# include <zencore/windows.h> +# include <dpapi.h> +#elif ZEN_PLATFORM_MAC +# include <CoreFoundation/CoreFoundation.h> +# include <Security/Security.h> +#elif ZEN_PLATFORM_LINUX && ZEN_USE_LIBSECRET +# include <libsecret/secret.h> +#endif ZEN_THIRD_PARTY_INCLUDES_END namespace zen { @@ -420,6 +434,291 @@ namespace crypto { } // namespace crypto +bool +SecureRandomBytes(MutableMemoryView Out) +{ + if (Out.GetSize() == 0) + { + return true; + } + +#if ZEN_USE_BCRYPT + NTSTATUS Status = + BCryptGenRandom(nullptr, static_cast<PUCHAR>(Out.GetData()), static_cast<ULONG>(Out.GetSize()), BCRYPT_USE_SYSTEM_PREFERRED_RNG); + return NT_SUCCESS(Status); +#elif ZEN_USE_OPENSSL + return RAND_bytes(static_cast<unsigned char*>(Out.GetData()), static_cast<int>(Out.GetSize())) == 1; +#else // ZEN_USE_MBEDTLS + mbedtls_entropy_context Entropy; + mbedtls_ctr_drbg_context Drbg; + mbedtls_entropy_init(&Entropy); + mbedtls_ctr_drbg_init(&Drbg); + auto _ = MakeGuard([&]() { + mbedtls_ctr_drbg_free(&Drbg); + mbedtls_entropy_free(&Entropy); + }); + if (mbedtls_ctr_drbg_seed(&Drbg, mbedtls_entropy_func, &Entropy, nullptr, 0) != 0) + { + return false; + } + return mbedtls_ctr_drbg_random(&Drbg, static_cast<unsigned char*>(Out.GetData()), Out.GetSize()) == 0; +#endif +} + +#if ZEN_PLATFORM_MAC || (ZEN_PLATFORM_LINUX && ZEN_USE_LIBSECRET) +namespace { + // Keychain-backed wrap/unwrap. Plaintext is stored in the OS keyring under a + // fixed service and a per-call random account id. The "wrapped" blob returned to + // the caller is just the account id bytes; the plaintext never lands on disk. + constexpr size_t kKeychainAccountBytes = 16; + constexpr char kKeychainServiceName[] = "org.unrealengine.zen.auth"; + + bool AccountStringFromBytes(const uint8_t* Bytes, size_t Size, char* OutHex, size_t OutHexSize) + { + if (OutHexSize < Size * 2 + 1) + { + return false; + } + for (size_t i = 0; i < Size; ++i) + { + static const char kHex[] = "0123456789abcdef"; + OutHex[i * 2] = kHex[Bytes[i] >> 4]; + OutHex[i * 2 + 1] = kHex[Bytes[i] & 0x0F]; + } + OutHex[Size * 2] = '\0'; + return true; + } +} // namespace +#endif + +#if ZEN_PLATFORM_LINUX && ZEN_USE_LIBSECRET +namespace { + // Schema name is shared across zen installs on the machine — uniqueness per + // install comes from the random `account` attribute. + const SecretSchema* ZenAuthSchema() + { + static const SecretSchema Schema = {"org.unrealengine.zen.AuthMachineKey", + SECRET_SCHEMA_NONE, + { + {"account", SECRET_SCHEMA_ATTRIBUTE_STRING}, + {nullptr, SecretSchemaAttributeType(0)}, + }, + // reserved + 0, + 0, + 0, + 0, + 0, + 0, + 0}; + return &Schema; + } +} // namespace +#endif + +bool +TryProtectData(MemoryView Plaintext, std::vector<uint8_t>& OutProtected) +{ +#if ZEN_PLATFORM_WINDOWS + DATA_BLOB In{static_cast<DWORD>(Plaintext.GetSize()), const_cast<BYTE*>(static_cast<const BYTE*>(Plaintext.GetData()))}; + DATA_BLOB Out{}; + if (!CryptProtectData(&In, L"zen auth machine key", nullptr, nullptr, nullptr, 0, &Out)) + { + return false; + } + auto _ = MakeGuard([&Out]() { LocalFree(Out.pbData); }); + OutProtected.assign(Out.pbData, Out.pbData + Out.cbData); + return true; +#elif ZEN_PLATFORM_MAC + uint8_t AccountBytes[kKeychainAccountBytes]; + if (!SecureRandomBytes(MutableMemoryView(AccountBytes, sizeof(AccountBytes)))) + { + return false; + } + char AccountHex[sizeof(AccountBytes) * 2 + 1]; + AccountStringFromBytes(AccountBytes, sizeof(AccountBytes), AccountHex, sizeof(AccountHex)); + + CFStringRef Account = CFStringCreateWithCString(nullptr, AccountHex, kCFStringEncodingASCII); + if (Account == nullptr) + { + return false; + } + auto _A = MakeGuard([&]() { CFRelease(Account); }); + + CFDataRef Secret = CFDataCreate(nullptr, static_cast<const UInt8*>(Plaintext.GetData()), static_cast<CFIndex>(Plaintext.GetSize())); + if (Secret == nullptr) + { + return false; + } + auto _S = MakeGuard([&]() { CFRelease(Secret); }); + + const void* Keys[] = {kSecClass, kSecAttrService, kSecAttrAccount, kSecValueData, kSecAttrAccessible}; + const void* Values[] = {kSecClassGenericPassword, + CFSTR("org.unrealengine.zen.auth"), + Account, + Secret, + kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly}; + CFDictionaryRef Attrs = CFDictionaryCreate(nullptr, + Keys, + Values, + sizeof(Keys) / sizeof(*Keys), + &kCFTypeDictionaryKeyCallBacks, + &kCFTypeDictionaryValueCallBacks); + if (Attrs == nullptr) + { + return false; + } + auto _D = MakeGuard([&]() { CFRelease(Attrs); }); + + OSStatus Status = SecItemAdd(Attrs, nullptr); + if (Status != errSecSuccess) + { + return false; + } + OutProtected.assign(AccountBytes, AccountBytes + sizeof(AccountBytes)); + return true; +#elif ZEN_PLATFORM_LINUX && ZEN_USE_LIBSECRET + uint8_t AccountBytes[kKeychainAccountBytes]; + if (!SecureRandomBytes(MutableMemoryView(AccountBytes, sizeof(AccountBytes)))) + { + return false; + } + char AccountHex[sizeof(AccountBytes) * 2 + 1]; + AccountStringFromBytes(AccountBytes, sizeof(AccountBytes), AccountHex, sizeof(AccountHex)); + + // libsecret password APIs take UTF-8 strings, so base64 the raw key material + // before handing it to the keyring. Decoded back on lookup. + gchar* Encoded = g_base64_encode(static_cast<const guchar*>(Plaintext.GetData()), Plaintext.GetSize()); + if (Encoded == nullptr) + { + return false; + } + auto _E = MakeGuard([&]() { g_free(Encoded); }); + + GError* Err = nullptr; + gboolean Ok = secret_password_store_sync(ZenAuthSchema(), + SECRET_COLLECTION_DEFAULT, + "zen auth machine key", + Encoded, + nullptr, + &Err, + "account", + AccountHex, + nullptr); + if (Err != nullptr) + { + g_error_free(Err); + } + if (!Ok) + { + return false; + } + OutProtected.assign(AccountBytes, AccountBytes + sizeof(AccountBytes)); + return true; +#else + (void)Plaintext; + (void)OutProtected; + return false; +#endif +} + +bool +TryUnprotectData(MemoryView Protected, std::vector<uint8_t>& OutPlaintext) +{ +#if ZEN_PLATFORM_WINDOWS + DATA_BLOB In{static_cast<DWORD>(Protected.GetSize()), const_cast<BYTE*>(static_cast<const BYTE*>(Protected.GetData()))}; + DATA_BLOB Out{}; + if (!CryptUnprotectData(&In, nullptr, nullptr, nullptr, nullptr, 0, &Out)) + { + return false; + } + auto _ = MakeGuard([&Out]() { LocalFree(Out.pbData); }); + OutPlaintext.assign(Out.pbData, Out.pbData + Out.cbData); + return true; +#elif ZEN_PLATFORM_MAC + if (Protected.GetSize() != kKeychainAccountBytes) + { + return false; + } + char AccountHex[kKeychainAccountBytes * 2 + 1]; + AccountStringFromBytes(static_cast<const uint8_t*>(Protected.GetData()), kKeychainAccountBytes, AccountHex, sizeof(AccountHex)); + + CFStringRef Account = CFStringCreateWithCString(nullptr, AccountHex, kCFStringEncodingASCII); + if (Account == nullptr) + { + return false; + } + auto _A = MakeGuard([&]() { CFRelease(Account); }); + + const void* Keys[] = {kSecClass, kSecAttrService, kSecAttrAccount, kSecReturnData, kSecMatchLimit}; + const void* Values[] = {kSecClassGenericPassword, CFSTR("org.unrealengine.zen.auth"), Account, kCFBooleanTrue, kSecMatchLimitOne}; + CFDictionaryRef Query = CFDictionaryCreate(nullptr, + Keys, + Values, + sizeof(Keys) / sizeof(*Keys), + &kCFTypeDictionaryKeyCallBacks, + &kCFTypeDictionaryValueCallBacks); + if (Query == nullptr) + { + return false; + } + auto _D = MakeGuard([&]() { CFRelease(Query); }); + + CFTypeRef Result = nullptr; + OSStatus Status = SecItemCopyMatching(Query, &Result); + if (Status != errSecSuccess || Result == nullptr) + { + return false; + } + auto _R = MakeGuard([&]() { CFRelease(Result); }); + + if (CFGetTypeID(Result) != CFDataGetTypeID()) + { + return false; + } + CFDataRef Data = static_cast<CFDataRef>(Result); + const UInt8* DataPtr = CFDataGetBytePtr(Data); + const CFIndex DataLen = CFDataGetLength(Data); + OutPlaintext.assign(DataPtr, DataPtr + DataLen); + return true; +#elif ZEN_PLATFORM_LINUX && ZEN_USE_LIBSECRET + if (Protected.GetSize() != kKeychainAccountBytes) + { + return false; + } + char AccountHex[kKeychainAccountBytes * 2 + 1]; + AccountStringFromBytes(static_cast<const uint8_t*>(Protected.GetData()), kKeychainAccountBytes, AccountHex, sizeof(AccountHex)); + + GError* Err = nullptr; + gchar* Encoded = secret_password_lookup_sync(ZenAuthSchema(), nullptr, &Err, "account", AccountHex, nullptr); + if (Err != nullptr) + { + g_error_free(Err); + } + if (Encoded == nullptr) + { + return false; + } + // `secret_password_free` zeroes memory before freeing; use it rather than g_free. + auto _E = MakeGuard([&]() { secret_password_free(Encoded); }); + + gsize DecodedLen = 0; + guchar* Decoded = g_base64_decode(Encoded, &DecodedLen); + if (Decoded == nullptr) + { + return false; + } + auto _D = MakeGuard([&]() { g_free(Decoded); }); + + OutPlaintext.assign(Decoded, Decoded + DecodedLen); + return true; +#else + (void)Protected; + (void)OutPlaintext; + return false; +#endif +} + MemoryView Aes::Encrypt(const AesKey256Bit& Key, const AesIV128Bit& IV, MemoryView In, MutableMemoryView Out, std::optional<std::string>& Reason) { @@ -502,6 +801,96 @@ TEST_CASE("crypto.aes") } } +TEST_CASE("crypto.securerandom") +{ + std::array<uint8_t, 64> A{}; + std::array<uint8_t, 64> B{}; + CHECK(SecureRandomBytes(MutableMemoryView(A.data(), A.size()))); + CHECK(SecureRandomBytes(MutableMemoryView(B.data(), B.size()))); + // Vanishingly small probability two 64-byte draws match. + CHECK(A != B); +} + +TEST_CASE("crypto.protectdata") +{ + const uint8_t Plain[48] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, + 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48}; + std::vector<uint8_t> Wrapped; + const bool WrapOk = TryProtectData(MakeMemoryView(Plain), Wrapped); +# if ZEN_PLATFORM_WINDOWS + REQUIRE(WrapOk); + CHECK(Wrapped.size() != sizeof(Plain)); // DPAPI envelope adds overhead + CHECK(memcmp(Wrapped.data(), Plain, std::min(Wrapped.size(), sizeof(Plain))) != 0); + + std::vector<uint8_t> Unwrapped; + REQUIRE(TryUnprotectData(MakeMemoryView(Wrapped), Unwrapped)); + CHECK(Unwrapped.size() == sizeof(Plain)); + CHECK(memcmp(Unwrapped.data(), Plain, sizeof(Plain)) == 0); +# elif ZEN_PLATFORM_MAC + // Keychain may not be accessible in headless / CI contexts. Round-trip is + // asserted only when Wrap succeeds; otherwise the test is a no-op. + if (WrapOk) + { + CHECK(Wrapped.size() == 16); // Keychain account id + std::vector<uint8_t> Unwrapped; + REQUIRE(TryUnprotectData(MakeMemoryView(Wrapped), Unwrapped)); + CHECK(Unwrapped.size() == sizeof(Plain)); + CHECK(memcmp(Unwrapped.data(), Plain, sizeof(Plain)) == 0); + + // Delete the Keychain entry we just added so tests don't accumulate residue. + static const char kHex[] = "0123456789abcdef"; + char AccountHex[33]; + for (size_t i = 0; i < 16; ++i) + { + AccountHex[i * 2] = kHex[Wrapped[i] >> 4]; + AccountHex[i * 2 + 1] = kHex[Wrapped[i] & 0x0F]; + } + AccountHex[32] = '\0'; + CFStringRef Account = CFStringCreateWithCString(nullptr, AccountHex, kCFStringEncodingASCII); + const void* Keys[] = {kSecClass, kSecAttrService, kSecAttrAccount}; + const void* Values[] = {kSecClassGenericPassword, CFSTR("org.unrealengine.zen.auth"), Account}; + CFDictionaryRef Query = + CFDictionaryCreate(nullptr, Keys, Values, 3, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks); + SecItemDelete(Query); + CFRelease(Query); + CFRelease(Account); + } +# elif ZEN_PLATFORM_LINUX && ZEN_USE_LIBSECRET + // libsecret round-trip only when a Secret Service daemon is reachable. On + // headless / container CI with no D-Bus session, WrapOk is false and we fall + // back to the raw-bytes path at the authutils level, so the test is a no-op. + if (WrapOk) + { + CHECK(Wrapped.size() == 16); + std::vector<uint8_t> Unwrapped; + REQUIRE(TryUnprotectData(MakeMemoryView(Wrapped), Unwrapped)); + CHECK(Unwrapped.size() == sizeof(Plain)); + CHECK(memcmp(Unwrapped.data(), Plain, sizeof(Plain)) == 0); + + // Clean up the keyring entry we just created. + static const char kHex[] = "0123456789abcdef"; + char AccountHex[33]; + for (size_t i = 0; i < 16; ++i) + { + AccountHex[i * 2] = kHex[Wrapped[i] >> 4]; + AccountHex[i * 2 + 1] = kHex[Wrapped[i] & 0x0F]; + } + AccountHex[32] = '\0'; + GError* Err = nullptr; + secret_password_clear_sync(ZenAuthSchema(), nullptr, &Err, "account", AccountHex, nullptr); + if (Err != nullptr) + { + g_error_free(Err); + } + } +# else + // No OS-level implementation compiled in; both calls must report failure. + CHECK(WrapOk == false); + std::vector<uint8_t> Unwrapped; + CHECK(TryUnprotectData(MakeMemoryView(Plain), Unwrapped) == false); +# endif +} + TEST_SUITE_END(); #endif diff --git a/src/zencore/include/zencore/crypto.h b/src/zencore/include/zencore/crypto.h index 88d156879..a5e23135f 100644 --- a/src/zencore/include/zencore/crypto.h +++ b/src/zencore/include/zencore/crypto.h @@ -8,6 +8,7 @@ #include <memory> #include <optional> +#include <vector> namespace zen { @@ -72,6 +73,21 @@ public: std::optional<std::string>& Reason); }; +// Fill Out with cryptographically secure random bytes from the platform RNG. +// Returns false if the platform RNG failed; on success Out is filled entirely. +bool SecureRandomBytes(MutableMemoryView Out); + +// Wrap Plaintext with per-user OS-protected storage (Windows DPAPI). On success +// OutProtected holds an opaque blob that only the current OS user on the current +// machine can UnprotectData. Returns false on platforms without an implementation +// (currently macOS and Linux) and when the platform call fails; callers should +// fall back to persisting the plaintext with restrictive file permissions. +bool TryProtectData(MemoryView Plaintext, std::vector<uint8_t>& OutProtected); + +// Inverse of TryProtectData. Returns false if the blob cannot be unwrapped by the +// current user/machine context. +bool TryUnprotectData(MemoryView Protected, std::vector<uint8_t>& OutPlaintext); + void crypto_forcelink(); } // namespace zen diff --git a/src/zencore/xmake.lua b/src/zencore/xmake.lua index fe12c14e8..c5a3ea562 100644 --- a/src/zencore/xmake.lua +++ b/src/zencore/xmake.lua @@ -39,6 +39,14 @@ target('zencore') if is_plat("linux", "macosx") then add_packages("openssl3") -- required for crypto end + + if is_plat("linux") and has_config("zenlibsecret") then + -- libsecret-1 is pulled from the system package (libsecret-1-dev on + -- Debian/Ubuntu, libsecret-devel on Fedora). xmake's package recipe + -- resolves include dirs and link flags through pkg-config. + add_requires("libsecret") + add_packages("libsecret") + end add_packages( "gsl-lite", @@ -79,6 +87,11 @@ target('zencore') add_syslinks("rt") end + if is_plat("macosx") then + -- Security.framework for TryProtectData/TryUnprotectData (Keychain) + add_frameworks("Security", "CoreFoundation") + end + if is_plat("windows") then add_syslinks("Advapi32") add_syslinks("Dbghelp") diff --git a/src/zenremotestore/builds/buildupdatefolder.cpp b/src/zenremotestore/builds/buildupdatefolder.cpp index e1a9a553d..98972740a 100644 --- a/src/zenremotestore/builds/buildupdatefolder.cpp +++ b/src/zenremotestore/builds/buildupdatefolder.cpp @@ -4582,7 +4582,7 @@ DownloadFolder(LoggerRef InLog, { const std::filesystem::path AbsoluteDownloadSpecPath = DownloadSpecPath.is_relative() ? MakeSafeAbsolutePath(Path / DownloadSpecPath) : MakeSafeAbsolutePath(DownloadSpecPath); - Manifest = ParseBuildManifest(DownloadSpecPath); + Manifest = ParseBuildManifest(AbsoluteDownloadSpecPath); } std::vector<ChunkedFolderContent> PartContents; diff --git a/thirdparty/cxxopts/include/cxxopts.hpp b/thirdparty/cxxopts/include/cxxopts.hpp index 0b272acec..189ba3ba7 100644 --- a/thirdparty/cxxopts/include/cxxopts.hpp +++ b/thirdparty/cxxopts/include/cxxopts.hpp @@ -1920,6 +1920,16 @@ class Options return *this; } + // Local patch: counterpart to allow_unrecognised_options so the flag can be + // reverted after a permissive parse. Not in upstream cxxopts 3.2.1; if that + // changes the upstream name wins. + Options& + disallow_unrecognised_options() + { + m_allow_unrecognised = false; + return *this; + } + Options& set_width(std::size_t width) { @@ -408,6 +408,13 @@ option("zenrpmalloc") option_end() add_define_by_config("ZEN_USE_RPMALLOC", "zenrpmalloc") +option("zenlibsecret") + set_default(false) + set_showmenu(true) + set_description("Use libsecret (Secret Service / gnome-keyring) on Linux for zen auth key wrapping. Requires libsecret-1-dev at build time.") +option_end() +add_define_by_config("ZEN_USE_LIBSECRET", "zenlibsecret") + if is_os("windows") then option("httpsys") set_default(true) |