Compare commits

...

4 Commits

Author SHA1 Message Date
Tatsuhiro Tsujikawa
781057e156 Update manual pages 2023-07-14 22:36:35 +09:00
Tatsuhiro Tsujikawa
2f87b9c4f8 Bump package and library versions 2023-07-14 22:31:21 +09:00
Tatsuhiro Tsujikawa
299d2fdaf9 doc: Bump boringssl 2023-07-14 22:21:19 +09:00
Tatsuhiro Tsujikawa
8720afa304 Fix memory leak
This commit fixes memory leak that happens when PUSH_PROMISE or
HEADERS frame cannot be sent, and nghttp2_on_stream_close_callback
fails with a fatal error.  For example, if GOAWAY frame has been
received, a HEADERS frame that opens new stream cannot be sent.

This issue has already been made public via CVE-2023-35945 [1] issued
by envoyproxy/envoy project.  During embargo period, the patch to fix
this bug was accidentally submitted to nghttp2/nghttp2 repository [2].
And they decided to disclose CVE early.  I was notified just 1.5 hours
before disclosure.  I had no time to respond.

PoC described in [1] is quite simple, but I think it is not enough to
trigger this bug.  While it is true that receiving GOAWAY prevents a
client from opening new stream, and nghttp2 enters error handling
branch, in order to cause the memory leak,
nghttp2_session_close_stream function must return a fatal error.
nghttp2 defines 2 fatal error codes:

- NGHTTP2_ERR_NOMEM
- NGHTTP2_ERR_CALLBACK_FAILURE

NGHTTP2_ERR_NOMEM, as its name suggests, indicates out of memory.  It
is unlikely that a process gets short of memory with this simple PoC
scenario unless application does something memory heavy processing.

NGHTTP2_ERR_CALLBACK_FAILURE is returned from application defined
callback function (nghttp2_on_stream_close_callback, in this case),
which indicates something fatal happened inside a callback, and a
connection must be closed immediately without any further action.  As
nghttp2_on_stream_close_error_callback documentation says, any error
code other than 0 or NGHTTP2_ERR_CALLBACK_FAILURE is treated as fatal
error code.  More specifically, it is treated as if
NGHTTP2_ERR_CALLBACK_FAILURE is returned.  I guess that envoy returns
NGHTTP2_ERR_CALLBACK_FAILURE or other error code which is translated
into NGHTTP2_ERR_CALLBACK_FAILURE.

[1] https://github.com/envoyproxy/envoy/security/advisories/GHSA-jfxv-29pc-x22r
[2] https://github.com/nghttp2/nghttp2/pull/1929
2023-07-14 21:57:59 +09:00
9 changed files with 48 additions and 14 deletions

View File

@@ -24,12 +24,12 @@
cmake_minimum_required(VERSION 3.0) cmake_minimum_required(VERSION 3.0)
# XXX using 1.8.90 instead of 1.9.0-DEV # XXX using 1.8.90 instead of 1.9.0-DEV
project(nghttp2 VERSION 1.55.0) project(nghttp2 VERSION 1.55.1)
# See versioning rule: # See versioning rule:
# https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html # https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
set(LT_CURRENT 38) set(LT_CURRENT 38)
set(LT_REVISION 2) set(LT_REVISION 3)
set(LT_AGE 24) set(LT_AGE 24)
set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH}) set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake" ${CMAKE_MODULE_PATH})

View File

@@ -129,7 +129,7 @@ following libraries are required:
* `OpenSSL with QUIC support * `OpenSSL with QUIC support
<https://github.com/quictls/openssl/tree/OpenSSL_1_1_1u+quic>`_; or <https://github.com/quictls/openssl/tree/OpenSSL_1_1_1u+quic>`_; or
`BoringSSL <https://boringssl.googlesource.com/boringssl/>`_ (commit `BoringSSL <https://boringssl.googlesource.com/boringssl/>`_ (commit
b0341041b03ea71d8371a9692aedae263fc06ee9) 80dcb67d4481fb1194b9669917e35580c32dc388)
* `ngtcp2 <https://github.com/ngtcp2/ngtcp2>`_ 0.17.x * `ngtcp2 <https://github.com/ngtcp2/ngtcp2>`_ 0.17.x
* `nghttp3 <https://github.com/ngtcp2/nghttp3>`_ 0.13.x * `nghttp3 <https://github.com/ngtcp2/nghttp3>`_ 0.13.x

