From 757178a551c2dcc24730dccb6ed1cd11005b3b8f Mon Sep 17 00:00:00 2001 From: Ed Tanous Date: Wed, 3 Apr 2024 14:32:38 -0700 Subject: Refactor tftp parser This function in the next patch will be used for more than just TFTP, so rename it to match intent, and refactor to use non-TFTP specific types. Tested: Rename only. Need help on TFTP setups if we need it. Change-Id: Ifc7485aa60ec53407c38b3d1bec530bdacf50075 Signed-off-by: Ed Tanous --- redfish-core/lib/update_service.hpp | 95 ++++++++++++++++++--------- test/redfish-core/lib/update_service_test.cpp | 39 ++++++----- 2 files changed, 86 insertions(+), 48 deletions(-) diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp index a735567c8e..df4043f369 100644 --- a/redfish-core/lib/update_service.hpp +++ b/redfish-core/lib/update_service.hpp @@ -19,6 +19,7 @@ #include "app.hpp" #include "dbus_utility.hpp" +#include "generated/enums/update_service.hpp" #include "multipart_parser.hpp" #include "ossl_random.hpp" #include "query.hpp" @@ -441,16 +442,10 @@ inline void monitorForSoftwareAvailable( std::bind_front(afterUpdateErrorMatcher, asyncResp, url)); } -struct TftpUrl -{ - std::string fwFile; - std::string tftpServer; -}; - -inline std::optional - parseTftpUrl(std::string imageURI, - std::optional transferProtocol, - crow::Response& res) +inline std::optional + parseSimpleUpdateUrl(std::string imageURI, + std::optional transferProtocol, + crow::Response& res) { if (imageURI.find("://") == std::string::npos) { @@ -467,7 +462,11 @@ inline std::optional return std::nullopt; } // OpenBMC currently only supports TFTP - if (*transferProtocol != "TFTP") + if (*transferProtocol == "TFTP") + { + imageURI = "tftp://" + imageURI; + } + else { messages::actionParameterNotSupported(res, "TransferProtocol", *transferProtocol); @@ -475,7 +474,6 @@ inline std::optional *transferProtocol); return std::nullopt; } - imageURI = "tftp://" + imageURI; } boost::system::result url = @@ -489,32 +487,52 @@ inline std::optional } url->normalize(); - if (url->scheme() != "tftp") + if (url->scheme() == "tftp") + { + if (url->encoded_path().size() < 2) + { + messages::actionParameterNotSupported(res, "ImageURI", + url->buffer()); + return std::nullopt; + } + } + else { messages::actionParameterNotSupported(res, "ImageURI", imageURI); return std::nullopt; } - std::string path(url->encoded_path()); - if (path.size() < 2) + + if (url->encoded_path().empty()) { - messages::actionParameterNotSupported(res, "ImageURI", imageURI); + messages::actionParameterValueTypeError(res, imageURI, "ImageURI", + "UpdateService.SimpleUpdate"); return std::nullopt; } - path.erase(0, 1); - std::string host(url->encoded_host_and_port()); - return TftpUrl{path, host}; + + return *url; } inline void doTftpUpdate(const crow::Request& req, const std::shared_ptr& asyncResp, - const TftpUrl& tftpUrl) + const boost::urls::url_view_base& url) { #ifndef BMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE messages::actionParameterNotSupported(asyncResp->res, "ImageURI", - tftpUrl.tftpServer); + url.buffer()); return; #endif - BMCWEB_LOG_DEBUG("Server: {} File: {}", tftpUrl.tftpServer, tftpUrl.fwFile); + + std::string path(url.encoded_path()); + if (path.size() < 2) + { + messages::actionParameterNotSupported(asyncResp->res, "ImageURI", + url.buffer()); + return; + } + // TFTP expects a path without a / + path.erase(0, 1); + std::string host(url.encoded_host_and_port()); + BMCWEB_LOG_DEBUG("Server: {} File: {}", host, path); // Setup callback for when new software detected // Give TFTP 10 minutes to complete @@ -545,7 +563,7 @@ inline void doTftpUpdate(const crow::Request& req, }, "xyz.openbmc_project.Software.Download", "/xyz/openbmc_project/software", "xyz.openbmc_project.Common.TFTP", - "DownloadViaTFTP", tftpUrl.fwFile, tftpUrl.tftpServer); + "DownloadViaTFTP", path, host); } inline void handleUpdateServiceSimpleUpdateAction( @@ -575,15 +593,22 @@ inline void handleUpdateServiceSimpleUpdateAction( return; } - std::optional ret = parseTftpUrl(imageURI, transferProtocol, - asyncResp->res); - if (!ret) + std::optional url = + parseSimpleUpdateUrl(imageURI, transferProtocol, asyncResp->res); + if (!url) { return; } - - BMCWEB_LOG_DEBUG("Server: {} File: {}", ret->tftpServer, ret->fwFile); - doTftpUpdate(req, asyncResp, *ret); + if (url->scheme() == "tftp") + { + doTftpUpdate(req, asyncResp, *url); + } + else + { + messages::actionParameterNotSupported(asyncResp->res, "ImageURI", + url->buffer()); + return; + } BMCWEB_LOG_DEBUG("Exit UpdateService.SimpleUpdate doPost"); } @@ -805,15 +830,21 @@ inline void asyncResp->res.jsonValue["MaxImageSizeBytes"] = bmcwebHttpReqBodyLimitMb * 1024 * 1024; -#ifdef BMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE // Update Actions object. nlohmann::json& updateSvcSimpleUpdate = asyncResp->res.jsonValue["Actions"]["#UpdateService.SimpleUpdate"]; updateSvcSimpleUpdate["target"] = "/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate"; - updateSvcSimpleUpdate["TransferProtocol@Redfish.AllowableValues"] = { - "TFTP"}; + + nlohmann::json::array_t allowed; + +#ifdef BMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE + allowed.emplace_back(update_service::TransferProtocolType::TFTP); #endif + + updateSvcSimpleUpdate["TransferProtocol@Redfish.AllowableValues"] = + std::move(allowed); + // Get the current ApplyTime value sdbusplus::asio::getProperty( *crow::connections::systemBus, "xyz.openbmc_project.Settings", diff --git a/test/redfish-core/lib/update_service_test.cpp b/test/redfish-core/lib/update_service_test.cpp index d56d709f5f..540313154f 100644 --- a/test/redfish-core/lib/update_service_test.cpp +++ b/test/redfish-core/lib/update_service_test.cpp @@ -16,38 +16,42 @@ TEST(UpdateService, ParseTFTPPostitive) crow::Response res; { // No protocol, schema on url - std::optional ret = parseTftpUrl("tftp://1.1.1.1/path", - std::nullopt, res); + std::optional ret = + parseSimpleUpdateUrl("tftp://1.1.1.1/path", std::nullopt, res); ASSERT_TRUE(ret); if (!ret) { return; } - EXPECT_EQ(ret->tftpServer, "1.1.1.1"); - EXPECT_EQ(ret->fwFile, "path"); + EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1"); + EXPECT_EQ(ret->encoded_path(), "/path"); + EXPECT_EQ(ret->scheme(), "tftp"); } { // Protocol, no schema on url - std::optional ret = parseTftpUrl("1.1.1.1/path", "TFTP", res); + std::optional ret = + parseSimpleUpdateUrl("1.1.1.1/path", "TFTP", res); ASSERT_TRUE(ret); if (!ret) { return; } - EXPECT_EQ(ret->tftpServer, "1.1.1.1"); - EXPECT_EQ(ret->fwFile, "path"); + EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1"); + EXPECT_EQ(ret->encoded_path(), "/path"); + EXPECT_EQ(ret->scheme(), "tftp"); } { // Both protocl and schema on url - std::optional ret = parseTftpUrl("tftp://1.1.1.1/path", "TFTP", - res); + std::optional ret = + parseSimpleUpdateUrl("tftp://1.1.1.1/path", "TFTP", res); ASSERT_TRUE(ret); if (!ret) { return; } - EXPECT_EQ(ret->tftpServer, "1.1.1.1"); - EXPECT_EQ(ret->fwFile, "path"); + EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1"); + EXPECT_EQ(ret->encoded_path(), "/path"); + EXPECT_EQ(ret->scheme(), "tftp"); } } @@ -55,16 +59,19 @@ TEST(UpdateService, ParseTFTPNegative) { crow::Response res; // No protocol, no schema - ASSERT_EQ(parseTftpUrl("1.1.1.1/path", std::nullopt, res), std::nullopt); + ASSERT_EQ(parseSimpleUpdateUrl("1.1.1.1/path", std::nullopt, res), + std::nullopt); // No host - ASSERT_EQ(parseTftpUrl("/path", "TFTP", res), std::nullopt); + ASSERT_EQ(parseSimpleUpdateUrl("/path", "TFTP", res), std::nullopt); // No host - ASSERT_EQ(parseTftpUrl("path", "TFTP", res), std::nullopt); + ASSERT_EQ(parseSimpleUpdateUrl("path", "TFTP", res), std::nullopt); // No path - ASSERT_EQ(parseTftpUrl("tftp://1.1.1.1", "TFTP", res), std::nullopt); - ASSERT_EQ(parseTftpUrl("tftp://1.1.1.1/", "TFTP", res), std::nullopt); + ASSERT_EQ(parseSimpleUpdateUrl("tftp://1.1.1.1", "TFTP", res), + std::nullopt); + ASSERT_EQ(parseSimpleUpdateUrl("tftp://1.1.1.1/", "TFTP", res), + std::nullopt); } } // namespace } // namespace redfish -- cgit v1.2.3