[libsoup/carlosgc/http2-strict-header-validation: 2/2] http2: add session property to disable RFC 9113 header value validation
- From: Carlos Garcia Campos <carlosgc src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [libsoup/carlosgc/http2-strict-header-validation: 2/2] http2: add session property to disable RFC 9113 header value validation
- Date: Fri, 30 Sep 2022 08:22:11 +0000 (UTC)
commit dacc655bf5139b70107e894535ca7806fa657a23
Author: Carlos Garcia Campos <cgarcia igalia com>
Date: Fri Sep 30 10:16:01 2022 +0200
http2: add session property to disable RFC 9113 header value validation
It's breaking many popular web sites, so browsers might want to
disable it.
Fixes #300
libsoup/http2/soup-client-message-io-http2.c | 34 +++++++++++--
libsoup/soup-connection-manager.c | 1 +
libsoup/soup-connection.c | 24 +++++++++
libsoup/soup-connection.h | 1 +
libsoup/soup-session.c | 53 +++++++++++++++++++
libsoup/soup-session.h | 3 ++
meson.build | 7 ++-
tests/http2-test.c | 76 ++++++++++++++++++++++++++++
8 files changed, 194 insertions(+), 5 deletions(-)
---
diff --git a/libsoup/http2/soup-client-message-io-http2.c b/libsoup/http2/soup-client-message-io-http2.c
index fdeb6841..1dd85c3a 100644
--- a/libsoup/http2/soup-client-message-io-http2.c
+++ b/libsoup/http2/soup-client-message-io-http2.c
@@ -545,6 +545,22 @@ on_header_callback (nghttp2_session *session,
return 0;
}
+static int
+on_invalid_header_callback (nghttp2_session *session,
+ const nghttp2_frame *frame,
+ const uint8_t *name,
+ size_t namelen,
+ const uint8_t *value,
+ size_t valuelen,
+ uint8_t flags,
+ void *user_data)
+{
+ SoupHTTP2MessageData *data = nghttp2_session_get_stream_user_data (session, frame->hd.stream_id);
+
+ h2_debug (user_data, data, "[HEADERS] Invalid header received: name=[%.*s] value=[%.*s]", namelen,
name, valuelen, value);
+ return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+}
+
static GError *
memory_stream_need_more_data_callback (SoupBodyInputStreamHttp2 *stream,
gboolean blocking,
@@ -1825,13 +1841,15 @@ static const SoupClientMessageIOFuncs io_funcs = {
};
static void
-soup_client_message_io_http2_init (SoupClientMessageIOHTTP2 *io)
+soup_client_message_io_http2_init (SoupClientMessageIOHTTP2 *io,
+ gboolean strict_header_validation)
{
soup_http2_debug_init ();
nghttp2_session_callbacks *callbacks;
NGCHECK (nghttp2_session_callbacks_new (&callbacks));
nghttp2_session_callbacks_set_on_header_callback (callbacks, on_header_callback);
+ nghttp2_session_callbacks_set_on_invalid_header_callback (callbacks, on_invalid_header_callback);
nghttp2_session_callbacks_set_on_frame_recv_callback (callbacks, on_frame_recv_callback);
nghttp2_session_callbacks_set_on_data_chunk_recv_callback (callbacks, on_data_chunk_recv_callback);
nghttp2_session_callbacks_set_on_begin_frame_callback (callbacks, on_begin_frame_callback);
@@ -1840,7 +1858,17 @@ soup_client_message_io_http2_init (SoupClientMessageIOHTTP2 *io)
nghttp2_session_callbacks_set_on_frame_send_callback (callbacks, on_frame_send_callback);
nghttp2_session_callbacks_set_on_stream_close_callback (callbacks, on_stream_close_callback);
- NGCHECK (nghttp2_session_client_new (&io->session, callbacks, io));
+#ifdef HAVE_NGHTTP2_OPTION_SET_NO_RFC9113_LEADING_AND_TRAILING_WS_VALIDATION
+ if (!strict_header_validation) {
+ nghttp2_option *option;
+
+ nghttp2_option_new (&option);
+ nghttp2_option_set_no_rfc9113_leading_and_trailing_ws_validation (option, 1);
+ NGCHECK (nghttp2_session_client_new2 (&io->session, callbacks, io, option));
+ nghttp2_option_del (option);
+ } else
+#endif
+ NGCHECK (nghttp2_session_client_new (&io->session, callbacks, io));
nghttp2_session_callbacks_del (callbacks);
io->messages = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL,
(GDestroyNotify)soup_http2_message_data_free);
@@ -1856,7 +1884,7 @@ SoupClientMessageIO *
soup_client_message_io_http2_new (SoupConnection *conn)
{
SoupClientMessageIOHTTP2 *io = g_new0 (SoupClientMessageIOHTTP2, 1);
- soup_client_message_io_http2_init (io);
+ soup_client_message_io_http2_init (io, soup_connection_get_http2_strict_header_validation (conn));
g_weak_ref_init (&io->conn, conn);
diff --git a/libsoup/soup-connection-manager.c b/libsoup/soup-connection-manager.c
index a200f221..64aab7be 100644
--- a/libsoup/soup-connection-manager.c
+++ b/libsoup/soup-connection-manager.c
@@ -466,6 +466,7 @@ soup_connection_manager_get_connection_locked (SoupConnectionManager *manager,
"ssl", soup_uri_is_https (host->uri),
"socket-properties", socket_props,
"force-http-version", force_http_version,
+ "http2-strict-header-validation",
soup_session_get_http2_strict_header_validation (item->session),
NULL);
g_signal_connect (conn, "disconnected",
diff --git a/libsoup/soup-connection.c b/libsoup/soup-connection.c
index 5deb3d35..45620c43 100644
--- a/libsoup/soup-connection.c
+++ b/libsoup/soup-connection.c
@@ -32,6 +32,7 @@ typedef struct {
guint64 id;
GSocketAddress *remote_address;
guint8 force_http_version;
+ gboolean http2_strict_header_validation;
GUri *proxy_uri;
gboolean ssl;
@@ -77,6 +78,7 @@ enum {
PROP_TLS_PROTOCOL_VERSION,
PROP_TLS_CIPHERSUITE_NAME,
PROP_FORCE_HTTP_VERSION,
+ PROP_HTTP2_STRICT_HEADER_VALIDATION,
PROP_CONTEXT,
LAST_PROPERTY
@@ -98,6 +100,7 @@ soup_connection_init (SoupConnection *conn)
priv->http_version = SOUP_HTTP_1_1;
priv->force_http_version = G_MAXUINT8;
+ priv->http2_strict_header_validation = TRUE;
priv->owner = g_thread_self ();
}
@@ -167,6 +170,9 @@ soup_connection_set_property (GObject *object, guint prop_id,
case PROP_FORCE_HTTP_VERSION:
priv->force_http_version = g_value_get_uchar (value);
break;
+ case PROP_HTTP2_STRICT_HEADER_VALIDATION:
+ priv->http2_strict_header_validation = g_value_get_boolean (value);
+ break;
case PROP_CONTEXT:
priv->idle_timeout_src = g_timeout_source_new (0);
g_source_set_ready_time (priv->idle_timeout_src, -1);
@@ -220,6 +226,9 @@ soup_connection_get_property (GObject *object, guint prop_id,
case PROP_FORCE_HTTP_VERSION:
g_value_set_uchar (value, priv->force_http_version);
break;
+ case PROP_HTTP2_STRICT_HEADER_VALIDATION:
+ g_value_set_boolean (value, priv->http2_strict_header_validation);
+ break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
break;
@@ -366,6 +375,13 @@ soup_connection_class_init (SoupConnectionClass *connection_class)
0, G_MAXUINT8, G_MAXUINT8,
G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY |
G_PARAM_STATIC_STRINGS);
+ properties[PROP_HTTP2_STRICT_HEADER_VALIDATION] =
+ g_param_spec_boolean ("http2-strict-header-validation",
+ "Strict HTTP/2 header validation",
+ "Whether to validate header values on HTTP/2 connections according to
RFC 9113",
+ TRUE,
+ G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY |
+ G_PARAM_STATIC_STRINGS);
properties[PROP_CONTEXT] =
g_param_spec_pointer ("context",
"Context",
@@ -1043,6 +1059,14 @@ soup_connection_disconnect (SoupConnection *conn)
soup_connection_disconnected (conn);
}
+gboolean
+soup_connection_get_http2_strict_header_validation (SoupConnection *conn)
+{
+ SoupConnectionPrivate *priv = soup_connection_get_instance_private (conn);
+
+ return priv->http2_strict_header_validation;
+}
+
GSocket *
soup_connection_get_socket (SoupConnection *conn)
{
diff --git a/libsoup/soup-connection.h b/libsoup/soup-connection.h
index 18a21749..afdeac89 100644
--- a/libsoup/soup-connection.h
+++ b/libsoup/soup-connection.h
@@ -47,6 +47,7 @@ gboolean soup_connection_tunnel_handshake (SoupConnection *conn,
GError **error);
void soup_connection_disconnect (SoupConnection *conn);
+gboolean soup_connection_get_http2_strict_header_validation (SoupConnection *conn);
GSocket *soup_connection_get_socket (SoupConnection *conn);
GIOStream *soup_connection_get_iostream (SoupConnection *conn);
GIOStream *soup_connection_steal_iostream (SoupConnection *conn);
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index e3577428..099d1c6f 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -83,6 +83,8 @@ typedef struct {
SoupSocketProperties *socket_props;
+ gboolean http2_strict_header_validation;
+
GMainContext *context;
GMutex queue_mutex;
GQueue *queue;
@@ -131,6 +133,7 @@ enum {
PROP_PROXY_RESOLVER,
PROP_MAX_CONNS,
PROP_MAX_CONNS_PER_HOST,
+ PROP_HTTP2_STRICT_HEADER_VALIDATION,
PROP_TLS_DATABASE,
PROP_TIMEOUT,
PROP_USER_AGENT,
@@ -298,6 +301,8 @@ soup_session_init (SoupSession *session)
*/
priv->proxy_use_default = TRUE;
priv->tlsdb_use_default = TRUE;
+
+ priv->http2_strict_header_validation = TRUE;
}
static void
@@ -417,6 +422,9 @@ soup_session_set_property (GObject *object, guint prop_id,
case PROP_MAX_CONNS_PER_HOST:
soup_connection_manager_set_max_conns_per_host (priv->conn_manager, g_value_get_int (value));
break;
+ case PROP_HTTP2_STRICT_HEADER_VALIDATION:
+ priv->http2_strict_header_validation = g_value_get_boolean (value);
+ break;
case PROP_TLS_DATABASE:
soup_session_set_tls_database (session, g_value_get_object (value));
break;
@@ -466,6 +474,9 @@ soup_session_get_property (GObject *object, guint prop_id,
case PROP_MAX_CONNS_PER_HOST:
g_value_set_int (value, soup_session_get_max_conns_per_host (session));
break;
+ case PROP_HTTP2_STRICT_HEADER_VALIDATION:
+ g_value_set_boolean (value, soup_session_get_http2_strict_header_validation (session));
+ break;
case PROP_TLS_DATABASE:
g_value_set_object (value, soup_session_get_tls_database (session));
break;
@@ -592,6 +603,29 @@ soup_session_get_max_conns_per_host (SoupSession *session)
return soup_connection_manager_get_max_conns_per_host (priv->conn_manager);
}
+/**
+ * soup_session_get_http2_strict_header_validation: (attributes
org.gtk.Method.get_property=http2-strict-header-validation)
+ * @session: a #SoupSession
+ *
+ * Whether to validate header values on HTTP/2 connections according to [RFC
9113](https://datatracker.ietf.org/doc/html/rfc9113#section-8.2.1),
+ * which, in addition to the validation specified in [RFC
7230](http://tools.ietf.org/html/rfc7230#section-3.2), it doesn't allow
+ * trailing and leading whitespaces in HTTP header fields.
+ *
+ * Returns: %TRUE if HTTP/2 strict header validation is enabled, or %FALSE otherwise
+ *
+ * Since: 3.4
+ */
+gboolean
+soup_session_get_http2_strict_header_validation (SoupSession *session)
+{
+ SoupSessionPrivate *priv;
+
+ g_return_val_if_fail (SOUP_IS_SESSION (session), 0);
+
+ priv = soup_session_get_instance_private (session);
+ return priv->http2_strict_header_validation;
+}
+
/**
* soup_session_set_proxy_resolver: (attributes org.gtk.Method.set_property=proxy-resolver)
* @session: a #SoupSession
@@ -2410,6 +2444,25 @@ soup_session_class_init (SoupSessionClass *session_class)
G_PARAM_READWRITE |
G_PARAM_CONSTRUCT_ONLY |
G_PARAM_STATIC_STRINGS);
+
+ /**
+ * SoupSession:http2-strict-header-validation: (attributes
org.gtk.Property.get=soup_session_get_http2_strict_header_validation)
+ *
+ * Whether to validate header values on HTTP/2 connections according to [RFC
9113](https://datatracker.ietf.org/doc/html/rfc9113#section-8.2.1),
+ * which, in addition to the validation specified in [RFC
7230](http://tools.ietf.org/html/rfc7230#section-3.2), it doesn't allow
+ * trailing and leading whitespaces in HTTP header fields.
+ *
+ * Since: 3.4
+ */
+ properties[PROP_HTTP2_STRICT_HEADER_VALIDATION] =
+ g_param_spec_boolean ("http2-strict-header-validation",
+ "Strict HTTP/2 header validation",
+ "Whether to validate header values on HTTP/2 connections according to
RFC 9113",
+ TRUE,
+ G_PARAM_READWRITE |
+ G_PARAM_CONSTRUCT_ONLY |
+ G_PARAM_STATIC_STRINGS);
+
/**
* SoupSession:idle-timeout: (attributes org.gtk.Property.get=soup_session_get_idle_timeout
org.gtk.Property.set=soup_session_set_idle_timeout)
*
diff --git a/libsoup/soup-session.h b/libsoup/soup-session.h
index 13e5a4f5..3ad6456c 100644
--- a/libsoup/soup-session.h
+++ b/libsoup/soup-session.h
@@ -69,6 +69,9 @@ guint soup_session_get_max_conns (SoupSession *sess
SOUP_AVAILABLE_IN_ALL
guint soup_session_get_max_conns_per_host (SoupSession *session);
+SOUP_AVAILABLE_IN_3_4
+gboolean soup_session_get_http2_strict_header_validation (SoupSession *session);
+
SOUP_AVAILABLE_IN_ALL
void soup_session_set_proxy_resolver (SoupSession *session,
GProxyResolver *proxy_resolver);
diff --git a/meson.build b/meson.build
index 3080594c..0487fbc1 100644
--- a/meson.build
+++ b/meson.build
@@ -110,7 +110,12 @@ gio_dep = dependency('gio-2.0', version : glib_required_version,
glib_deps = [glib_dep, gmodule_dep, gobject_dep, gio_dep]
+cdata = configuration_data()
+
libnghttp2_dep = dependency('libnghttp2')
+if cc.has_function('nghttp2_option_set_no_rfc9113_leading_and_trailing_ws_validation', prefix : '#include
<nghttp2/nghttp2.h>', dependencies : libnghttp2_dep)
+ cdata.set('HAVE_NGHTTP2_OPTION_SET_NO_RFC9113_LEADING_AND_TRAILING_WS_VALIDATION', '1')
+endif
sqlite_dep = dependency('sqlite3', required: false)
@@ -126,8 +131,6 @@ if not sqlite_dep.found()
sqlite_dep = dependency('sqlite3')
endif
-cdata = configuration_data()
-
brotlidec_dep = dependency('libbrotlidec', required : get_option('brotli'))
if brotlidec_dep.found()
cdata.set('WITH_BROTLI', true)
diff --git a/tests/http2-test.c b/tests/http2-test.c
index 12012c3b..6c05eb2a 100644
--- a/tests/http2-test.c
+++ b/tests/http2-test.c
@@ -986,6 +986,67 @@ do_invalid_header_received_test (Test *test, gconstpointer data)
g_object_unref (msg);
}
+#ifdef HAVE_NGHTTP2_OPTION_SET_NO_RFC9113_LEADING_AND_TRAILING_WS_VALIDATION
+static void
+do_invalid_header_rfc9113_received_test (gconstpointer data)
+{
+ SoupSession *session;
+ gboolean async = GPOINTER_TO_INT (data);
+ GUri *uri;
+ SoupMessage *msg;
+ GBytes *response;
+ GError *error = NULL;
+
+ session = soup_test_session_new (NULL);
+ uri = g_uri_parse_relative (base_uri, "/invalid-header-rfc9113", SOUP_HTTP_URI_FLAGS, NULL);
+ msg = soup_message_new_from_uri (SOUP_METHOD_GET, uri);
+
+ if (async)
+ response = soup_test_session_async_send (session, msg, NULL, &error);
+ else
+ response = soup_session_send_and_read (session, msg, NULL, &error);
+
+ g_assert_null (response);
+ g_error_matches (error, G_IO_ERROR, G_IO_ERROR_FAILED);
+ g_assert_cmpstr (error->message, ==, "HTTP/2 Error: PROTOCOL_ERROR");
+ g_clear_error (&error);
+ g_object_unref (msg);
+ soup_test_session_abort_unref (session);
+
+ session = soup_test_session_new ("http2-strict-header-validation", FALSE, NULL);
+ msg = soup_message_new_from_uri (SOUP_METHOD_GET, uri);
+
+ if (async)
+ response = soup_test_session_async_send (session, msg, NULL, &error);
+ else
+ response = soup_session_send_and_read (session, msg, NULL, &error);
+
+ g_assert_nonnull (response);
+ g_assert_no_error (error);
+ g_clear_error (&error);
+ g_object_unref (msg);
+ g_uri_unref (uri);
+
+ /* An invalid header according to RFC 7230 is still invalid */
+ uri = g_uri_parse_relative (base_uri, "/invalid-header", SOUP_HTTP_URI_FLAGS, NULL);
+ msg = soup_message_new_from_uri (SOUP_METHOD_GET, uri);
+
+ if (async)
+ response = soup_test_session_async_send (session, msg, NULL, &error);
+ else
+ response = soup_session_send_and_read (session, msg, NULL, &error);
+
+ g_assert_null (response);
+ g_error_matches (error, G_IO_ERROR, G_IO_ERROR_FAILED);
+ g_assert_cmpstr (error->message, ==, "HTTP/2 Error: PROTOCOL_ERROR");
+ g_clear_error (&error);
+ g_object_unref (msg);
+ soup_test_session_abort_unref (session);
+
+ g_uri_unref (uri);
+}
+#endif
+
static void
content_sniffed (SoupMessage *msg,
char *content_type,
@@ -1211,6 +1272,15 @@ server_handler (SoupServer *server,
/* Use soup_message_headers_append_common to skip the validation check. */
soup_message_headers_append_common (response_headers, SOUP_HEADER_CONTENT_TYPE, "\r");
soup_server_message_set_status (msg, SOUP_STATUS_OK, NULL);
+ } else if (strcmp (path, "/invalid-header-rfc9113") == 0) {
+ SoupMessageHeaders *response_headers;
+
+ response_headers = soup_server_message_get_response_headers (msg);
+ soup_message_headers_append (response_headers, "Invalid-Header-Value", "foo ");
+ soup_server_message_set_response (msg, "text/plain",
+ SOUP_MEMORY_STATIC,
+ "Success!", 8);
+ soup_server_message_set_status (msg, SOUP_STATUS_OK, NULL);
}
}
@@ -1346,6 +1416,12 @@ main (int argc, char **argv)
setup_session,
do_invalid_header_received_test,
teardown_session);
+#ifdef HAVE_NGHTTP2_OPTION_SET_NO_RFC9113_LEADING_AND_TRAILING_WS_VALIDATION
+ g_test_add_data_func ("/http2/invalid-header-rfc9113-received/async", GINT_TO_POINTER (TRUE),
+ do_invalid_header_rfc9113_received_test);
+ g_test_add_data_func ("/http2/invalid-header-rfc9113-received/sync", GINT_TO_POINTER (FALSE),
+ do_invalid_header_rfc9113_received_test);
+#endif
g_test_add ("/http2/sniffer/async", Test, NULL,
setup_session,
do_sniffer_async_test,
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
