QUIC: fix unintended address/port match for empty DCID

Commit "QUIC: fix decryption when the client uses an empty SCID"
addressed the root cause that prevented connections from being correctly
linked. However another trace with an empty DCID was still properly
linked even in presence of the bug.

It turns out that an earlier optimization has an unintended change.
If a short packet was preceded by any packet with a DCID of exactly 20
bytes, then a connection with an empty CID is looked up as expected, by
`quic_connection_find_dcid(pinfo, NULL, from_server)`. However if no
earlier DCID of exactly 20 bytes exists, then a lookup by address/port
would occur. That is why earlier traces still decrypt successfully.

Restore the intended behavior to ensure that (1) invalid DCIDs in a Long
Header packet are ignored, and (2) Short Header Packets are not
accidentally matched to a wrong session based on an address/port match.
The latter could occur if the same src/dst address/port tuple is reused
across different QUIC connections when all CIDs are not 20 bytes.

Change-Id: Ida2523a0922314c7a455dec7e1f8f0442be27e94
Ping-Bug: 13881
Fixes: v2.9.0rc0-1878-gfc9e404ab2 ("QUIC: small connection tracking optimization")
Reviewed-on: https://code.wireshark.org/review/37845
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
This commit is contained in:
Peter Wu 2020-07-12 23:30:21 +02:00 committed by Anders Broman
parent 608835bf56
commit 44ed20a97c
1 changed files with 5 additions and 1 deletions

View File

@ -743,7 +743,11 @@ quic_connection_find_dcid(packet_info *pinfo, const quic_cid_t *dcid, gboolean *
quic_info_data_t *conn = NULL;
gboolean check_ports = FALSE;
if (dcid && dcid->len > 0 && quic_cids_is_known_length(dcid)) {
if (dcid && dcid->len > 0) {
// Optimization: avoid lookup for invalid CIDs.
if (!quic_cids_is_known_length(dcid)) {
return NULL;
}
conn = (quic_info_data_t *) wmem_map_lookup(quic_client_connections, dcid);
if (conn) {
// DCID recognized by client, so it was from server.