View File

@@ -25,7 +25,7 @@ dnl Do not change user variables!
dnl https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html dnl https://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html
AC_PREREQ(2.61) AC_PREREQ(2.61)
AC_INIT([nghttp2], [1.55.0], [t-tujikawa@users.sourceforge.net]) AC_INIT([nghttp2], [1.55.1], [t-tujikawa@users.sourceforge.net])
AC_CONFIG_AUX_DIR([.]) AC_CONFIG_AUX_DIR([.])
AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_MACRO_DIR([m4])
AC_CONFIG_HEADERS([config.h]) AC_CONFIG_HEADERS([config.h])
@@ -45,7 +45,7 @@ m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])])
dnl See versioning rule: dnl See versioning rule:
dnl https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html dnl https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
AC_SUBST(LT_CURRENT, 38) AC_SUBST(LT_CURRENT, 38)
AC_SUBST(LT_REVISION, 2) AC_SUBST(LT_REVISION, 3)
AC_SUBST(LT_AGE, 24) AC_SUBST(LT_AGE, 24)
major=`echo $PACKAGE_VERSION |cut -d. -f1 | sed -e "s/[^0-9]//g"` major=`echo $PACKAGE_VERSION |cut -d. -f1 | sed -e "s/[^0-9]//g"`

View File

@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
.\" new: \\n[rst2man-indent\\n[rst2man-indent-level]] .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
.in \\n[rst2man-indent\\n[rst2man-indent-level]]u .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
.. ..
.TH "H2LOAD" "1" "Jul 12, 2023" "1.55.0" "nghttp2" .TH "H2LOAD" "1" "Jul 14, 2023" "1.55.1" "nghttp2"
.SH NAME .SH NAME
h2load \- HTTP/2 benchmarking tool h2load \- HTTP/2 benchmarking tool
.SH SYNOPSIS .SH SYNOPSIS

View File

@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
.\" new: \\n[rst2man-indent\\n[rst2man-indent-level]] .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
.in \\n[rst2man-indent\\n[rst2man-indent-level]]u .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
.. ..
.TH "NGHTTP" "1" "Jul 12, 2023" "1.55.0" "nghttp2" .TH "NGHTTP" "1" "Jul 14, 2023" "1.55.1" "nghttp2"
.SH NAME .SH NAME
nghttp \- HTTP/2 client nghttp \- HTTP/2 client
.SH SYNOPSIS .SH SYNOPSIS

View File

@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
.\" new: \\n[rst2man-indent\\n[rst2man-indent-level]] .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
.in \\n[rst2man-indent\\n[rst2man-indent-level]]u .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
.. ..
.TH "NGHTTPD" "1" "Jul 12, 2023" "1.55.0" "nghttp2" .TH "NGHTTPD" "1" "Jul 14, 2023" "1.55.1" "nghttp2"
.SH NAME .SH NAME
nghttpd \- HTTP/2 server nghttpd \- HTTP/2 server
.SH SYNOPSIS .SH SYNOPSIS

View File

@@ -27,7 +27,7 @@ level margin: \\n[rst2man-indent\\n[rst2man-indent-level]]
.\" new: \\n[rst2man-indent\\n[rst2man-indent-level]] .\" new: \\n[rst2man-indent\\n[rst2man-indent-level]]
.in \\n[rst2man-indent\\n[rst2man-indent-level]]u .in \\n[rst2man-indent\\n[rst2man-indent-level]]u
.. ..
.TH "NGHTTPX" "1" "Jul 12, 2023" "1.55.0" "nghttp2" .TH "NGHTTPX" "1" "Jul 14, 2023" "1.55.1" "nghttp2"
.SH NAME .SH NAME
nghttpx \- HTTP/2 proxy nghttpx \- HTTP/2 proxy
.SH SYNOPSIS .SH SYNOPSIS

View File

