common: Restrict frame embedding to same origin

Declare `X-Frame-Options: sameorigin` [1] so that cockpit frames can
only be embedded into pages coming from the same origin. This is similar
to setting CORP in commit 2b38b8d (which applies to `<script>`,
`<img>`, etc.).

The main use case for embedding is to run cockpit-ws behind a reverse
proxy, while also serving other pages. Cross-origin embedding is
discouraged these days to prevent "clickjacking".

Cross-origin embedding already did not work in most cases: Frames would
always just show the login page.  However, this looks confusing and is
unclean. With X-Frame-Options, the browser instead shows an explanatory
error page.

Mention the same origin requirement in the embedding documentation.

Fixes #16122
https://bugzilla.redhat.com/show_bug.cgi?id=1980688
CVE-2021-3660

[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options

doc/guide/embedding.xml

@@ -5,7 +5,9 @@
   <title>Embedding and Integrating Cockpit</title>
 
   <para>Cockpit can be embedded in other web applications either as a whole or specific
-    Cockpit components can be integrated.</para>
+    Cockpit components can be integrated. Due to frame security policy restrictions,
+    this only works if Cockpit and the web application have the <emphasis>same origin</emphasis>;
+    this is commonly achieved by running both from a common reverse proxy.</para>
 
   <section id="embedding-full">
     <title>Embedding the Cockpit Interface</title>

pkg/base1/test-http.js

@@ -208,6 +208,7 @@ QUnit.test("headers", function (assert) {
                     "Referrer-Policy": "no-referrer",
                     "X-DNS-Prefetch-Control": "off",
                     "X-Content-Type-Options": "nosniff",
+                    "X-Frame-Options": "sameorigin",
                     "Cross-Origin-Resource-Policy": "same-origin",
                 }, "got back headers");
             })
@@ -250,6 +251,7 @@ QUnit.test("connection headers", function (assert) {
                     "Referrer-Policy": "no-referrer",
                     "X-DNS-Prefetch-Control": "off",
                     "X-Content-Type-Options": "nosniff",
+                    "X-Frame-Options": "sameorigin",
                     "Cross-Origin-Resource-Policy": "same-origin",
                 }, "got back combined headers");
             })

src/bridge/test-httpstream.c

@@ -34,7 +34,7 @@
 extern gboolean cockpit_webserver_want_certificate;
 
 /* JSON dict snippet for headers that are present in every request */
