nodejs/node

http2: cannot negotiate ALPN besides http/1.1

Open

#26,835 opened on Mar 21, 2019

View on GitHub
 (9 comments) (0 reactions) (0 assignees)JavaScript (117,218 stars) (35,535 forks)batch import
help wantedhttp2

Description

The documentation for 'unknownProtocol' says this:

The 'unknownProtocol' event is emitted when a connecting client fails to negotiate an allowed protocol (i.e. HTTP/2 or HTTP/1.1). The event handler receives the socket for handling. If no listener is registered for this event, the connection is terminated.

The logic seems wrong though. It only passes through nothing (no protocol negotiated) or http/1.1, everything else is ignored:

https://github.com/nodejs/node/blob/11f8024d992c385d3db196ab64678311bfdabd84/lib/internal/http2/core.js#L2614-L2634

Caveat: if the check is loosened, care should be taken not to introduce an information leak.

For an attacker it should not be possible to deduce whether the server has { allowHTTP1: true } and an 'unknownProtocol' listener installed by sending messages with the ALPN proto set to http/1.1 and e.g. hax/13.37, and then comparing the responses he gets back.

Contributor guide

Tech stack
javascriptnodejs
Domain
backendsecurity
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
3
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
1-3 hours
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
basic understanding of HTTP/2 ALPNfamiliarity with Node.js http2 module
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
40
Research direction
Investigate the ALPN negotiation logic in lib/internal/http2/core.js at lines 2614-2634. The issue highlights that only http/1.1 is allowed, but other protocols should be passed to the 'unknownProtocol' event. Ensure any fix does not leak information about server configuration, e.g., by not distinguishing between 'http/1.1' and unknown protocols. Consider adding tests for various ALPN strings.