From 25a8a5c6e47268933b9b4433a9590ccfd9c04c83 Mon Sep 17 00:00:00 2001
From: Martin Pitt <mpitt@redhat.com>
Date: Tue, 14 Sep 2021 09:02:33 +0200
Subject: [PATCH] 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 2b38b8de92f9a (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

Conflict:NA
Reference:https://github.com/cockpit-project/cockpit/pull/16342/commits/25a8a5c6e47268933b9b4433a9590ccfd9c04c83
---
 doc/guide/embedding.xml         | 4 +++-
 src/bridge/test-httpstream.c    | 2 +-
 src/bridge/test-packages.c      | 3 +++
 src/common/cockpitwebresponse.c | 6 ++++++
 src/common/test-webresponse.c   | 3 +++
 src/ws/test-channelresponse.c   | 3 +++
 6 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/doc/guide/embedding.xml b/doc/guide/embedding.xml
index 99b30dc..7fe8aa9 100644
--- a/doc/guide/embedding.xml
+++ b/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>
diff --git a/src/bridge/test-httpstream.c b/src/bridge/test-httpstream.c
index 47af9ea..6844f72 100644
--- a/src/bridge/test-httpstream.c
+++ b/src/bridge/test-httpstream.c
@@ -31,7 +31,7 @@
 #include <string.h>
 
 /* 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,
diff --git a/src/bridge/test-packages.c b/src/bridge/test-packages.c
index 769c51c..b96a3b4 100644
--- a/src/bridge/test-packages.c
+++ b/src/bridge/test-packages.c
@@ -45,6 +45,9 @@
 #define CHECKSUM_RELOAD_UPDATED "0d1c0b7c6133cc7c3956197fd8a76bef68b158bd78beac75cfa80b75c36aa827"
 #define CHECKSUM_CSP            "25cab69451c3667cb9ed33f006fc7003c248f1029dae4a763bbadb0c4cafaf8d"
 
+/* 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\",\"X-Frame-Options\": \"sameorigin\""
+
 extern const gchar **cockpit_bridge_data_dirs;
 extern const gchar *cockpit_bridge_local_address;
 extern gint cockpit_bridge_packages_port;
diff --git a/src/common/cockpitwebresponse.c b/src/common/cockpitwebresponse.c
index 0e757f3..ddf1911 100644
--- a/src/common/cockpitwebresponse.c
+++ b/src/common/cockpitwebresponse.c
@@ -735,6 +735,7 @@ enum {
     HEADER_CACHE_CONTROL = 1 << 3,
     HEADER_DNS_PREFETCH_CONTROL = 1 << 4,
     HEADER_REFERRER_POLICY = 1 << 5,
+    HEADER_X_FRAME_OPTIONS = 1 << 8,
 };
 
 static GString *
@@ -773,6 +774,8 @@ append_header (GString *string,
     return HEADER_DNS_PREFETCH_CONTROL;
   if (g_ascii_strcasecmp ("Referrer-Policy", name) == 0)
     return HEADER_REFERRER_POLICY;
+   if (g_ascii_strcasecmp ("X-Frame-Options", name) == 0)
+    return HEADER_X_FRAME_OPTIONS;
   else if (g_ascii_strcasecmp ("Connection", name) == 0)
     g_critical ("Don't set Connection header manually. This is a programmer error.");
   return 0;
@@ -875,6 +878,9 @@ finish_headers (CockpitWebResponse *self,
     g_string_append (string, "X-DNS-Prefetch-Control: off\r\n");
   if ((seen & HEADER_REFERRER_POLICY) == 0)
     g_string_append (string, "Referrer-Policy: no-referrer\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);
diff --git a/src/common/test-webresponse.c b/src/common/test-webresponse.c
index ab59c13..a797f2b 100644
--- a/src/common/test-webresponse.c
+++ b/src/common/test-webresponse.c
@@ -34,6 +34,9 @@
 #include <stdlib.h>
 #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\nX-Frame-Options: sameorigin\r\n\r\n"
+
 static gchar *srcdir;
 
 typedef struct {
diff --git a/src/ws/test-channelresponse.c b/src/ws/test-channelresponse.c
index 86c0fff..763913b 100644
--- a/src/ws/test-channelresponse.c
+++ b/src/ws/test-channelresponse.c
@@ -51,6 +51,9 @@
 
 #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\nX-Frame-Options: sameorigin\r\n"
+
 typedef struct {
   CockpitWebService *service;
   GIOStream *io;
-- 
2.27.0