Skip to content

Fix Missing ECH (0xfe0d) and ALPS (0x44cd) extensions - #42

Merged
vlvkobal merged 1 commit into
FoxIO-LLC:mainfrom
12189108:main
Jan 13, 2026
Merged

Fix Missing ECH (0xfe0d) and ALPS (0x44cd) extensions#42
vlvkobal merged 1 commit into
FoxIO-LLC:mainfrom
12189108:main

Conversation

@12189108

@12189108 12189108 commented Jan 10, 2026

Copy link
Copy Markdown

Fix missing ECH and ALPS extensions

  • Replace SSL_client_hello_get1_extensions_present() with raw ClientHello parsing
  • Capture all TLS extensions including ECH (0xfe0d) and ALPS (0x44cd)
  • Add SCSV detection (TLS_EMPTY_RENEGOTIATION_INFO_SCSV)
  • Implement QUIC CRYPTO frame reassembly for fragmented ClientHello
  • Support both TLS and QUIC connections with complete extension visibility

Patch generated from nginx 1.28.1 source code

@vlvkobal

Copy link
Copy Markdown
Collaborator

Hi @12189108, Thanks for the fix!

Two quick questions on buffer safety:

  1. QUIC (ngx_event_quic_ssl.c): The alloc is based on the header length, but memcpy is based on the incoming frame len. If a frame sends more data than the header claimed, isn't that an overflow?
  2. ALPN (ngx_http_ssl_module.c): Is it safe to trust in[0] without checking inlen?

@vlvkobal

Copy link
Copy Markdown
Collaborator

Also, please update NGINX_VERSION.

@12189108

Copy link
Copy Markdown
Author

@vlvkobal , Thanks for raising these security issues! Fixed:

QUIC overflow: added bounds check before memcpy to prevent frames from exceeding header's declared length. Truncates and logs alert on overflow.

ALPN overflow: This was introduced by a previous commit, not by me. Fixed by validating inlen > 0 before accessing in[0] and ensuring buffer has sufficient data (inlen >= alpn_len + 1).

Both fixes handle malformed input gracefully while maintaining compatibility.

Note: Patches have been regenerated against the latest nginx code.

1. TLS/QUIC Extension Parsing:
   - Replaced SSL_client_hello_get1_extensions_present() with raw ClientHello parsing to capture all TLS extensions, specifically ECH (0xfe0d) and ALPS (0x44cd).
   - Added detection for TLS_EMPTY_RENEGOTIATION_INFO_SCSV.
   - Implemented QUIC CRYPTO frame reassembly for fragmented ClientHello messages.
   - Ensured complete extension visibility for both TLS and QUIC connections.

2. Security Fixes:
   - QUIC Overflow: Added bounds check before memcpy to prevent frames from exceeding the header's declared length. The system now truncates and logs an alert on overflow.
   - ALPN Overflow: Fixed a regression from a previous commit by validating inlen > 0 before accessing in[0] and ensuring the buffer has sufficient data (inlen >= alpn_len + 1). Both fixes handle malformed input gracefully.

3. Dependencies:
   - Bumped Nginx version to 1.28.1 in Dockerfile.
   - Bumped OpenSSL version to 3.5.4 in Dockerfile.
@vlvkobal
vlvkobal merged commit e0c1eab into FoxIO-LLC:main Jan 13, 2026
1 check passed
@vlvkobal

Copy link
Copy Markdown
Collaborator

Could you add tests for the ECH and ALPS extensions?

@vlvkobal vlvkobal linked an issue Jan 13, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]Missing ECH (0xfe0d) and ALPS (0x44cd) extensions

2 participants