QUIC: fixed migration during NAT rebinding.

The RFC 9000 allows a packet from known CID arrive from unknown path:

    These requirements regarding connection ID reuse apply only to the
    sending of packets, as unintentional changes in path without a change
    in connection ID are possible.  For example, after a period of
    network inactivity, NAT rebinding might cause packets to be sent on a
    new path when the client resumes sending.

Before the patch, such packets were rejected with an error in the
ngx_quic_check_migration() function.  Removing the check makes the
separate function excessive - remaining checks are early migration
check and "disable_active_migration" check.  The latter is a transport
parameter sent to client and it should not be used by server.

The server should send "disable_active_migration" "if the endpoint does
not support active connection migration" (18.2). The support status depends
on nginx configuration: to have migration working with multiple workers,
you need bpf helper, available on recent Linux systems.  The patch does
not set "disable_active_migration" automatically and leaves it for the
administrator. By default, active migration is enabled.

RFC 900 says that it is ok to migrate if the peer violates
"disable_active_migration" flag requirements:

   If the peer violates this requirement,

   the endpoint MUST either drop the incoming packets on that path without
   generating a Stateless Reset

   OR

   proceed with path validation and allow the peer to migrate.  Generating a
   Stateless Reset or closing the connection would allow third parties in the
   network to cause connections to close by spoofing or otherwise manipulating
   observed traffic.

So, nginx adheres to the second option and proceeds to path validation.

Note:

The ngtcp2 may be used for testing both active migration and NAT rebinding:

ngtcp2/client --change-local-addr=200ms --delay-stream=500ms <ip> <port> <url>

ngtcp2/client --change-local-addr=200ms --delay-stream=500ms --nat-rebinding \
              <ip> <port> <url>

diff --git a/src/event/quic/ngx_event_quic.c b/src/event/quic/ngx_event_quic.c
index 53e50a8b91bebbf61209164335ae162d118a82d7..03d703b2c08f6f436d5cdb0ca6d7283f2ce0faf3 100644 (file)
--- a/src/event/quic/ngx_event_quic.c
+++ b/src/event/quic/ngx_event_quic.c
@@ -812,11 +812,6 @@ ngx_quic_process_packet(ngx_connection_t *c, ngx_quic_conf_t *conf,
             return NGX_DECLINED;
         }
 
