|
13 | 13 |
|
14 | 14 | import os |
15 | 15 | import time |
| 16 | +import warnings |
16 | 17 | from collections.abc import Mapping, MutableMapping |
17 | 18 | from typing import Any, cast |
18 | 19 |
|
|
34 | 35 | logger = get_logger(__name__) |
35 | 36 |
|
36 | 37 |
|
| 38 | +def _warn_provisioned_config_execution(bundle_root: str) -> None: |
| 39 | + """ |
| 40 | + Warn that the bundle under ``bundle_root`` is about to be executed. |
| 41 | +
|
| 42 | + In federated learning the whole app directory -- configs included -- is provisioned by the FL |
| 43 | + system, and the aggregation server dispatches `initialize`/`train` tasks that the client runs |
| 44 | + on its own, so there is no per-round human interaction to catch a poisoned config. |
| 45 | + """ |
| 46 | + warnings.warn( |
| 47 | + f"executing the bundle config under {bundle_root}, which is provisioned by the FL system: " |
| 48 | + 'any `"_target_"` value in it is resolved to an importable callable and invoked with no ' |
| 49 | + 'allow list, and any `"$"`-prefixed value is passed to Python `eval()`. A malicious or ' |
| 50 | + "compromised aggregation server therefore gets code execution on this client, without any " |
| 51 | + "per-round human interaction. Only join a federation whose server and app-provisioning " |
| 52 | + "channel you trust (see " |
| 53 | + "https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-x6pr-233j-x5cw).", |
| 54 | + stacklevel=3, |
| 55 | + ) |
| 56 | + |
| 57 | + |
37 | 58 | def convert_global_weights(global_weights: Mapping, local_var_dict: MutableMapping) -> tuple[MutableMapping, int]: |
38 | 59 | """Helper function to convert global weights to local weights format""" |
39 | 60 | # Before loading weights, tensors might need to be reshaped to support HE for secure aggregation. |
@@ -86,6 +107,15 @@ class MonaiAlgoStats(ClientAlgoStats): |
86 | 107 | """ |
87 | 108 | Implementation of ``ClientAlgoStats`` to allow federated learning with MONAI bundle configurations. |
88 | 109 |
|
| 110 | + Security note: the bundle under `bundle_root` is provisioned by the FL system -- `initialize()` |
| 111 | + resolves it against `extra[ExtraItems.APP_ROOT]`, which the aggregation server supplies -- and |
| 112 | + executing it runs whatever its config contains: any `"_target_"` value is resolved to an |
| 113 | + importable callable and invoked with no allow list, and any `"$"`-prefixed value is passed to |
| 114 | + Python `eval()`. A malicious or compromised server therefore gets code execution on this client, |
| 115 | + with no per-round human interaction. Executing a config raises a warning -- once per call site, |
| 116 | + as Python's default warning filter suppresses repeats |
| 117 | + (see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-x6pr-233j-x5cw). |
| 118 | +
|
89 | 119 | Args: |
90 | 120 | bundle_root: directory path of the bundle. |
91 | 121 | config_train_filename: bundle training config path relative to bundle_root. Can be a list of files; |
@@ -135,18 +165,29 @@ def initialize(self, extra=None): |
135 | 165 | Args: |
136 | 166 | extra: Dict with additional information that should be provided by FL system, |
137 | 167 | i.e., `ExtraItems.CLIENT_NAME`, `ExtraItems.APP_ROOT` and `ExtraItems.LOGGING_FILE`. |
138 | | - You can diable the logging logic in the monai bundle by setting {ExtraItems.LOGGING_FILE} to False. |
| 168 | + `{ExtraItems.LOGGING_FILE}` defaults to False here, and an explicit `None` is |
| 169 | + treated the same way, so the bundle's own "configs/logging.conf" is not applied: |
| 170 | + it is provisioned by the FL system and `logging.config.fileConfig` runs the INI's |
| 171 | + `class=`/`args=` fields through `eval()` |
| 172 | + (see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-wvpx-5qmp-46g3). |
| 173 | + Set it to a logging config file path to opt back in to configuring logging. |
139 | 174 |
|
140 | 175 | """ |
141 | 176 | if extra is None: |
142 | 177 | extra = {} |
143 | 178 | self.client_name = extra.get(ExtraItems.CLIENT_NAME, "noname") |
144 | | - logging_file = extra.get(ExtraItems.LOGGING_FILE, None) |
| 179 | + logging_file = extra.get(ExtraItems.LOGGING_FILE, False) |
| 180 | + if logging_file is None: |
| 181 | + # `ConfigWorkflow` reads `None` as "fall back to the bundle's own configs/logging.conf", |
| 182 | + # the FL-provisioned file this default exists to keep away from `fileConfig`. Passing the |
| 183 | + # key explicitly as `None` has to mean the same as leaving it out. |
| 184 | + logging_file = False |
145 | 185 | self.logger.info(f"Initializing {self.client_name} ...") |
146 | 186 |
|
147 | 187 | # FL platform needs to provide filepath to configuration files |
148 | 188 | self.app_root = extra.get(ExtraItems.APP_ROOT, "") |
149 | 189 | self.bundle_root = os.path.join(self.app_root, self.bundle_root) |
| 190 | + _warn_provisioned_config_execution(self.bundle_root) |
150 | 191 |
|
151 | 192 | if self.workflow is None: |
152 | 193 | config_train_files = self._add_config_files(self.config_train_filename) |
@@ -313,6 +354,15 @@ class MonaiAlgo(ClientAlgo, MonaiAlgoStats): |
313 | 354 | """ |
314 | 355 | Implementation of ``ClientAlgo`` to allow federated learning with MONAI bundle configurations. |
315 | 356 |
|
| 357 | + Security note: the bundle under `bundle_root` is provisioned by the FL system -- `initialize()` |
| 358 | + resolves it against `extra[ExtraItems.APP_ROOT]`, which the aggregation server supplies -- and |
| 359 | + executing it runs whatever its config contains: any `"_target_"` value is resolved to an |
| 360 | + importable callable and invoked with no allow list, and any `"$"`-prefixed value is passed to |
| 361 | + Python `eval()`. A malicious or compromised server therefore gets code execution on this client, |
| 362 | + with no per-round human interaction. Executing a config raises a warning -- once per call site, |
| 363 | + as Python's default warning filter suppresses repeats |
| 364 | + (see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-x6pr-233j-x5cw). |
| 365 | +
|
316 | 366 | Args: |
317 | 367 | bundle_root: directory path of the bundle. |
318 | 368 | local_epochs: number of local epochs to execute during each round of local training; defaults to 1. |
@@ -416,19 +466,30 @@ def initialize(self, extra=None): |
416 | 466 | Args: |
417 | 467 | extra: Dict with additional information that should be provided by FL system, |
418 | 468 | i.e., `ExtraItems.CLIENT_NAME`, `ExtraItems.APP_ROOT` and `ExtraItems.LOGGING_FILE`. |
419 | | - You can diable the logging logic in the monai bundle by setting {ExtraItems.LOGGING_FILE} to False. |
| 469 | + `{ExtraItems.LOGGING_FILE}` defaults to False here, and an explicit `None` is |
| 470 | + treated the same way, so the bundle's own "configs/logging.conf" is not applied: |
| 471 | + it is provisioned by the FL system and `logging.config.fileConfig` runs the INI's |
| 472 | + `class=`/`args=` fields through `eval()` |
| 473 | + (see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-wvpx-5qmp-46g3). |
| 474 | + Set it to a logging config file path to opt back in to configuring logging. |
420 | 475 |
|
421 | 476 | """ |
422 | 477 | self._set_cuda_device() |
423 | 478 | if extra is None: |
424 | 479 | extra = {} |
425 | 480 | self.client_name = extra.get(ExtraItems.CLIENT_NAME, "noname") |
426 | | - logging_file = extra.get(ExtraItems.LOGGING_FILE, None) |
| 481 | + logging_file = extra.get(ExtraItems.LOGGING_FILE, False) |
| 482 | + if logging_file is None: |
| 483 | + # `ConfigWorkflow` reads `None` as "fall back to the bundle's own configs/logging.conf", |
| 484 | + # the FL-provisioned file this default exists to keep away from `fileConfig`. Passing the |
| 485 | + # key explicitly as `None` has to mean the same as leaving it out. |
| 486 | + logging_file = False |
427 | 487 | timestamp = time.strftime("%Y%m%d_%H%M%S") |
428 | 488 | self.logger.info(f"Initializing {self.client_name} ...") |
429 | 489 | # FL platform needs to provide filepath to configuration files |
430 | 490 | self.app_root = extra.get(ExtraItems.APP_ROOT, "") |
431 | 491 | self.bundle_root = os.path.join(self.app_root, self.bundle_root) |
| 492 | + _warn_provisioned_config_execution(self.bundle_root) |
432 | 493 |
|
433 | 494 | if self.train_workflow is None and self.config_train_filename is not None: |
434 | 495 | config_train_files = self._add_config_files(self.config_train_filename) |
|
0 commit comments