diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ee40889f31..febfb4931044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [7.0.5] + +[7.0.5]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.5 + +### Security + +- Host-created files (ledger chunks, snapshots, PID file, and node certificate/key files) are now created with restrictive permissions (`0600`) instead of relying on the process `umask`. Existing deployments will not see existing files affected; only newly created files will have these restricted permissions (#7916). + ## [7.0.4] [7.0.4]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.4 diff --git a/python/pyproject.toml b/python/pyproject.toml index b8144b614556..e8809e787664 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "ccf" -version = "7.0.4" +version = "7.0.5" authors = [ { name="CCF Team", email="CCF-Sec@microsoft.com" }, ] diff --git a/src/ds/files.h b/src/ds/files.h index 00e967edbe48..d19e1550f2f6 100644 --- a/src/ds/files.h +++ b/src/ds/files.h @@ -2,14 +2,23 @@ // Licensed under the Apache 2.0 License. #pragma once +#include +#include +#include #include +#include +#include #include #include #include #include #include +#include #include #include +#include +#include +#include #include #define FMT_HEADER_ONLY @@ -19,6 +28,45 @@ namespace files { namespace fs = std::filesystem; + static constexpr mode_t private_file_permissions = S_IRUSR | S_IWUSR; + + static int open_fd( + const fs::path& file, + int flags, + mode_t permissions = private_file_permissions) + { + return ::open(file.c_str(), flags, permissions); + } + + static FILE* open_file( + const fs::path& file, + int flags, + const char* mode, + mode_t permissions = private_file_permissions) + { + const auto fd = open_fd(file, flags, permissions); + if (fd == -1) + { + return nullptr; + } + + errno = 0; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + auto* f = fdopen(fd, mode); + if (f == nullptr) + { + auto fdopen_errno = errno; + // Preserve the original fdopen() failure when it set errno, and only + // fall back to close()'s errno if fdopen() did not. + if (close(fd) != 0 && fdopen_errno == 0) + { + fdopen_errno = errno; + } + errno = fdopen_errno != 0 ? fdopen_errno : EIO; + } + + return f; + } /** * @brief Checks if a path exists @@ -114,32 +162,62 @@ namespace files return nlohmann::json::parse(v.begin(), v.end()); } + static void dump(std::span data, const fs::path& file) + { + auto* f = open_file(file, O_WRONLY | O_CREAT | O_TRUNC, "wb"); + if (f == nullptr) + { + throw std::logic_error(fmt::format( + "Failed to open file {} for writing: {}", + file.string(), + std::strerror(errno))); // NOLINT(concurrency-mt-unsafe) + } + + errno = 0; + const auto bytes_written = + fwrite(data.data(), sizeof(std::byte), data.size(), f); + const auto write_errno = errno; + errno = 0; + // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) + const auto close_rc = fclose(f); + const auto close_errno = errno; + if (bytes_written != data.size() || close_rc != 0) + { + if (bytes_written != data.size()) + { + errno = write_errno != 0 ? write_errno : EIO; + } + else + { + errno = close_errno != 0 ? close_errno : EIO; + } + throw std::logic_error(fmt::format( + "Failed to write to file {}: {}", + file.string(), + std::strerror(errno))); // NOLINT(concurrency-mt-unsafe) + } + } + /** - * @brief Writes the content of a vector to a file + * @brief Writes the content of a byte span to a file * - * @param data vector to write + * @param data bytes to write * @param file the path */ - static void dump(const std::vector& data, const std::string& file) + static void dump(std::span data, const fs::path& file) { - using namespace std; - ofstream f(file, ios::binary | ios::trunc); - f.write(reinterpret_cast(data.data()), data.size()); - if (!f) - { - throw logic_error("Failed to write to file: " + file); - } + dump(std::as_bytes(data), file); } /** - * @brief Writes the content of a string to a file + * @brief Writes the content of a string view to a file * - * @param data string to write + * @param data string view to write * @param file the path */ - static void dump(const std::string& data, const std::string& file) + static void dump(std::string_view data, const fs::path& file) { - dump(std::vector(data.begin(), data.end()), file); + dump(std::as_bytes(std::span(data)), file); } static void rename(const fs::path& src, const fs::path& dst) diff --git a/src/host/ledger.h b/src/host/ledger.h index 73381117e154..0e07eddb2152 100644 --- a/src/host/ledger.h +++ b/src/host/ledger.h @@ -21,7 +21,6 @@ #include #include #include -#include #include #include @@ -120,13 +119,12 @@ namespace asynchost auto file_path = dir / file_name; - // Use exclusive-create mode ("x") to atomically fail if the file - // already exists, avoiding a separate fs::exists() stat call. + // Use O_EXCL to atomically fail if the file already exists, and create + // with restrictive permissions (0600) rather than relying on umask. { TimeBoundLogger log_if_slow( - fmt::format("Creating ledger file - fopen({})", file_path)); - // NOLINTNEXTLINE(cppcoreguidelines-owning-memory) - file = fopen(file_path.c_str(), "w+bx"); + fmt::format("Creating ledger file - open({})", file_path)); + file = files::open_file(file_path, O_RDWR | O_CREAT | O_EXCL, "w+b"); } if (file == nullptr) { diff --git a/src/host/lfs_file_handler.h b/src/host/lfs_file_handler.h index 24ff4105a711..338869cd940f 100644 --- a/src/host/lfs_file_handler.h +++ b/src/host/lfs_file_handler.h @@ -2,12 +2,12 @@ // Licensed under the Apache 2.0 License. #pragma once +#include "ds/files.h" #include "ds/messaging.h" #include "indexing/lfs_ringbuffer_types.h" #include "time_bound_logger.h" #include -#include namespace asynchost { @@ -50,16 +50,12 @@ namespace asynchost const auto target_path = root_dir / key; { TimeBoundLogger log_if_slow(fmt::format( - "Writing LFS file ({} bytes) - ofstream({})", + "Writing LFS file ({} bytes) - {}", encrypted.size(), target_path)); - std::ofstream f(target_path, std::ios::trunc | std::ios::binary); LOG_TRACE_FMT( "Writing {} byte file to {}", encrypted.size(), target_path); - f.write( - reinterpret_cast(encrypted.data()), - encrypted.size()); - f.close(); + files::dump(encrypted, target_path); } }); diff --git a/src/snapshots/snapshot_manager.h b/src/snapshots/snapshot_manager.h index 8f3cd44b86ab..ce2e88930e10 100644 --- a/src/snapshots/snapshot_manager.h +++ b/src/snapshots/snapshot_manager.h @@ -4,6 +4,7 @@ #include "ccf/ds/nonstd.h" #include "consensus/ledger_enclave_types.h" +#include "ds/files.h" #include "host/time_bound_logger.h" #include "snapshots/filenames.h" @@ -185,8 +186,8 @@ namespace snapshots { asynchost::TimeBoundLogger log_open_if_slow( fmt::format("Opening snapshot file - open({})", file_name)); - snapshot_fd = open( - full_snapshot_path.c_str(), O_CREAT | O_EXCL | O_WRONLY, 0664); + snapshot_fd = + files::open_fd(full_snapshot_path, O_CREAT | O_EXCL | O_WRONLY); } if (snapshot_fd == -1) {