diff options
| -rw-r--r-- | http/http_connection.hpp | 115 | ||||
| -rw-r--r-- | http/http_request.hpp | 8 | ||||
| -rw-r--r-- | include/authorization.hpp | 130 | ||||
| -rw-r--r-- | include/forward_unauthorized.hpp | 9 | ||||
| -rw-r--r-- | include/http_utility.hpp | 3 |
5 files changed, 144 insertions, 121 deletions
diff --git a/http/http_connection.hpp b/http/http_connection.hpp index a1a7045c03..a4723f121a 100644 --- a/http/http_connection.hpp +++ b/http/http_connection.hpp @@ -69,7 +69,6 @@ class Connection : parser.emplace(std::piecewise_construct, std::make_tuple()); parser->body_limit(httpReqBodyLimit); parser->header_limit(httpHeaderLimit); - req.emplace(parser->get()); #ifdef BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION prepareMutualTls(); @@ -263,15 +262,16 @@ class Connection : } sslUser.resize(lastChar); std::string unsupportedClientId = ""; - session = persistent_data::SessionStore::getInstance() - .generateUserSession( - sslUser, req->ipAddress.to_string(), - unsupportedClientId, - persistent_data::PersistenceType::TIMEOUT); - if (auto sp = session.lock()) + userSession = persistent_data::SessionStore::getInstance() + .generateUserSession( + sslUser, req->ipAddress.to_string(), + unsupportedClientId, + persistent_data::PersistenceType::TIMEOUT); + if (userSession != nullptr) { - BMCWEB_LOG_DEBUG << this - << " Generating TLS session: " << sp->uniqueId; + BMCWEB_LOG_DEBUG + << this + << " Generating TLS session: " << userSession->uniqueId; } return true; }); @@ -319,9 +319,9 @@ class Connection : bool isInvalidRequest = false; // Check for HTTP version 1.1. - if (req->version() == 11) + if (parser->get().version() == 11) { - if (req->getHeaderValue(boost::beast::http::field::host).empty()) + if (parser->get()[boost::beast::http::field::host].empty()) { isInvalidRequest = true; res.result(boost::beast::http::status::bad_request); @@ -329,9 +329,23 @@ class Connection : } BMCWEB_LOG_INFO << "Request: " - << " " << this << " HTTP/" << req->version() / 10 << "." - << req->version() % 10 << ' ' << req->methodString() - << " " << req->target() << " " << req->ipAddress; + << " " << this << " HTTP/" + << parser->get().version() / 10 << "." + << parser->get().version() % 10 << ' ' + << parser->get().method_string() << " " + << parser->get().target() << " " << req->ipAddress; + req.emplace(parser->release()); + req->session = userSession; + try + { + // causes life time issue + req->urlView = boost::urls::url_view(req->target()); + req->url = req->urlView.encoded_path(); + } + catch (std::exception& p) + { + BMCWEB_LOG_ERROR << p.what(); + } needToCallAfterHandlers = false; @@ -397,11 +411,13 @@ class Connection : { adaptor.next_layer().close(); #ifdef BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION - if (auto sp = session.lock()) + if (userSession != nullptr) { - BMCWEB_LOG_DEBUG << this - << " Removing TLS session: " << sp->uniqueId; - persistent_data::SessionStore::getInstance().removeSession(sp); + BMCWEB_LOG_DEBUG + << this + << " Removing TLS session: " << userSession->uniqueId; + persistent_data::SessionStore::getInstance().removeSession( + userSession); } #endif // BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION } @@ -437,7 +453,7 @@ class Connection : } if (res.body().empty() && !res.jsonValue.empty()) { - if (http_helpers::requestPrefersHtml(*req)) + if (http_helpers::requestPrefersHtml(req->getHeaderValue("Accept"))) { prettyPrintJson(res); } @@ -477,6 +493,17 @@ class Connection : void readClientIp() { + boost::asio::ip::address ip; + boost::system::error_code ec = getClientIp(ip); + if (ec) + { + return; + } + req->ipAddress = ip; + } + + boost::system::error_code getClientIp(boost::asio::ip::address& ip) + { boost::system::error_code ec; BMCWEB_LOG_DEBUG << "Fetch the client IP address"; boost::asio::ip::tcp::endpoint endpoint = @@ -488,11 +515,10 @@ class Connection : // will be empty. BMCWEB_LOG_ERROR << "Failed to get the client's IP Address. ec : " << ec; + return ec; } - else - { - req->ipAddress = endpoint.address(); - } + ip = endpoint.address(); + return ec; } private: @@ -519,7 +545,8 @@ class Connection : { // if the adaptor isn't open anymore, and wasn't handed to a // websocket, treat as an error - if (!isAlive() && !req->isUpgrade()) + if (!isAlive() && + !boost::beast::websocket::is_upgrade(parser->get())) { errorWhileReading = true; } @@ -534,24 +561,12 @@ class Connection : return; } - if (!req) - { - close(); - return; - } - - // Note, despite the bmcweb coding policy on use of exceptions - // for error handling, this one particular use of exceptions is - // deemed acceptible, as it solved a significant error handling - // problem that resulted in seg faults, the exact thing that the - // exceptions rule is trying to avoid. If at some point, - // boost::urls makes the parser object public (or we port it - // into bmcweb locally) this will be replaced with - // parser::parse, which returns a status code - + boost::beast::http::verb method = parser->get().method(); + readClientIp(); try { - req->urlView = boost::urls::url_view(req->target()); + req->urlView = + boost::urls::url_view(parser->get().target()); req->url = req->urlView.encoded_path(); } catch (std::exception& p) @@ -559,9 +574,15 @@ class Connection : BMCWEB_LOG_ERROR << p.what(); } - crow::authorization::authenticate(*req, res, session); - - bool loggedIn = req && req->session; + boost::asio::ip::address ip; + if (getClientIp(ip)) + { + BMCWEB_LOG_DEBUG << "Unable to get client IP"; + } + userSession = crow::authorization::authenticate( + req->url, ip, res, method, parser->get().base(), + userSession); + bool loggedIn = userSession != nullptr; if (loggedIn) { startDeadline(loggedInAttempts); @@ -622,8 +643,7 @@ class Connection : if (isAlive()) { cancelDeadlineTimer(); - bool loggedIn = req && req->session; - if (loggedIn) + if (userSession != nullptr) { startDeadline(loggedInAttempts); } @@ -692,7 +712,7 @@ class Connection : // newly created parser buffer.consume(buffer.size()); - req.emplace(parser->get()); + req.emplace(parser->release()); doReadHeaders(); }); } @@ -762,7 +782,6 @@ class Connection : private: Adaptor adaptor; Handler* handler; - // Making this a std::optional allows it to be efficiently destroyed and // re-created on Connection reset std::optional< @@ -778,7 +797,7 @@ class Connection : std::optional<crow::Request> req; crow::Response res; - std::weak_ptr<persistent_data::UserSession> session; + std::shared_ptr<persistent_data::UserSession> userSession; std::optional<size_t> timerCancelKey; diff --git a/http/http_request.hpp b/http/http_request.hpp index 44ed2e30f5..fecb9de3fb 100644 --- a/http/http_request.hpp +++ b/http/http_request.hpp @@ -18,7 +18,7 @@ namespace crow struct Request { - boost::beast::http::request<boost::beast::http::string_body>& req; + boost::beast::http::request<boost::beast::http::string_body> req; boost::beast::http::fields& fields; std::string_view url{}; boost::urls::url_view urlView{}; @@ -34,9 +34,9 @@ struct Request std::string userRole{}; Request( - boost::beast::http::request<boost::beast::http::string_body>& reqIn) : - req(reqIn), - fields(reqIn.base()), body(reqIn.body()) + boost::beast::http::request<boost::beast::http::string_body> reqIn) : + req(std::move(reqIn)), + fields(req.base()), body(req.body()) {} boost::beast::http::verb method() const diff --git a/include/authorization.hpp b/include/authorization.hpp index b1d1097068..ecbdca04c0 100644 --- a/include/authorization.hpp +++ b/include/authorization.hpp @@ -93,36 +93,37 @@ static std::shared_ptr<persistent_data::UserSession> BMCWEB_LOG_DEBUG << "[AuthMiddleware] Token authentication"; std::string_view token = authHeader.substr(strlen("Token ")); - auto session = + auto sessionOut = persistent_data::SessionStore::getInstance().loginSessionByToken(token); - return session; + return sessionOut; } #endif #ifdef BMCWEB_ENABLE_XTOKEN_AUTHENTICATION static std::shared_ptr<persistent_data::UserSession> - performXtokenAuth(const crow::Request& req) + performXtokenAuth(const boost::beast::http::header<true>& reqHeader) { BMCWEB_LOG_DEBUG << "[AuthMiddleware] X-Auth-Token authentication"; - std::string_view token = req.getHeaderValue("X-Auth-Token"); + std::string_view token = reqHeader["X-Auth-Token"]; if (token.empty()) { return nullptr; } - auto session = + auto sessionOut = persistent_data::SessionStore::getInstance().loginSessionByToken(token); - return session; + return sessionOut; } #endif #ifdef BMCWEB_ENABLE_COOKIE_AUTHENTICATION static std::shared_ptr<persistent_data::UserSession> - performCookieAuth(const crow::Request& req) + performCookieAuth(boost::beast::http::verb method, + const boost::beast::http::header<true>& reqHeader) { BMCWEB_LOG_DEBUG << "[AuthMiddleware] Cookie authentication"; - std::string_view cookieValue = req.getHeaderValue("Cookie"); + std::string_view cookieValue = reqHeader["Cookie"]; if (cookieValue.empty()) { return nullptr; @@ -142,20 +143,20 @@ static std::shared_ptr<persistent_data::UserSession> std::string_view authKey = cookieValue.substr(startIndex, endIndex - startIndex); - std::shared_ptr<persistent_data::UserSession> session = + std::shared_ptr<persistent_data::UserSession> sessionOut = persistent_data::SessionStore::getInstance().loginSessionByToken( authKey); - if (session == nullptr) + if (sessionOut == nullptr) { return nullptr; } #ifndef BMCWEB_INSECURE_DISABLE_CSRF_PREVENTION // RFC7231 defines methods that need csrf protection - if (req.method() != boost::beast::http::verb::get) + if (method != boost::beast::http::verb::get) { - std::string_view csrf = req.getHeaderValue("X-XSRF-TOKEN"); + std::string_view csrf = reqHeader["X-XSRF-TOKEN"]; // Make sure both tokens are filled - if (csrf.empty() || session->csrfToken.empty()) + if (csrf.empty() || sessionOut->csrfToken.empty()) { return nullptr; } @@ -165,31 +166,33 @@ static std::shared_ptr<persistent_data::UserSession> return nullptr; } // Reject if csrf token not available - if (!crow::utility::constantTimeStringCompare(csrf, session->csrfToken)) + if (!crow::utility::constantTimeStringCompare(csrf, + sessionOut->csrfToken)) { return nullptr; } } #endif - return session; + return sessionOut; } #endif #ifdef BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION static std::shared_ptr<persistent_data::UserSession> - performTLSAuth(const crow::Request& req, Response& res, + performTLSAuth(Response& res, + const boost::beast::http::header<true>& reqHeader, const std::weak_ptr<persistent_data::UserSession>& session) { if (auto sp = session.lock()) { // set cookie only if this is req from the browser. - if (req.getHeaderValue("User-Agent").empty()) + if (reqHeader["User-Agent"].empty()) { BMCWEB_LOG_DEBUG << " TLS session: " << sp->uniqueId << " will be used for this request."; return sp; } - std::string_view cookieValue = req.getHeaderValue("Cookie"); + std::string_view cookieValue = reqHeader["Cookie"]; if (cookieValue.empty() || cookieValue.find("SESSION=") == std::string::npos) { @@ -211,18 +214,17 @@ static std::shared_ptr<persistent_data::UserSession> #endif // checks if request can be forwarded without authentication -static bool isOnWhitelist(const crow::Request& req) +static bool isOnWhitelist(std::string_view url, boost::beast::http::verb method) { - // it's allowed to GET root node without authentication - if (boost::beast::http::verb::get == req.method()) + if (boost::beast::http::verb::get == method) { - if (req.url == "/redfish/v1" || req.url == "/redfish/v1/" || - req.url == "/redfish" || req.url == "/redfish/" || - req.url == "/redfish/v1/odata" || req.url == "/redfish/v1/odata/") + if (url == "/redfish/v1" || url == "/redfish/v1/" || + url == "/redfish" || url == "/redfish/" || + url == "/redfish/v1/odata" || url == "/redfish/v1/odata/") { return true; } - if (crow::webroutes::routes.find(std::string(req.url)) != + if (crow::webroutes::routes.find(std::string(url)) != crow::webroutes::routes.end()) { return true; @@ -231,11 +233,11 @@ static bool isOnWhitelist(const crow::Request& req) // it's allowed to POST on session collection & login without // authentication - if (boost::beast::http::verb::post == req.method()) + if (boost::beast::http::verb::post == method) { - if ((req.url == "/redfish/v1/SessionService/Sessions") || - (req.url == "/redfish/v1/SessionService/Sessions/") || - (req.url == "/login")) + if ((url == "/redfish/v1/SessionService/Sessions") || + (url == "/redfish/v1/SessionService/Sessions/") || + (url == "/login")) { return true; } @@ -244,66 +246,68 @@ static bool isOnWhitelist(const crow::Request& req) return false; } -static void authenticate( - crow::Request& req, Response& res, - [[maybe_unused]] const std::weak_ptr<persistent_data::UserSession>& session) +static std::shared_ptr<persistent_data::UserSession> authenticate( + std::string_view url, boost::asio::ip::address& ipAddress, Response& res, + boost::beast::http::verb method, + const boost::beast::http::header<true>& reqHeader, + [[maybe_unused]] const std::shared_ptr<persistent_data::UserSession>& + session) { - if (isOnWhitelist(req)) + if (isOnWhitelist(url, method)) { - return; + return nullptr; } - const persistent_data::AuthConfigMethods& authMethodsConfig = persistent_data::SessionStore::getInstance().getAuthMethodsConfig(); + std::shared_ptr<persistent_data::UserSession> sessionOut = nullptr; #ifdef BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION - if (req.session == nullptr && authMethodsConfig.tls) + if (authMethodsConfig.tls) { - req.session = performTLSAuth(req, res, session); + sessionOut = performTLSAuth(res, reqHeader, session); } #endif #ifdef BMCWEB_ENABLE_XTOKEN_AUTHENTICATION - if (req.session == nullptr && authMethodsConfig.xtoken) + if (sessionOut == nullptr && authMethodsConfig.xtoken) { - req.session = performXtokenAuth(req); + sessionOut = performXtokenAuth(reqHeader); } #endif #ifdef BMCWEB_ENABLE_COOKIE_AUTHENTICATION - if (req.session == nullptr && authMethodsConfig.cookie) + if (sessionOut == nullptr && authMethodsConfig.cookie) { - req.session = performCookieAuth(req); + sessionOut = performCookieAuth(method, reqHeader); } #endif - if (req.session == nullptr) + std::string_view authHeader = reqHeader["Authorization"]; + + if (!authHeader.empty()) { - std::string_view authHeader = req.getHeaderValue("Authorization"); - if (!authHeader.empty()) + // Reject any kind of auth other than basic or token + if (boost::starts_with(authHeader, "Token ") && + authMethodsConfig.sessionToken) { - // Reject any kind of auth other than basic or token - if (boost::starts_with(authHeader, "Token ") && - authMethodsConfig.sessionToken) - { #ifdef BMCWEB_ENABLE_SESSION_AUTHENTICATION - req.session = performTokenAuth(authHeader); + sessionOut = performTokenAuth(authHeader); #endif - } - else if (boost::starts_with(authHeader, "Basic ") && - authMethodsConfig.basic) - { + } + else if (boost::starts_with(authHeader, "Basic ") && + authMethodsConfig.basic) + { #ifdef BMCWEB_ENABLE_BASIC_AUTHENTICATION - req.session = performBasicAuth(req.ipAddress, authHeader); + sessionOut = performBasicAuth(ipAddress, authHeader); #endif - } + } + if (sessionOut != nullptr) + { + return sessionOut; } } - - if (req.session == nullptr) - { - BMCWEB_LOG_WARNING << "[AuthMiddleware] authorization failed"; - forward_unauthorized::sendUnauthorized(req, res); - res.end(); - return; - } + BMCWEB_LOG_WARNING << "[AuthMiddleware] authorization failed"; + forward_unauthorized::sendUnauthorized(url, reqHeader["User-Agent"], + reqHeader["accept"], res); + res.end(); + return nullptr; } } // namespace authorization diff --git a/include/forward_unauthorized.hpp b/include/forward_unauthorized.hpp index 46a25d4b42..29fef337f0 100644 --- a/include/forward_unauthorized.hpp +++ b/include/forward_unauthorized.hpp @@ -8,18 +8,19 @@ namespace forward_unauthorized static bool hasWebuiRoute = false; -inline void sendUnauthorized(const crow::Request& req, crow::Response& res) +inline void sendUnauthorized(std::string_view url, std::string_view userAgent, + std::string_view accept, crow::Response& res) { // If it's a browser connecting, don't send the HTTP authenticate // header, to avoid possible CSRF attacks with basic auth - if (http_helpers::requestPrefersHtml(req)) + if (http_helpers::requestPrefersHtml(accept)) { // If we have a webui installed, redirect to that login page if (hasWebuiRoute) { res.result(boost::beast::http::status::temporary_redirect); res.addHeader("Location", - "/#/login?next=" + http_helpers::urlEncode(req.url)); + "/#/login?next=" + http_helpers::urlEncode(url)); } else { @@ -35,7 +36,7 @@ inline void sendUnauthorized(const crow::Request& req, crow::Response& res) // only send the WWW-authenticate header if this isn't a xhr // from the browser. Most scripts, tend to not set a user-agent header. // So key off that to know whether or not we need to suggest basic auth - if (req.getHeaderValue("User-Agent").empty()) + if (userAgent.empty()) { res.addHeader("WWW-Authenticate", "Basic"); } diff --git a/include/http_utility.hpp b/include/http_utility.hpp index 119a1eefb3..ef65e23419 100644 --- a/include/http_utility.hpp +++ b/include/http_utility.hpp @@ -5,9 +5,8 @@ namespace http_helpers { -inline bool requestPrefersHtml(const crow::Request& req) +inline bool requestPrefersHtml(std::string_view header) { - std::string_view header = req.getHeaderValue("accept"); std::vector<std::string> encodings; // chrome currently sends 6 accepts headers, firefox sends 4. encodings.reserve(6); |