[main] Skip auth filter for cellnet protocol-level bye messages

nvflare/fuel/f3/cellnet/core_cell.py

@@ -28,6 +28,8 @@
 from nvflare.fuel.f3.cellnet.defs import (
     AbortRun,
     AuthenticationError,
+    CellChannel,
+    CellChannelTopic,
     CellPropertyKey,
     InvalidRequest,
     InvalidSession,
@@ -60,9 +62,7 @@
 from nvflare.fuel.utils.log_utils import get_obj_logger
 from nvflare.security.logging import secure_format_exception, secure_format_traceback
 
-_CHANNEL = "cellnet.channel"
 _TOPIC_BULK = "bulk"
-_TOPIC_BYE = "bye"
 _SM_CHANNEL = "credential_manager"
 _SM_TOPIC = "key_exchange"
 
@@ -197,7 +197,7 @@ def send(self):
         tms = [m.to_dict() for m in messages_to_send]
         bulk_msg = Message(None, tms)
         send_errs = self.cell.fire_and_forget(
-            channel=_CHANNEL, topic=_TOPIC_BULK, targets=[self.target], message=bulk_msg
+            channel=CellChannel.CELLNET, topic=_TOPIC_BULK, targets=[self.target], message=bulk_msg
         )
         if send_errs[self.target]:
             log_messaging_error(
@@ -497,8 +497,8 @@ def __init__(
         self.adhoc_connector_lock = threading.Lock()
         self.root_change_lock = threading.Lock()
 
-        self.register_request_cb(channel=_CHANNEL, topic=_TOPIC_BULK, cb=self._receive_bulk_message)
-        self.register_request_cb(channel=_CHANNEL, topic=_TOPIC_BYE, cb=self._peer_goodbye)
+        self.register_request_cb(channel=CellChannel.CELLNET, topic=_TOPIC_BULK, cb=self._receive_bulk_message)
+        self.register_request_cb(channel=CellChannel.CELLNET, topic=CellChannelTopic.Bye, cb=self._peer_goodbye)
 
         self.cleanup_waiter = None
         self.msg_stats_pool = StatsPoolManager.add_time_hist_pool(
@@ -974,8 +974,8 @@ def stop(self):
                 targets = [peer_name for peer_name in self.agents.keys()]
                 self.logger.debug(f"broadcasting goodbye to {targets}")
                 self.broadcast_request(
-                    channel=_CHANNEL,
-                    topic=_TOPIC_BYE,
+                    channel=CellChannel.CELLNET,
+                    topic=CellChannelTopic.Bye,
                     targets=targets,
                     request=Message(),
                     timeout=0.5,

nvflare/fuel/f3/cellnet/defs.py

@@ -167,12 +167,14 @@ class CellChannel:
     RETURN_ONLY = "return_only"
     EDGE_REQUEST = "edge_request"
     HCI = "hci_channel"
+    CELLNET = "cellnet.channel"
 
 
 class CellChannelTopic:
 
     Challenge = "challenge"
     Register = "register"
+    Bye = "bye"
     Quit = "quit"
     GET_TASK = "get_task"
     SUBMIT_RESULT = "submit_result"

nvflare/fuel/hci/client/api.py

@@ -459,6 +459,7 @@ def connect(self, timeout=None):
             cb=validate_auth_headers,
             token_verifier=token_verifier,
             logger=self.logger,
+            local_cell_fqcn=self.cell.get_fqcn(),
         )
         self.debug(f"Successfully authenticated to {self.server_identity}: {token=} {ssid=}")
 

nvflare/private/fed/app/relay/relay.py

@@ -221,6 +221,7 @@ def main(args):
             cb=_validate_auth_headers,
             token_verifier=token_verifier,
             logger=logger,
+            local_cell_fqcn=my_fqcn,
         )
 
     logger.info(f"Successfully authenticated to {server_identity}: {token=} {ssid=}")
@@ -232,13 +233,16 @@ def main(args):
     logger.info(f"Relay {my_fqcn} stopped.")
 
 
-def _validate_auth_headers(message: CellMessage, token_verifier: TokenVerifier, logger):
-    """Validate auth headers from messages that go through the server.
+def _validate_auth_headers(message: CellMessage, token_verifier: TokenVerifier, logger, local_cell_fqcn=None):
+    """Validate auth headers from messages that go through the relay.
     Args:
         message: the message to validate
+        token_verifier: verifier for the token and signature
+        logger: logger
+        local_cell_fqcn: FQCN of this relay's cell, used to scope the cellnet bye auth bypass
     Returns:
     """
-    return validate_auth_headers(message, token_verifier, logger)
+    return validate_auth_headers(message, token_verifier, logger, local_cell_fqcn=local_cell_fqcn)
 
 
 if __name__ == "__main__":

nvflare/private/fed/authenticator.py

@@ -355,6 +355,7 @@ def validate_auth_headers(
     token_verifier: TokenVerifier,
     logger,
     client_fqcn_resolver: Optional[Callable[[str, str], Optional[str]]] = None,
+    local_cell_fqcn: Optional[str] = None,
 ):
     """Validate auth headers from messages that go through the server.
 
@@ -364,6 +365,9 @@ def validate_auth_headers(
         client_fqcn_resolver: optional resolver used to bind a client token to its registered CellNet origin.
             Return None only when the token/name cannot be resolved; return MISSING_CLIENT_FQCN for a registered
             client with no stored origin so validation fails closed.
+        local_cell_fqcn: the FQCN of the cell that owns this auth filter. Used to bypass auth ONLY for a
+            cellnet ``bye`` that actually terminates at this cell (DESTINATION == local_cell_fqcn). When None,
+            the bye bypass never triggers and byes fall through to the normal auth check.
 
     Returns:
     """
@@ -379,6 +383,29 @@ def validate_auth_headers(
         logger.debug(f"skip special message {topic=} {channel=}")
         return None
 
+    # Cellnet protocol-level goodbye is broadcast by Cell.stop() with an empty
+    # Message() that carries no FL-level auth headers. Only bypass auth when the
+    # bye actually TERMINATES at this cell, i.e. DESTINATION == this cell's own
+    # FQCN. This is the true "direct-neighbor" property: the in-filter runs
+    # before CoreCell's forward decision (``if destination != my_fqcn: forward``),
+    # so a bye whose DESTINATION is this cell is handled locally by
+    # _peer_goodbye and never forwarded. A crafted bye that names some OTHER
+    # cell as DESTINATION (to have this cell forward it and evict that cell's
+    # upstream agent) has DESTINATION != local_cell_fqcn and falls through to the
+    # normal auth check. Comparing DESTINATION to a sender-controlled TO_CELL
+    # header would NOT give this guarantee, since both are attacker-controlled;
+    # only comparing against the receiver's own FQCN does. When local_cell_fqcn
+    # is unknown (None), the bypass never triggers (byes get normal auth).
+    destination = message.get_header(MessageHeaderKey.DESTINATION)
+    if (
+        topic == CellChannelTopic.Bye
+        and channel == CellChannel.CELLNET
+        and local_cell_fqcn is not None
+        and destination == local_cell_fqcn
+    ):
+        logger.debug(f"skip direct-neighbor cellnet bye {topic=} {channel=} {destination=}")
+        return None
+
     client_name = message.get_header(CellMessageHeaderKeys.CLIENT_NAME)
     err_text = f"unauthenticated msg ({channel=} {topic=}) received from {origin}"
     if not client_name:

nvflare/private/fed/server/fed_server.py

@@ -454,6 +454,7 @@ def _validate_auth_headers(self, message: Message):
             token_verifier=token_verifier,
             logger=self.logger,
             client_fqcn_resolver=self._resolve_client_fqcn_for_auth,
+            local_cell_fqcn=self.cell.get_fqcn() if getattr(self, "cell", None) else None,
         )
         if not reply:
             self._strip_peer_transit_reply_auth_headers(message)

tests/unit_test/fuel/hci/admin_api_test.py

@@ -32,6 +32,9 @@ def __init__(self, **kwargs):
         def register_request_cb(self, **kwargs):
             pass
 
+        def get_fqcn(self):
+            return "admin@nvidia.com"
+
         def start(self):
             pass
 

tests/unit_test/private/fed/authenticator_test.py

@@ -17,7 +17,7 @@
 import pytest
 
 from nvflare.apis.fl_constant import ServerCommandNames
-from nvflare.fuel.f3.cellnet.defs import CellChannel, MessageHeaderKey, ReturnCode
+from nvflare.fuel.f3.cellnet.defs import CellChannel, CellChannelTopic, MessageHeaderKey, ReturnCode
 from nvflare.fuel.f3.message import Message
 from nvflare.fuel.f3.streaming.stream_const import STREAM_CHANNEL, STREAM_DATA_TOPIC
 from nvflare.private.defs import CellMessageHeaderKeys
@@ -186,3 +186,54 @@ def test_validate_auth_headers_rejects_registered_token_with_missing_origin_bind
 
 def test_validate_auth_headers_keeps_compatibility_when_origin_cannot_be_resolved():
     assert _validate("site-b", lambda _client_name, _token: None) is None
+
+
+def _make_bye_message(destination, to_cell=None):
+    # Cellnet Cell.stop() broadcasts an empty Message() carrying no FL auth headers;
+    # the transport stamps DESTINATION/TO_CELL. A crafted bye can set them to anything.
+    headers = {
+        MessageHeaderKey.CHANNEL: CellChannel.CELLNET,
+        MessageHeaderKey.TOPIC: CellChannelTopic.Bye,
+        MessageHeaderKey.ORIGIN: "attacker",
+        MessageHeaderKey.DESTINATION: destination,
+    }
+    if to_cell is not None:
+        headers[MessageHeaderKey.TO_CELL] = to_cell
+    return Message(headers=headers)
+
+
+def _validate_bye(destination, local_cell_fqcn, to_cell=None):
+    return validate_auth_headers(
+        message=_make_bye_message(destination, to_cell=to_cell),
+        token_verifier=_TokenVerifier(),
+        logger=logging.getLogger(__name__),
+        local_cell_fqcn=local_cell_fqcn,
+    )
+
+
+def test_validate_auth_headers_bypasses_bye_terminating_at_this_cell():
+    # A legitimate bye is a direct-neighbor message: DESTINATION == the receiving cell's own FQCN.
+    assert _validate_bye(destination="server", local_cell_fqcn="server") is None
+
+
+def test_validate_auth_headers_rejects_forwarded_bye_for_other_cell():
+    # A crafted bye that names another cell as DESTINATION (so this cell would forward it and evict
+    # that cell's upstream agent) must NOT bypass auth — DESTINATION != this cell's FQCN.
+    reply = _validate_bye(destination="site-1", local_cell_fqcn="server")
+
+    assert reply.get_header(MessageHeaderKey.RETURN_CODE) == ReturnCode.UNAUTHENTICATED
+
+
+def test_validate_auth_headers_rejects_bye_forged_to_cell_matching_destination():
+    # The old guard compared two sender-controlled headers (TO_CELL == DESTINATION); an attacker can
+    # satisfy that while DESTINATION still points at another cell. The FQCN-based guard rejects it.
+    reply = _validate_bye(destination="site-1", local_cell_fqcn="server", to_cell="site-1")
+
+    assert reply.get_header(MessageHeaderKey.RETURN_CODE) == ReturnCode.UNAUTHENTICATED
+
+
+def test_validate_auth_headers_does_not_bypass_bye_when_local_fqcn_unknown():
+    # Without a known local FQCN the bypass must not trigger (fail closed).
+    reply = _validate_bye(destination="server", local_cell_fqcn=None)
+
+    assert reply.get_header(MessageHeaderKey.RETURN_CODE) == ReturnCode.UNAUTHENTICATED