From 9997edeb3f0bc2f57e31593bb3bc14fc8d72eae0 Mon Sep 17 00:00:00 2001 From: Oleksandr Grytsov Date: Wed, 4 Nov 2026 20:29:13 +0200 Subject: [PATCH] common: ocispec: return integrity error on file read Integrity recovery is triggered by eIntegrity, but the OCI spec loaders read files with std::ifstream, which doesn't report read errors: a failed read ends up as a JSON parse error. Add utils::ReadJsonFromFile, which reads the file with fs::ReadFile and parses its content, and use it in all the loaders. EIO and EBADMSG are returned as eIntegrity, ENOENT as eNotFound, any other errno as is. Malformed JSON that is read successfully is still returned as eInvalidArgument. This change requires fs::ReadFile of aos_core_lib_cpp that returns eIntegrity and eNotFound. Signed-off-by: Oleksandr Grytsov Reviewed-by: Mykhailo Lohvynenko Reviewed-by: Mykola Solianko --- src/common/ocispec/imageconfig.cpp | 13 +--- src/common/ocispec/imageindex.cpp | 13 +--- src/common/ocispec/imagemanifest.cpp | 13 +--- src/common/ocispec/itemconfig.cpp | 13 +--- src/common/ocispec/runtimeconfig.cpp | 12 +--- src/common/ocispec/tests/ocispec.cpp | 103 +++++++++++++++++++++++++++ src/common/utils/json.cpp | 23 ++++++ src/common/utils/json.hpp | 11 +++ src/common/utils/tests/json.cpp | 42 +++++++++++ 9 files changed, 189 insertions(+), 54 deletions(-) diff --git a/src/common/ocispec/imageconfig.cpp b/src/common/ocispec/imageconfig.cpp index 67d3fef8b..2d123a52e 100644 --- a/src/common/ocispec/imageconfig.cpp +++ b/src/common/ocispec/imageconfig.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -118,14 +115,8 @@ Poco::JSON::Object RootfsToJSON(const aos::oci::Rootfs& rootfs) Error OCISpec::LoadImageConfig(const String& path, aos::oci::ImageConfig& imageConfig) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/imageindex.cpp b/src/common/ocispec/imageindex.cpp index 02d6e92ff..7574054d2 100644 --- a/src/common/ocispec/imageindex.cpp +++ b/src/common/ocispec/imageindex.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -56,14 +53,8 @@ Poco::JSON::Object IndexContentDescriptorToJSONObject(const aos::oci::IndexConte Error OCISpec::LoadImageIndex(const String& path, aos::oci::ImageIndex& index) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/imagemanifest.cpp b/src/common/ocispec/imagemanifest.cpp index a40fdd258..708560ef3 100644 --- a/src/common/ocispec/imagemanifest.cpp +++ b/src/common/ocispec/imagemanifest.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -23,14 +20,8 @@ namespace aos::common::oci { Error OCISpec::LoadImageManifest(const String& path, aos::oci::ImageManifest& manifest) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/itemconfig.cpp b/src/common/ocispec/itemconfig.cpp index 5a69bc958..c080cfa80 100644 --- a/src/common/ocispec/itemconfig.cpp +++ b/src/common/ocispec/itemconfig.cpp @@ -4,9 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include -#include - #include #include #include @@ -458,14 +455,8 @@ Poco::JSON::Object AlertRulesToJSON(const AlertRules& rules) Error OCISpec::LoadItemConfig(const String& path, aos::oci::ItemConfig& itemConfig) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/runtimeconfig.cpp b/src/common/ocispec/runtimeconfig.cpp index a545ce406..657bf1ab1 100644 --- a/src/common/ocispec/runtimeconfig.cpp +++ b/src/common/ocispec/runtimeconfig.cpp @@ -4,8 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include - #include #include #include @@ -928,14 +926,8 @@ Poco::JSON::Object VMToJSON(const aos::oci::VM& vm) Error OCISpec::LoadRuntimeConfig(const String& path, aos::oci::RuntimeConfig& runtimeConfig) { try { - std::ifstream file(path.CStr()); - - if (!file.is_open()) { - AOS_ERROR_THROW(ErrorEnum::eNotFound, "failed to open file"); - } - - auto [var, err] = utils::ParseJson(file); - AOS_ERROR_CHECK_AND_THROW(err, "failed to parse json"); + auto [var, err] = utils::ReadJsonFromFile(path.CStr()); + AOS_ERROR_CHECK_AND_THROW(err, "failed to load json file"); Poco::JSON::Object::Ptr object = var.extract(); utils::CaseInsensitiveObjectWrapper wrapper(object); diff --git a/src/common/ocispec/tests/ocispec.cpp b/src/common/ocispec/tests/ocispec.cpp index 1c4a359ec..4f8ec21fd 100644 --- a/src/common/ocispec/tests/ocispec.cpp +++ b/src/common/ocispec/tests/ocispec.cpp @@ -4,6 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include +#include + +#include +#include +#include +#include +#include + #include #include @@ -16,6 +25,25 @@ using namespace testing; +namespace { + +std::atomic sReadErrno {0}; + +} // namespace + +// File read is intercepted to inject the read errors of the integrity checked storage, which can't be reproduced with a +// regular file. +extern "C" ssize_t read(int fd, void* buf, size_t count) +{ + if (const auto readErrno = sReadErrno.load(); readErrno != 0) { + errno = readErrno; + + return -1; + } + + return static_cast(syscall(SYS_read, fd, buf, count)); +} + namespace aos::common::jsonprovider { namespace { @@ -318,6 +346,29 @@ std::string MakeItemConfigWithPermissions(const std::string& permissions) )"; } +using LoadFunc = std::function; + +template +LoadFunc MakeLoadFunc(Method method) +{ + return [method](oci::OCISpec& ociSpec, const String& path) { + auto object = std::make_unique(); + + return (ociSpec.*method)(path, *object); + }; +} + +std::vector GetLoadFuncs() +{ + return { + MakeLoadFunc(&oci::OCISpec::LoadImageIndex), + MakeLoadFunc(&oci::OCISpec::LoadImageManifest), + MakeLoadFunc(&oci::OCISpec::LoadImageConfig), + MakeLoadFunc(&oci::OCISpec::LoadItemConfig), + MakeLoadFunc(&oci::OCISpec::LoadRuntimeConfig), + }; +} + class OCISpecTest : public Test { public: void SetUp() override @@ -332,6 +383,8 @@ class OCISpecTest : public Test { fs::WriteStringToFile(cItemConfigPath, cItemConfig, S_IRUSR | S_IWUSR); } + void TearDown() override { sReadErrno = 0; } + oci::OCISpec mOCISpec; }; @@ -582,6 +635,56 @@ TEST_F(OCISpecTest, LoadItemConfigPermissionsExceedLimitsFails) } } +TEST_F(OCISpecTest, LoadReadErrorIsIntegrity) +{ + for (const auto readErrno : {EIO, EBADMSG}) { + for (const auto& load : GetLoadFuncs()) { + sReadErrno = readErrno; + const auto err = load(mOCISpec, cImageIndexPath); + sReadErrno = 0; + + EXPECT_TRUE(err.Is(ErrorEnum::eIntegrity)) + << "errno: " << readErrno << ", error: " << tests::utils::ErrorToStr(err); + } + } +} + +TEST_F(OCISpecTest, LoadOtherReadErrorIsReturnedAsIs) +{ + for (const auto& load : GetLoadFuncs()) { + sReadErrno = EPERM; + const auto err = load(mOCISpec, cImageIndexPath); + sReadErrno = 0; + + EXPECT_EQ(err.Value(), ErrorEnum::eRuntime) << tests::utils::ErrorToStr(err); + EXPECT_EQ(err.Code(), EPERM) << tests::utils::ErrorToStr(err); + } +} + +TEST_F(OCISpecTest, LoadMissingFileIsNotFound) +{ + const auto cMissingPath = fs::JoinPath(cTestBaseDir, "missing.json"); + + for (const auto& load : GetLoadFuncs()) { + const auto err = load(mOCISpec, cMissingPath); + + EXPECT_TRUE(err.Is(ErrorEnum::eNotFound)) << tests::utils::ErrorToStr(err); + } +} + +TEST_F(OCISpecTest, LoadMalformedFileIsNotIntegrity) +{ + const auto cMalformedPath = fs::JoinPath(cTestBaseDir, "malformed.json"); + + ASSERT_TRUE(fs::WriteStringToFile(cMalformedPath, "{\"schemaVersion\": ", S_IRUSR | S_IWUSR).IsNone()); + + for (const auto& load : GetLoadFuncs()) { + const auto err = load(mOCISpec, cMalformedPath); + + EXPECT_TRUE(err.Is(ErrorEnum::eInvalidArgument)) << tests::utils::ErrorToStr(err); + } +} + TEST_F(OCISpecTest, ServiceConfigFromFileRunParams) { const std::vector runParams = { diff --git a/src/common/utils/json.cpp b/src/common/utils/json.cpp index 1cb95921f..b1ca1c639 100644 --- a/src/common/utils/json.cpp +++ b/src/common/utils/json.cpp @@ -6,12 +6,16 @@ */ #include +#include #include #include +#include #include #include +#include + #include "exception.hpp" #include "json.hpp" @@ -133,6 +137,25 @@ Error WriteJsonToFile(const Poco::JSON::Object::Ptr& json, const std::string& pa return ErrorEnum::eNone; } +aos::RetWithError ReadJsonFromFile(const std::string& path) +{ + std::error_code ec; + + const auto size = std::filesystem::file_size(path, ec); + + // The size error is not returned: if the file can't be accessed, ReadFile fails with the proper error. + std::string content(ec ? 0 : size, '\0'); + auto buffer = Array(reinterpret_cast(content.data()), content.size()); + + if (auto err = fs::ReadFile(path.c_str(), buffer); !err.IsNone()) { + return {{}, AOS_ERROR_WRAP(err)}; + } + + content.resize(buffer.Size()); + + return ParseJson(content); +} + Poco::Dynamic::Var FindByPath(const Poco::Dynamic::Var object, const std::vector& keys) { if (keys.empty()) { diff --git a/src/common/utils/json.hpp b/src/common/utils/json.hpp index fd7d81e4a..32f8b2617 100644 --- a/src/common/utils/json.hpp +++ b/src/common/utils/json.hpp @@ -55,6 +55,17 @@ aos::RetWithError ParseJson(std::istream& in) noexcept; */ Error WriteJsonToFile(const Poco::JSON::Object::Ptr& json, const std::string& path); +/** + * Reads and parses json from file. + * + * The errors of the file open and read are returned as follows: EIO and EBADMSG as ErrorEnum::eIntegrity, ENOENT as + * ErrorEnum::eNotFound, any other errno as is. + * + * @param path path to the file. + * @return aos::RetWithError. + */ +aos::RetWithError ReadJsonFromFile(const std::string& path); + /** * Finds value of the json by path * diff --git a/src/common/utils/tests/json.cpp b/src/common/utils/tests/json.cpp index e19bd0ccd..198909ede 100644 --- a/src/common/utils/tests/json.cpp +++ b/src/common/utils/tests/json.cpp @@ -5,6 +5,9 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include + +#include #include #include @@ -284,6 +287,45 @@ TEST_F(JsonTest, WriteJsonToFileFails) EXPECT_EQ(WriteJsonToFile(object, path), aos::ErrorEnum::eFailed); } +TEST_F(JsonTest, ReadJsonFromFileSucceeds) +{ + const std::string path = "read_json_from_file.json"; + + Poco::JSON::Object::Ptr object = new Poco::JSON::Object; + object->set("key", "value"); + + ASSERT_EQ(WriteJsonToFile(object, path), aos::ErrorEnum::eNone); + + auto [var, err] = ReadJsonFromFile(path); + + EXPECT_EQ(err, aos::ErrorEnum::eNone); + EXPECT_EQ(var.extract()->getValue("key"), "value"); + + std::remove(path.c_str()); +} + +TEST_F(JsonTest, ReadJsonFromFileNotFound) +{ + auto [var, err] = ReadJsonFromFile("read_json_from_file_missing.json"); + + EXPECT_EQ(err, aos::ErrorEnum::eNotFound); +} + +TEST_F(JsonTest, ReadJsonFromFileReturnsErrnoAsIs) +{ + const std::string path = "read_json_from_file_link_loop.json"; + + std::remove(path.c_str()); + + ASSERT_EQ(symlink(path.c_str(), path.c_str()), 0); + + auto [var, err] = ReadJsonFromFile(path); + + EXPECT_EQ(err.Code(), ELOOP); + + std::remove(path.c_str()); +} + TEST_F(JsonTest, ToJSONArray) { auto array