[PATCH] Upstream: fixed SSL initialization on read events

[Maxim Dounin]

Hello!

On Tue, Feb 10, 2026 at 12:49:49PM +0300, Vladimir Homutov via nginx-devel wrote:

> On Tue, Feb 10, 2026 at 05:56:15AM +0300, Maxim Dounin wrote:
> > Hello!
> >
> > On Mon, Feb 09, 2026 at 04:58:34PM +0300, Vladimir Homutov via nginx-devel wrote:
> >
> > > On Sun, Feb 08, 2026 at 11:23:13PM +0300, Maxim Dounin wrote:
> > > > # HG changeset patch
> > > > # User Maxim Dounin <mdounin at mdounin.ru>
> > > > # Date 1770581815 -10800
> > > > #      Sun Feb 08 23:16:55 2026 +0300
> > > > # Node ID bce5954f74a115503fd22d955d0fcea18f6c77cb
> > > > # Parent  b44cd6f3bc983859f3a213fbff19b4761ac59ec8
> > > > Upstream: fixed SSL initialization on read events.
> > > >
> > > > Previously, SSL initialization only happened on write events, which are
> > > > reported once a TCP connection is established.  However, if some data
> > > > are received before the write event is reported, the read event might be
> > > > reported first, potentially resulting in a plain text response being
> > > > accepted when it shouldn't (CVE-2026-1642).
> > > >
> > > > Normally SSL servers do not send anything before ClientHello from the
> > > > client, though formally they are allowed to (for example, HelloRequest
> > > > messages may be sent at any time).  As such, the fix is to do proper SSL
> > > > initialization on read events as well.  This ensures correct and identical
> > > > behaviour regardless of the order of events being reported.
> > >
> > > Hello,
> > >
> > > while the fix does the job, I think that the root reason
> > > is the fact that we set u->read_event_handler to ngx_http_upstream_process_header
> > > in ngx_http_upstream_connect(), when we haven't yet sent the request.
> > >
> > > And we have no idea what the request will be (maybe plaintext, maybe
> > > SSL, maybe some other protocol).
> > >
> > > It looks like the reasonable default for u->read_event_handler should be
> > > setting something like 'reject_premature_response'.
> > >
> > > And when we later connect and send the request, we may override it with
> > > appropriate handlers according to the desired protocol.
> >
> > I don't think that rejecting premature responses is a correct
> > approach, since at least in case of SSL server can legitimately
> > send data first: as mentioned in the commit log, HelloRequest
> > messages in TLSv1.2 may be sent at any time, see here:
> >
> > https://datatracker.ietf.org/doc/html/rfc5246#section-7.4.1.1
> >
> > While it says that "Servers SHOULD NOT send a HelloRequest
> > immediately upon the client's initial connection", servers still
> > can send it, either immediately or after some timeout, and this is
> > not expected to break things (but it will if we'll reject any
> > data before the write handler is triggered).
> >
> > Also I tend to think that in HTTP returning a response without an
> > actual request on the wire is also valid - e.g., 408 might be
> > returned due to a timeout before the request is received, or 503
> > might be sent due to some server issues without trying to read an
> > actual request.
> >
> > What probably can be done here is a block reading handler, so
> > reading will actually happen, but only after the write event
> > handler is triggered (and we'll decide what to do with the
> > connection).  It will indeed simplify things though, including
> > fixing some other issues (notably proper reinitialization when
> > switching to next upstream after a premature response, and
> > $upstream_connect_time calculation).
> >
> > A downside of this approach is that it doesn't allow optimized
> > handling of premature error responses.  On the other hand, it does
> > not look like a big issue, as such responses aren't common (if
> > exist at all).
> >
> > The patch below implements this approach.  What do you think?
> >
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at mdounin.ru>
> > # Date 1770691814 -10800
> > #      Tue Feb 10 05:50:14 2026 +0300
> > # Node ID 36c9c3fae57f47a5a6710cc754e52f79ae8dc0fe
> > # Parent  b44cd6f3bc983859f3a213fbff19b4761ac59ec8
> > Upstream: blocked read events till sending request.

[...]

> yes, this makes sense.
> 
> looks good for me.

Committed, thanks for looking into it.

-- 
Maxim Dounin
http://mdounin.ru/