-#define STATIC_HEADERS "\"Cross-Origin-Resource-Policy\":\"same-origin\",\"Referrer-Policy\":\"no-referrer\",\"X-Content-Type-Options\":\"nosniff\",\"X-DNS-Prefetch-Control\":\"off\""
+#define STATIC_HEADERS "\"Cross-Origin-Resource-Policy\":\"same-origin\",\"Referrer-Policy\":\"no-referrer\",\"X-Content-Type-Options\":\"nosniff\",\"X-DNS-Prefetch-Control\":\"off\",\"X-Frame-Options\":\"sameorigin\""
 
 static void
 on_closed_set_flag (CockpitChannel *channel,

src/bridge/test-packages.c

@@ -47,7 +47,7 @@
 #define CHECKSUM_CSP            "80921dc3cde9ff9f2acd2a5851f9b2a3b25ea7b4577128461d9e32fbdd671e16"
 
 /* JSON dict snippet for headers that are present in every request */
-#define STATIC_HEADERS "\"X-DNS-Prefetch-Control\":\"off\",\"Referrer-Policy\":\"no-referrer\",\"X-Content-Type-Options\":\"nosniff\",\"Cross-Origin-Resource-Policy\": \"same-origin\""
+#define STATIC_HEADERS "\"X-DNS-Prefetch-Control\":\"off\",\"Referrer-Policy\":\"no-referrer\",\"X-Content-Type-Options\":\"nosniff\",\"Cross-Origin-Resource-Policy\": \"same-origin\",\"X-Frame-Options\": \"sameorigin\""
 #define STATIC_HEADERS_CACHECONTROL STATIC_HEADERS ",\"Cache-Control\":\"no-cache, no-store\""
 
 extern const gchar **cockpit_bridge_data_dirs;

src/common/cockpitwebresponse.c

@@ -747,6 +747,7 @@ enum {
     HEADER_REFERRER_POLICY = 1 << 5,
     HEADER_CONTENT_TYPE_OPTIONS = 1 << 6,
     HEADER_CROSS_ORIGIN_RESOURCE_POLICY = 1 << 7,
+    HEADER_X_FRAME_OPTIONS = 1 << 8,
 };
 
 static GString *
@@ -789,6 +790,8 @@ append_header (GString *string,
     return HEADER_CONTENT_TYPE_OPTIONS;
   if (g_ascii_strcasecmp ("Cross-Origin-Resource-Policy", name) == 0)
     return HEADER_CROSS_ORIGIN_RESOURCE_POLICY;
+  if (g_ascii_strcasecmp ("X-Frame-Options", name) == 0)
+    return HEADER_X_FRAME_OPTIONS;
   if (g_ascii_strcasecmp ("Content-Length", name) == 0 ||
       g_ascii_strcasecmp ("Transfer-Encoding", name) == 0 ||
       g_ascii_strcasecmp ("Connection", name) == 0)
@@ -900,6 +903,9 @@ finish_headers (CockpitWebResponse *self,
    * be able to read any resource. This does *not* affect embedding with <iframe> */
   if ((seen & HEADER_CROSS_ORIGIN_RESOURCE_POLICY) == 0)
     g_string_append (string, "Cross-Origin-Resource-Policy: same-origin\r\n");
+  /* This is the counterpart for iframe embedding, line of defence against clickjacking */
+  if ((seen & HEADER_X_FRAME_OPTIONS) == 0)
+    g_string_append (string, "X-Frame-Options: sameorigin\r\n");
 
   g_string_append (string, "\r\n");
   return g_string_free_to_bytes (string);

src/common/test-webresponse.c

@@ -35,7 +35,7 @@
 #include <string.h>
 
 /* headers that are present in every request */
-#define STATIC_HEADERS "X-DNS-Prefetch-Control: off\r\nReferrer-Policy: no-referrer\r\nX-Content-Type-Options: nosniff\r\nCross-Origin-Resource-Policy: same-origin\r\n\r\n"
+#define STATIC_HEADERS "X-DNS-Prefetch-Control: off\r\nReferrer-Policy: no-referrer\r\nX-Content-Type-Options: nosniff\r\nCross-Origin-Resource-Policy: same-origin\r\nX-Frame-Options: sameorigin\r\n\r\n"
 static gchar *srcdir;
 
 typedef struct {

src/ws/test-channelresponse.c

@@ -53,7 +53,7 @@
 #define PASSWORD "this is the password"
 
 /* headers that are present in every request */
-#define STATIC_HEADERS "X-Content-Type-Options: nosniff\r\nX-DNS-Prefetch-Control: off\r\nReferrer-Policy: no-referrer\r\nCross-Origin-Resource-Policy: same-origin\r\n"
+#define STATIC_HEADERS "X-Content-Type-Options: nosniff\r\nX-DNS-Prefetch-Control: off\r\nReferrer-Policy: no-referrer\r\nCross-Origin-Resource-Policy: same-origin\r\nX-Frame-Options: sameorigin\r\n"
 
 typedef struct {
   CockpitWebService *service;

test/verify/check-connection

@@ -411,8 +411,9 @@ class TestConnection(MachineCase):
         self.assertIn(
             "default-src 'self' https://127.0.0.1:9090; connect-src 'self' https://127.0.0.1:9090 wss://127.0.0.1:9090", headers)
         self.assertIn("Access-Control-Allow-Origin: https://127.0.0.1:9090", headers)
-        # CORP is also set for dynamic paths
+        # CORP and Frame-Options are also set for dynamic paths
         self.assertIn("Cross-Origin-Resource-Policy: same-origin", headers)
+        self.assertIn("X-Frame-Options: sameorigin", headers)
 
         self.allow_journal_messages(
             ".*Peer failed to perform TLS handshake",
@@ -627,6 +628,7 @@ class TestConnection(MachineCase):
         self.assertIn("HTTP/1.1 200 OK\r\n", headers)
         self.assertIn("Content-Type: text/html\r\n", headers)
         self.assertIn("Cross-Origin-Resource-Policy: same-origin\r\n", headers)
+        self.assertIn("X-Frame-Options: sameorigin\r\n", headers)
         # login.html is not always accessible as a file (e.g. on CoreOS), so just assert a reasonable content length
         self.assertIn("Content-Length: ", headers)
         length = int(headers.split('Content-Length: ', 1)[1].split()[0])

test/verify/check-embed

@@ -94,20 +94,13 @@ Shell=/shell/index.html
         b.open("http://localhost:12346/index.html")
         b.set_val("#embed-address", "http://{0}:{1}".format(m.web_address, m.web_port))
         b.click("#embed-full")
-        # FIXME (#16122): we should not even get that far, frame loading should already be blocked here
         b.wait_visible("iframe[name='embed-full'][loaded]")
         b.switch_to_frame("embed-full")
 
-        # second line of defense: existing login cookie does not work (default browser protection)
-        b.wait_visible("#login")
-        b.set_val("#login-user-input", "admin")
-        b.set_val("#login-password-input", "foobar")
-        b.click('#login-button')
-        b.expect_load_frame("embed-full")
-
-        # third line of defense: login succeeds and creates a PAM session; but loading session UI does not
-        # (again, due to default browser protection)
-        b.wait_visible("#login")
+        # X-Frame-Options sameorigin blocks frame
+        if b.cdp.browser == "firefox":
+            b.wait_visible("body.neterror")
+        self.assertFalse(b.is_present("#login"))
 
 
 if __name__ == '__main__':