@@ -3296,6 +3296,7 @@ static ssize_t nghttp2_session_mem_send_internal(nghttp2_session *session,
if (rv < 0) { if (rv < 0) {
int32_t opened_stream_id = 0; int32_t opened_stream_id = 0;
uint32_t error_code = NGHTTP2_INTERNAL_ERROR; uint32_t error_code = NGHTTP2_INTERNAL_ERROR;
int rv2 = 0;
DEBUGF("send: frame preparation failed with %s\n", DEBUGF("send: frame preparation failed with %s\n",
nghttp2_strerror(rv)); nghttp2_strerror(rv));
@@ -3338,19 +3339,18 @@ static ssize_t nghttp2_session_mem_send_internal(nghttp2_session *session,
} }
if (opened_stream_id) { if (opened_stream_id) {
/* careful not to override rv */ /* careful not to override rv */
int rv2;
rv2 = nghttp2_session_close_stream(session, opened_stream_id, rv2 = nghttp2_session_close_stream(session, opened_stream_id,
error_code); error_code);
if (nghttp2_is_fatal(rv2)) {
return rv2;
}
} }
nghttp2_outbound_item_free(item, mem); nghttp2_outbound_item_free(item, mem);
nghttp2_mem_free(mem, item); nghttp2_mem_free(mem, item);
active_outbound_item_reset(aob, mem); active_outbound_item_reset(aob, mem);
if (nghttp2_is_fatal(rv2)) {
return rv2;
}
if (rv == NGHTTP2_ERR_HEADER_COMP) { if (rv == NGHTTP2_ERR_HEADER_COMP) {
/* If header compression error occurred, should terminiate /* If header compression error occurred, should terminiate
connection. */ connection. */

View File

@@ -584,6 +584,15 @@ static int on_stream_close_callback(nghttp2_session *session, int32_t stream_id,
return 0; return 0;
} }
static int fatal_error_on_stream_close_callback(nghttp2_session *session,
int32_t stream_id,
uint32_t error_code,
void *user_data) {
on_stream_close_callback(session, stream_id, error_code, user_data);
return NGHTTP2_ERR_CALLBACK_FAILURE;
}
static ssize_t pack_extension_callback(nghttp2_session *session, uint8_t *buf, static ssize_t pack_extension_callback(nghttp2_session *session, uint8_t *buf,
size_t len, const nghttp2_frame *frame, size_t len, const nghttp2_frame *frame,
void *user_data) { void *user_data) {
@@ -4296,6 +4305,8 @@ void test_nghttp2_session_on_goaway_received(void) {
nghttp2_frame frame; nghttp2_frame frame;
int i; int i;
nghttp2_mem *mem; nghttp2_mem *mem;
const uint8_t *data;
ssize_t datalen;
mem = nghttp2_mem_default(); mem = nghttp2_mem_default();
user_data.frame_recv_cb_called = 0; user_data.frame_recv_cb_called = 0;
@@ -4337,6 +4348,29 @@ void test_nghttp2_session_on_goaway_received(void) {
nghttp2_frame_goaway_free(&frame.goaway, mem); nghttp2_frame_goaway_free(&frame.goaway, mem);
nghttp2_session_del(session); nghttp2_session_del(session);
/* Make sure that no memory leak when stream_close callback fails
with a fatal error */
memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
callbacks.on_stream_close_callback = fatal_error_on_stream_close_callback;
memset(&user_data, 0, sizeof(user_data));
nghttp2_session_client_new(&session, &callbacks, &user_data);
nghttp2_frame_goaway_init(&frame.goaway, 0, NGHTTP2_NO_ERROR, NULL, 0);
CU_ASSERT(0 == nghttp2_session_on_goaway_received(session, &frame));
nghttp2_submit_request(session, NULL, reqnv, ARRLEN(reqnv), NULL, NULL);
datalen = nghttp2_session_mem_send(session, &data);
CU_ASSERT(NGHTTP2_ERR_CALLBACK_FAILURE == datalen);
CU_ASSERT(1 == user_data.stream_close_cb_called);
nghttp2_frame_goaway_free(&frame.goaway, mem);
nghttp2_session_del(session);
} }
void test_nghttp2_session_on_window_update_received(void) { void test_nghttp2_session_on_window_update_received(void) {