Support RespondToUnauthenticatedClients PATCH
RespondToUnauthenticatedClients allows users to explicitly select mTLS
as their only authentication mechanism, thus significantly reducing
their code exposure to unauthenticated clients.
From the Redfish specification
```
The RespondToUnauthenticatedClients property within the
ClientCertificate property within the MFA property of the AccountService
resource controls the response behavior when an invalid certificate is
provided by the client.
• If the property contains true or is not
supported by the service, the service shall not fail the TLS handshake.
This is to allow the service to send error messages or unauthenticated
resources to the client.
• If the property contains false , the service
shall fail the TLS handshake.
```
This commit implements that behavior.
This also has some added benefits in that we no longer have to check the
filesystem for every connection, as TLS is controlled explicitly, and
not whether or not a root cert is in place.
Note, this also implements a TODO to disable cookie auth when using
mTLS. Clients can still use IsAuthenticated to determine if they are
authenticated on request.
Tested:
Run scripts/generate_auth_certs.py to set up a root certificate and
client certificate. This verifies that mTLS as optional has not been
broken. Script succeeds.
```
PATCH /redfish/v1/AccountService
{"MultiFactorAuth": {"ClientCertificate": {"RespondToUnauthenticatedClients": false}}}
```
GET /redfish/v1
without a client certificate now fails with an ssl verification error
GET /redfish/v1
with a client certificate returns the result
```
PATCH /redfish/v1/AccountService
{"MultiFactorAuth": {"ClientCertificate": {"RespondToUnauthenticatedClients": false}}}
With certificate returns non mTLS functionality.
```
Change-Id: I5a9d6d6b1698bff83ab62b1f760afed6555849c9
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/http/app.hpp b/http/app.hpp
index 29cb4de..b5f5c13 100644
--- a/http/app.hpp
+++ b/http/app.hpp
@@ -83,6 +83,16 @@
router.validate();
}
+ void loadCertificate()
+ {
+ BMCWEB_LOG_DEBUG("Loading certificate");
+ if (!server)
+ {
+ return;
+ }
+ server->loadCertificate();
+ }
+
std::optional<boost::asio::ip::tcp::acceptor> setupSocket()
{
if (io == nullptr)
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 2050afd..e591455 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -63,18 +63,13 @@
public:
Connection(Handler* handlerIn, boost::asio::steady_timer&& timerIn,
std::function<std::string()>& getCachedDateStrF,
- Adaptor adaptorIn) :
+ Adaptor&& adaptorIn) :
adaptor(std::move(adaptorIn)),
handler(handlerIn), timer(std::move(timerIn)),
getCachedDateStr(getCachedDateStrF)
{
initParser();
- if constexpr (BMCWEB_MUTUAL_TLS_AUTH)
- {
- prepareMutualTls();
- }
-
connectionCount++;
BMCWEB_LOG_DEBUG("{} Connection created, total {}", logPtr(this),
@@ -99,55 +94,61 @@
bool tlsVerifyCallback(bool preverified,
boost::asio::ssl::verify_context& ctx)
{
- // We always return true to allow full auth flow for resources that
- // don't require auth
+ BMCWEB_LOG_DEBUG("{} tlsVerifyCallback called with preverified {}",
+ logPtr(this), preverified);
if (preverified)
{
mtlsSession = verifyMtlsUser(ip, ctx);
if (mtlsSession)
{
- BMCWEB_LOG_DEBUG("{} Generating TLS session: {}", logPtr(this),
+ BMCWEB_LOG_DEBUG("{} Generated TLS session: {}", logPtr(this),
mtlsSession->uniqueId);
}
}
+ const persistent_data::AuthConfigMethods& c =
+ persistent_data::SessionStore::getInstance().getAuthMethodsConfig();
+ if (c.tlsStrict)
+ {
+ return preverified;
+ }
+ // If tls strict mode is disabled
+ // We always return true to allow full auth flow for resources that
+ // don't require auth
return true;
}
- void prepareMutualTls()
+ bool prepareMutualTls()
{
if constexpr (IsTls<Adaptor>::value)
{
- std::error_code error;
- std::filesystem::path caPath(ensuressl::trustStorePath);
- auto caAvailable = !std::filesystem::is_empty(caPath, error);
- caAvailable = caAvailable && !error;
- if (caAvailable && persistent_data::SessionStore::getInstance()
- .getAuthMethodsConfig()
- .tls)
- {
- adaptor.set_verify_mode(boost::asio::ssl::verify_peer);
- std::string id = "bmcweb";
+ BMCWEB_LOG_DEBUG("prepareMutualTls");
- const char* cStr = id.c_str();
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- const auto* idC = reinterpret_cast<const unsigned char*>(cStr);
- int ret = SSL_set_session_id_context(
- adaptor.native_handle(), idC,
- static_cast<unsigned int>(id.length()));
- if (ret == 0)
- {
- BMCWEB_LOG_ERROR("{} failed to set SSL id", logPtr(this));
- }
+ constexpr std::string_view id = "bmcweb";
+
+ const char* idPtr = id.data();
+ const auto* idCPtr = std::bit_cast<const unsigned char*>(idPtr);
+ auto idLen = static_cast<unsigned int>(id.length());
+ int ret = SSL_set_session_id_context(adaptor.native_handle(),
+ idCPtr, idLen);
+ if (ret == 0)
+ {
+ BMCWEB_LOG_ERROR("{} failed to set SSL id", logPtr(this));
+ return false;
}
- adaptor.set_verify_callback(
- std::bind_front(&self_type::tlsVerifyCallback, this));
- }
- }
+ BMCWEB_LOG_DEBUG("set_verify_callback");
- Adaptor& socket()
- {
- return adaptor;
+ boost::system::error_code ec;
+ adaptor.set_verify_callback(
+ std::bind_front(&self_type::tlsVerifyCallback, this), ec);
+ if (ec)
+ {
+ BMCWEB_LOG_ERROR("Failed to set verify callback {}", ec);
+ return false;
+ }
+ }
+
+ return true;
}
void start()
@@ -161,6 +162,15 @@
return;
}
+ if constexpr (BMCWEB_MUTUAL_TLS_AUTH)
+ {
+ if (!prepareMutualTls())
+ {
+ BMCWEB_LOG_ERROR("{} Failed to prepare mTLS", logPtr(this));
+ return;
+ }
+ }
+
startDeadline();
readClientIp();
@@ -332,6 +342,13 @@
void hardClose()
{
+ if (mtlsSession != nullptr)
+ {
+ BMCWEB_LOG_DEBUG("{} Removing TLS session: {}", logPtr(this),
+ mtlsSession->uniqueId);
+ persistent_data::SessionStore::getInstance().removeSession(
+ mtlsSession);
+ }
BMCWEB_LOG_DEBUG("{} Closing socket", logPtr(this));
boost::beast::get_lowest_layer(adaptor).close();
}
@@ -350,13 +367,7 @@
void gracefulClose()
{
BMCWEB_LOG_DEBUG("{} Socket close requested", logPtr(this));
- if (mtlsSession != nullptr)
- {
- BMCWEB_LOG_DEBUG("{} Removing TLS session: {}", logPtr(this),
- mtlsSession->uniqueId);
- persistent_data::SessionStore::getInstance().removeSession(
- mtlsSession);
- }
+
if constexpr (IsTls<Adaptor>::value)
{
adaptor.async_shutdown(std::bind_front(
@@ -517,14 +528,14 @@
return;
}
- if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>)
+ constexpr bool isTest =
+ std::is_same_v<Adaptor, boost::beast::test::stream>;
+
+ if constexpr (!BMCWEB_INSECURE_DISABLE_AUTH && !isTest)
{
- 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);
- }
+ boost::beast::http::verb method = parser->get().method();
+ userSession = crow::authentication::authenticate(
+ ip, res, method, parser->get().base(), mtlsSession);
}
std::string_view expect =
diff --git a/http/http_server.hpp b/http/http_server.hpp
index 6d725c9..b22fae2 100644
--- a/http/http_server.hpp
+++ b/http/http_server.hpp
@@ -10,6 +10,7 @@
#include <boost/asio/ssl/context.hpp>
#include <boost/asio/ssl/stream.hpp>
#include <boost/asio/steady_timer.hpp>
+#include <boost/beast/core/stream_traits.hpp>
#include <atomic>
#include <chrono>
@@ -27,6 +28,8 @@
template <typename Handler, typename Adaptor = boost::asio::ip::tcp::socket>
class Server
{
+ using self_t = Server<Handler, Adaptor>;
+
public:
Server(Handler* handlerIn, boost::asio::ip::tcp::acceptor&& acceptorIn,
std::shared_ptr<boost::asio::ssl::context> adaptorCtxIn,
@@ -100,14 +103,6 @@
{
BMCWEB_LOG_INFO("Receivied reload signal");
loadCertificate();
- boost::system::error_code ec2;
- acceptor.cancel(ec2);
- if (ec2)
- {
- BMCWEB_LOG_ERROR(
- "Error while canceling async operations:{}",
- ec2.message());
- }
startAsyncWaitForSignal();
}
else
@@ -122,16 +117,20 @@
{
ioService->stop();
}
+ using Socket = boost::beast::lowest_layer_type<Adaptor>;
+ using SocketPtr = std::unique_ptr<Socket>;
- void doAccept()
+ void afterAccept(SocketPtr socket, const boost::system::error_code& ec)
{
- if (ioService == nullptr)
+ if (ec)
{
- BMCWEB_LOG_CRITICAL("IoService was null");
+ BMCWEB_LOG_ERROR("Failed to accept socket {}", ec);
return;
}
+
boost::asio::steady_timer timer(*ioService);
std::shared_ptr<Connection<Adaptor, Handler>> connection;
+
if constexpr (std::is_same<Adaptor,
boost::asio::ssl::stream<
boost::asio::ip::tcp::socket>>::value)
@@ -144,24 +143,36 @@
}
connection = std::make_shared<Connection<Adaptor, Handler>>(
handler, std::move(timer), getCachedDateStr,
- Adaptor(*ioService, *adaptorCtx));
+ Adaptor(std::move(*socket), *adaptorCtx));
}
else
{
connection = std::make_shared<Connection<Adaptor, Handler>>(
handler, std::move(timer), getCachedDateStr,
- Adaptor(*ioService));
+ Adaptor(std::move(*socket)));
}
+
+ boost::asio::post(*ioService, [connection] { connection->start(); });
+
+ doAccept();
+ }
+
+ void doAccept()
+ {
+ if (ioService == nullptr)
+ {
+ BMCWEB_LOG_CRITICAL("IoService was null");
+ return;
+ }
+
+ SocketPtr socket = std::make_unique<Socket>(*ioService);
+ // Keep a raw pointer so when the socket is moved, the pointer is still
+ // valid
+ Socket* socketPtr = socket.get();
+
acceptor.async_accept(
- boost::beast::get_lowest_layer(connection->socket()),
- [this, connection](const boost::system::error_code& ec) {
- if (!ec)
- {
- boost::asio::post(*ioService,
- [connection] { connection->start(); });
- }
- doAccept();
- });
+ *socketPtr,
+ std::bind_front(&self_t::afterAccept, this, std::move(socket)));
}
private:
diff --git a/include/authentication.hpp b/include/authentication.hpp
index 5c7ec19..221e197 100644
--- a/include/authentication.hpp
+++ b/include/authentication.hpp
@@ -185,28 +185,18 @@
inline std::shared_ptr<persistent_data::UserSession>
performTLSAuth(Response& res,
- const boost::beast::http::header<true>& reqHeader,
- const std::weak_ptr<persistent_data::UserSession>& session)
+ const std::shared_ptr<persistent_data::UserSession>& session)
{
- if (auto sp = session.lock())
+ if (session != nullptr)
{
- // set cookie only if this is req from the browser.
- if (reqHeader["User-Agent"].empty())
- {
- BMCWEB_LOG_DEBUG(" TLS session: {} will be used for this request.",
- sp->uniqueId);
- return sp;
- }
- // TODO: change this to not switch to cookie auth
- bmcweb::setSessionCookies(res, *sp);
res.addHeader(boost::beast::http::field::set_cookie,
"IsAuthenticated=true; Secure");
BMCWEB_LOG_DEBUG(
" TLS session: {} with cookie will be used for this request.",
- sp->uniqueId);
- return sp;
+ session->uniqueId);
}
- return nullptr;
+
+ return session;
}
// checks if request can be forwarded without authentication
@@ -265,7 +255,7 @@
{
if (authMethodsConfig.tls)
{
- sessionOut = performTLSAuth(res, reqHeader, session);
+ sessionOut = performTLSAuth(res, session);
}
}
if constexpr (BMCWEB_XTOKEN_AUTH)
diff --git a/include/persistent_data.hpp b/include/persistent_data.hpp
index 3b98e1a..fc69aed 100644
--- a/include/persistent_data.hpp
+++ b/include/persistent_data.hpp
@@ -226,6 +226,7 @@
authConfig["SessionToken"] = c.sessionToken;
authConfig["BasicAuth"] = c.basic;
authConfig["TLS"] = c.tls;
+ authConfig["TLSStrict"] = c.tlsStrict;
authConfig["TLSCommonNameParseMode"] =
static_cast<int>(c.mTLSCommonNameParsingMode);
diff --git a/include/sessions.hpp b/include/sessions.hpp
index e6e4a68..9f93259 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -184,12 +184,20 @@
struct AuthConfigMethods
{
+ // Authentication paths
bool basic = BMCWEB_BASIC_AUTH;
bool sessionToken = BMCWEB_SESSION_AUTH;
bool xtoken = BMCWEB_XTOKEN_AUTH;
bool cookie = BMCWEB_COOKIE_AUTH;
bool tls = BMCWEB_MUTUAL_TLS_AUTH;
+ // Whether or not unauthenticated TLS should be accepted
+ // true = reject connections if mutual tls is not provided
+ // false = allow connection, and allow user to use other auth method
+ // Always default to false, because root certificates will not
+ // be provisioned at startup
+ bool tlsStrict = false;
+
MTLSCommonNameParseMode mTLSCommonNameParsingMode =
getMTLSCommonNameParseMode(
BMCWEB_MUTUAL_TLS_COMMON_NAME_PARSING_DEFAULT);
@@ -221,6 +229,10 @@
{
tls = *value;
}
+ else if (element.first == "TLSStrict")
+ {
+ tlsStrict = *value;
+ }
}
const uint64_t* intValue =
element.second.get_ptr<const uint64_t*>();
diff --git a/include/ssl_key_handler.hpp b/include/ssl_key_handler.hpp
index a8bcfa5..ce1e638 100644
--- a/include/ssl_key_handler.hpp
+++ b/include/ssl_key_handler.hpp
@@ -4,6 +4,7 @@
#include "logging.hpp"
#include "ossl_random.hpp"
+#include "persistent_data.hpp"
#include <boost/beast/core/file_posix.hpp>
@@ -604,11 +605,23 @@
BMCWEB_LOG_CRITICAL("Couldn't get server context");
return nullptr;
}
+ const persistent_data::AuthConfigMethods& c =
+ persistent_data::SessionStore::getInstance().getAuthMethodsConfig();
- // BIG WARNING: This needs to stay disabled, as there will always be
- // unauthenticated endpoints
- // mSslContext->set_verify_mode(boost::asio::ssl::verify_peer);
+ boost::asio::ssl::verify_mode mode = boost::asio::ssl::verify_peer;
+ if (c.tlsStrict)
+ {
+ BMCWEB_LOG_DEBUG("Setting verify peer");
+ mode |= boost::asio::ssl::verify_fail_if_no_peer_cert;
+ }
+ boost::system::error_code ec;
+ sslCtx.set_verify_mode(mode, ec);
+ if (ec)
+ {
+ BMCWEB_LOG_DEBUG("Failed to set verify mode {}", ec.message());
+ return nullptr;
+ }
SSL_CTX_set_options(sslCtx.native_handle(), SSL_OP_NO_RENEGOTIATION);
if constexpr (BMCWEB_EXPERIMENTAL_HTTP2)
diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index b3b848f..a0725e6 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -23,6 +23,7 @@
#include "persistent_data.hpp"
#include "query.hpp"
#include "registries/privilege_registry.hpp"
+#include "sessions.hpp"
#include "utils/collection.hpp"
#include "utils/dbus_utils.hpp"
#include "utils/json_utils.hpp"
@@ -1339,7 +1340,8 @@
nlohmann::json::object_t clientCertificate;
clientCertificate["Enabled"] = authMethodsConfig.tls;
- clientCertificate["RespondToUnauthenticatedClients"] = true;
+ clientCertificate["RespondToUnauthenticatedClients"] =
+ !authMethodsConfig.tlsStrict;
using account_service::CertificateMappingAttribute;
@@ -1468,6 +1470,38 @@
authMethodsConfig.mTLSCommonNameParsingMode = parseMode;
}
+inline void handleRespondToUnauthenticatedClientsPatch(
+ App& app, const crow::Request& req, crow::Response& res,
+ bool respondToUnauthenticatedClients)
+{
+ if (req.session != nullptr)
+ {
+ // Sanity check. If the user isn't currently authenticated with mutual
+ // TLS, they very likely are about to permanently lock themselves out.
+ // Make sure they're using mutual TLS before allowing locking.
+ if (req.session->sessionType != persistent_data::SessionType::MutualTLS)
+ {
+ messages::propertyValueExternalConflict(
+ res,
+ "MultiFactorAuth/ClientCertificate/RespondToUnauthenticatedClients",
+ respondToUnauthenticatedClients);
+ return;
+ }
+ }
+
+ persistent_data::AuthConfigMethods& authMethodsConfig =
+ persistent_data::SessionStore::getInstance().getAuthMethodsConfig();
+
+ // Change the settings
+ authMethodsConfig.tlsStrict = !respondToUnauthenticatedClients;
+
+ // Write settings to disk
+ persistent_data::getConfig().writeData();
+
+ // Trigger a reload, to apply the new settings to new connections
+ app.loadCertificate();
+}
+
inline void handleAccountServicePatch(
App& app, const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
@@ -1482,6 +1516,7 @@
std::optional<uint16_t> maxPasswordLength;
LdapPatchParams ldapObject;
std::optional<std::string> certificateMappingAttribute;
+ std::optional<bool> respondToUnauthenticatedClients;
LdapPatchParams activeDirectoryObject;
AuthMethods auth;
std::optional<std::string> httpBasicAuth;
@@ -1501,6 +1536,7 @@
"ActiveDirectory/ServiceAddresses", activeDirectoryObject.serviceAddressList,
"ActiveDirectory/ServiceEnabled", activeDirectoryObject.serviceEnabled,
"MultiFactorAuth/ClientCertificate/CertificateMappingAttribute", certificateMappingAttribute,
+ "MultiFactorAuth/ClientCertificate/RespondToUnauthenticatedClients", respondToUnauthenticatedClients,
"LDAP/Authentication/AuthenticationType", ldapObject.authType,
"LDAP/Authentication/Password", ldapObject.password,
"LDAP/Authentication/Username", ldapObject.userName,
@@ -1540,6 +1576,12 @@
}
}
+ if (respondToUnauthenticatedClients)
+ {
+ handleRespondToUnauthenticatedClientsPatch(
+ app, req, asyncResp->res, *respondToUnauthenticatedClients);
+ }
+
if (certificateMappingAttribute)
{
handleCertificateMappingAttributePatch(asyncResp->res,