firmware: add verify blob_id
Add verification blob_id into the blob list. This blob_id will require
special handling in a few actions to be added later.
Goal behavior:
- on open, if all others closed, allows open (now only one of 3 can be
opened at once).
- on commit, starts verification process.
- on close, clears out any outstanding state (but doesn't abort
anything).
- on delete, returns failure.
Change-Id: Ifc759c1051cf1748624ccdb5f7dda0a9ea1681d4
Signed-off-by: Patrick Venture <venture@google.com>
diff --git a/firmware_handler.cpp b/firmware_handler.cpp
index fda533e..74328f3 100644
--- a/firmware_handler.cpp
+++ b/firmware_handler.cpp
@@ -28,6 +28,7 @@
namespace blobs
{
+const std::string FirmwareBlobHandler::verifyBlobID = "/flash/verify";
const std::string FirmwareBlobHandler::hashBlobID = "/flash/hash";
const std::string FirmwareBlobHandler::activeImageBlobID =
"/flash/active/image";
@@ -53,6 +54,7 @@
{
blobs.push_back(item.blobName);
}
+ blobs.push_back(verifyBlobID); /* Add blob_id to always exist. */
if (0 == std::count(blobs.begin(), blobs.end(), hashBlobID))
{
@@ -106,6 +108,15 @@
{
const std::string* toDelete;
+ /* You cannot delete the verify blob -- trying to delete it, currently has
+ * no impact.
+ * TODO: Should trying to delete this cause an abort?
+ */
+ if (path == verifyBlobID)
+ {
+ return false;
+ }
+
if (path == hashBlobID || path == activeHashBlobID)
{
/* They're deleting the hash. */
@@ -146,11 +157,17 @@
bool FirmwareBlobHandler::stat(const std::string& path, struct BlobMeta* meta)
{
/* We know we support this path because canHandle is called ahead */
- if (path == FirmwareBlobHandler::activeImageBlobID)
+ if (path == verifyBlobID)
+ {
+ /* We need to return information for the verify state -- did they call
+ * commit() did things start?
+ */
+ }
+ else if (path == activeImageBlobID)
{
/* We need to return information for the image that's staged. */
}
- else if (path == FirmwareBlobHandler::activeHashBlobID)
+ else if (path == activeHashBlobID)
{
/* We need to return information for the hash that's staged. */
}
@@ -266,6 +283,27 @@
return false;
}
+ /* Handle opening the verifyBlobId --> we know the image and hash aren't
+ * open because of the fileOpen check.
+ *
+ * The file must be opened for writing, but no transport mechanism specified
+ * since it's irrelevant.
+ */
+ if (path == verifyBlobID)
+ {
+ /* In this case, there's no image handler to use, or data handler,
+ * simply set up a session.
+ */
+ verifyImage.flags = flags;
+ verifyImage.state = Session::State::open;
+
+ lookup[session] = &verifyImage;
+
+ fileOpen = true;
+
+ return true;
+ }
+
/* There are two abstractions at play, how you get the data and how you
* handle that data. such that, whether the data comes from the PCI bridge
* or LPC bridge is not connected to whether the data goes into a static
@@ -441,16 +479,38 @@
}
/*
- * If this command is called on the session for the hash image, it'll
+ * If this command is called on the session for the verifyBlobID, it'll
* trigger a systemd service `verify_image.service` to attempt to verify
- * the image. Before doing this, if the transport mechanism is not IPMI
- * BT, it'll shut down the mechanism used for transport preventing the
- * host from updating anything.
+ * the image.
+ *
+ * For this file to have opened, the other two must be closed, which means any
+ * out-of-band transport mechanism involved is closed.
*/
bool FirmwareBlobHandler::commit(uint16_t session,
const std::vector<uint8_t>& data)
{
- return false;
+ auto item = lookup.find(session);
+ if (item == lookup.end())
+ {
+ return false;
+ }
+
+ /* You can only commit on the verifyBlodId */
+ if (item->second->activePath != verifyBlobID)
+ {
+ return false;
+ }
+
+ /* Can only be called once per verification. */
+ if (state == UpdateState::verificationStarted)
+ {
+ return false;
+ }
+
+ /* Set state to committing. */
+ item->second->flags |= StateFlags::committing;
+
+ return triggerVerification();
}
/*
@@ -472,11 +532,28 @@
return false;
}
+ /* Are you closing the verify blob? */
+ if (item->second->activePath == verifyBlobID)
+ {
+ /* If they close this blob before verification finishes, that's an
+ * abort.
+ * TODO: implement this, for now just let them close the file.
+ */
+ if (state == UpdateState::verificationStarted)
+ {
+ return false;
+ }
+ }
+
if (item->second->dataHandler)
{
item->second->dataHandler->close();
}
- item->second->imageHandler->close();
+ if (item->second->imageHandler)
+ {
+ item->second->imageHandler->close();
+ }
+
item->second->state = Session::State::closed;
/* Do not delete the active blob_id from the list of blob_ids, because that
* blob_id indicates there is data stored. Delete will destroy it.
@@ -508,4 +585,12 @@
return {};
}
+bool FirmwareBlobHandler::triggerVerification()
+{
+ state = UpdateState::verificationStarted;
+
+ /* TODO: implement this. */
+ return true;
+}
+
} // namespace blobs
diff --git a/firmware_handler.hpp b/firmware_handler.hpp
index c87e635..36faa05 100644
--- a/firmware_handler.hpp
+++ b/firmware_handler.hpp
@@ -75,6 +75,9 @@
lpc = (1 << 10), /* Expect to send contents over LPC bridge. */
};
+ /* TODO: All of the states may not be required - if we add abort() commands
+ * appropriately.
+ */
/** The state of the firmware update process. */
enum UpdateState
{
@@ -123,8 +126,8 @@
std::uint16_t bitmask) :
handlers(firmwares),
blobIDs(blobs), transports(transports), bitmask(bitmask),
- activeImage(activeImageBlobID), activeHash(activeHashBlobID), lookup(),
- state(UpdateState::notYetStarted)
+ activeImage(activeImageBlobID), activeHash(activeHashBlobID),
+ verifyImage(verifyBlobID), lookup(), state(UpdateState::notYetStarted)
{
}
~FirmwareBlobHandler() = default;
@@ -150,6 +153,9 @@
bool stat(uint16_t session, struct BlobMeta* meta) override;
bool expire(uint16_t session) override;
+ bool triggerVerification();
+
+ static const std::string verifyBlobID;
static const std::string hashBlobID;
static const std::string activeImageBlobID;
static const std::string activeHashBlobID;
@@ -179,6 +185,9 @@
/** Active hash session. */
Session activeHash;
+ /** Session for verification. */
+ Session verifyImage;
+
/** A quick method for looking up a session's mechanisms and details. */
std::map<std::uint16_t, Session*> lookup;
diff --git a/test/Makefile.am b/test/Makefile.am
index 8d01588..2af6033 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -19,6 +19,7 @@
firmware_close_unittest \
firmware_delete_unittest \
firmware_sessionstat_unittest \
+ firmware_commit_unittest \
file_handler_unittest
TESTS = $(check_PROGRAMS)
@@ -50,5 +51,8 @@
firmware_sessionstat_unittest_SOURCES = firmware_sessionstat_unittest.cpp
firmware_sessionstat_unittest_LDADD = $(top_builddir)/firmware_handler.o
+firmware_commit_unittest_SOURCES = firmware_commit_unittest.cpp
+firmware_commit_unittest_LDADD = $(top_builddir)/firmware_handler.o
+
file_handler_unittest_SOURCES = file_handler_unittest.cpp
file_handler_unittest_LDADD = $(top_builddir)/file_handler.o -lstdc++fs
diff --git a/test/firmware_close_unittest.cpp b/test/firmware_close_unittest.cpp
index 5bbd333..546d6df 100644
--- a/test/firmware_close_unittest.cpp
+++ b/test/firmware_close_unittest.cpp
@@ -42,7 +42,7 @@
/* The active hash blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeHashBlobID));
@@ -84,7 +84,7 @@
/* The active hash blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeHashBlobID));
diff --git a/test/firmware_commit_unittest.cpp b/test/firmware_commit_unittest.cpp
new file mode 100644
index 0000000..d7bb744
--- /dev/null
+++ b/test/firmware_commit_unittest.cpp
@@ -0,0 +1,111 @@
+#include "data_mock.hpp"
+#include "firmware_handler.hpp"
+#include "image_mock.hpp"
+
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace blobs
+{
+using ::testing::Return;
+
+TEST(FirmwareHandlerCommitTest, VerifyCannotCommitOnFlashImage)
+{
+ /* Verify the flash image returns failure on this command. It's a fairly
+ * artificial test.
+ */
+ ImageHandlerMock imageMock1, imageMock2;
+ std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock1},
+ {"asdf", &imageMock2},
+ };
+
+ std::vector<DataHandlerPack> data = {
+ {FirmwareBlobHandler::UpdateFlags::ipmi, nullptr},
+ };
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+
+ EXPECT_CALL(imageMock2, open("asdf")).WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->open(
+ 0, OpenFlags::write | FirmwareBlobHandler::UpdateFlags::ipmi, "asdf"));
+
+ EXPECT_FALSE(handler->commit(0, {}));
+}
+
+TEST(FirmwareHandlerCommitTest, VerifyCannotCommitOnHashFile)
+{
+ /* Verify the hash file returns failure on this command. It's a fairly
+ * artificial test.
+ */
+ ImageHandlerMock imageMock1, imageMock2;
+ std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock1},
+ {"asdf", &imageMock2},
+ };
+
+ std::vector<DataHandlerPack> data = {
+ {FirmwareBlobHandler::UpdateFlags::ipmi, nullptr},
+ };
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+
+ EXPECT_CALL(imageMock1, open(FirmwareBlobHandler::hashBlobID))
+ .WillOnce(Return(true));
+
+ EXPECT_TRUE(handler->open(
+ 0, OpenFlags::write | FirmwareBlobHandler::UpdateFlags::ipmi,
+ FirmwareBlobHandler::hashBlobID));
+
+ EXPECT_FALSE(handler->commit(0, {}));
+}
+
+TEST(FirmwareHandlerCommitTest, VerifyCommitAcceptedOnVerifyBlob)
+{
+ /* Verify the verify blob lets you call this command, and it returns
+ * success.
+ */
+ ImageHandlerMock imageMock1, imageMock2;
+ std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock1},
+ {"asdf", &imageMock2},
+ };
+
+ std::vector<DataHandlerPack> data = {
+ {FirmwareBlobHandler::UpdateFlags::ipmi, nullptr},
+ };
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+
+ EXPECT_TRUE(
+ handler->open(0, OpenFlags::write, FirmwareBlobHandler::verifyBlobID));
+
+ EXPECT_TRUE(handler->commit(0, {}));
+}
+
+TEST(FirmwareHandlerCommitTest, VerifyCommitCanOnlyBeCalledOnce)
+{
+ /* Verify you cannot call the commit() command once verification is started.
+ */
+ ImageHandlerMock imageMock1, imageMock2;
+ std::vector<HandlerPack> blobs = {
+ {FirmwareBlobHandler::hashBlobID, &imageMock1},
+ {"asdf", &imageMock2},
+ };
+
+ std::vector<DataHandlerPack> data = {
+ {FirmwareBlobHandler::UpdateFlags::ipmi, nullptr},
+ };
+
+ auto handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
+
+ EXPECT_TRUE(
+ handler->open(0, OpenFlags::write, FirmwareBlobHandler::verifyBlobID));
+
+ EXPECT_TRUE(handler->commit(0, {}));
+ EXPECT_FALSE(handler->commit(0, {}));
+}
+
+} // namespace blobs
diff --git a/test/firmware_delete_unittest.cpp b/test/firmware_delete_unittest.cpp
index 0697bbd..a5550a9 100644
--- a/test/firmware_delete_unittest.cpp
+++ b/test/firmware_delete_unittest.cpp
@@ -39,7 +39,7 @@
/* The active hash blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeHashBlobID));
@@ -48,7 +48,7 @@
EXPECT_TRUE(handler->close(0));
currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeHashBlobID));
@@ -56,7 +56,7 @@
EXPECT_TRUE(handler->deleteBlob(FirmwareBlobHandler::activeHashBlobID));
currentBlobs = handler->getBlobIds();
- EXPECT_EQ(2, currentBlobs.size());
+ EXPECT_EQ(3, currentBlobs.size());
EXPECT_EQ(0, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeHashBlobID));
}
diff --git a/test/firmware_handler_unittest.cpp b/test/firmware_handler_unittest.cpp
index f29668b..ce65ad4 100644
--- a/test/firmware_handler_unittest.cpp
+++ b/test/firmware_handler_unittest.cpp
@@ -49,9 +49,11 @@
handler = FirmwareBlobHandler::CreateFirmwareBlobHandler(blobs, data);
auto result = handler->getBlobIds();
- EXPECT_EQ(2, result.size());
- EXPECT_EQ(2, std::count(result.begin(), result.end(), "asdf") +
+ EXPECT_EQ(3, result.size());
+ EXPECT_EQ(3, std::count(result.begin(), result.end(), "asdf") +
std::count(result.begin(), result.end(),
- FirmwareBlobHandler::hashBlobID));
+ FirmwareBlobHandler::hashBlobID) +
+ std::count(result.begin(), result.end(),
+ FirmwareBlobHandler::verifyBlobID));
}
} // namespace blobs
diff --git a/test/firmware_open_unittest.cpp b/test/firmware_open_unittest.cpp
index 1975740..81a1977 100644
--- a/test/firmware_open_unittest.cpp
+++ b/test/firmware_open_unittest.cpp
@@ -37,7 +37,7 @@
/* The active image blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeImageBlobID));
}
@@ -66,7 +66,7 @@
/* The active hash blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeHashBlobID));
}
@@ -100,7 +100,7 @@
/* The active hash blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeHashBlobID));
}
@@ -130,7 +130,7 @@
/* The active hash blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(2, currentBlobs.size());
+ EXPECT_EQ(3, currentBlobs.size());
}
TEST(FirmwareHandlerOpenTest, OpenEverythingSucceedsVerifyOpenFileCheck)
@@ -157,7 +157,7 @@
/* The active image blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeImageBlobID));
@@ -202,7 +202,7 @@
/* The active image blob_id was added. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(3, currentBlobs.size());
+ EXPECT_EQ(4, currentBlobs.size());
EXPECT_EQ(1, std::count(currentBlobs.begin(), currentBlobs.end(),
FirmwareBlobHandler::activeImageBlobID));
@@ -240,7 +240,7 @@
/* Verify blob_id list doesn't grow. */
auto currentBlobs = handler->getBlobIds();
- EXPECT_EQ(2, currentBlobs.size());
+ EXPECT_EQ(3, currentBlobs.size());
}
TEST(FirmwareHandlerOpenTest, OpenWithoutWriteFails)