packages/cockpit
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix cve-2021-3660

Browse Source
This commit is contained in:
wangkerong 2021-09-24 14:19:51 +08:00
parent 48e20f0e0a
commit 08b6ef87fc
3 changed files with 335 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

184
backport-0001-CVE-2021-3660.patch Normal file
View File

@ -0,0 +1,184 @@
From b0b57c8093f180fa7d67fdd4efa2f1a13e296930 Mon Sep 17 00:00:00 2001
From: Martin Pitt <mpitt@redhat.com>
Date: Tue, 14 Sep 2021 11:03:32 +0200
Subject: [PATCH] bridge: Robust JSON object comparison in test-httpstream.c
Comparing the received control message JSON against the expected value
in string form has always been brittle: It assumes a
predictable/reproducible GLib dictionary serialization. This will break
across different GLib versions once we add a new static header.
So change the tests to compare the control message with
`cockpit_assert_json_eq()` (which will do parsing and deep object
comparison) instead of plain string comparison. Introduce a new
assert_channel_response() helper for this.
Conflict:NA
Reference:https://github.com/cockpit-project/cockpit/pull/16348/commits/b0b57c8093f180fa7d67fdd4efa2f1a13e296930
---
src/bridge/test-httpstream.c | 69 ++++++++++++++++++++----------------
1 file changed, 38 insertions(+), 31 deletions(-)
diff --git a/src/bridge/test-httpstream.c b/src/bridge/test-httpstream.c
index 53cd7bf..47af9ea 100644
--- a/src/bridge/test-httpstream.c
+++ b/src/bridge/test-httpstream.c
@@ -30,6 +30,9 @@
#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\""
+
static void
on_closed_set_flag (CockpitChannel *channel,
const gchar *problem,
@@ -64,6 +67,24 @@ non_local_ip (void)
return str;
}
+/* Compare subsequent "{ control JSON }" and "body" channel messages against expected values. */
+static void
+assert_channel_response (MockTransport *transport,
+ const gchar *channel_id,
+ const gchar *expected_control,
+ const gchar *expected_body)
+{
+ GBytes *data;
+
+ data = mock_transport_pop_channel (transport, channel_id);
+ JsonNode *node = cockpit_json_parse (g_bytes_get_data (data, NULL), -1, NULL);
+ cockpit_assert_json_eq (json_node_get_object (node), expected_control);
+ json_node_unref (node);
+
+ data = mock_transport_pop_channel (transport, channel_id);
+ cockpit_assert_bytes_eq (data, expected_body, -1);
+}
+
static void
setup_general (TestGeneral *tt,
gconstpointer host_fixture)
@@ -116,8 +137,6 @@ test_host_header (TestGeneral *tt,
JsonObject *options;
const gchar *control;
gboolean closed;
- GBytes *data;
- guint count;
if (tt->host == NULL)
{
@@ -154,11 +173,9 @@ test_host_header (TestGeneral *tt,
g_signal_connect (channel, "closed", G_CALLBACK (on_closed_set_flag), &closed);
while (!closed)
g_main_context_iteration (NULL, TRUE);
-
- data = mock_transport_combine_output (tt->transport, "444", &count);
- cockpit_assert_bytes_eq (data, "{\"status\":200,\"reason\":\"OK\",\"headers\":{\"X-DNS-Prefetch-Control\":\"off\",\"Referrer-Policy\":\"no-referrer\"}}Da Da Da", -1);
- g_assert_cmpuint (count, ==, 2);
- g_bytes_unref (data);
+ assert_channel_response (tt->transport, "444",
+ "{\"status\":200,\"reason\":\"OK\",\"headers\":{" STATIC_HEADERS "}}",
+ "Da Da Da");
}
static gboolean
@@ -340,11 +357,8 @@ test_http_chunked (void)
TestResult *tr = g_slice_new (TestResult);
GBytes *bytes = NULL;
- GBytes *data = NULL;
const gchar *control;
- gchar *expected = g_strdup_printf ("{\"status\":200,\"reason\":\"OK\",\"headers\":{\"X-DNS-Prefetch-Control\":\"off\",\"Referrer-Policy\":\"no-referrer\"}}%0*d", MAGIC_NUMBER, 0);
- guint count;
guint port;
web_server = cockpit_web_server_new (NULL, 0, NULL, NULL, NULL);
@@ -387,12 +401,11 @@ test_http_chunked (void)
g_main_context_iteration (NULL, TRUE);
g_assert_cmpstr (tr->problem, ==, NULL);
- data = mock_transport_combine_output (transport, "444", &count);
- cockpit_assert_bytes_eq (data, expected, -1);
- g_assert_cmpuint (count, ==, 2);
+ g_autofree gchar *expected_body = g_strdup_printf ("%0*d", MAGIC_NUMBER, 0);
- g_bytes_unref (data);
- g_free (expected);
+ assert_channel_response (transport, "444",
+ "{\"status\":200,\"reason\":\"OK\",\"headers\":{" STATIC_HEADERS "}}",
+ expected_body);
g_object_unref (transport);
g_object_add_weak_pointer (G_OBJECT (channel), (gpointer *)&channel);
@@ -509,7 +522,6 @@ test_tls_basic (TestTls *test,
JsonObject *options;
const gchar *control;
GBytes *bytes;
- GBytes *data;
options = json_object_new ();
json_object_set_int_member (options, "port", test->port);
@@ -537,10 +549,9 @@ test_tls_basic (TestTls *test,
while (closed == FALSE)
g_main_context_iteration (NULL, TRUE);
- data = mock_transport_combine_output (test->transport, "444", NULL);
- cockpit_assert_bytes_eq (data, "{\"status\":200,\"reason\":\"OK\",\"headers\":{\"X-DNS-Prefetch-Control\":\"off\",\"Referrer-Policy\":\"no-referrer\"}}Oh Marmalaade!", -1);
-
- g_bytes_unref (data);
+ assert_channel_response (test->transport, "444",
+ "{\"status\":200,\"reason\":\"OK\",\"headers\":{" STATIC_HEADERS "}}",
+ "Oh Marmalaade!");
g_object_add_weak_pointer (G_OBJECT (channel), (gpointer *)&channel);
g_object_unref (channel);
@@ -668,7 +679,6 @@ test_tls_certificate (TestTls *test,
GTlsCertificate *cert;
const gchar *control;
GBytes *bytes;
- GBytes *data;
tls = cockpit_json_parse_object (json, -1, &error);
g_assert_no_error (error);
@@ -699,10 +709,9 @@ test_tls_certificate (TestTls *test,
while (closed == FALSE)
g_main_context_iteration (NULL, TRUE);
- data = mock_transport_combine_output (test->transport, "444", NULL);
- cockpit_assert_bytes_eq (data, "{\"status\":200,\"reason\":\"OK\",\"headers\":{\"X-DNS-Prefetch-Control\":\"off\",\"Referrer-Policy\":\"no-referrer\"}}Oh Marmalaade!", -1);
-
- g_bytes_unref (data);
+ assert_channel_response (test->transport, "444",
+ "{\"status\":200,\"reason\":\"OK\",\"headers\":{" STATIC_HEADERS "}}",
+ "Oh Marmalaade!");
g_assert (test->peer != NULL);
@@ -733,7 +742,6 @@ test_tls_authority_good (TestTls *test,
GError *error = NULL;
const gchar *control;
GBytes *bytes;
- GBytes *data;
tls = cockpit_json_parse_object (json, -1, &error);
g_assert_no_error (error);
@@ -763,11 +771,10 @@ test_tls_authority_good (TestTls *test,
while (closed == FALSE)
g_main_context_iteration (NULL, TRUE);
-
- data = mock_transport_combine_output (test->transport, "444", NULL);
- cockpit_assert_bytes_eq (data, "{\"status\":200,\"reason\":\"OK\",\"headers\":{\"X-DNS-Prefetch-Control\":\"off\",\"Referrer-Policy\":\"no-referrer\"}}Oh Marmalaade!", -1);
-
- g_bytes_unref (data);
+
+ assert_channel_response (test->transport, "444",
+ "{\"status\":200,\"reason\":\"OK\",\"headers\":{" STATIC_HEADERS "}}",
+ "Oh Marmalaade!");
g_object_add_weak_pointer (G_OBJECT (channel), (gpointer *)&channel);
g_object_unref (channel);
--
2.27.0

142
backport-0002-CVE-2021-3660.patch Normal file
View File

@ -0,0 +1,142 @@
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

10
cockpit.spec
View File

@ -1,13 +1,15 @@
%bcond_with pcp
Name: cockpit
Version: 178
Release: 11
Release: 12
Summary: A easy-to-use, integrated, glanceable, and open web-based interface for Linux servers
License: LGPLv2+
URL: https://cockpit-project.org/
Source0: https://github.com/cockpit-project/cockpit/releases/download/%{version}/cockpit-%{version}.tar.xz
Patch6000: CVE-2019-3804.patch
Patch6001: backport-0001-CVE-2021-3660.patch
Patch6002: backport-0002-CVE-2021-3660.patch
BuildRequires: gcc
BuildRequires: pkgconfig(gio-unix-2.0) pkgconfig(json-glib-1.0) pkgconfig(polkit-agent-1) >= 0.105 pam-devel
@ -179,6 +181,12 @@ test -f %{_bindir}/firewall-cmd && firewall-cmd --reload --quiet || true
%doc %{_mandir}/man8/{cockpit-ws.8.gz,remotectl.8.gz,pam_ssh_add.8.gz}
%changelog
* Fri Sep 24 2021 wangkerong <wangkerong@huawei.com> - 178-12
- Type:CVE
- Id:CVE-2021-3660
- SUG:NA
- DESC:fix CVE-2021-3660
* Sat Sep 4 2021 hanhui <hanhui15@huawei.com> - 178-11
- strip binary file