diff options
37 files changed, 822 insertions, 415 deletions
diff --git a/Redfish.md b/Redfish.md index 5e410ad616..cfee9d5110 100644 --- a/Redfish.md +++ b/Redfish.md @@ -62,6 +62,10 @@ Fields common to all schemas - LDAP - MaxPasswordLength - MinPasswordLength +- MultiFactorAuth/ClientCertificate/Certificates +- MultiFactorAuth/ClientCertificate/CertificateMappingAttribute +- MultiFactorAuth/ClientCertificate/Enabled +- MultiFactorAuth/ClientCertificate/RespondToUnauthenticatedClients - Oem/OpenBMC/AuthMethods/BasicAuth - Oem/OpenBMC/AuthMethods/Cookie - Oem/OpenBMC/AuthMethods/SessionToken @@ -70,6 +74,31 @@ Fields common to all schemas - Roles - ServiceEnabled +### /redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates + +- Members +- Members@odata.count + +### /redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates/{Certificate} + +- CertificateString +- Id +- Issuer/City +- Issuer/CommonName +- Issuer/Country +- Issuer/Organization +- Issuer/OrganizationalUnit +- Issuer/State +- KeyUsage +- Subject/City +- Subject/Country +- Subject/CommonName +- Subject/Organization +- Subject/OrganizationalUnit +- Subject/State +- ValidNotAfter +- ValidNotBefore + ### /redfish/v1/AggregationService/ #### AggregationService diff --git a/config/bmcweb_config.h.in b/config/bmcweb_config.h.in index 81a61c8b17..adaf46120b 100644 --- a/config/bmcweb_config.h.in +++ b/config/bmcweb_config.h.in @@ -40,6 +40,7 @@ constexpr const bool BMCWEB_REDFISH_HOST_LOGGER = @REDFISH_HOST_LOGGER@; constexpr const bool BMCWEB_REDFISH_NEW_POWERSUBSYSTEM_THERMALSUBSYSTEM = @REDFISH_NEW_POWERSUBSYSTEM_THERMALSUBSYSTEM@; constexpr const bool BMCWEB_REDFISH_OEM_MANAGER_FAN_DATA = @REDFISH_OEM_MANAGER_FAN_DATA@; constexpr const bool BMCWEB_REDFISH_PROVISIONING_FEATURE = @REDFISH_PROVISIONING_FEATURE@; +constexpr const bool BMCWEB_REDFISH_UPDATESERVICE_USE_DBUS = @REDFISH_UPDATESERVICE_USE_DBUS@; constexpr const bool BMCWEB_REDFISH = @REDFISH@; constexpr const bool BMCWEB_REST = @REST@; constexpr const bool BMCWEB_SESSION_AUTH = @SESSION_AUTH@; diff --git a/config/meson.build b/config/meson.build index 4b862b49eb..9f66d0eaa4 100644 --- a/config/meson.build +++ b/config/meson.build @@ -33,6 +33,7 @@ feature_options = [ 'redfish-new-powersubsystem-thermalsubsystem', 'redfish-oem-manager-fan-data', 'redfish-provisioning-feature', + 'redfish-updateservice-use-dbus', 'redfish', 'rest', 'session-auth', diff --git a/http/app.hpp b/http/app.hpp index eea13058e1..29cb4de9eb 100644 --- a/http/app.hpp +++ b/http/app.hpp @@ -54,14 +54,14 @@ class App App& operator=(const App&&) = delete; template <typename Adaptor> - void handleUpgrade(Request& req, + void handleUpgrade(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, Adaptor&& adaptor) { router.handleUpgrade(req, asyncResp, std::forward<Adaptor>(adaptor)); } - void handle(Request& req, + void handle(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) { router.handle(req, asyncResp); diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp index 2c60c1268e..446c38f302 100644 --- a/http/http2_connection.hpp +++ b/http/http2_connection.hpp @@ -37,7 +37,7 @@ namespace crow struct Http2StreamData { - Request req; + std::shared_ptr<Request> req = std::make_shared<Request>(); std::optional<bmcweb::HttpBody::reader> reqReader; Response res; std::optional<bmcweb::HttpBody::writer> writer; @@ -170,7 +170,7 @@ class HTTP2Connection : Http2StreamData& stream = it->second; Response& res = stream.res; res = std::move(completedRes); - crow::Request& thisReq = stream.req; + crow::Request& thisReq = *stream.req; completeResponseFields(thisReq, res); res.addHeader(boost::beast::http::field::date, getCachedDateStr()); @@ -243,7 +243,7 @@ class HTTP2Connection : return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } } - crow::Request& thisReq = it->second.req; + crow::Request& thisReq = *it->second.req; thisReq.ioService = static_cast<decltype(thisReq.ioService)>( &adaptor.get_executor().context()); BMCWEB_LOG_DEBUG("Handling {} \"{}\"", logPtr(&thisReq), @@ -262,31 +262,30 @@ class HTTP2Connection : }); auto asyncResp = std::make_shared<bmcweb::AsyncResp>(std::move(it->second.res)); -#ifndef BMCWEB_INSECURE_DISABLE_AUTHX - thisReq.session = crow::authentication::authenticate( - {}, asyncResp->res, thisReq.method(), thisReq.req, nullptr); - if (!crow::authentication::isOnAllowlist(thisReq.url().path(), - thisReq.method()) && - thisReq.session == nullptr) - { - BMCWEB_LOG_WARNING("Authentication failed"); - forward_unauthorized::sendUnauthorized( - thisReq.url().encoded_path(), - thisReq.getHeaderValue("X-Requested-With"), - thisReq.getHeaderValue("Accept"), asyncResp->res); - } - else -#endif // BMCWEB_INSECURE_DISABLE_AUTHX + if constexpr (!BMCWEB_INSECURE_DISABLE_AUTH) { - std::string_view expected = thisReq.getHeaderValue( - boost::beast::http::field::if_none_match); - BMCWEB_LOG_DEBUG("Setting expected hash {}", expected); - if (!expected.empty()) + thisReq.session = crow::authentication::authenticate( + {}, asyncResp->res, thisReq.method(), thisReq.req, nullptr); + if (!crow::authentication::isOnAllowlist(thisReq.url().path(), + thisReq.method()) && + thisReq.session == nullptr) { - asyncResp->res.setExpectedHash(expected); + BMCWEB_LOG_WARNING("Authentication failed"); + forward_unauthorized::sendUnauthorized( + thisReq.url().encoded_path(), + thisReq.getHeaderValue("X-Requested-With"), + thisReq.getHeaderValue("Accept"), asyncResp->res); + return 0; } - handler->handle(thisReq, asyncResp); } + std::string_view expected = + thisReq.getHeaderValue(boost::beast::http::field::if_none_match); + BMCWEB_LOG_DEBUG("Setting expected hash {}", expected); + if (!expected.empty()) + { + asyncResp->res.setExpectedHash(expected); + } + handler->handle(it->second.req, asyncResp); return 0; } @@ -306,8 +305,8 @@ class HTTP2Connection : if (!reqReader) { reqReader.emplace( - bmcweb::HttpBody::reader(thisStream->second.req.req.base(), - thisStream->second.req.req.body())); + bmcweb::HttpBody::reader(thisStream->second.req->req.base(), + thisStream->second.req->req.body())); } boost::beast::error_code ec; reqReader->put(boost::asio::const_buffer(data, len), ec); @@ -425,7 +424,7 @@ class HTTP2Connection : return -1; } - crow::Request& thisReq = thisStream->second.req; + crow::Request& thisReq = *thisStream->second.req; if (nameSv == ":path") { @@ -493,7 +492,7 @@ class HTTP2Connection : Http2StreamData& stream = streams[frame.hd.stream_id]; // http2 is by definition always tls - stream.req.isSecure = true; + stream.req->isSecure = true; } return 0; } diff --git a/http/http_body.hpp b/http/http_body.hpp index 46070cc60f..f4bfed58ec 100644 --- a/http/http_body.hpp +++ b/http/http_body.hpp @@ -178,7 +178,11 @@ class HttpBody::writer value_type& body; size_t sent = 0; - constexpr static size_t readBufSize = 4096; + // 64KB This number is arbitrary, and selected to try to optimize for larger + // files and fewer loops over per-connection reduction in memory usage. + // Nginx uses 16-32KB here, so we're in the range of what other webservers + // do. + constexpr static size_t readBufSize = 1024UL * 64UL; std::array<char, readBufSize> fileReadBuf{}; public: diff --git a/http/http_client.hpp b/http/http_client.hpp index ac231b42e7..0bfb3818df 100644 --- a/http/http_client.hpp +++ b/http/http_client.hpp @@ -44,7 +44,6 @@ #include <cstdlib> #include <functional> -#include <iostream> #include <memory> #include <queue> #include <string> diff --git a/http/http_connection.hpp b/http/http_connection.hpp index e02c518ebb..4d9c9806ef 100644 --- a/http/http_connection.hpp +++ b/http/http_connection.hpp @@ -30,6 +30,7 @@ #include <atomic> #include <chrono> +#include <memory> #include <vector> namespace crow @@ -232,7 +233,7 @@ class Connection : { return; } - req = crow::Request(parser->release(), reqEc); + req = std::make_shared<crow::Request>(parser->release(), reqEc); if (reqEc) { BMCWEB_LOG_DEBUG("Request failed to construct{}", reqEc.message()); @@ -240,15 +241,15 @@ class Connection : completeRequest(res); return; } - req.session = userSession; + req->session = userSession; // Fetch the client IP address - req.ipAddress = ip; + req->ipAddress = ip; // Check for HTTP version 1.1. - if (req.version() == 11) + if (req->version() == 11) { - if (req.getHeaderValue(boost::beast::http::field::host).empty()) + if (req->getHeaderValue(boost::beast::http::field::host).empty()) { res.result(boost::beast::http::status::bad_request); completeRequest(res); @@ -257,11 +258,11 @@ class Connection : } BMCWEB_LOG_INFO("Request: {} HTTP/{}.{} {} {} {}", logPtr(this), - req.version() / 10, req.version() % 10, - req.methodString(), req.target(), - req.ipAddress.to_string()); + req->version() / 10, req->version() % 10, + req->methodString(), req->target(), + req->ipAddress.to_string()); - req.ioService = static_cast<decltype(req.ioService)>( + req->ioService = static_cast<decltype(req->ioService)>( &adaptor.get_executor().context()); if (res.completed) @@ -269,23 +270,24 @@ class Connection : completeRequest(res); return; } - keepAlive = req.keepAlive(); + keepAlive = req->keepAlive(); if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>) { -#ifndef BMCWEB_INSECURE_DISABLE_AUTHX - if (!crow::authentication::isOnAllowlist(req.url().path(), - req.method()) && - req.session == nullptr) + if constexpr (!BMCWEB_INSECURE_DISABLE_AUTH) { - BMCWEB_LOG_WARNING("Authentication failed"); - forward_unauthorized::sendUnauthorized( - req.url().encoded_path(), - req.getHeaderValue("X-Requested-With"), - req.getHeaderValue("Accept"), res); - completeRequest(res); - return; + if (!crow::authentication::isOnAllowlist(req->url().path(), + req->method()) && + req->session == nullptr) + { + BMCWEB_LOG_WARNING("Authentication failed"); + forward_unauthorized::sendUnauthorized( + req->url().encoded_path(), + req->getHeaderValue("X-Requested-With"), + req->getHeaderValue("Accept"), res); + completeRequest(res); + return; + } } -#endif // BMCWEB_INSECURE_DISABLE_AUTHX } auto asyncResp = std::make_shared<bmcweb::AsyncResp>(); BMCWEB_LOG_DEBUG("Setting completion handler"); @@ -294,11 +296,11 @@ class Connection : self->completeRequest(thisRes); }); bool isSse = - isContentTypeAllowed(req.getHeaderValue("Accept"), + isContentTypeAllowed(req->getHeaderValue("Accept"), http_helpers::ContentType::EventStream, false); std::string_view upgradeType( - req.getHeaderValue(boost::beast::http::field::upgrade)); - if ((req.isUpgrade() && + req->getHeaderValue(boost::beast::http::field::upgrade)); + if ((req->isUpgrade() && bmcweb::asciiIEquals(upgradeType, "websocket")) || isSse) { @@ -320,7 +322,7 @@ class Connection : return; } std::string_view expected = - req.getHeaderValue(boost::beast::http::field::if_none_match); + req->getHeaderValue(boost::beast::http::field::if_none_match); if (!expected.empty()) { res.setExpectedHash(expected); @@ -371,7 +373,7 @@ class Connection : res = std::move(thisRes); res.keepAlive(keepAlive); - completeResponseFields(req, res); + completeResponseFields(*req, res); res.addHeader(boost::beast::http::field::date, getCachedDateStr()); doWrite(); @@ -405,12 +407,13 @@ class Connection : private: uint64_t getContentLengthLimit() { -#ifndef BMCWEB_INSECURE_DISABLE_AUTHX - if (userSession == nullptr) + if constexpr (!BMCWEB_INSECURE_DISABLE_AUTH) { - return loggedOutPostBodyLimit; + if (userSession == nullptr) + { + return loggedOutPostBodyLimit; + } } -#endif return httpReqBodyLimit; } @@ -514,8 +517,18 @@ class Connection : return; } + if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>) + { + if constexpr (!BMCWEB_INSECURE_DISABLE_AUTH) + { + boost::beast::http::verb method = parser->get().method(); + userSession = crow::authentication::authenticate( + ip, res, method, parser->get().base(), mtlsSession); + } + } + std::string_view expect = - req.getHeaderValue(boost::beast::http::field::expect); + parser->get()[boost::beast::http::field::expect]; if (bmcweb::asciiIEquals(expect, "100-continue")) { res.result(boost::beast::http::status::continue_); @@ -523,15 +536,6 @@ class Connection : return; } - if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>) - { -#ifndef BMCWEB_INSECURE_DISABLE_AUTHX - boost::beast::http::verb method = parser->get().method(); - userSession = crow::authentication::authenticate( - ip, res, method, parser->get().base(), mtlsSession); -#endif // BMCWEB_INSECURE_DISABLE_AUTHX - } - if (!handleContentLengthError()) { return; @@ -644,8 +648,7 @@ class Connection : userSession = nullptr; - // Destroy the Request via the std::optional - req.clear(); + req->clear(); doReadHeaders(); } @@ -732,7 +735,7 @@ class Connection : boost::beast::flat_static_buffer<8192> buffer; - crow::Request req; + std::shared_ptr<crow::Request> req; crow::Response res; std::shared_ptr<persistent_data::UserSession> userSession; diff --git a/http/http_request.hpp b/http/http_request.hpp index d1a6ac053a..d2a1e259fc 100644 --- a/http/http_request.hpp +++ b/http/http_request.hpp @@ -18,7 +18,8 @@ namespace crow struct Request { - boost::beast::http::request<bmcweb::HttpBody> req; + using Body = boost::beast::http::request<bmcweb::HttpBody>; + Body req; private: boost::urls::url urlBase; @@ -32,9 +33,7 @@ struct Request std::shared_ptr<persistent_data::UserSession> session; std::string userRole; - Request(boost::beast::http::request<bmcweb::HttpBody> reqIn, - std::error_code& ec) : - req(std::move(reqIn)) + Request(Body reqIn, std::error_code& ec) : req(std::move(reqIn)) { if (!setUrlInfo()) { diff --git a/http/logging.hpp b/http/logging.hpp index cf908771bb..768014a3b9 100644 --- a/http/logging.hpp +++ b/http/logging.hpp @@ -65,7 +65,7 @@ const void* logPtr(T p) } template <LogLevel level, typename... Args> -inline void vlog(std::format_string<Args...> format, Args... args, +inline void vlog(std::format_string<Args...>&& format, Args&&... args, const std::source_location& loc) noexcept { if constexpr (bmcwebCurrentLoggingLevel < level) @@ -78,9 +78,28 @@ inline void vlog(std::format_string<Args...> format, Args... args, constexpr std::string_view levelString = mapLogLevelFromName[stringIndex]; std::string_view filename = loc.file_name(); filename = filename.substr(filename.rfind('/') + 1); - std::cout << std::format("[{} {}:{}] ", levelString, filename, loc.line()) - << std::format(format, std::forward<Args>(args)...) << '\n' - << std::flush; + std::string logLocation; + try + { + // TODO, multiple static analysis tools flag that this could potentially + // throw Based on the documentation, it shouldn't throw, so long as none + // of the formatters throw, so unclear at this point why this try/catch + // is required, but add it to silence the static analysis tools. + logLocation = std::format("[{} {}:{}] ", levelString, filename, + loc.line()); + logLocation += std::format(std::move(format), + std::forward<Args>(args)...); + } + catch (const std::format_error& /*error*/) + { + logLocation += "Failed to format"; + // Nothing more we can do here if logging is broken. + } + logLocation += '\n'; + // Intentionally ignore error return. + fwrite(logLocation.data(), sizeof(std::string::value_type), + logLocation.size(), stdout); + fflush(stdout); } } // namespace crow @@ -92,7 +111,8 @@ struct BMCWEB_LOG_CRITICAL const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Critical, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Critical, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -104,7 +124,8 @@ struct BMCWEB_LOG_ERROR const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Error, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Error, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -116,7 +137,8 @@ struct BMCWEB_LOG_WARNING const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Warning, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Warning, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -128,7 +150,8 @@ struct BMCWEB_LOG_INFO const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Info, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Info, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; @@ -140,7 +163,8 @@ struct BMCWEB_LOG_DEBUG const std::source_location& loc = std::source_location::current()) noexcept { - crow::vlog<crow::LogLevel::Debug, Args...>(format, args..., loc); + crow::vlog<crow::LogLevel::Debug, Args...>( + std::move(format), std::forward<Args>(args)..., loc); } }; diff --git a/http/parsing.hpp b/http/parsing.hpp index cf813537ed..d59621104c 100644 --- a/http/parsing.hpp +++ b/http/parsing.hpp @@ -32,9 +32,10 @@ inline JsonParseResult parseRequestAsJson(const crow::Request& req, req.getHeaderValue(boost::beast::http::field::content_type))) { BMCWEB_LOG_WARNING("Failed to parse content type on request"); -#ifndef BMCWEB_INSECURE_IGNORE_CONTENT_TYPE - return JsonParseResult::BadContentType; -#endif + if constexpr (!BMCWEB_INSECURE_IGNORE_CONTENT_TYPE) + { + return JsonParseResult::BadContentType; + } } jsonOut = nlohmann::json::parse(req.body(), nullptr, false); if (jsonOut.is_discarded()) diff --git a/http/routing.hpp b/http/routing.hpp index fa6db58bfc..7e481a0dd2 100644 --- a/http/routing.hpp +++ b/http/routing.hpp @@ -491,7 +491,7 @@ class Router return route; } - FindRouteResponse findRoute(Request& req) const + FindRouteResponse findRoute(const Request& req) const { FindRouteResponse findRoute; @@ -529,11 +529,11 @@ class Router } template <typename Adaptor> - void handleUpgrade(Request& req, + void handleUpgrade(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, Adaptor&& adaptor) { - std::optional<HttpVerb> verb = httpVerbFromBoost(req.method()); + std::optional<HttpVerb> verb = httpVerbFromBoost(req->method()); if (!verb || static_cast<size_t>(*verb) >= perMethods.size()) { asyncResp->res.result(boost::beast::http::status::not_found); @@ -543,11 +543,12 @@ class Router Trie& trie = perMethod.trie; std::vector<BaseRule*>& rules = perMethod.rules; - Trie::FindResult found = trie.find(req.url().encoded_path()); + Trie::FindResult found = trie.find(req->url().encoded_path()); unsigned ruleIndex = found.ruleIndex; if (ruleIndex == 0U) { - BMCWEB_LOG_DEBUG("Cannot match rules {}", req.url().encoded_path()); + BMCWEB_LOG_DEBUG("Cannot match rules {}", + req->url().encoded_path()); asyncResp->res.result(boost::beast::http::status::not_found); return; } @@ -563,7 +564,7 @@ class Router { BMCWEB_LOG_DEBUG( "Rule found but method mismatch: {} with {}({}) / {}", - req.url().encoded_path(), req.methodString(), + req->url().encoded_path(), req->methodString(), static_cast<uint32_t>(*verb), methods); asyncResp->res.result(boost::beast::http::status::not_found); return; @@ -574,25 +575,24 @@ class Router // TODO(ed) This should be able to use std::bind_front, but it doesn't // appear to work with the std::move on adaptor. - validatePrivilege( - req, asyncResp, rule, - [&rule, asyncResp, adaptor = std::forward<Adaptor>(adaptor)]( - Request& thisReq) mutable { - rule.handleUpgrade(thisReq, asyncResp, std::move(adaptor)); + validatePrivilege(req, asyncResp, rule, + [req, &rule, asyncResp, + adaptor = std::forward<Adaptor>(adaptor)]() mutable { + rule.handleUpgrade(*req, asyncResp, std::move(adaptor)); }); } - void handle(Request& req, + void handle(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) { - std::optional<HttpVerb> verb = httpVerbFromBoost(req.method()); + std::optional<HttpVerb> verb = httpVerbFromBoost(req->method()); if (!verb || static_cast<size_t>(*verb) >= perMethods.size()) { asyncResp->res.result(boost::beast::http::status::not_found); return; } - FindRouteResponse foundRoute = findRoute(req); + FindRouteResponse foundRoute = findRoute(*req); if (foundRoute.route.rule == nullptr) { @@ -600,13 +600,13 @@ class Router // route if (foundRoute.allowHeader.empty()) { - foundRoute.route = findRouteByIndex(req.url().encoded_path(), + foundRoute.route = findRouteByIndex(req->url().encoded_path(), notFoundIndex); } else { // See if we have a method not allowed (405) handler - foundRoute.route = findRouteByIndex(req.url().encoded_path(), + foundRoute.route = findRouteByIndex(req->url().encoded_path(), methodNotAllowedIndex); } } @@ -640,14 +640,15 @@ class Router BMCWEB_LOG_DEBUG("Matched rule '{}' {} / {}", rule.rule, static_cast<uint32_t>(*verb), rule.getMethods()); - if (req.session == nullptr) + if (req->session == nullptr) { - rule.handle(req, asyncResp, params); + rule.handle(*req, asyncResp, params); return; } - validatePrivilege(req, asyncResp, rule, - [&rule, asyncResp, params](Request& thisReq) mutable { - rule.handle(thisReq, asyncResp, params); + validatePrivilege( + req, asyncResp, rule, + [req, asyncResp, &rule, params = std::move(params)]() { + rule.handle(*req, asyncResp, params); }); } diff --git a/http/verb.hpp b/http/verb.hpp index 5859d10b0d..b96008de18 100644 --- a/http/verb.hpp +++ b/http/verb.hpp @@ -2,7 +2,6 @@ #include <boost/beast/http/verb.hpp> -#include <iostream> #include <optional> #include <string_view> diff --git a/http/websocket.hpp b/http/websocket.hpp index e669ffa611..e8d7b1259e 100644 --- a/http/websocket.hpp +++ b/http/websocket.hpp @@ -93,21 +93,21 @@ class ConnectionImpl : public Connection ws.set_option(boost::beast::websocket::stream_base::decorator( [session{session}, protocolHeader](boost::beast::websocket::response_type& m) { - -#ifndef BMCWEB_INSECURE_DISABLE_CSRF_PREVENTION - if (session != nullptr) + if constexpr (!BMCWEB_INSECURE_DISABLE_CSRF) { - // use protocol for csrf checking - if (session->cookieAuth && - !crow::utility::constantTimeStringCompare( - protocolHeader, session->csrfToken)) + if (session != nullptr) { - BMCWEB_LOG_ERROR("Websocket CSRF error"); - m.result(boost::beast::http::status::unauthorized); - return; + // use protocol for csrf checking + if (session->cookieAuth && + !crow::utility::constantTimeStringCompare( + protocolHeader, session->csrfToken)) + { + BMCWEB_LOG_ERROR("Websocket CSRF error"); + m.result(boost::beast::http::status::unauthorized); + return; + } } } -#endif if (!protocolHeader.empty()) { m.insert(bf::sec_websocket_protocol, protocolHeader); diff --git a/include/async_resolve.hpp b/include/async_resolve.hpp index 2d9899d1a4..dbc0cfc749 100644 --- a/include/async_resolve.hpp +++ b/include/async_resolve.hpp @@ -8,7 +8,6 @@ #include <sdbusplus/message.hpp> #include <charconv> -#include <iostream> #include <memory> namespace async_resolve diff --git a/include/authentication.hpp b/include/authentication.hpp index 6483365bef..0617cf3729 100644 --- a/include/authentication.hpp +++ b/include/authentication.hpp @@ -153,7 +153,7 @@ inline std::shared_ptr<persistent_data::UserSession> } sessionOut->cookieAuth = true; - if constexpr (BMCWEB_INSECURE_DISABLE_CSRF) + if constexpr (!BMCWEB_INSECURE_DISABLE_CSRF) { // RFC7231 defines methods that need csrf protection if (method != boost::beast::http::verb::get) @@ -177,6 +177,7 @@ inline std::shared_ptr<persistent_data::UserSession> } } } + return sessionOut; } return nullptr; } diff --git a/include/dbus_monitor.hpp b/include/dbus_monitor.hpp index 25f0c7f772..db75fce823 100644 --- a/include/dbus_monitor.hpp +++ b/include/dbus_monitor.hpp @@ -164,11 +164,6 @@ inline void requestRoutes(App& app) { interfaceCount = 1; } - // Reserve our matches upfront. For each path there is 1 for - // interfacesAdded, and InterfaceCount number for - // PropertiesChanged - thisSession.matches.reserve(thisSession.matches.size() + - paths->size() * (1U + interfaceCount)); // These regexes derived on the rules here: // https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-names diff --git a/include/dbus_privileges.hpp b/include/dbus_privileges.hpp index a58f9bebd0..4cafe52ec6 100644 --- a/include/dbus_privileges.hpp +++ b/include/dbus_privileges.hpp @@ -106,19 +106,18 @@ inline bool return true; } -template <typename CallbackFn> -void afterGetUserInfo(Request& req, - const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, - BaseRule& rule, CallbackFn callback, - const boost::system::error_code& ec, - const dbus::utility::DBusPropertiesMap& userInfoMap) +inline bool + afterGetUserInfo(Request& req, + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + BaseRule& rule, const boost::system::error_code& ec, + const dbus::utility::DBusPropertiesMap& userInfoMap) { if (ec) { BMCWEB_LOG_ERROR("GetUserInfo failed..."); asyncResp->res.result( boost::beast::http::status::internal_server_error); - return; + return false; } if (!populateUserInfo(req, userInfoMap)) @@ -126,7 +125,7 @@ void afterGetUserInfo(Request& req, BMCWEB_LOG_ERROR("Failed to populate user information"); asyncResp->res.result( boost::beast::http::status::internal_server_error); - return; + return false; } if (!isUserPrivileged(req, asyncResp, rule)) @@ -134,28 +133,30 @@ void afterGetUserInfo(Request& req, // User is not privileged BMCWEB_LOG_ERROR("Insufficient Privilege"); asyncResp->res.result(boost::beast::http::status::forbidden); - return; + return false; } - callback(req); + + return true; } template <typename CallbackFn> -void validatePrivilege(Request& req, +void validatePrivilege(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, BaseRule& rule, CallbackFn&& callback) { - if (req.session == nullptr) + if (req->session == nullptr) { return; } - std::string username = req.session->username; + std::string username = req->session->username; crow::connections::systemBus->async_method_call( - [req{std::move(req)}, asyncResp, &rule, - callback = std::forward<CallbackFn>(callback)]( + [req, asyncResp, &rule, callback = std::forward<CallbackFn>(callback)]( const boost::system::error_code& ec, const dbus::utility::DBusPropertiesMap& userInfoMap) mutable { - afterGetUserInfo(req, asyncResp, rule, - std::forward<CallbackFn>(callback), ec, userInfoMap); + if (afterGetUserInfo(*req, asyncResp, rule, ec, userInfoMap)) + { + callback(); + } }, "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user", "xyz.openbmc_project.User.Manager", "GetUserInfo", username); diff --git a/include/obmc_console.hpp b/include/obmc_console.hpp index fb363be275..842cbe6307 100644 --- a/include/obmc_console.hpp +++ b/include/obmc_console.hpp @@ -7,6 +7,12 @@ #include <boost/asio/local/stream_protocol.hpp> #include <boost/container/flat_map.hpp> +#include <boost/system/error_code.hpp> + +#include <array> +#include <memory> +#include <string> +#include <string_view> namespace crow { diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp index f0b72fbcc4..72e1c31c06 100644 --- a/include/openbmc_dbus_rest.hpp +++ b/include/openbmc_dbus_rest.hpp @@ -821,7 +821,11 @@ inline int convertJsonToDbus(sd_bus_message* m, const std::string& argType, { return -ERANGE; } - sd_bus_message_append_basic(m, argCode[0], doubleValue); + r = sd_bus_message_append_basic(m, argCode[0], doubleValue); + if (r < 0) + { + return r; + } } else if (argCode.starts_with("a")) { @@ -2373,7 +2377,11 @@ inline void return; } - convertDBusToJSON("v", msg, propertyItem); + int r = convertDBusToJSON("v", msg, propertyItem); + if (r < 0) + { + BMCWEB_LOG_ERROR("Couldn't convert vector to json"); + } }); } property = property->NextSiblingElement("property"); diff --git a/include/ossl_random.hpp b/include/ossl_random.hpp index 0e28944d83..4d4bc04a06 100644 --- a/include/ossl_random.hpp +++ b/include/ossl_random.hpp @@ -7,7 +7,6 @@ extern "C" #include <openssl/rand.h> } -#include <iostream> #include <limits> #include <string> diff --git a/include/vm_websocket.hpp b/include/vm_websocket.hpp index b489a4265f..4d86c3e231 100644 --- a/include/vm_websocket.hpp +++ b/include/vm_websocket.hpp @@ -449,7 +449,9 @@ inline void // If the socket file exists (i.e. after bmcweb crash), // we cannot reuse it. - std::remove(socket.c_str()); + std::error_code ec2; + std::filesystem::remove(socket.c_str(), ec2); + // Ignore failures. File might not exist. sessions[&conn] = std::make_shared<NbdProxyServer>(conn, socket, endpointId, path); @@ -588,9 +590,10 @@ inline void requestRoutes(App& app) return; } - boost::asio::buffer_copy(handler->inputBuffer->prepare(data.size()), - boost::asio::buffer(data)); - handler->inputBuffer->commit(data.size()); + size_t copied = boost::asio::buffer_copy( + handler->inputBuffer->prepare(data.size()), + boost::asio::buffer(data)); + handler->inputBuffer->commit(copied); handler->doWrite(); }); } diff --git a/meson_options.txt b/meson_options.txt index 8a497ed8cc..11b476f9a4 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -242,6 +242,20 @@ option( ) option( + 'redfish-updateservice-use-dbus', + type: 'feature', + value: 'disabled', + description: '''Enables xyz.openbmc_project.Software.Update D-Bus interface + to propagate UpdateService requests to the corresponding + updater daemons instead of moving files to /tmp/images dir. + This option is temporary, should not be enabled on any + production systems. The code will be moved to the normal + code update flow and the option will be removed at the end + of Q3 2024. + ''' +) + +option( 'https_port', type: 'integer', min: 1, diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp index 702ca268fd..2ea7430072 100644 --- a/redfish-core/include/event_service_manager.hpp +++ b/redfish-core/include/event_service_manager.hpp @@ -60,7 +60,6 @@ static constexpr const char* eventServiceFile = static constexpr const uint8_t maxNoOfSubscriptions = 20; static constexpr const uint8_t maxNoOfSSESubscriptions = 10; -#ifndef BMCWEB_ENABLE_REDFISH_DBUS_LOG_ENTRIES // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) static std::optional<boost::asio::posix::stream_descriptor> inotifyConn; static constexpr const char* redfishEventLogDir = "/var/log"; @@ -258,7 +257,6 @@ inline int formatEventLogEntry(const std::string& logEntryID, } } // namespace event_log -#endif inline bool isFilterQuerySpecialChar(char c) { @@ -426,7 +424,6 @@ class Subscription : public persistent_data::UserSubscription return sendEvent(std::move(strMsg)); } -#ifndef BMCWEB_ENABLE_REDFISH_DBUS_LOG_ENTRIES void filterAndSendEventLogs( const std::vector<EventLogObjectsType>& eventRecords) { @@ -492,7 +489,6 @@ class Subscription : public persistent_data::UserSubscription sendEvent(std::move(strMsg)); eventSeqNum++; } -#endif void filterAndSendReports(const std::string& reportId, const telemetry::TimestampReadings& var) @@ -682,11 +678,11 @@ class EventServiceManager updateNoOfSubscribersCount(); -#ifndef BMCWEB_ENABLE_REDFISH_DBUS_LOG_ENTRIES - - cacheRedfishLogFile(); + if constexpr (!BMCWEB_REDFISH_DBUS_LOG) + { + cacheRedfishLogFile(); + } -#endif // Update retry configuration. subValue->updateRetryConfig(retryAttempts, retryTimeoutInterval); } @@ -942,12 +938,13 @@ class EventServiceManager updateSubscriptionData(); } -#ifndef BMCWEB_ENABLE_REDFISH_DBUS_LOG_ENTRIES - if (redfishLogFilePosition != 0) + if constexpr (!BMCWEB_REDFISH_DBUS_LOG) { - cacheRedfishLogFile(); + if (redfishLogFilePosition != 0) + { + cacheRedfishLogFile(); + } } -#endif // Update retry configuration. subValue->updateRetryConfig(retryAttempts, retryTimeoutInterval); @@ -1097,8 +1094,6 @@ class EventServiceManager } } -#ifndef BMCWEB_ENABLE_REDFISH_DBUS_LOG_ENTRIES - void resetRedfishFilePosition() { // Control would be here when Redfish file is created. @@ -1336,7 +1331,6 @@ class EventServiceManager return 0; } -#endif static void getReadingsForReport(sdbusplus::message_t& msg) { if (msg.is_method_error()) diff --git a/redfish-core/include/query.hpp b/redfish-core/include/query.hpp index 063f1a8ef2..2d280cb787 100644 --- a/redfish-core/include/query.hpp +++ b/redfish-core/include/query.hpp @@ -16,7 +16,6 @@ #include <functional> #include <memory> -#include <new> #include <optional> #include <string> #include <string_view> @@ -31,11 +30,10 @@ namespace redfish { -inline void - afterIfMatchRequest(crow::App& app, - const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, - crow::Request& req, const std::string& ifMatchHeader, - const crow::Response& resIn) +inline void afterIfMatchRequest( + crow::App& app, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + const std::shared_ptr<crow::Request>& req, const std::string& ifMatchHeader, + const crow::Response& resIn) { std::string computedEtag = resIn.computeEtag(); BMCWEB_LOG_DEBUG("User provided if-match etag {} computed etag {}", @@ -46,7 +44,7 @@ inline void return; } // Restart the request without if-match - req.clearHeader(boost::beast::http::field::if_match); + req->clearHeader(boost::beast::http::field::if_match); BMCWEB_LOG_DEBUG("Restarting request"); app.handle(req, asyncResp); } @@ -84,8 +82,10 @@ inline bool handleIfMatch(crow::App& app, const crow::Request& req, boost::system::error_code ec; // Try to GET the same resource - crow::Request newReq( - {boost::beast::http::verb::get, req.url().encoded_path(), 11}, ec); + auto getReq = std::make_shared<crow::Request>( + crow::Request::Body{boost::beast::http::verb::get, + req.url().encoded_path(), 11}, + ec); if (ec) { @@ -94,17 +94,21 @@ inline bool handleIfMatch(crow::App& app, const crow::Request& req, } // New request has the same credentials as the old request - newReq.session = req.session; + getReq->session = req.session; // Construct a new response object to fill in, and check the hash of before // we modify the Resource. std::shared_ptr<bmcweb::AsyncResp> getReqAsyncResp = std::make_shared<bmcweb::AsyncResp>(); + // Ideally we would have a shared_ptr to the original Request which we could + // modify to remove the If-Match and restart it. But instead we have to make + // a full copy to restart it. getReqAsyncResp->res.setCompleteRequestHandler(std::bind_front( - afterIfMatchRequest, std::ref(app), asyncResp, req, ifMatch)); + afterIfMatchRequest, std::ref(app), asyncResp, + std::make_shared<crow::Request>(req), std::move(ifMatch))); - app.handle(newReq, getReqAsyncResp); + app.handle(getReq, getReqAsyncResp); return false; } diff --git a/redfish-core/include/registries.hpp b/redfish-core/include/registries.hpp index bad31c4324..b0ddb68fdd 100644 --- a/redfish-core/include/registries.hpp +++ b/redfish-core/include/registries.hpp @@ -20,7 +20,6 @@ #include <array> #include <charconv> #include <cstddef> -#include <iostream> #include <numeric> #include <span> #include <string> diff --git a/redfish-core/include/utils/collection.hpp b/redfish-core/include/utils/collection.hpp index 2e5563b4c9..9efa7d3ef3 100644 --- a/redfish-core/include/utils/collection.hpp +++ b/redfish-core/include/utils/collection.hpp @@ -27,6 +27,12 @@ inline void handleCollectionMembers( const boost::system::error_code& ec, const dbus::utility::MapperGetSubTreePathsResponse& objects) { + if (jsonKeyName.empty()) + { + messages::internalError(asyncResp->res); + BMCWEB_LOG_ERROR("Json Key called empty. Did you mean /Members?"); + return; + } nlohmann::json::json_pointer jsonCountKeyName = jsonKeyName; std::string back = jsonCountKeyName.back(); jsonCountKeyName.pop_back(); diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp index eadb66ed5c..3b04e21991 100644 --- a/redfish-core/include/utils/query_param.hpp +++ b/redfish-core/include/utils/query_param.hpp @@ -503,7 +503,8 @@ inline bool processOnly(crow::App& app, crow::Response& res, // TODO(Ed) copy request headers? // newReq.session = req.session; std::error_code ec; - crow::Request newReq({boost::beast::http::verb::get, *url, 11}, ec); + auto newReq = std::make_shared<crow::Request>( + crow::Request::Body{boost::beast::http::verb::get, *url, 11}, ec); if (ec) { messages::internalError(res); @@ -854,8 +855,10 @@ class MultiAsyncResp : public std::enable_shared_from_this<MultiAsyncResp> const std::string subQuery = node.uri + *queryStr; BMCWEB_LOG_DEBUG("URL of subquery: {}", subQuery); std::error_code ec; - crow::Request newReq({boost::beast::http::verb::get, subQuery, 11}, - ec); + auto newReq = std::make_shared<crow::Request>( + crow::Request::Body{boost::beast::http::verb::get, subQuery, + 11}, + ec); if (ec) { messages::internalError(finalRes->res); diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp index 972512b145..1d3ef411a8 100644 --- a/redfish-core/lib/account_service.hpp +++ b/redfish-core/lib/account_service.hpp @@ -16,6 +16,7 @@ #pragma once #include "app.hpp" +#include "certificate_service.hpp" #include "dbus_utility.hpp" #include "error_messages.hpp" #include "generated/enums/account_service.hpp" @@ -23,13 +24,17 @@ #include "persistent_data.hpp" #include "query.hpp" #include "registries/privilege_registry.hpp" +#include "utils/collection.hpp" #include "utils/dbus_utils.hpp" #include "utils/json_utils.hpp" +#include <boost/url/format.hpp> +#include <boost/url/url.hpp> #include <sdbusplus/asio/property.hpp> #include <sdbusplus/unpack_properties.hpp> #include <array> +#include <memory> #include <optional> #include <ranges> #include <string> @@ -1174,6 +1179,80 @@ inline void handleAccountServiceHead( } inline void + getClientCertificates(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + const nlohmann::json::json_pointer& keyLocation) +{ + boost::urls::url url( + "/redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates"); + std::array<std::string_view, 1> interfaces = { + "xyz.openbmc_project.Certs.Certificate"}; + std::string path = "/xyz/openbmc_project/certs/authority/truststore"; + + collection_util::getCollectionToKey(asyncResp, url, interfaces, path, + keyLocation); +} + +inline void handleAccountServiceClientCertificatesInstanceHead( + App& app, const crow::Request& req, + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + const std::string& /*id*/) +{ + if (!redfish::setUpRedfishRoute(app, req, asyncResp)) + { + return; + } + + asyncResp->res.addHeader( + boost::beast::http::field::link, + "</redfish/v1/JsonSchemas/Certificate/Certificate.json>; rel=describedby"); +} + +inline void handleAccountServiceClientCertificatesInstanceGet( + App& app, const crow::Request& req, + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, const std::string& id) +{ + if (!redfish::setUpRedfishRoute(app, req, asyncResp)) + { + return; + } + BMCWEB_LOG_DEBUG("ClientCertificate Certificate ID={}", id); + const boost::urls::url certURL = boost::urls::format( + "/redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates/{}", + id); + std::string objPath = + sdbusplus::message::object_path(certs::authorityObjectPath) / id; + getCertificateProperties( + asyncResp, objPath, + "xyz.openbmc_project.Certs.Manager.Authority.Truststore", id, certURL, + "Client Certificate"); +} + +inline void handleAccountServiceClientCertificatesHead( + App& app, const crow::Request& req, + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) +{ + if (!redfish::setUpRedfishRoute(app, req, asyncResp)) + { + return; + } + + asyncResp->res.addHeader( + boost::beast::http::field::link, + "</redfish/v1/JsonSchemas/CertificateCollection/CertificateCollection.json>; rel=describedby"); +} + +inline void handleAccountServiceClientCertificatesGet( + App& app, const crow::Request& req, + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) +{ + if (!redfish::setUpRedfishRoute(app, req, asyncResp)) + { + return; + } + getClientCertificates(asyncResp, "/Members"_json_pointer); +} + +inline void handleAccountServiceGet(App& app, const crow::Request& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) { @@ -1214,6 +1293,23 @@ inline void allowed.emplace_back(account_service::BasicAuthState::Disabled); json["HTTPBasicAuth@AllowableValues"] = std::move(allowed); + nlohmann::json::object_t clientCertificate; + clientCertificate["Enabled"] = authMethodsConfig.tls; + clientCertificate["RespondToUnauthenticatedClients"] = true; + clientCertificate["CertificateMappingAttribute"] = + account_service::CertificateMappingAttribute::CommonName; + nlohmann::json::object_t certificates; + certificates["@odata.id"] = + "/redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates"; + certificates["@odata.type"] = + "#CertificateCollection.CertificateCollection"; + clientCertificate["Certificates"] = std::move(certificates); + json["MultiFactorAuth"]["ClientCertificate"] = std::move(clientCertificate); + + getClientCertificates( + asyncResp, + "/MultiFactorAuth/ClientCertificate/Certificates/Members"_json_pointer); + json["Oem"]["OpenBMC"]["@odata.type"] = "#OpenBMCAccountService.v1_0_0.AccountService"; json["Oem"]["OpenBMC"]["@odata.id"] = @@ -1249,7 +1345,7 @@ inline void return; } - BMCWEB_LOG_DEBUG("Got {}properties for AccountService", + BMCWEB_LOG_DEBUG("Got {} properties for AccountService", propertiesList.size()); const uint8_t* minPasswordLength = nullptr; @@ -2026,6 +2122,34 @@ inline void requestAccountServiceRoutes(App& app) .methods(boost::beast::http::verb::patch)( std::bind_front(handleAccountServicePatch, std::ref(app))); + BMCWEB_ROUTE( + app, + "/redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates") + .privileges(redfish::privileges::headCertificateCollection) + .methods(boost::beast::http::verb::head)(std::bind_front( + handleAccountServiceClientCertificatesHead, std::ref(app))); + + BMCWEB_ROUTE( + app, + "/redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates") + .privileges(redfish::privileges::getCertificateCollection) + .methods(boost::beast::http::verb::get)(std::bind_front( + handleAccountServiceClientCertificatesGet, std::ref(app))); + + BMCWEB_ROUTE( + app, + "/redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates/<str>") + .privileges(redfish::privileges::headCertificate) + .methods(boost::beast::http::verb::head)(std::bind_front( + handleAccountServiceClientCertificatesInstanceHead, std::ref(app))); + + BMCWEB_ROUTE( + app, + "/redfish/v1/AccountService/MultiFactorAuth/ClientCertificate/Certificates/<str>/") + .privileges(redfish::privileges::getCertificate) + .methods(boost::beast::http::verb::get)(std::bind_front( + handleAccountServiceClientCertificatesInstanceGet, std::ref(app))); + BMCWEB_ROUTE(app, "/redfish/v1/AccountService/Accounts/") .privileges(redfish::privileges::headManagerAccountCollection) .methods(boost::beast::http::verb::head)( diff --git a/redfish-core/lib/ethernet.hpp b/redfish-core/lib/ethernet.hpp index 876bc2edb4..533c7f5c2e 100644 --- a/redfish-core/lib/ethernet.hpp +++ b/redfish-core/lib/ethernet.hpp @@ -47,6 +47,12 @@ enum class LinkType Global }; +enum class IpVersion +{ + IpV4, + IpV6 +}; + /** * Structure for keeping IPv4 data required by Redfish */ @@ -700,7 +706,30 @@ inline void extractIPData(const std::string& ethifaceId, } /** - * @brief Deletes given IPv4 interface + * @brief Modifies the default gateway assigned to the NIC + * + * @param[in] ifaceId Id of network interface whose default gateway is to be + * changed + * @param[in] gateway The new gateway value. Assigning an empty string + * causes the gateway to be deleted + * @param[io] asyncResp Response object that will be returned to client + * + * @return None + */ +inline void updateIPv4DefaultGateway( + const std::string& ifaceId, const std::string& gateway, + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) +{ + setDbusProperty( + asyncResp, "xyz.openbmc_project.Network", + sdbusplus::message::object_path("/xyz/openbmc_project/network") / + ifaceId, + "xyz.openbmc_project.Network.EthernetInterface", "DefaultGateway", + "Gateway", gateway); +} + +/** + * @brief Deletes given static IP address for the interface * * @param[in] ifaceId Id of interface whose IP should be deleted * @param[in] ipHash DBus Hash id of IP that should be deleted @@ -724,17 +753,6 @@ inline void deleteIPAddress(const std::string& ifaceId, "xyz.openbmc_project.Object.Delete", "Delete"); } -inline void updateIPv4DefaultGateway( - const std::string& ifaceId, const std::string& gateway, - const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) -{ - setDbusProperty( - asyncResp, "xyz.openbmc_project.Network", - sdbusplus::message::object_path("/xyz/openbmc_project/network") / - ifaceId, - "xyz.openbmc_project.Network.EthernetInterface", "DefaultGateway", - "Gateway", gateway); -} /** * @brief Creates a static IPv4 entry * @@ -757,7 +775,6 @@ inline void createIPv4(const std::string& ifaceId, uint8_t prefixLength, messages::internalError(asyncResp->res); return; } - updateIPv4DefaultGateway(ifaceId, gateway, asyncResp); }; crow::connections::systemBus->async_method_call( @@ -769,24 +786,19 @@ inline void createIPv4(const std::string& ifaceId, uint8_t prefixLength, } /** - * @brief Deletes the IPv6 entry for this interface and creates a replacement - * static IPv6 entry + * @brief Deletes the IP entry for this interface and creates a replacement + * static entry * - * @param[in] ifaceId Id of interface upon which to create the IPv6 entry - * @param[in] id The unique hash entry identifying the DBus entry - * @param[in] prefixLength IPv6 prefix syntax for the subnet mask - * @param[in] address IPv6 address to assign to this interface - * @param[io] asyncResp Response object that will be returned to client + * @param[in] ifaceId Id of interface upon which to create the IPv6 entry + * @param[in] id The unique hash entry identifying the DBus entry + * @param[in] prefixLength Prefix syntax for the subnet mask + * @param[in] address Address to assign to this interface + * @param[in] numStaticAddrs Count of IPv4 static addresses + * @param[io] asyncResp Response object that will be returned to client * * @return None */ -enum class IpVersion -{ - IpV4, - IpV6 -}; - inline void deleteAndCreateIPAddress( IpVersion version, const std::string& ifaceId, const std::string& id, uint8_t prefixLength, const std::string& address, @@ -1391,6 +1403,10 @@ inline void handleDHCPPatch(const std::string& ifaceId, bool ipv4Active = translateDhcpEnabledToBool(ethData.dhcpEnabled, true); bool ipv6Active = translateDhcpEnabledToBool(ethData.dhcpEnabled, false); + if (ipv4Active) + { + updateIPv4DefaultGateway(ifaceId, "", asyncResp); + } bool nextv4DHCPState = v4dhcpParms.dhcpv4Enabled ? *v4dhcpParms.dhcpv4Enabled : ipv4Active; @@ -1489,6 +1505,7 @@ inline std::vector<IPv6AddressData>::const_iterator getNextStaticIpEntry( inline void handleIPv4StaticPatch( const std::string& ifaceId, std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>& input, + const EthernetInterfaceData& ethData, const std::vector<IPv4AddressData>& ipv4Data, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) { @@ -1500,6 +1517,19 @@ inline void handleIPv4StaticPatch( std::vector<IPv4AddressData>::const_iterator nicIpEntry = getNextStaticIpEntry(ipv4Data.cbegin(), ipv4Data.cend()); + bool gatewayValueAssigned{}; + std::string activePath{}; + std::string activeGateway{}; + if (!ethData.defaultGateway.empty() && ethData.defaultGateway != "0.0.0.0") + { + // The NIC is already configured with a default gateway. Use this if + // the leading entry in the PATCH is '{}', which is preserving an active + // static address. + activeGateway = ethData.defaultGateway; + activePath = "IPv4StaticAddresses/1"; + gatewayValueAssigned = true; + } + for (std::variant<nlohmann::json::object_t, std::nullptr_t>& thisJson : input) { @@ -1507,7 +1537,32 @@ inline void handleIPv4StaticPatch( std::to_string(entryIdx); nlohmann::json::object_t* obj = std::get_if<nlohmann::json::object_t>(&thisJson); - if (obj != nullptr && !obj->empty()) + if (obj == nullptr) + { + if (nicIpEntry != ipv4Data.cend()) + { + deleteIPAddress(ifaceId, nicIpEntry->id, asyncResp); + nicIpEntry = getNextStaticIpEntry(++nicIpEntry, + ipv4Data.cend()); + if (!gatewayValueAssigned && (nicIpEntry == ipv4Data.cend())) + { + // All entries have been processed, and this last has + // requested the IP address be deleted. No prior entry + // performed an action that created or modified a + // gateway. Deleting this IP address means the default + // gateway entry has to be removed as well. + updateIPv4DefaultGateway(ifaceId, "", asyncResp); + } + entryIdx++; + continue; + } + // Received a DELETE action on an entry not assigned to the NIC + messages::resourceCannotBeDeleted(asyncResp->res); + return; + } + + // An Add/Modify action is requested + if (!obj->empty()) { std::optional<std::string> address; std::optional<std::string> subnetMask; @@ -1596,6 +1651,29 @@ inline void handleIPv4StaticPatch( return; } + if (gatewayValueAssigned) + { + if (activeGateway != gateway) + { + // A NIC can only have a single active gateway value. + // If any gateway in the array of static addresses + // mismatch the PATCH is in error. + std::string arg1 = pathString + "/Gateway"; + std::string arg2 = activePath + "/Gateway"; + messages::propertyValueConflict(asyncResp->res, arg1, arg2); + return; + } + } + else + { + // Capture the very first gateway value from the incoming + // JSON record and use it at the default gateway. + updateIPv4DefaultGateway(ifaceId, *gateway, asyncResp); + activeGateway = *gateway; + activePath = pathString; + gatewayValueAssigned = true; + } + if (nicIpEntry != ipv4Data.cend()) { deleteAndCreateIPAddress(IpVersion::IpV4, ifaceId, @@ -1613,31 +1691,21 @@ inline void handleIPv4StaticPatch( } else { - if (nicIpEntry == ipv4Data.cend()) - { - // Requesting a DELETE/DO NOT MODIFY action for an item - // that isn't present on the eth(n) interface. Input JSON is - // in error, so bail out. - if (obj == nullptr) - { - messages::resourceCannotBeDeleted(asyncResp->res); - return; - } - messages::propertyValueFormatError(asyncResp->res, *obj, - pathString); - return; - } - - if (obj == nullptr) - { - deleteIPAddress(ifaceId, nicIpEntry->id, asyncResp); - } + // Received {}, do not modify this address if (nicIpEntry != ipv4Data.cend()) { nicIpEntry = getNextStaticIpEntry(++nicIpEntry, ipv4Data.cend()); + entryIdx++; + } + else + { + // Requested a DO NOT MODIFY action on an entry not assigned + // to the NIC + messages::propertyValueFormatError(asyncResp->res, *obj, + pathString); + return; } - entryIdx++; } } } @@ -2268,8 +2336,8 @@ inline void requestEthernetInterfacesRoutes(App& app) if (ipv4StaticAddresses) { - handleIPv4StaticPatch(ifaceId, *ipv4StaticAddresses, ipv4Data, - asyncResp); + handleIPv4StaticPatch(ifaceId, *ipv4StaticAddresses, ethData, + ipv4Data, asyncResp); } if (staticNameServers) diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp index 1453a4b11f..058852c62c 100644 --- a/redfish-core/lib/log_services.hpp +++ b/redfish-core/lib/log_services.hpp @@ -1313,7 +1313,7 @@ inline void requestRoutesSystemLogServiceCollection(App& app) logServiceArray.emplace_back(std::move(dumpLog)); } - if constexpr (BMCWEB_REDFISH_DUMP_LOG) + if constexpr (BMCWEB_REDFISH_CPU_LOG) { nlohmann::json::object_t crashdump; crashdump["@odata.id"] = diff --git a/redfish-core/lib/update_service.hpp b/redfish-core/lib/update_service.hpp index 6895cb4949..81534f94de 100644 --- a/redfish-core/lib/update_service.hpp +++ b/redfish-core/lib/update_service.hpp @@ -40,9 +40,12 @@ #include <array> #include <filesystem> +#include <functional> +#include <memory> #include <optional> #include <string> #include <string_view> +#include <vector> namespace redfish { @@ -83,6 +86,121 @@ inline void activateImage(const std::string& objPath, }); } +inline bool handleCreateTask(const boost::system::error_code& ec2, + sdbusplus::message_t& msg, + const std::shared_ptr<task::TaskData>& taskData) +{ + if (ec2) + { + return task::completed; + } + + std::string iface; + dbus::utility::DBusPropertiesMap values; + + std::string index = std::to_string(taskData->index); + msg.read(iface, values); + + if (iface == "xyz.openbmc_project.Software.Activation") + { + const std::string* state = nullptr; + for (const auto& property : values) + { + if (property.first == "Activation") + { + state = std::get_if<std::string>(&property.second); + if (state == nullptr) + { + taskData->messages.emplace_back(messages::internalError()); + return task::completed; + } + } + } + + if (state == nullptr) + { + return !task::completed; + } + + if (state->ends_with("Invalid") || state->ends_with("Failed")) + { + taskData->state = "Exception"; + taskData->status = "Warning"; + taskData->messages.emplace_back(messages::taskAborted(index)); + return task::completed; + } + + if (state->ends_with("Staged")) + { + taskData->state = "Stopping"; + taskData->messages.emplace_back(messages::taskPaused(index)); + + // its staged, set a long timer to + // allow them time to complete the + // update (probably cycle the + // system) if this expires then + // task will be canceled + taskData->extendTimer(std::chrono::hours(5)); + return !task::completed; + } + + if (state->ends_with("Active")) + { + taskData->messages.emplace_back(messages::taskCompletedOK(index)); + taskData->state = "Completed"; + return task::completed; + } + } + else if (iface == "xyz.openbmc_project.Software.ActivationProgress") + { + const uint8_t* progress = nullptr; + for (const auto& property : values) + { + if (property.first == "Progress") + { + progress = std::get_if<uint8_t>(&property.second); + if (progress == nullptr) + { + taskData->messages.emplace_back(messages::internalError()); + return task::completed; + } + } + } + + if (progress == nullptr) + { + return !task::completed; + } + taskData->percentComplete = *progress; + taskData->messages.emplace_back( + messages::taskProgressChanged(index, *progress)); + + // if we're getting status updates it's + // still alive, update timer + taskData->extendTimer(std::chrono::minutes(5)); + } + + // as firmware update often results in a + // reboot, the task may never "complete" + // unless it is an error + + return !task::completed; +} + +inline void createTask(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + task::Payload&& payload, + const sdbusplus::message::object_path& objPath) +{ + std::shared_ptr<task::TaskData> task = task::TaskData::createTask( + std::bind_front(handleCreateTask), + "type='signal',interface='org.freedesktop.DBus.Properties'," + "member='PropertiesChanged',path='" + + objPath.str + "'"); + task->startTimer(std::chrono::minutes(5)); + task->populateResp(asyncResp->res); + task->payload.emplace(std::move(payload)); +} + // Note that asyncResp can be either a valid pointer or nullptr. If nullptr // then no asyncResp updates will occur static void @@ -142,125 +260,7 @@ static void activateImage(objPath.str, objInfo[0].first); if (asyncResp) { - std::shared_ptr<task::TaskData> task = - task::TaskData::createTask( - [](const boost::system::error_code& ec2, - sdbusplus::message_t& msg, - const std::shared_ptr<task::TaskData>& - taskData) { - if (ec2) - { - return task::completed; - } - - std::string iface; - dbus::utility::DBusPropertiesMap values; - - std::string index = std::to_string(taskData->index); - msg.read(iface, values); - - if (iface == "xyz.openbmc_project.Software.Activation") - { - const std::string* state = nullptr; - for (const auto& property : values) - { - if (property.first == "Activation") - { - state = std::get_if<std::string>( - &property.second); - if (state == nullptr) - { - taskData->messages.emplace_back( - messages::internalError()); - return task::completed; - } - } - } - - if (state == nullptr) - { - return !task::completed; - } - - if (state->ends_with("Invalid") || - state->ends_with("Failed")) - { - taskData->state = "Exception"; - taskData->status = "Warning"; - taskData->messages.emplace_back( - messages::taskAborted(index)); - return task::completed; - } - - if (state->ends_with("Staged")) - { - taskData->state = "Stopping"; - taskData->messages.emplace_back( - messages::taskPaused(index)); - - // its staged, set a long timer to - // allow them time to complete the - // update (probably cycle the - // system) if this expires then - // task will be canceled - taskData->extendTimer(std::chrono::hours(5)); - return !task::completed; - } - - if (state->ends_with("Active")) - { - taskData->messages.emplace_back( - messages::taskCompletedOK(index)); - taskData->state = "Completed"; - return task::completed; - } - } - else if ( - iface == - "xyz.openbmc_project.Software.ActivationProgress") - { - const uint8_t* progress = nullptr; - for (const auto& property : values) - { - if (property.first == "Progress") - { - progress = - std::get_if<uint8_t>(&property.second); - if (progress == nullptr) - { - taskData->messages.emplace_back( - messages::internalError()); - return task::completed; - } - } - } - - if (progress == nullptr) - { - return !task::completed; - } - taskData->percentComplete = *progress; - taskData->messages.emplace_back( - messages::taskProgressChanged(index, - *progress)); - - // if we're getting status updates it's - // still alive, update timer - taskData->extendTimer(std::chrono::minutes(5)); - } - - // as firmware update often results in a - // reboot, the task may never "complete" - // unless it is an error - - return !task::completed; - }, - "type='signal',interface='org.freedesktop.DBus.Properties'," - "member='PropertiesChanged',path='" + - objPath.str + "'"); - task->startTimer(std::chrono::minutes(5)); - task->populateResp(asyncResp->res); - task->payload.emplace(std::move(payload)); + createTask(asyncResp, std::move(payload), objPath); } fwUpdateInProgress = false; }); @@ -464,11 +464,15 @@ inline std::optional<boost::urls::url> res, imageURI, "ImageURI", "UpdateService.SimpleUpdate"); return std::nullopt; } - // OpenBMC currently only supports TFTP + // OpenBMC currently only supports TFTP or HTTPS if (*transferProtocol == "TFTP") { imageURI = "tftp://" + imageURI; } + else if (*transferProtocol == "HTTPS") + { + imageURI = "https://" + imageURI; + } else { messages::actionParameterNotSupported(res, "TransferProtocol", @@ -499,6 +503,14 @@ inline std::optional<boost::urls::url> return std::nullopt; } } + else if (url->scheme() == "https") + { + // Empty paths default to "/" + if (url->encoded_path().empty()) + { + url->set_encoded_path("/"); + } + } else { messages::actionParameterNotSupported(res, "ImageURI", imageURI); @@ -515,15 +527,23 @@ inline std::optional<boost::urls::url> return *url; } +inline void doHttpsUpdate(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + const boost::urls::url_view_base& url) +{ + messages::actionParameterNotSupported(asyncResp->res, "ImageURI", + url.buffer()); +} + inline void doTftpUpdate(const crow::Request& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, const boost::urls::url_view_base& url) { -#ifndef BMCWEB_INSECURE_ENABLE_REDFISH_FW_TFTP_UPDATE - messages::actionParameterNotSupported(asyncResp->res, "ImageURI", - url.buffer()); - return; -#endif + if (!BMCWEB_INSECURE_TFTP_UPDATE) + { + messages::actionParameterNotSupported(asyncResp->res, "ImageURI", + url.buffer()); + return; + } std::string path(url.encoded_path()); if (path.size() < 2) @@ -606,6 +626,10 @@ inline void handleUpdateServiceSimpleUpdateAction( { doTftpUpdate(req, asyncResp, *url); } + else if (url->scheme() == "https") + { + doHttpsUpdate(asyncResp, *url); + } else { messages::actionParameterNotSupported(asyncResp->res, "ImageURI", @@ -666,22 +690,27 @@ inline void setApplyTime(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, "RequestedApplyTime", "ApplyTime", applyTimeNewVal); } -inline void - updateMultipartContext(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, - const crow::Request& req, - const MultipartParser& parser) +struct MultiPartUpdateParameters +{ + std::optional<std::string> applyTime; + std::string uploadData; + std::vector<boost::urls::url> targets; +}; + +inline std::optional<MultiPartUpdateParameters> + extractMultipartUpdateParameters( + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + MultipartParser parser) { - const std::string* uploadData = nullptr; - std::optional<std::string> applyTime = "OnReset"; - bool targetFound = false; - for (const FormPart& formpart : parser.mime_fields) + MultiPartUpdateParameters multiRet; + for (FormPart& formpart : parser.mime_fields) { boost::beast::http::fields::const_iterator it = formpart.fields.find("Content-Disposition"); if (it == formpart.fields.end()) { BMCWEB_LOG_ERROR("Couldn't find Content-Disposition"); - return; + return std::nullopt; } BMCWEB_LOG_INFO("Parsing value {}", it->value()); @@ -702,63 +731,95 @@ inline void if (param.second == "UpdateParameters") { - std::vector<std::string> targets; + std::vector<std::string> tempTargets; nlohmann::json content = nlohmann::json::parse(formpart.content); nlohmann::json::object_t* obj = content.get_ptr<nlohmann::json::object_t*>(); if (obj == nullptr) { - messages::propertyValueFormatError(asyncResp->res, targets, - "UpdateParameters"); - return; + messages::propertyValueTypeError( + asyncResp->res, formpart.content, "UpdateParameters"); + return std::nullopt; } if (!json_util::readJsonObject( - *obj, asyncResp->res, "Targets", targets, - "@Redfish.OperationApplyTime", applyTime)) + *obj, asyncResp->res, "Targets", tempTargets, + "@Redfish.OperationApplyTime", multiRet.applyTime)) { - return; + return std::nullopt; } - if (targets.size() != 1) + + for (size_t urlIndex = 0; urlIndex < tempTargets.size(); + urlIndex++) { - messages::propertyValueFormatError(asyncResp->res, targets, - "Targets"); - return; + const std::string& target = tempTargets[urlIndex]; + boost::system::result<boost::urls::url_view> url = + boost::urls::parse_origin_form(target); + if (!url) + { + messages::propertyValueFormatError( + asyncResp->res, target, + std::format("Targets/{}", urlIndex)); + return std::nullopt; + } + multiRet.targets.emplace_back(*url); } - if (targets[0] != "/redfish/v1/Managers/bmc") + if (multiRet.targets.size() != 1) { - messages::propertyValueNotInList(asyncResp->res, targets[0], - "Targets/0"); - return; + messages::propertyValueFormatError( + asyncResp->res, multiRet.targets, "Targets"); + return std::nullopt; + } + if (multiRet.targets[0].path() != "/redfish/v1/Managers/bmc") + { + messages::propertyValueNotInList( + asyncResp->res, multiRet.targets[0], "Targets/0"); + return std::nullopt; } - targetFound = true; } else if (param.second == "UpdateFile") { - uploadData = &(formpart.content); + multiRet.uploadData = std::move(formpart.content); } } } - if (uploadData == nullptr) + if (multiRet.uploadData.empty()) { BMCWEB_LOG_ERROR("Upload data is NULL"); messages::propertyMissing(asyncResp->res, "UpdateFile"); - return; + return std::nullopt; } - if (!targetFound) + if (multiRet.targets.empty()) + { + messages::propertyMissing(asyncResp->res, "Targets"); + return std::nullopt; + } + return multiRet; +} + +inline void + updateMultipartContext(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + const crow::Request& req, MultipartParser&& parser) +{ + std::optional<MultiPartUpdateParameters> multipart = + extractMultipartUpdateParameters(asyncResp, std::move(parser)); + if (!multipart) { - messages::propertyMissing(asyncResp->res, "targets"); return; } + if (!multipart->applyTime) + { + multipart->applyTime = "OnReset"; + } - setApplyTime(asyncResp, *applyTime); + setApplyTime(asyncResp, *multipart->applyTime); // Setup callback for when new software detected monitorForSoftwareAvailable(asyncResp, req, "/redfish/v1/UpdateService"); - uploadImageFile(asyncResp->res, *uploadData); + uploadImageFile(asyncResp->res, multipart->uploadData); } inline void @@ -797,7 +858,7 @@ inline void return; } - updateMultipartContext(asyncResp, req, parser); + updateMultipartContext(asyncResp, req, std::move(parser)); } else { @@ -841,6 +902,7 @@ inline void "/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate"; nlohmann::json::array_t allowed; + allowed.emplace_back(update_service::TransferProtocolType::HTTPS); if constexpr (BMCWEB_INSECURE_PUSH_STYLE_NOTIFICATION) { diff --git a/redfish-core/src/redfish.cpp b/redfish-core/src/redfish.cpp index e7a58c1f41..47d6fc2d07 100644 --- a/redfish-core/src/redfish.cpp +++ b/redfish-core/src/redfish.cpp @@ -130,7 +130,7 @@ RedfishService::RedfishService(App& app) requestRoutesFaultLogDumpClear(app); } - if constexpr (BMCWEB_REDFISH_DBUS_LOG) + if constexpr (!BMCWEB_REDFISH_DBUS_LOG) { requestRoutesJournalEventLogEntryCollection(app); requestRoutesJournalEventLogEntry(app); diff --git a/test/http/http2_connection_test.cpp b/test/http/http2_connection_test.cpp index 34dc7cd20e..0385ed62b9 100644 --- a/test/http/http2_connection_test.cpp +++ b/test/http/http2_connection_test.cpp @@ -38,16 +38,17 @@ using ::testing::UnorderedElementsAre; struct FakeHandler { bool called = false; - void handle(Request& req, + void handle(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp) { called = true; - EXPECT_EQ(req.url().buffer(), "/redfish/v1/"); - EXPECT_EQ(req.methodString(), "GET"); - EXPECT_EQ(req.getHeaderValue(boost::beast::http::field::user_agent), + EXPECT_EQ(req->url().buffer(), "/redfish/v1/"); + EXPECT_EQ(req->methodString(), "GET"); + EXPECT_EQ(req->getHeaderValue(boost::beast::http::field::user_agent), "curl/8.5.0"); - EXPECT_EQ(req.getHeaderValue(boost::beast::http::field::accept), "*/*"); - EXPECT_EQ(req.getHeaderValue(":authority"), "localhost:18080"); + EXPECT_EQ(req->getHeaderValue(boost::beast::http::field::accept), + "*/*"); + EXPECT_EQ(req->getHeaderValue(":authority"), "localhost:18080"); asyncResp->res.write("StringOutput"); } }; diff --git a/test/http/http_connection_test.cpp b/test/http/http_connection_test.cpp index caf50c8a62..393e209b8a 100644 --- a/test/http/http_connection_test.cpp +++ b/test/http/http_connection_test.cpp @@ -23,7 +23,7 @@ namespace crow struct FakeHandler { static void - handleUpgrade(Request& /*req*/, + handleUpgrade(const std::shared_ptr<Request>& /*req*/, const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/, boost::beast::test::stream&& /*adaptor*/) { @@ -31,16 +31,16 @@ struct FakeHandler EXPECT_FALSE(true); } - void handle(Request& req, + void handle(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/) { - EXPECT_EQ(req.method(), boost::beast::http::verb::get); - EXPECT_EQ(req.target(), "/"); - EXPECT_EQ(req.getHeaderValue(boost::beast::http::field::host), + EXPECT_EQ(req->method(), boost::beast::http::verb::get); + EXPECT_EQ(req->target(), "/"); + EXPECT_EQ(req->getHeaderValue(boost::beast::http::field::host), "openbmc_project.xyz"); - EXPECT_FALSE(req.keepAlive()); - EXPECT_EQ(req.version(), 11); - EXPECT_EQ(req.body(), ""); + EXPECT_FALSE(req->keepAlive()); + EXPECT_EQ(req->version(), 11); + EXPECT_EQ(req->body(), ""); called = true; } diff --git a/test/http/router_test.cpp b/test/http/router_test.cpp index f23331782b..23725416af 100644 --- a/test/http/router_test.cpp +++ b/test/http/router_test.cpp @@ -87,7 +87,8 @@ TEST(Router, OverlapingRoutes) { constexpr std::string_view url = "/foo/bar"; - Request req{{boost::beast::http::verb::get, url, 11}, ec}; + auto req = std::make_shared<Request>( + Request::Body{boost::beast::http::verb::get, url, 11}, ec); std::shared_ptr<bmcweb::AsyncResp> asyncResp = std::make_shared<bmcweb::AsyncResp>(); @@ -112,7 +113,8 @@ TEST(Router, 404) constexpr std::string_view url = "/foo/bar"; - Request req{{boost::beast::http::verb::get, url, 11}, ec}; + auto req = std::make_shared<Request>( + Request::Body{boost::beast::http::verb::get, url, 11}, ec); router.newRuleTagged<getParameterTag(url)>("/foo/<path>") .notFound()(nullCallback); @@ -142,7 +144,8 @@ TEST(Router, 405) constexpr std::string_view url = "/foo/bar"; - Request req{{boost::beast::http::verb::patch, url, 11}, ec}; + auto req = std::make_shared<Request>( + Request::Body{boost::beast::http::verb::patch, url, 11}, ec); router.newRuleTagged<getParameterTag(url)>(std::string(url)) .methods(boost::beast::http::verb::get)(nullCallback); diff --git a/test/redfish-core/lib/update_service_test.cpp b/test/redfish-core/lib/update_service_test.cpp index 87af3f116a..11b4479a3d 100644 --- a/test/redfish-core/lib/update_service_test.cpp +++ b/test/redfish-core/lib/update_service_test.cpp @@ -43,7 +43,7 @@ TEST(UpdateService, ParseTFTPPostitive) EXPECT_EQ(ret->scheme(), "tftp"); } { - // Both protocl and schema on url + // Both protocol and schema on url std::optional<boost::urls::url> ret = parseSimpleUpdateUrl("tftp://1.1.1.1/path", "TFTP", res); ASSERT_TRUE(ret); @@ -57,6 +57,63 @@ TEST(UpdateService, ParseTFTPPostitive) } } +TEST(UpdateService, ParseHTTPSPostitive) +{ + crow::Response res; + { + // No protocol, schema on url + std::optional<boost::urls::url> ret = + parseSimpleUpdateUrl("https://1.1.1.1/path", std::nullopt, res); + ASSERT_TRUE(ret); + if (!ret) + { + return; + } + EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1"); + EXPECT_EQ(ret->encoded_path(), "/path"); + EXPECT_EQ(ret->scheme(), "https"); + } + { + // Protocol, no schema on url + std::optional<boost::urls::url> ret = + parseSimpleUpdateUrl("1.1.1.1/path", "HTTPS", res); + ASSERT_TRUE(ret); + if (!ret) + { + return; + } + EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1"); + EXPECT_EQ(ret->encoded_path(), "/path"); + EXPECT_EQ(ret->scheme(), "https"); + } + { + // Both protocol and schema on url with path + std::optional<boost::urls::url> ret = + parseSimpleUpdateUrl("https://1.1.1.1/path", "HTTPS", res); + ASSERT_TRUE(ret); + if (!ret) + { + return; + } + EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1"); + EXPECT_EQ(ret->encoded_path(), "/path"); + EXPECT_EQ(ret->scheme(), "https"); + } + { + // Both protocol and schema on url without path + std::optional<boost::urls::url> ret = + parseSimpleUpdateUrl("https://1.1.1.1", "HTTPS", res); + ASSERT_TRUE(ret); + if (!ret) + { + return; + } + EXPECT_EQ(ret->encoded_host_and_port(), "1.1.1.1"); + EXPECT_EQ(ret->encoded_path(), "/"); + EXPECT_EQ(ret->scheme(), "https"); + } +} + TEST(UpdateService, ParseTFTPNegative) { crow::Response res; |