Make QUIC connection ID creation explicit

[bneradt]

Default-constructing a QUIC connection ID generates random bytes, even
when the value is only a placeholder that will be overwritten. Several
generation paths also randomize the same ID a second time.

This patch makes empty, decoded, and newly generated IDs explicit. It
deletes default construction, adds a checked CSPRNG-backed factory, and
adds coverage for the connection ID representation and initialization
contract.

Fixes: #5504

[Copilot]

Pull request overview

This PR makes QUIC connection ID (CID) initialization explicit across ATS’s QUIC and QMUX codepaths, removing implicit/random default construction and centralizing CID generation behind a CSPRNG-backed factory to avoid unnecessary work and double-randomization (Fixes: #5504).

Changes:

• Delete QUICConnectionId default construction and replace call sites with explicit ZERO() (empty) or random() (generated) initialization.
• Implement QUICConnectionId::random() using OpenSSL RAND_bytes and remove the legacy randomize() implementation.
• Add a Catch2 unit test validating the CID representation/initialization contract and wire it into test_net.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/iocore/net/unit_tests/test_QUICConnectionId.cc	Adds unit coverage for explicit CID initialization, decoding, hashing, and random generation.
src/iocore/net/QUICPacketHandler.cc	Switches retry/new-connection CID creation to QUICConnectionId::random() to avoid placeholder randomization and double-randomization.
src/iocore/net/QUICNetProcessor.cc	Uses explicit random CID generation for client connect.
src/iocore/net/QUICNetVConnection.cc	Replaces CID randomization/default returns with explicit random() and ZERO() placeholders.
src/iocore/net/OpenSSLQUICNetVConnection.cc	Uses random() for local CID generation and replaces {} returns with ZERO().
src/iocore/net/P_QUICNetVConnection.h	Adds in-class ZERO() initialization for CID members now that default construction is deleted.
src/iocore/net/quic/QUICTypes.cc	Removes default-ctor randomization and legacy randomize(), adds OpenSSL-backed random().
include/iocore/net/quic/QUICTypes.h	Declares random(), deletes default ctor, removes randomize(), and zero-initializes internal storage.
src/iocore/net/qmux/QMuxConnection.cc	Initializes synthetic CID explicitly via random() in the ctor initializer list.
src/iocore/net/CMakeLists.txt	Adds the new QUICConnectionId unit test to test_net when QUIC or QMUX is enabled.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/iocore/net/unit_tests/test_QUICConnectionId.cc:63

• The test mutates the global QUICConnectionId::SCID_LEN and manually restores it. If this section ever gains an early-exit (e.g., via REQUIRE/throw), the restore could be skipped and leak global state into other tests. Use an RAII guard to restore SCID_LEN on scope exit.

  {
    uint8_t const  previous_scid_len = QUICConnectionId::SCID_LEN;
    ts::PostScript restore_scid_len([previous_scid_len]() -> void { QUICConnectionId::SCID_LEN = previous_scid_len; });

    QUICConnectionId::SCID_LEN = 18;

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/iocore/net/QUICNetVConnection.cc:476

• initial_source_connection_id() currently always returns ZERO(), but init() sets _initial_source_connection_id to the generated local connection ID. Returning ZERO breaks the connection ID initialization contract for QUICHE builds.

QUICConnectionId
QUICNetVConnection::initial_source_connection_id() const
{
  return QUICConnectionId::ZERO();
}

src/iocore/net/QUICNetVConnection.cc:482

• connection_id() currently always returns ZERO(), but init() generates and registers _quic_connection_id. Returning ZERO prevents callers from getting a stable per-connection ID (e.g., for logging / session IDs) in QUICHE builds.

QUICConnectionId
QUICNetVConnection::connection_id() const
{
  return QUICConnectionId::ZERO();
}

src/iocore/net/QUICNetVConnection.cc:458

• original_connection_id() returns a zero ID even though init() stores the original CID in _original_quic_connection_id. This loses connection-id information (and can collapse per-connection identifiers to 0) for QUICHE builds.

This issue also appears in the following locations of the same file:

• line 472
• line 478

QUICConnectionId
QUICNetVConnection::original_connection_id() const
{
  return QUICConnectionId::ZERO();
}

src/iocore/net/quic/QUICTypes.cc:730

• The byte-buffer constructor both clamps len via std::min and also asserts len <= MAX_LENGTH. In non-assert builds this can silently truncate oversized IDs, which is inconsistent with the new “explicit initialization” contract. Prefer enforcing the contract with a release assert and storing the exact length.

QUICConnectionId::QUICConnectionId(const uint8_t *buf, uint8_t len) : _len(std::min<uint8_t>(len, MAX_LENGTH))
{
  ink_assert(len <= QUICConnectionId::MAX_LENGTH);
  memcpy(this->_id, buf, this->_len);
}