-        rc = ngx_quic_check_migration(c, pkt);
-        if (rc != NGX_OK) {
-            return rc;
-        }
-
         if (pkt->level != ssl_encryption_application) {
 
             if (pkt->version != qc->version) {
@@ -825,6 +820,18 @@ ngx_quic_process_packet(ngx_connection_t *c, ngx_quic_conf_t *conf,
                 return NGX_DECLINED;
             }
 
+            if (pkt->first) {
+                if (ngx_quic_find_path(c, c->udp->dgram->sockaddr,
+                                       c->udp->dgram->socklen)
+                    == NULL)
+                {
+                    /* packet comes from unknown path, possibly migration */
+                    ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
+                                  "quic too early migration attempt");
+                    return NGX_DECLINED;
+                }
+            }
+
             if (ngx_quic_check_csid(qc, pkt) != NGX_OK) {
                 return NGX_DECLINED;
             }

diff --git a/src/event/quic/ngx_event_quic_migration.c b/src/event/quic/ngx_event_quic_migration.c
index 4f7ab2c973d78a20fec9797e346e0b737bc5283a..887824573d26d5869688ae372caea44671aad14e 100644 (file)
--- a/src/event/quic/ngx_event_quic_migration.c
+++ b/src/event/quic/ngx_event_quic_migration.c
@@ -278,66 +278,6 @@ ngx_quic_find_path(ngx_connection_t *c, struct sockaddr *sockaddr,
 }
 
 
-ngx_int_t
-ngx_quic_check_migration(ngx_connection_t *c, ngx_quic_header_t *pkt)
-{
-    ngx_quic_path_t        *path;
-    ngx_quic_socket_t      *qsock;
-    ngx_quic_connection_t  *qc;
-
-    qc = ngx_quic_get_connection(c);
-
-    qsock = ngx_quic_get_socket(c);
-
-    if (c->udp->dgram == NULL) {
-        /* 2nd QUIC packet in first UDP datagram */
-        return NGX_OK;
-    }
-
-    path = ngx_quic_find_path(c, c->udp->dgram->sockaddr,
-                              c->udp->dgram->socklen);
-    if (path == NULL) {
-        /* packet comes from unknown path, possibly migration */
-
-        if (qc->tp.disable_active_migration) {
-            ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
-                           "quic migration disabled, dropping packet "
-                           "from unknown path");
-            return NGX_DECLINED;
-        }
-
-        if (pkt->level != ssl_encryption_application) {
-            ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
-                           "quic too early migration attempt");
-            return NGX_DECLINED;
-        }
-
-        return NGX_OK;
-    }
-
-    /* packet from known path */
-
-    if (qsock->path == NULL) {
-        /* client switched to previously unused server id */
-        return NGX_OK;
-    }
-
-    if (path == qsock->path) {
-        /* regular packet to expected path */
-        return NGX_OK;
-    }
-
-    /* client is trying to use server id already used on other path */
-
-    ngx_log_debug4(NGX_LOG_DEBUG_EVENT, c->log, 0,
-                   "quic attempt to use socket #%uL:%uL:%uL with path #%uL",
-                   qsock->sid.seqnum, qsock->cid->seqnum,
-                   qsock->path->seqnum, path->seqnum);
-
-    return NGX_DECLINED;
-}
-
-
 ngx_int_t
 ngx_quic_update_paths(ngx_connection_t *c, ngx_quic_header_t *pkt)
 {
@@ -348,9 +288,10 @@ ngx_quic_update_paths(ngx_connection_t *c, ngx_quic_header_t *pkt)
     ngx_quic_connection_t  *qc;
 
     qsock = ngx_quic_get_socket(c);
-    path = qsock->path;
 
-    if (path) {
+    if (c->udp->dgram == NULL && qsock->path) {
+        /* 1st ever packet in connection, path already exists */
+        path = qsock->path;
         goto update;
     }
 
@@ -363,6 +304,20 @@ ngx_quic_update_paths(ngx_connection_t *c, ngx_quic_header_t *pkt)
         if (path == NULL) {
             return NGX_ERROR;
         }
+
+        if (qsock->path) {
+            /* NAT rebinding case: packet to same CID, but from new address */
+
+            ngx_quic_unref_path(c, qsock->path);
+
+            qsock->path = path;
+            path->refcnt++;
+
+            goto update;
+        }
+
+    } else if (qsock->path) {
+        goto update;
     }
 
     /* prefer unused client IDs if available */

diff --git a/src/event/quic/ngx_event_quic_migration.h b/src/event/quic/ngx_event_quic_migration.h
index 7c37c90857e56200c8f2a6ca524dad7bd80026a0..4ad3213b6b25bbb5207145e17828273b20016170 100644 (file)
--- a/src/event/quic/ngx_event_quic_migration.h
+++ b/src/event/quic/ngx_event_quic_migration.h
@@ -30,11 +30,11 @@ ngx_int_t ngx_quic_handle_path_challenge_frame(ngx_connection_t *c,
 ngx_int_t ngx_quic_handle_path_response_frame(ngx_connection_t *c,
     ngx_quic_path_challenge_frame_t *f);
 
+ngx_quic_path_t *ngx_quic_find_path(ngx_connection_t *c,
+    struct sockaddr *sockaddr, socklen_t socklen);
 ngx_quic_path_t *ngx_quic_add_path(ngx_connection_t *c,
     struct sockaddr *sockaddr, socklen_t socklen);
 
-ngx_int_t ngx_quic_check_migration(ngx_connection_t *c,
-    ngx_quic_header_t *pkt);
 ngx_int_t ngx_quic_update_paths(ngx_connection_t *c, ngx_quic_header_t *pkt);
 ngx_int_t ngx_quic_handle_migration(ngx_connection_t *c,
     ngx_quic_header_t *pkt);