Add support for tarball contents signature
Currently only supported to allow optional image files in BMC tarball.
In order to ensure that the contents of the tarball are the expected
ones as a full package, an additional signature file has been created
for all the signature files in the tarball,
(ex: image-full.sig = hash(file1.sig + file2.sig + file3.sig...)).
Need to check the existence of the file and the signature verification
passed.
Also, added unit test case for the mergeFiles method.
Tested:
Enable `WANT_SIGNATURE_FULL_VERIFY` and ran the following command:
curl -k -H "X-Auth-Token: $token" -H "Content-Type: application/octet-stream"
-X POST -T obmc-phosphor-image-fp5280g2.static.mtd.tar
https://${bmc}/redfish/v1/UpdateService
{
"@odata.id": "/redfish/v1/TaskService/Tasks/1",
"@odata.type": "#Task.v1_4_3.Task",
"Id": "1",
"TaskState": "Running",
"TaskStatus": "OK"
}
And Log output:
`Successfully completed Signature vaildation.`
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I0e658b9dd90ea405a9c8292f29183ab516a0fa31
diff --git a/image_verify.cpp b/image_verify.cpp
index d9679c9..b92956a 100644
--- a/image_verify.cpp
+++ b/image_verify.cpp
@@ -3,6 +3,7 @@
#include "image_verify.hpp"
#include "images.hpp"
+#include "utils.hpp"
#include "version.hpp"
#include <fcntl.h>
@@ -85,6 +86,41 @@
return std::make_pair(std::move(hashpath), std::move(keyPath));
}
+bool Signature::verifyFullImage()
+{
+ bool ret = true;
+#ifdef WANT_SIGNATURE_FULL_VERIFY
+ std::vector<std::string> fullImages = {
+ fs::path(imageDirPath) / "image-bmc.sig",
+ fs::path(imageDirPath) / "image-hostfw.sig",
+ fs::path(imageDirPath) / "image-kernel.sig",
+ fs::path(imageDirPath) / "image-rofs.sig",
+ fs::path(imageDirPath) / "image-rwfs.sig",
+ fs::path(imageDirPath) / "image-u-boot.sig",
+ fs::path(imageDirPath) / "MANIFEST.sig",
+ fs::path(imageDirPath) / "publickey.sig"};
+
+ // Merge files
+ std::string tmpFullFile = "/tmp/image-full";
+ utils::mergeFiles(fullImages, tmpFullFile);
+
+ // Validate the full image files
+ fs::path pkeyFullFile(tmpFullFile);
+
+ std::string imageFullSig = "image-full.sig";
+ fs::path pkeyFullFileSig(imageDirPath / imageFullSig);
+ pkeyFullFileSig.replace_extension(SIGNATURE_FILE_EXT);
+
+ // image specific publickey file name.
+ fs::path publicKeyFile(imageDirPath / PUBLICKEY_FILE_NAME);
+
+ ret = verifyFile(pkeyFullFile, pkeyFullFileSig, publicKeyFile, hashType);
+ fs::remove(tmpFullFile);
+#endif
+
+ return ret;
+}
+
bool Signature::verify()
{
try
@@ -144,6 +180,12 @@
}
}
+ if (verifyFullImage() == false)
+ {
+ log<level::ERR>("Image full file Signature Validation failed");
+ return false;
+ }
+
log<level::DEBUG>("Successfully completed Signature vaildation.");
return true;
diff --git a/image_verify.hpp b/image_verify.hpp
index 02e1093..a995e5a 100644
--- a/image_verify.hpp
+++ b/image_verify.hpp
@@ -199,6 +199,13 @@
*/
CustomMap mapFile(const fs::path& path, size_t size);
+ /**
+ * @brief Verify the full file signature using public key and hash function
+ *
+ * @return true if signature verification was successful, false if not
+ */
+ bool verifyFullImage();
+
/** @brief Directory where software images are placed*/
fs::path imageDirPath;
diff --git a/meson.build b/meson.build
index c74a927..f06090e 100644
--- a/meson.build
+++ b/meson.build
@@ -54,6 +54,7 @@
# Configurable features
conf.set('HOST_BIOS_UPGRADE', get_option('host-bios-upgrade').enabled())
conf.set('WANT_SIGNATURE_VERIFY', get_option('verify-signature').enabled())
+conf.set('WANT_SIGNATURE_FULL_VERIFY', get_option('verify-full-signature').enabled())
# Configurable variables
conf.set('ACTIVE_BMC_MAX_ALLOWED', get_option('active-bmc-max-allowed'))
@@ -172,6 +173,7 @@
if get_option('verify-signature').enabled()
image_updater_sources += files(
+ 'utils.cpp',
'image_verify.cpp',
'openssl_alloc.cpp'
)
@@ -246,6 +248,7 @@
gtest = dependency('gtest', main: true, disabler: true, required: build_tests)
include_srcs = declare_dependency(sources: [
+ 'utils.cpp',
'image_verify.cpp',
'images.cpp',
'version.cpp']
diff --git a/meson_options.txt b/meson_options.txt
index d37e681..8b40d0e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -22,6 +22,9 @@
option('verify-signature', type: 'feature',
description: 'Enable image signature validation.')
+option('verify-full-signature', type: 'feature',
+ description: 'Enable image full signature validation.')
+
# Variables
option(
'active-bmc-max-allowed', type: 'integer',
diff --git a/test/utest.cpp b/test/utest.cpp
index 94e7c96..b91a461 100644
--- a/test/utest.cpp
+++ b/test/utest.cpp
@@ -1,4 +1,5 @@
#include "image_verify.hpp"
+#include "utils.hpp"
#include "version.hpp"
#include <openssl/sha.h>
@@ -9,6 +10,7 @@
#include <iostream>
#include <sstream>
#include <string>
+#include <vector>
#include <gtest/gtest.h>
@@ -262,3 +264,72 @@
command("rm -rf " + signedConfOpenBMCPath.string());
EXPECT_FALSE(signature->verify());
}
+
+class FileTest : public testing::Test
+{
+ protected:
+ std::string readFile(fs::path path)
+ {
+ std::ifstream f(path, std::ios::in);
+ const auto sz = fs::file_size(path);
+ std::string result(sz, '\0');
+ f.read(result.data(), sz);
+
+ return result;
+ }
+
+ void command(const std::string& cmd)
+ {
+ auto val = std::system(cmd.c_str());
+ if (val)
+ {
+ std::cout << "COMMAND Error: " << val << std::endl;
+ }
+ }
+
+ virtual void SetUp()
+ {
+ // Create test base directory.
+ tmpDir = fs::temp_directory_path() / "testFileXXXXXX";
+ if (!mkdtemp(tmpDir.data()))
+ {
+ throw "Failed to create tmp dir";
+ }
+
+ std::string file1 = tmpDir + "/file1";
+ std::string file2 = tmpDir + "/file2";
+ command("echo \"File Test1\n\n\" > " + file1);
+ command("echo \"FileTe st2\n\nte st2\" > " + file2);
+
+ srcFiles.push_back(file1);
+ srcFiles.push_back(file2);
+ }
+
+ virtual void TearDown()
+ {
+ fs::remove_all(tmpDir);
+ }
+
+ std::vector<std::string> srcFiles;
+ std::string tmpDir;
+};
+
+TEST_F(FileTest, TestMergeFiles)
+{
+ std::string retFile = tmpDir + "/retFile";
+ for (auto file : srcFiles)
+ {
+ command("cat " + file + " >> " + retFile);
+ }
+
+ std::string dstFile = tmpDir + "/dstFile";
+ utils::mergeFiles(srcFiles, dstFile);
+
+ ASSERT_NE(fs::file_size(retFile), static_cast<uintmax_t>(-1));
+ ASSERT_NE(fs::file_size(dstFile), static_cast<uintmax_t>(-1));
+ ASSERT_EQ(fs::file_size(retFile), fs::file_size(dstFile));
+
+ std::string ssRetFile = readFile(fs::path(retFile));
+ std::string ssDstFile = readFile(fs::path(dstFile));
+ ASSERT_EQ(ssRetFile, ssDstFile);
+}
\ No newline at end of file
diff --git a/utils.cpp b/utils.cpp
index d3faed3..b9611a3 100644
--- a/utils.cpp
+++ b/utils.cpp
@@ -41,4 +41,29 @@
return response[0].first;
}
+void mergeFiles(std::vector<std::string>& srcFiles, std::string& dstFile)
+{
+ std::ofstream outFile(dstFile, std::ios::out);
+ for (auto& file : srcFiles)
+ {
+ std::ifstream inFile;
+ inFile.open(file, std::ios_base::in);
+ if (!inFile)
+ {
+ continue;
+ }
+
+ inFile.peek();
+ if (inFile.eof())
+ {
+ inFile.close();
+ continue;
+ }
+
+ outFile << inFile.rdbuf();
+ inFile.close();
+ }
+ outFile.close();
+}
+
} // namespace utils
diff --git a/utils.hpp b/utils.hpp
index 7a4150d..13f653c 100644
--- a/utils.hpp
+++ b/utils.hpp
@@ -2,6 +2,7 @@
#include <sdbusplus/server.hpp>
+#include <fstream>
#include <map>
#include <string>
@@ -16,4 +17,12 @@
std::string getService(sdbusplus::bus::bus& bus, const std::string& path,
const std::string& interface);
+/**
+ * @brief Merge more files
+ *
+ * @param[in] srcFiles - source files
+ * @param[out] dstFile - destination file
+ * @return
+ **/
+void mergeFiles(std::vector<std::string>& srcFiles, std::string& dstFile);
} // namespace utils