From f71e021e3bb3a486b6d5e376970fd4974392f896 Mon Sep 17 00:00:00 2001 From: MarkLee131 Date: Wed, 29 Apr 2026 20:29:47 +0800 Subject: [PATCH] HTTPSock: emit standard hardening response headers Close #2012. Add X-Frame-Options: SAMEORIGIN, X-Content-Type-Options: nosniff and Referrer-Policy: same-origin to every response so webadmin and module pages are framed/sniff-protected by default. Add no-store Cache-Control and Pragma: no-cache on dynamic responses so shared workstations can't replay authenticated pages from browser history. Skip the cache headers for 304 and for static asset MIME types (image, font, text/css, application/javascript) that the existing ETag/Last-Modified path on PrintFile already handles. Per review feedback: the emitter is a private WriteHardeningHeaders that writes each line via the socket directly from PrintHeader, not a public helper returning a temporary VCString. Callers can override a default value with AddHeader, or suppress one outright with the new public OmitHardeningHeader(name). Tests: drive PrintHeader on a CHTTPSock subclass that captures Write() calls, then assert with gmock matchers (Contains(StartsWith(...))). --- include/znc/HTTPSock.h | 11 ++++ src/HTTPSock.cpp | 34 ++++++++++++ test/HTTPSockTest.cpp | 117 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+) diff --git a/include/znc/HTTPSock.h b/include/znc/HTTPSock.h index d2b2edd4..1d22e9aa 100644 --- a/include/znc/HTTPSock.h +++ b/include/znc/HTTPSock.h @@ -56,6 +56,11 @@ class CHTTPSock : public CSocket { * inputs that could split the response into a separate header or body * (RFC 7230 disallows obs-fold; treat any bare CR/LF as invalid). */ static bool IsValidHeaderField(const CString& s); + /** Suppress one of the default security/cache hardening headers for + * the next response. Use this when the caller does not want a value + * sent at all (a different value can instead be supplied via + * AddHeader). Must be called before PrintHeader. */ + void OmitHardeningHeader(const CString& sName); void SetContentType(const CString& sContentType); bool PrintNotFound(); @@ -121,6 +126,11 @@ class CHTTPSock : public CSocket { void WriteFileUncompressed(CFile& File); void WriteFileGzipped(CFile& File); + /** Write the security/cache hardening header lines for the current + * response. Skips any name that the caller has already populated via + * AddHeader, or has explicitly omitted via OmitHardeningHeader. */ + void WriteHardeningHeaders(unsigned int uStatusId); + protected: void PrintPage(const CString& sPage); void Init(); @@ -142,6 +152,7 @@ class CHTTPSock : public CSocket { std::map m_msvsPOSTParams; std::map m_msvsGETParams; MCString m_msHeaders; + SCString m_ssOmitHardening; bool m_bHTTP10Client; CString m_sIfNoneMatch; bool m_bAcceptGzip; diff --git a/src/HTTPSock.cpp b/src/HTTPSock.cpp index 2ff3a643..672f67ea 100644 --- a/src/HTTPSock.cpp +++ b/src/HTTPSock.cpp @@ -740,6 +740,8 @@ bool CHTTPSock::PrintHeader(off_t uContentLength, const CString& sContentType, } Write("Content-Type: " + m_sContentType + "\r\n"); + WriteHardeningHeaders(uStatusId); + for (const auto& it : m_msResponseCookies) { Write("Set-Cookie: " + it.first.Escape_n(CString::EURL) + "=" + it.second.Escape_n(CString::EURL) + "; HttpOnly; path=/;" + @@ -776,6 +778,38 @@ void CHTTPSock::AddHeader(const CString& sName, const CString& sValue) { m_msHeaders[sName] = sValue; } +void CHTTPSock::OmitHardeningHeader(const CString& sName) { + m_ssOmitHardening.insert(sName); +} + +void CHTTPSock::WriteHardeningHeaders(unsigned int uStatusId) { + auto writeIfWanted = [&](const CString& sName, const CString& sValue) { + if (m_msHeaders.find(sName) != m_msHeaders.end()) return; + if (m_ssOmitHardening.find(sName) != m_ssOmitHardening.end()) return; + Write(sName + ": " + sValue + "\r\n"); + }; + + // Always-on defaults: callers can override via AddHeader, or skip + // entirely via OmitHardeningHeader, before PrintHeader runs. + writeIfWanted("X-Frame-Options", "SAMEORIGIN"); + writeIfWanted("X-Content-Type-Options", "nosniff"); + writeIfWanted("Referrer-Policy", "same-origin"); + + // Don't cache authenticated/dynamic responses. Skip for 304 and for + // static asset MIME types that the ETag/Last-Modified path handles + // explicitly via PrintFile. + const bool bStaticLike = + uStatusId == 304 || m_sContentType.StartsWith("image/") || + m_sContentType.StartsWith("font/") || + m_sContentType.StartsWith("text/css") || + m_sContentType.StartsWith("application/javascript"); + if (!bStaticLike) { + writeIfWanted("Cache-Control", + "no-store, no-cache, must-revalidate, max-age=0"); + writeIfWanted("Pragma", "no-cache"); + } +} + bool CHTTPSock::Redirect(const CString& sURL) { if (SentHeader()) { DEBUG("Redirect() - Header was already sent"); diff --git a/test/HTTPSockTest.cpp b/test/HTTPSockTest.cpp index 420d847b..ae41a1a6 100644 --- a/test/HTTPSockTest.cpp +++ b/test/HTTPSockTest.cpp @@ -14,8 +14,14 @@ * limitations under the License. */ +#include #include #include +#include + +using ::testing::Contains; +using ::testing::Not; +using ::testing::StartsWith; // Validation contract used by AddHeader to keep CR/LF (and therefore // response-splitting bytes) out of the response stream (#2010). @@ -37,3 +43,114 @@ TEST(HTTPSockTest, IsValidHeaderField) { EXPECT_FALSE(CHTTPSock::IsValidHeaderField("trailing\n")); EXPECT_FALSE(CHTTPSock::IsValidHeaderField("\rleading")); } + +namespace { + +// Minimal CHTTPSock subclass for tests: captures every Write() call as a +// separate vector entry, and stubs out the pure-virtual hooks. Each Write +// call from PrintHeader corresponds to one header line, so matchers like +// Contains(StartsWith("X-Frame-Options:")) work directly on m_vsLines. +class CCapturingHTTPSock : public CHTTPSock { + public: + CCapturingHTTPSock() : CHTTPSock(nullptr, "") {} + + void OnPageRequest(const CString& sURI) override {} + Csock* GetSockObj(const CString& sHost, unsigned short uPort) override { + return nullptr; + } + // PrintHeader writes one CString per header line; capture each call. + using CHTTPSock::Write; + bool Write(const CString& sData) override { + m_vsLines.push_back(sData); + return true; + } + + VCString m_vsLines; +}; + +class HTTPSockHeadersTest : public ::testing::Test { + protected: + HTTPSockHeadersTest() { CZNC::CreateInstance(); } + ~HTTPSockHeadersTest() override { CZNC::DestroyInstance(); } +}; + +} // namespace + +// Hardening response headers introduced for #2012. The fix's contract: +// - emit X-Frame-Options, X-Content-Type-Options, Referrer-Policy on every +// response (unless the caller already set them or asked to omit them); +// - emit no-store Cache-Control + Pragma for dynamic responses, but skip +// for 304 and for static asset MIME types whose freshness is handled by +// ETag/Last-Modified; +// - never duplicate a header the caller already added via AddHeader; +// - skip a header entirely when the caller calls OmitHardeningHeader. +TEST_F(HTTPSockHeadersTest, HardeningHeadersDefaultDynamicResponse) { + CCapturingHTTPSock sock; + sock.PrintHeader(0, "text/html"); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("X-Frame-Options: SAMEORIGIN"))); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("X-Content-Type-Options: nosniff"))); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("Referrer-Policy: same-origin"))); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("Cache-Control:"))); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("Pragma: no-cache"))); +} + +TEST_F(HTTPSockHeadersTest, HardeningHeadersSkipCacheControlOn304) { + CCapturingHTTPSock sock; + sock.PrintHeader(0, "text/html", 304, "Not Modified"); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("X-Frame-Options:"))); + EXPECT_THAT(sock.m_vsLines, Not(Contains(StartsWith("Cache-Control:")))); + EXPECT_THAT(sock.m_vsLines, Not(Contains(StartsWith("Pragma:")))); +} + +TEST_F(HTTPSockHeadersTest, HardeningHeadersSkipCacheControlForStaticAssets) { + for (const CString& sCT : {CString("image/png"), CString("image/svg+xml"), + CString("font/woff2"), CString("text/css"), + CString("application/javascript")}) { + CCapturingHTTPSock sock; + sock.PrintHeader(0, sCT); + EXPECT_THAT(sock.m_vsLines, Not(Contains(StartsWith("Cache-Control:")))) + << "for content type " << sCT; + // Security headers still emitted even for static-like responses. + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("X-Content-Type-Options: nosniff"))) + << "for content type " << sCT; + } +} + +TEST_F(HTTPSockHeadersTest, HardeningHeadersDeferToCallerXFrameOptions) { + // Caller-supplied X-Frame-Options should suppress the default so we + // don't emit a duplicate (or worse, a conflicting) header. + CCapturingHTTPSock sock; + sock.AddHeader("X-Frame-Options", "DENY"); + sock.PrintHeader(0, "text/html"); + // The caller's value goes out via the m_msHeaders loop, not via the + // hardening emitter, so the SAMEORIGIN default must not appear. + EXPECT_THAT(sock.m_vsLines, Not(Contains(StartsWith("X-Frame-Options: SAMEORIGIN")))); + EXPECT_THAT(sock.m_vsLines, Contains(CString("X-Frame-Options: DENY\r\n"))); + // Other defaults still emitted. + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("X-Content-Type-Options: nosniff"))); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("Referrer-Policy: same-origin"))); +} + +TEST_F(HTTPSockHeadersTest, HardeningHeadersDeferToCallerCacheControl) { + CCapturingHTTPSock sock; + sock.AddHeader("Cache-Control", "max-age=300"); + sock.PrintHeader(0, "text/html"); + // No no-store default; only the caller's value should be emitted. + EXPECT_THAT(sock.m_vsLines, Not(Contains(StartsWith("Cache-Control: no-store")))); + EXPECT_THAT(sock.m_vsLines, Contains(CString("Cache-Control: max-age=300\r\n"))); + // Pragma is independently controlled and still emitted as a default. + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("Pragma: no-cache"))); +} + +TEST_F(HTTPSockHeadersTest, HardeningHeadersOmittedByCaller) { + // OmitHardeningHeader skips the default outright (no value emitted). + CCapturingHTTPSock sock; + sock.OmitHardeningHeader("X-Frame-Options"); + sock.OmitHardeningHeader("Cache-Control"); + sock.PrintHeader(0, "text/html"); + EXPECT_THAT(sock.m_vsLines, Not(Contains(StartsWith("X-Frame-Options:")))); + EXPECT_THAT(sock.m_vsLines, Not(Contains(StartsWith("Cache-Control:")))); + // Other defaults unaffected. + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("X-Content-Type-Options: nosniff"))); + EXPECT_THAT(sock.m_vsLines, Contains(StartsWith("Pragma: no-cache"))); +}