Add minimal handling of NEW_CONNECTION_ID frames
authorTomas Mraz <tomas@openssl.org>
Fri, 5 May 2023 14:51:28 +0000 (16:51 +0200)
committerHugo Landau <hlandau@openssl.org>
Wed, 17 May 2023 13:04:18 +0000 (14:04 +0100)
We actively use only the latest DCID received. And retire only
DCIDs requested by the peer to be retired.

Also changed the active_conn_id_limit to 2 as the minimum value allowed.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20892)

include/internal/quic_channel.h
ssl/quic/quic_channel.c
ssl/quic/quic_channel_local.h
ssl/quic/quic_rx_depack.c
ssl/quic/quic_wire.c

index a33416fd2b43fdaefef5e763ddc8845c98b42348..d1a231fcc8868a62748f1a761e3bf70eff48a9a5 100644 (file)
@@ -220,6 +220,8 @@ void ossl_quic_channel_raise_protocol_error(QUIC_CHANNEL *ch,
 /* For RXDP use. */
 void ossl_quic_channel_on_remote_conn_close(QUIC_CHANNEL *ch,
                                             OSSL_QUIC_FRAME_CONN_CLOSE *f);
+void ossl_quic_channel_on_new_conn_id(QUIC_CHANNEL *ch,
+                                      OSSL_QUIC_FRAME_NEW_CONN_ID *f);
 
 /*
  * Queries and Accessors
index cb1c99bfcf9444f5e881535d610ecf0c54c66f32..4b378bf40a64e39e43a49932614dcffcf08602d7 100644 (file)
@@ -1212,7 +1212,7 @@ static int ch_generate_transport_params(QUIC_CHANNEL *ch)
         goto err;
 
     if (!ossl_quic_wire_encode_transport_param_int(&wpkt, QUIC_TPARAM_ACTIVE_CONN_ID_LIMIT,
-                                                   4))
+                                                   2))
         goto err;
 
     if (!ossl_quic_wire_encode_transport_param_int(&wpkt, QUIC_TPARAM_INITIAL_MAX_DATA,
@@ -1240,16 +1240,17 @@ static int ch_generate_transport_params(QUIC_CHANNEL *ch)
                                                    ossl_quic_rxfc_get_cwm(&ch->max_streams_uni_rxfc)))
         goto err;
 
+    if (!WPACKET_finish(&wpkt))
+        goto err;
+
+    wpkt_valid = 0;
+
     if (!WPACKET_get_total_written(&wpkt, &buf_len))
         goto err;
 
     ch->local_transport_params = (unsigned char *)buf_mem->data;
     buf_mem->data = NULL;
 
-    if (!WPACKET_finish(&wpkt))
-        goto err;
-
-    wpkt_valid = 0;
 
     if (!ossl_quic_tls_set_transport_params(ch->qtls, ch->local_transport_params,
                                             buf_len))
@@ -1470,7 +1471,7 @@ static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
 
     if (ossl_quic_pkt_type_is_encrypted(ch->qrx_pkt->hdr->type)) {
         if (!ch->have_received_enc_pkt) {
-            ch->init_scid = ch->qrx_pkt->hdr->src_conn_id;
+            ch->cur_remote_dcid = ch->init_scid = ch->qrx_pkt->hdr->src_conn_id;
             ch->have_received_enc_pkt = 1;
 
             /*
@@ -2095,6 +2096,104 @@ void ossl_quic_channel_on_remote_conn_close(QUIC_CHANNEL *ch,
     ch_start_terminating(ch, &tcause, 0);
 }
 
+static void free_frame_data(unsigned char *buf, size_t buf_len, void *arg)
+{
+    OPENSSL_free(buf);
+}
+
+static int ch_enqueue_retire_conn_id(QUIC_CHANNEL *ch, uint64_t seq_num)
+{
+    BUF_MEM *buf_mem;
+    WPACKET wpkt;
+    size_t l;
+
+    if ((buf_mem = BUF_MEM_new()) == NULL)
+        return 0;
+
+    if (!WPACKET_init(&wpkt, buf_mem))
+        goto err;
+
+    if (!ossl_quic_wire_encode_frame_retire_conn_id(&wpkt, seq_num)) {
+        WPACKET_cleanup(&wpkt);
+        goto err;
+    }
+
+    WPACKET_finish(&wpkt);
+    if (!WPACKET_get_total_written(&wpkt, &l))
+        goto err;
+
+    if (ossl_quic_cfq_add_frame(ch->cfq, 1, QUIC_PN_SPACE_APP,
+                                OSSL_QUIC_FRAME_TYPE_RETIRE_CONN_ID,
+                                (unsigned char *)buf_mem->data, l,
+                                free_frame_data, NULL) == NULL)
+        goto err;
+
+    buf_mem->data = NULL;
+    BUF_MEM_free(buf_mem);
+    return 1;
+
+err:
+    ossl_quic_channel_raise_protocol_error(ch,
+                                           QUIC_ERR_INTERNAL_ERROR,
+                                           OSSL_QUIC_FRAME_TYPE_NEW_CONN_ID,
+                                           "internal error enqueueing retire conn id");
+    BUF_MEM_free(buf_mem);
+    return 0;
+}
+
+void ossl_quic_channel_on_new_conn_id(QUIC_CHANNEL *ch,
+                                      OSSL_QUIC_FRAME_NEW_CONN_ID *f)
+{
+    uint64_t new_remote_seq_num = ch->cur_remote_seq_num;
+    uint64_t new_retire_prior_to = ch->cur_retire_prior_to;
+
+    if (!ossl_quic_channel_is_active(ch))
+        return;
+
+    /* We allow only two active connection ids; first check some constraints */
+
+    if (ch->cur_remote_dcid.id_len == 0) {
+        /* Changing from 0 length connection id is disallowed */
+        ossl_quic_channel_raise_protocol_error(ch,
+                                               QUIC_ERR_PROTOCOL_VIOLATION,
+                                               OSSL_QUIC_FRAME_TYPE_NEW_CONN_ID,
+                                               "zero length connection id in use");
+
+        return;
+    }
+
+    if (f->seq_num > new_remote_seq_num)
+        new_remote_seq_num = f->seq_num;
+    if (f->retire_prior_to > new_retire_prior_to)
+        new_retire_prior_to = f->retire_prior_to;
+
+    if (new_remote_seq_num - new_retire_prior_to > 1
+        /*
+         * RFC allows connection termination if we see 2 times the limit
+         * of conn ids to retire. We are a little bit more liberal.
+         */
+        || new_retire_prior_to - ch->cur_retire_prior_to > 10) {
+        /* Violation of our active conn id limit */
+        ossl_quic_channel_raise_protocol_error(ch,
+                                               QUIC_ERR_CONNECTION_ID_LIMIT_ERROR,
+                                               OSSL_QUIC_FRAME_TYPE_NEW_CONN_ID,
+                                               "active_connection_id limit violated");
+
+        return;
+    }
+
+    if (new_remote_seq_num > ch->cur_remote_seq_num) {
+        ch->cur_remote_seq_num = new_remote_seq_num;
+        ch->cur_remote_dcid = f->conn_id;
+        ossl_quic_tx_packetiser_set_cur_dcid(ch->txp, &ch->cur_remote_dcid);
+    }
+    while (new_retire_prior_to > ch->cur_retire_prior_to) {
+        if (!ch_enqueue_retire_conn_id(ch, ch->cur_retire_prior_to))
+            break;
+        ++ch->cur_retire_prior_to;
+    }
+}
+
 static void ch_raise_net_error(QUIC_CHANNEL *ch)
 {
     QUIC_TERMINATE_CAUSE tcause = {0};
index 379528b516dabff22ecc1396e5e1eba527b99414..0eb47f3f13c28c887a4c12d87f52b8685583d5f9 100644 (file)
@@ -122,8 +122,10 @@ struct quic_channel_st {
      */
     QUIC_CONN_ID                    retry_scid;
 
-    /* Server only: The DCID we currently use to talk to the peer. */
+    /* The DCID we currently use to talk to the peer and its sequence num. */
     QUIC_CONN_ID                    cur_remote_dcid;
+    uint64_t                        cur_remote_seq_num;
+    uint64_t                        cur_retire_prior_to;
     /* Server only: The DCID we currently expect the peer to use to talk to us. */
     QUIC_CONN_ID                    cur_local_dcid;
 
index 4b9805b01cac91802d27f0e51af841c736e532d8..8bedc1c26bea5e1a337974bb9c1f9bf484ec9239 100644 (file)
@@ -660,7 +660,7 @@ static int depack_do_frame_new_conn_id(PACKET *pkt,
     /* This frame makes the packet ACK eliciting */
     ackm_data->is_ack_eliciting = 1;
 
-    /* TODO(QUIC): ADD CODE to send |frame_data.data| to the ch manager */
+    ossl_quic_channel_on_new_conn_id(ch, &frame_data);
 
     return 1;
 }
@@ -682,7 +682,7 @@ static int depack_do_frame_retire_conn_id(PACKET *pkt,
     /* This frame makes the packet ACK eliciting */
     ackm_data->is_ack_eliciting = 1;
 
-    /* TODO(QUIC): ADD CODE to send |seq_num| to the ch manager */
+    /* TODO(QUIC): Post MVP ADD CODE to send |seq_num| to the ch manager */
     return 1;
 }
 
index b4d69f4949292eef479cae35cd36ca4633f789d8..5d35c98b67d11323308633c83231576824b5fdcf 100644 (file)
@@ -746,6 +746,7 @@ int ossl_quic_wire_decode_frame_new_conn_id(PACKET *pkt,
     if (!expect_frame_header(pkt, OSSL_QUIC_FRAME_TYPE_NEW_CONN_ID)
             || !PACKET_get_quic_vlint(pkt, &f->seq_num)
             || !PACKET_get_quic_vlint(pkt, &f->retire_prior_to)
+            || f->seq_num < f->retire_prior_to
             || !PACKET_get_1(pkt, &len)
             || len > QUIC_MAX_CONN_ID_LEN)
         return 0;