TLS 1.3 server does not reliably reject ClientHello with non-null compression methods (possible regression of %236600)

Description

RFC 8446 Section 4.1.2 requires:

For every TLS 1.3 ClientHello, this vector MUST contain exactly one byte, set to zero, which corresponds to the "null" compression method in prior versions of TLS. If a TLS 1.3 ClientHello is received with any other value in this field, the server MUST abort the handshake with an "illegal_parameter" alert.

OpenSSL 3.1.8 does not consistently enforce this requirement. Under a specific parameter combination (non-null compression_methods, FFDHE3072 key share, ClientHello fragmented into 50-byte TLS records, with several optional extensions present), the server keeps the socket open without sending the required illegal_parameter alert.

Issue #6600 reported the same violation against master/1.1.1 and was closed as fixed. This report documents a recurrence (or incomplete fix) in the 3.1.x branch.

Affected Versions

• Confirmed: OpenSSL 3.1.8 (openssl-3.1.8-r1 on alpine:3.19)
• Claimed fixed: >= 1.1.1 (per #6600 resolution)

Steps to Reproduce

# Terminal 1: start TLS 1.3-only server
openssl req -x509 -newkey rsa:2048 -keyout /tmp/srv.key -out /tmp/srv.crt \
  -days 1 -nodes -subj "/CN=test"
openssl s_server -tls1_3 -accept 0.0.0.0:4433 -key /tmp/srv.key -cert /tmp/srv.crt

# Terminal 2: repro_compression.py  (requires: python3)
# Sends a TLS 1.3 ClientHello with compression_methods=[DEFLATE,null] fragmented
# into 50-byte TLS records, with FFDHE3072 key share and optional extensions.
# This is the exact parameter combination that triggers the bug.
import socket, struct, os, time, select, subprocess, tempfile

def u16(n): return struct.pack('>H', n)
def u8(n):  return struct.pack('B', n)

GREASE = b'\x0a\x0a'
random_bytes = os.urandom(32)
session_id   = os.urandom(32)

cipher_suites = GREASE + b'\x13\x01'                    # TLS_AES_128_GCM_SHA256
compression_methods = bytes([0x02, 0x01, 0x00])         # [DEFLATE, null] -- invalid for TLS 1.3

exts = b''
exts += struct.pack('>HHB', 0x002b, 3, 2) + b'\x03\x04'   # supported_versions: TLS 1.3
groups = GREASE + b'\x01\x01\x00\x1d\x00\x17'
exts += struct.pack('>HH', 0x000a, 2+len(groups)) + u16(len(groups)) + groups  # supported_groups
ffdhe3072_pub = os.urandom(384)                          # 384-byte FFDHE3072 public key
ks = b'\x01\x01' + u16(384) + ffdhe3072_pub
exts += struct.pack('>HH', 0x0033, 2+len(ks)) + u16(len(ks)) + ks             # key_share
sig = GREASE + b'\x04\x03\x05\x03\x06\x03\x08\x07\x08\x08\x04\x01\x05\x01\x06\x01'
exts += struct.pack('>HH', 0x000d, 2+len(sig)) + u16(len(sig)) + sig          # sig_algs
exts += struct.pack('>HHB', 0x002d, 2, 1) + b'\x01'    # psk_key_exchange_modes
exts += struct.pack('>HH', 0x0017, 0)                   # extended_master_secret
exts += struct.pack('>HH', 0x0023, 0)                   # session_ticket
exts += struct.pack('>HHB', 0xff01, 1, 0)               # renegotiation_info
alpn = b'\x08http/1.1'
exts += struct.pack('>HH', 0x0010, 4+len(alpn)) + u16(2+len(alpn)) + u16(len(alpn)) + alpn
exts += struct.pack('>HHB', 0x000f, 1, 1)               # heartbeat
exts += struct.pack('>HHB', 0x0001, 1, 2)               # max_fragment_length: 512
pad = max(0, 600 - (4+32+1+32+2+len(cipher_suites)+len(compression_methods)+2+len(exts)) - 4)
exts += struct.pack('>HH', 0x0015, pad) + b'\x00'*pad   # padding

body = (b'\x03\x03' + random_bytes + u8(len(session_id)) + session_id +
        u16(len(cipher_suites)) + cipher_suites + compression_methods +
        u16(len(exts)) + exts)
handshake = b'\x01' + struct.pack('>I', len(body))[1:] + body

# Fragment into 50-byte TLS records -- this is the key trigger
wire = b''
for i in range(0, len(handshake), 50):
    c = handshake[i:i+50]
    wire += b'\x16\x03\x01' + u16(len(c)) + c
wire += b'\x14\x03\x03\x00\x01\x01'                    # compat CCS

s = socket.create_connection(('127.0.0.1', 4433))
s.sendall(wire)

chunks = b''
deadline = time.time() + 4
while time.time() < deadline:
    r, _, _ = select.select([s], [], [], 0.2)
    if r:
        try:
            d = s.recv(4096)
            if not d: break
            chunks += d
            if chunks[0] == 0x15: break
        except OSError: break

s.close()
if not chunks:
    print("FAIL: no response -- socket stayed open, no alert sent")
elif chunks[0] == 0x15 and len(chunks) >= 7:
    lvl, desc = chunks[5], chunks[6]
    print(f"Alert: level={lvl} desc={desc}")
    print("PASS" if lvl == 2 and desc == 47 else "UNEXPECTED alert")
else:
    print(f"Other: {chunks[:8].hex()}")

Expected:

Alert: level=2 desc=47
PASS

Actual (OpenSSL 3.1.8, s_server -tls1_3):

FAIL: no response -- socket stayed open, no alert sent

Reproduction with tls-anvil (exhaustive)

tls-anvil against openssl s_server -tls1_3 (strength=1):

Test:   TLS13.ClientHello.invalidCompression
Result: PARTIALLY_FAILED
        1 out of 11 parameter combinations -> socket still open, no alert sent
        Remaining 10/11 -> correct illegal_parameter(47) alert

Failure message:
  "Expected a fatal alert but no messages have been received and socket is still open"

Failing combination:
  CIPHER_SUITE=TLS_AES_128_GCM_SHA256, NAMED_GROUP=FFDHE3072,
  RECORD_LENGTH=50, MAX_FRAGMENT_LENGTH=TWO_9, all optional extensions enabled

Code Location (Hypothesis)

The check exists in ssl/statem/statem_srvr.c:

if (SSL_CONNECTION_IS_TLS13(s)) {
    if (clienthello->compressions_len != 1) {
        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,
            SSL_R_INVALID_COMPRESSION_ALGORITHM);
        goto err;
    }
}

The hypothesis is that SSL_CONNECTION_IS_TLS13(s) at this point in the parsing flow depends on supported_versions having already been parsed and the TLS 1.3 version confirmed. When the ClientHello is heavily fragmented (50-byte records) and contains many extensions, the version identity may not yet be established when this check runs, causing the guard to evaluate as false and the TLS 1.2 path (which permits non-null compression) to be taken instead.

This analysis is based on code inspection; the exact failing condition was identified via tls-anvil exhaustive testing rather than a debugger trace.