Fix compile issue on DISABLE_XSS_PREVENTION
Fixes #178
Every few months, this option breaks because of some combination of
compiler options. I'm hoping that this is a more permenant fix, and
will keep it working forever.
Functionally, this commit changes a couple things.
1. It fixes the regression that snuck into this option, by making the
req variable optional using the c++17 [[maybe_unused]] syntax.
2. It promotes the BMCWEB_INSECURE_DISABLE_XSS_PREVENTION into the
config.h file, and a constexpr variable rather than a #define. This has
the benefit that both the code paths in question will compiled
regardless of whether or not they're used, thus ensuring they stay
buildable forever. The optimization path will still delete the code
later, but we won't have so many one-off build options breaking. We
should move all the other feature driven #ifdefs to this pattern in the
future.
3. As a mechnaical change to #2, this adds a config.h.in, which delcares
the various variables as their respective constexpr types. This allows
the constants to be used in a cleaner way.
As an aside, at some point, DISABLE_XSS_PREVENTION should really move to
a non-persistent runtime option rather than a compile time option. Too
many people get hung up on having to recompile their BMC, and moving it
to runtime under admin credentials is no more a security risk.
As another aside, we should move all the other #ifdef style options to
this pattern. It seems like it would help with keeping all options
buildable, and is definitely more modern than #ifdefs for features,
especially if they don't require #include changes or linker changes.
Tested:
enabled meson option insecure-disable-xss, and verified code builds and
works again.
Change-Id: Id03faa17cffdbabaf4e5b0d46b24bb58b7f44669
Signed-off-by: Ed Tanous <edtanous@google.com>
diff --git a/config.h.in b/config.h.in
new file mode 100644
index 0000000..394cfdf
--- /dev/null
+++ b/config.h.in
@@ -0,0 +1,12 @@
+#pragma once
+
+#include <cstdint>
+
+// clang-format off
+constexpr const int bmcwebInsecureDisableXssPrevention =
+ @BMCWEB_INSECURE_DISABLE_XSS_PREVENTION@;
+
+constexpr const size_t bmcwebHttpReqBodyLimitMb = @BMCWEB_HTTP_REQ_BODY_LIMIT_MB@;
+
+constexpr const char* mesonInstallPrefix = "@MESON_INSTALL_PREFIX@";
+// clang-format on
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index e5cd74f..6840fc0 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -38,9 +38,9 @@
static std::atomic<int> connectionCount;
#endif
-// request body limit size set by the BMCWEB_HTTP_REQ_BODY_LIMIT_MB option
+// request body limit size set by the bmcwebHttpReqBodyLimitMb option
constexpr unsigned int httpReqBodyLimit =
- 1024 * 1024 * BMCWEB_HTTP_REQ_BODY_LIMIT_MB;
+ 1024 * 1024 * bmcwebHttpReqBodyLimitMb;
constexpr uint64_t loggedOutPostBodyLimit = 4096;
@@ -410,7 +410,7 @@
BMCWEB_LOG_INFO << "Response: " << this << ' ' << req->url << ' '
<< res.resultInt() << " keepalive=" << req->keepAlive();
- addSecurityHeaders(res);
+ addSecurityHeaders(*req, res);
if (needToCallAfterHandlers)
{
diff --git a/include/cors_preflight.hpp b/include/cors_preflight.hpp
index 0a5c3cc..844fd38 100644
--- a/include/cors_preflight.hpp
+++ b/include/cors_preflight.hpp
@@ -10,7 +10,7 @@
{
BMCWEB_ROUTE(app, "<str>")
.methods(boost::beast::http::verb::options)(
- [](const crow::Request& req, crow::Response& res) {
+ [](const crow::Request&, crow::Response& res, const std::string&) {
// An empty body handler that simply returns the headers bmcweb
// uses This allows browsers to do their CORS preflight checks
res.end();
diff --git a/include/security_headers.hpp b/include/security_headers.hpp
index 13ec893..e3c472e 100644
--- a/include/security_headers.hpp
+++ b/include/security_headers.hpp
@@ -2,7 +2,8 @@
#include <http_response.hpp>
-inline void addSecurityHeaders(crow::Response& res)
+inline void addSecurityHeaders(const crow::Request& req [[maybe_unused]],
+ crow::Response& res)
{
/*
TODO(ed) these should really check content types. for example,
@@ -22,40 +23,41 @@
"mode=block");
res.addHeader("X-Content-Type-Options", "nosniff");
-#ifndef BMCWEB_INSECURE_DISABLE_XSS_PREVENTION
- res.addHeader("Content-Security-Policy", "default-src 'none'; "
- "img-src 'self' data:; "
- "font-src 'self'; "
- "style-src 'self'; "
- "script-src 'self'; "
- "connect-src 'self' wss:");
- // The KVM currently needs to load images from base64 encoded
- // strings. img-src 'self' data: is used to allow that.
- // https://stackoverflow.com/questions/18447970/content-security-policy-data-not-working-for-base64-images-in-chrome-28
+ if (bmcwebInsecureDisableXssPrevention)
+ {
+ res.addHeader("Content-Security-Policy", "default-src 'none'; "
+ "img-src 'self' data:; "
+ "font-src 'self'; "
+ "style-src 'self'; "
+ "script-src 'self'; "
+ "connect-src 'self' wss:");
+ // The KVM currently needs to load images from base64 encoded
+ // strings. img-src 'self' data: is used to allow that.
+ // https://stackoverflow.com/questions/18447970/content-security-policy-data-not-working-for-base64-images-in-chrome-28
+ }
+ else
+ {
+ // If XSS is disabled, we need to allow loading from addresses other
+ // than self, as the BMC will be hosted elsewhere.
+ res.addHeader("Content-Security-Policy", "default-src 'none'; "
+ "img-src *; "
+ "font-src *; "
+ "style-src *; "
+ "script-src *; "
+ "connect-src *");
-#else
- // If XSS is disabled, we need to allow loading from addresses other
- // than self, as the BMC will be hosted elsewhere.
- res.addHeader("Content-Security-Policy", "default-src 'none'; "
- "img-src *; "
- "font-src *; "
- "style-src *; "
- "script-src *; "
- "connect-src *");
-
- const std::string_view origin = req.getHeaderValue("Origin");
- res.addHeader(bf::access_control_allow_origin, origin);
- res.addHeader(bf::access_control_allow_methods, "GET, "
- "POST, "
- "PUT, "
- "PATCH, "
- "DELETE");
- res.addHeader(bf::access_control_allow_credentials, "true");
- res.addHeader(bf::access_control_allow_headers, "Origin, "
- "Content-Type, "
- "Accept, "
- "Cookie, "
- "X-XSRF-TOKEN");
-
-#endif
+ const std::string_view origin = req.getHeaderValue("Origin");
+ res.addHeader(bf::access_control_allow_origin, origin);
+ res.addHeader(bf::access_control_allow_methods, "GET, "
+ "POST, "
+ "PUT, "
+ "PATCH, "
+ "DELETE");
+ res.addHeader(bf::access_control_allow_credentials, "true");
+ res.addHeader(bf::access_control_allow_headers, "Origin, "
+ "Content-Type, "
+ "Accept, "
+ "Cookie, "
+ "X-XSRF-TOKEN");
+ }
}
diff --git a/meson.build b/meson.build
index 8c766bf..a08c8dd 100644
--- a/meson.build
+++ b/meson.build
@@ -59,7 +59,6 @@
'insecure-disable-auth' : '-DBMCWEB_INSECURE_DISABLE_AUTHENTICATION',
'insecure-disable-csrf' : '-DBMCWEB_INSECURE_DISABLE_CSRF_PREVENTION',
'insecure-disable-ssl' : '-DBMCWEB_INSECURE_DISABLE_SSL',
-'insecure-disable-xss' : '-DBMCWEB_INSECURE_DISABLE_XSS_PREVENTION',
'host-serial-socket' : '-DBMCWEB_ENABLE_HOST_SERIAL_WEBSOCKET',
'ibm-management-console' : '-DBMCWEB_ENABLE_IBM_MANAGEMENT_CONSOLE',
'kvm' : '-DBMCWEB_ENABLE_KVM' ,
@@ -336,9 +335,12 @@
# Gather the Configuration data
conf_data = configuration_data()
-conf_data.set('BMCWEB_HTTP_REQ_BODY_LIMIT_MB',get_option('http-body-limit'))
-conf_data.set('MESON_INSTALL_PREFIX',get_option('prefix'))
-configure_file(output: 'config.h',
+conf_data.set('BMCWEB_HTTP_REQ_BODY_LIMIT_MB', get_option('http-body-limit'))
+xss_enabled = get_option('insecure-disable-xss')
+conf_data.set10('BMCWEB_INSECURE_DISABLE_XSS_PREVENTION', xss_enabled)
+conf_data.set('MESON_INSTALL_PREFIX', get_option('prefix'))
+configure_file(input: 'config.h.in',
+ output: 'config.h',
configuration: conf_data)
# Configure and install systemd unit files
diff --git a/src/webserver_main.cpp b/src/webserver_main.cpp
index b5bc28c..014c2ff 100644
--- a/src/webserver_main.cpp
+++ b/src/webserver_main.cpp
@@ -2,6 +2,7 @@
#include <app.hpp>
#include <boost/asio/io_context.hpp>
+#include <cors_preflight.hpp>
#include <dbus_monitor.hpp>
#include <dbus_singleton.hpp>
#include <hostname_monitor.hpp>
@@ -99,9 +100,10 @@
crow::ibm_mc_lock::Lock::getInstance();
#endif
-#ifdef BMCWEB_INSECURE_DISABLE_XSS_PREVENTION
- cors_preflight::requestRoutes(app);
-#endif
+ if (bmcwebInsecureDisableXssPrevention)
+ {
+ cors_preflight::requestRoutes(app);
+ }
crow::login_routes::requestRoutes(app);