Revert "BUG/MEDIUM: quic: missing check of dcid for init pkt including a token"

This reverts commit 072e774939.

Doing h2load with h3 tests we notice this behavior:

Client ---- INIT no token SCID = a , DCID = A ---> Server (1)
Client <--- RETRY+TOKEN DCID = a, SCID = B    ---- Server (2)
Client ---- INIT+TOKEN SCID = a , DCID = B    ---> Server (3)
Client <--- INIT DCID = a, SCID = C           ---- Server (4)
Client ---- INIT+TOKEN SCID = a, DCID = C     ---> Server (5)

With (5) dropped by haproxy due to token validation.

Indeed the previous patch adds SCID of retry packet sent to the aad
of the token ciphering aad. It was useful to validate the next INIT
packets including the token are sent by the client using the new
provided SCID for DCID as mantionned into the RFC 9000.
But this stateless information is lost on received INIT packets
following the first outgoing INIT packet from the server because
the client is also supposed to re-use a second time the lastest
received SCID for its new DCID. This will break the token validation
on those last packets and they will be dropped by haproxy.

It was discussed there:
https://mailarchive.ietf.org/arch/msg/quic/7kXVvzhNCpgPk6FwtyPuIC6tRk0/

To resume: this is not the role of the server to verify the re-use of
retry's SCID for DCID in further client's INIT packets.

The previous patch must be reverted in all versions where it was
backported (supposed until 2.6)
This commit is contained in:
Emeric Brun 2023-09-28 15:29:53 +02:00 committed by Willy Tarreau
parent d956db6638
commit 3c250cb847
3 changed files with 9 additions and 16 deletions

View File

@ -44,7 +44,6 @@ void qc_prep_hdshk_fast_retrans(struct quic_conn *qc,
struct list *ifrms, struct list *hfrms);
int quic_generate_retry_token_aad(unsigned char *aad,
uint32_t version,
const struct quic_cid *dcid,
const struct quic_cid *scid,
const struct sockaddr_storage *addr);
int send_retry(int fd, struct sockaddr_storage *addr,

View File

@ -1716,8 +1716,7 @@ static int quic_retry_token_check(struct quic_rx_packet *pkt,
const uint64_t tokenlen = pkt->token_len;
unsigned char buf[128];
unsigned char aad[sizeof(uint32_t) + QUIC_CID_MAXLEN +
sizeof(in_port_t) + sizeof(struct in6_addr) +
QUIC_CID_MAXLEN];
sizeof(in_port_t) + sizeof(struct in6_addr)];
size_t aadlen;
const unsigned char *salt;
unsigned char key[QUIC_TLS_KEY_LEN];
@ -1758,7 +1757,7 @@ static int quic_retry_token_check(struct quic_rx_packet *pkt,
goto err;
}
aadlen = quic_generate_retry_token_aad(aad, qv->num, &pkt->dcid, &pkt->scid, &dgram->saddr);
aadlen = quic_generate_retry_token_aad(aad, qv->num, &pkt->scid, &dgram->saddr);
salt = token + tokenlen - QUIC_RETRY_TOKEN_SALTLEN;
if (!quic_tls_derive_retry_token_secret(EVP_sha256(), key, sizeof key, iv, sizeof iv,
salt, QUIC_RETRY_TOKEN_SALTLEN, sec, seclen)) {

View File

@ -1553,8 +1553,7 @@ int send_stateless_reset(struct listener *l, struct sockaddr_storage *dstaddr,
*/
int quic_generate_retry_token_aad(unsigned char *aad,
uint32_t version,
const struct quic_cid *dcid,
const struct quic_cid *scid,
const struct quic_cid *cid,
const struct sockaddr_storage *addr)
{
unsigned char *p;
@ -1562,11 +1561,9 @@ int quic_generate_retry_token_aad(unsigned char *aad,
p = aad;
*(uint32_t *)p = htonl(version);
p += sizeof version;
memcpy(p, dcid->data, dcid->len);
p += dcid->len;
p += quic_saddr_cpy(p, addr);
memcpy(p, scid->data, scid->len);
p += scid->len;
memcpy(p, cid->data, cid->len);
p += cid->len;
return p - aad;
}
@ -1581,15 +1578,13 @@ int quic_generate_retry_token_aad(unsigned char *aad,
static int quic_generate_retry_token(unsigned char *token, size_t len,
const uint32_t version,
const struct quic_cid *odcid,
const struct quic_cid *scid,
const struct quic_cid *dcid,
struct sockaddr_storage *addr)
{
int ret = 0;
unsigned char *p;
unsigned char aad[sizeof(uint32_t) + QUIC_CID_MAXLEN +
sizeof(in_port_t) + sizeof(struct in6_addr) +
QUIC_CID_MAXLEN];
unsigned char aad[sizeof(uint32_t) + sizeof(in_port_t) +
sizeof(struct in6_addr) + QUIC_CID_MAXLEN];
size_t aadlen;
unsigned char salt[QUIC_RETRY_TOKEN_SALTLEN];
unsigned char key[QUIC_TLS_KEY_LEN];
@ -1609,7 +1604,7 @@ static int quic_generate_retry_token(unsigned char *token, size_t len,
if (1 + odcid->len + 1 + sizeof(timestamp) + QUIC_TLS_TAG_LEN + QUIC_RETRY_TOKEN_SALTLEN > len)
goto err;
aadlen = quic_generate_retry_token_aad(aad, version, scid, dcid, addr);
aadlen = quic_generate_retry_token_aad(aad, version, dcid, addr);
/* TODO: RAND_bytes() should be replaced */
if (RAND_bytes(salt, sizeof salt) != 1) {
TRACE_ERROR("RAND_bytes()", QUIC_EV_CONN_TXPKT);
@ -1701,7 +1696,7 @@ int send_retry(int fd, struct sockaddr_storage *addr,
/* token */
if (!(token_len = quic_generate_retry_token(&buf[i], sizeof(buf) - i, qv->num,
&pkt->dcid, &scid, &pkt->scid, addr))) {
&pkt->dcid, &pkt->scid, addr))) {
TRACE_ERROR("quic_generate_retry_token() failed", QUIC_EV_CONN_TXPKT);
goto out;
}