Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specifying the HTTP/2 SETTINGS_MAX_HEADER_LIST_SIZE #2826

Closed
qinyushun opened this issue May 5, 2022 · 5 comments · Fixed by #2828
Closed

Allow specifying the HTTP/2 SETTINGS_MAX_HEADER_LIST_SIZE #2826

qinyushun opened this issue May 5, 2022 · 5 comments · Fixed by #2828
Labels
A-http2 Area: HTTP/2 specific. A-server Area: server. C-feature Category: feature. This is adding a new feature. E-easy Effort: easy. A task that would be a great starting point for a new contributor.

Comments

@qinyushun
Copy link

qinyushun commented May 5, 2022

Version
0.14.12

Description
Hyper does not open the customization of the max_header_list_size method in the H2 third-party software.
The H2 interface is as follows:

/// Sets the max size of received header frames.
    ///
    /// This advisory setting informs a peer of the maximum size of header list
    /// that the sender is prepared to accept, in octets. The value is based on
    /// the uncompressed size of header fields, including the length of the name
    /// and value in octets plus an overhead of 32 octets for each header field.
    ///
    /// This setting is also used to limit the maximum amount of data that is
    /// buffered to decode HEADERS frames.
    ///
    /// # Examples
    ///
    /// ```
    /// # use tokio::io::{AsyncRead, AsyncWrite};
    /// # use h2::server::*;
    /// #
    /// # fn doc<T: AsyncRead + AsyncWrite + Unpin>(my_io: T)
    /// # -> Handshake<T>
    /// # {
    /// // `server_fut` is a future representing the completion of the HTTP/2.0
    /// // handshake.
    /// let server_fut = Builder::new()
    ///     .max_header_list_size(16 * 1024)
    ///     .handshake(my_io);
    /// # server_fut
    /// # }
    /// #
    /// # pub fn main() {}
    /// ```
    pub fn max_header_list_size(&mut self, max: u32) -> &mut Self {
        self.settings.set_max_header_list_size(Some(max));
        self
    }

The default value of max_header_list_size in h2 is 16m.
As a result, HTTP2 attacks may occur when Hyper is used.

Attack scenario: Hyper is used as the server and continues to send continuance frames without ending. The data of each continuance frame is 10 KB. Each thread sends 1023 continuance frames, that is, about 10 MB. Multiple threads are started to send continuance frames. As a result, the memory of the service is exploded.

@qinyushun qinyushun added the C-bug Category: bug. Something is wrong. This is bad! label May 5, 2022
@lidong14
Copy link

lidong14 commented May 6, 2022

@seanmonstar Unlimited max_header_list_size leads to uncontrolled memory,It is not safe for the service

@seanmonstar
Copy link
Member

Adding a builder option would be straightforward, and welcome!

@seanmonstar seanmonstar added A-http2 Area: HTTP/2 specific. E-easy Effort: easy. A task that would be a great starting point for a new contributor. C-feature Category: feature. This is adding a new feature. A-server Area: server. and removed C-bug Category: bug. Something is wrong. This is bad! labels May 6, 2022
@qinyushun
Copy link
Author

The max_header_list_size option of H2 is critical. Why did you not add it? What are the considerations?

h2/src/codec/framed_read.rs
...
                // Extend the buf
                if partial.buf.is_empty() {
                    partial.buf = bytes.split_off(frame::HEADER_LEN);
                } else {
                    if partial.frame.is_over_size() {
                        // If there was left over bytes previously, they may be
                        // needed to continue decoding, even though we will
                        // be ignoring this frame. This is done to keep the HPACK
                        // decoder state up-to-date.
                        //
                        // Still, we need to be careful, because if a malicious
                        // attacker were to try to send a gigantic string, such
                        // that it fits over multiple header blocks, we could
                        // grow memory uncontrollably again, and that'd be a shame.
                        //
                        // Instead, we use a simple heuristic to determine if
                        // we should continue to ignore decoding, or to tell
                        // the attacker to go away.
                        if partial.buf.len() + bytes.len() > self.max_header_list_size {
                            proto_err!(conn: "CONTINUATION frame header block size over ignorable limit");
                            return Err(Connection(Reason::COMPRESSION_ERROR));
                        }
                    }
                    partial.buf.extend_from_slice(&bytes[frame::HEADER_LEN..]);
                }
...

@seanmonstar
Copy link
Member

A pull request is always welcome :)

seanmonstar pushed a commit that referenced this issue May 18, 2022
)

This allows setting the HTTP/2 `SETTINGS_MAX_HEADER_LIST_SIZE` which advertises to the peer the maximum header size allowed, and internally is enforced.

Closes #2826
@seanmonstar seanmonstar changed the title Hyper does not open the customization of the max_header_list_size method in the H2 third-party software. As a result, HTTP2 attacks may occur when Hyper is used. Allow specifying the HTTP/2 SETTINGS_MAX_HEADER_LIST_SIZE May 18, 2022
@JanZerebecki
Copy link

Note, the client is vulnerable to this, too, but is not yet fixed.

I don't currently have a need for it being fixed, just wanted to document it, as discussion elsewhere was confused about this:

Both directions of HTTP have headers, v2 specs the related advisory setting for both directions. The underlying h2 does have this setting for both client and server since almost its beginning, which comes with a default limit of 16MB.

For this to be a problem the application does need to have some thing else going on whose denial of service by memory exhaustion would be a problem. This something else might be requests to other servers that are not in the same security compartment. Or having taken a lock that needs to be released. Or it might be even a totally unrelated thing, like showing a GUI. Based on the 16MB default and the maximum outstanding in flight requests and the available memory you can calculate if this can be a problem. So a CLI application that just one command, which does one request, then exits anyway and is prepared to be killed at any time (instead of needing to avoid allocation failures) doesn't need this setting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-http2 Area: HTTP/2 specific. A-server Area: server. C-feature Category: feature. This is adding a new feature. E-easy Effort: easy. A task that would be a great starting point for a new contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants