mirror of https://github.com/openssl/openssl.git
0fa07898e1
Recently, our overnight QUIC interop runs began failing in CI when an openssl server was tested against an ngtcp2 client: https://github.com/openssl/openssl/actions/runs/16739736813 The underlying cause bears some explination for historical purposes The problem began happening with a recent update to ngtcp2 in which ngtcp2 updated its wolfssl tls backend to support ML-KEM, which caused ngtcp to emit a client hello message that offered several groups (including X25519MLKEM768) but only provided a keyshare for x25519. This in turn triggered the openssl server to respond with a hello retry request (HRR), requesting an ML-KEM keyshare instead, which ngtcp2 obliged. However all subsequent frames from the client were discarded by the server, due to failing packet body decryption. The problem was tracked down to a mismatch in the initial vectors used by the client and server, leading to an AEAD tag mismatch. Packet protection keys generate their IV's in QUIC by xoring the packet number of the received frame with the base IV as derived via HKDF in the tls layer. The underlying problem was that openssl hit a very odd corner case with how we compute the packet number of the received frame. To save space, QUIC encodes packet numbers using a variable length integer, and only sends the changed bits in the packet number. This requires that the receiver (openssl) store the largest received pn of the connection, which we nominally do. However, in default_port_packet_handler (where QUIC frames are processed prior to having an established channel allocated) we use a temporary qrx to validate the packet protection of those frames. This temporary qrx may be incorporated into the channel in some cases, but is not in the case of a valid frame that generates an HRR at the TLS layer. In this case, the channel allocates its own qrx independently. When this occurs, the largest_pn value of the temporary qrx is lost, and subsequent frames are unable to be received, as the newly allocated qrx belives that the larges_pn for a given pn_space is 0, rather than the value received in the initial frame (which was a complete 32 bit value, rather than just the changed lower 8 bits). As a result the IV construction produced the wrong value, and the decrypt failed on those subsequent frames. Up to this point, that wasn't even a problem, as most quic implementations start their packet numbering at 0, so the next packet could still have its packet number computed properly. The combination of ngtcp using large random values for initial packet numbers, along with the HRR triggering a separate qrx creation on a channel led to the discovery of this discrepancy. The fix seems pretty straightforward. When we detect in port_default_packet_handler, that we have a separate qrx in the new channel, we migrate processed packets from the temporary qrx to the canonical channel qrx. In addition to doing that, we also need to migrate the largest_pn array from the temporary qrx to the channel_qrx so that subsequent frame reception is guaranteed to compute the received frame packet number properly, and as such, compute the proper IV for packet protection decryption. Fixes openssl/project#1296 Reviewed-by: Saša Nedvědický <sashan@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/28189) |
||
|---|---|---|
| .. | ||
| __DECC_INCLUDE_EPILOGUE.H | ||
| __DECC_INCLUDE_PROLOGUE.H | ||
| asn1.h | ||
| bio.h | ||
| bio_addr.h | ||
| bio_tfo.h | ||
| cms.h | ||
| common.h | ||
| comp.h | ||
| conf.h | ||
| constant_time.h | ||
| core.h | ||
| crmf.h | ||
| cryptlib.h | ||
| dane.h | ||
| deprecated.h | ||
| der.h | ||
| deterministic_nonce.h | ||
| dso.h | ||
| dsoerr.h | ||
| e_os.h | ||
| e_winsock.h | ||
| encoder.h | ||
| endian.h | ||
| err.h | ||
| ffc.h | ||
| fips.h | ||
| hashfunc.h | ||
| hashtable.h | ||
| hpke_util.h | ||
| json_enc.h | ||
| ktls.h | ||
| list.h | ||
| namemap.h | ||
| nelem.h | ||
| numbers.h | ||
| o_dir.h | ||
| packet.h | ||
| packet_quic.h | ||
| param_build_set.h | ||
| param_names.h.in | ||
| params.h | ||
| passphrase.h | ||
| priority_queue.h | ||
| property.h | ||
| propertyerr.h | ||
| provider.h | ||
| qlog.h | ||
| qlog_event_helpers.h | ||
| qlog_events.h | ||
| quic_ackm.h | ||
| quic_cc.h | ||
| quic_cfq.h | ||
| quic_channel.h | ||
| quic_demux.h | ||
| quic_engine.h | ||
| quic_error.h | ||
| quic_fc.h | ||
| quic_fifd.h | ||
| quic_lcidm.h | ||
| quic_port.h | ||
| quic_predef.h | ||
| quic_rcidm.h | ||
| quic_reactor.h | ||
| quic_reactor_wait_ctx.h | ||
| quic_record_rx.h | ||
| quic_record_tx.h | ||
| quic_record_util.h | ||
| quic_rx_depack.h | ||
| quic_sf_list.h | ||
| quic_srt_gen.h | ||
| quic_srtm.h | ||
| quic_ssl.h | ||
| quic_statm.h | ||
| quic_stream.h | ||
| quic_stream_map.h | ||
| quic_thread_assist.h | ||
| quic_tls.h | ||
| quic_trace.h | ||
| quic_tserver.h | ||
| quic_txp.h | ||
| quic_txpim.h | ||
| quic_types.h | ||
| quic_vlint.h | ||
| quic_wire.h | ||
| quic_wire_pkt.h | ||
| rcu.h | ||
| recordmethod.h | ||
| refcount.h | ||
| ring_buf.h | ||
| rio_notifier.h | ||
| safe_math.h | ||
| sha3.h | ||
| sizes.h | ||
| skey.h | ||
| sm3.h | ||
| sockets.h | ||
| ssl.h | ||
| ssl3_cbc.h | ||
| ssl_unwrap.h | ||
| sslconf.h | ||
| statem.h | ||
| symhacks.h | ||
| thread.h | ||
| thread_arch.h | ||
| thread_once.h | ||
| time.h | ||
| tlsgroups.h | ||
| to_hex.h | ||
| tsan_assist.h | ||
| uint_set.h | ||
| unicode.h | ||