Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions framework/docs/source/ref-exit-codes/603.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#################################################
[603] COMMON_TLS_ROOT_CERTIFICATES_INCOMPATIBLE
#################################################

*************
Description
*************

The ``--root-certificates`` option was provided together with ``--insecure``. These
options are mutually exclusive because ``--insecure`` disables TLS.

****************
How to Resolve
****************

Choose one of the following:
Comment thread
panh99 marked this conversation as resolved.
- Remove ``--insecure`` to use TLS with ``--root-certificates``
- Remove ``--root-certificates`` to run without TLS
17 changes: 17 additions & 0 deletions framework/docs/source/ref-exit-codes/604.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
###########################
[604] COMMON_PATH_INVALID
###########################

*************
Description
*************

The provided path is invalid or does not point to the expected file.

****************
How to Resolve
****************

Verify that the path exists, points to the expected file type, and is readable by the
current process. Check the error message for the specific option or file path that
caused the failure.
7 changes: 7 additions & 0 deletions framework/py/flwr/common/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ def add_args_flwr_app_common(parser: argparse.ArgumentParser) -> None:
"is not encrypted. By default, the server runs with HTTPS enabled. "
"Use this flag only if you understand the risks.",
)
parser.add_argument(
"--root-certificates",
metavar="ROOT_CERT",
type=str,
help="Path to the PEM-encoded root certificate file used to establish "
"a secure TLS connection.",
)
parser.add_argument(
"--parent-pid",
type=int,
Expand Down
8 changes: 8 additions & 0 deletions framework/py/flwr/common/exit/exit_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ class ExitCode:
COMMON_ADDRESS_INVALID = 600
COMMON_MISSING_EXTRA_REST = 601
COMMON_TLS_NOT_SUPPORTED = 602
COMMON_TLS_ROOT_CERTIFICATES_INCOMPATIBLE = 603
COMMON_PATH_INVALID = 604

# Simulation exit codes (700-799)
SIMULATION_EXCEPTION = 700
Expand Down Expand Up @@ -164,6 +166,12 @@ def __new__(cls) -> ExitCode:
`pip install "flwr[rest]"`.
""",
ExitCode.COMMON_TLS_NOT_SUPPORTED: "Please use the '--insecure' flag.",
ExitCode.COMMON_TLS_ROOT_CERTIFICATES_INCOMPATIBLE: (
"The '--root-certificates' option cannot be used together with '--insecure'."
),
ExitCode.COMMON_PATH_INVALID: (
"The provided path is invalid or does not point to the expected file."
),
# Simulation exit codes (700-799)
ExitCode.SIMULATION_EXCEPTION: (
"An unhandled exception occurred when running the simulation."
Expand Down
4 changes: 3 additions & 1 deletion framework/py/flwr/server/grid/grpc_grid.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,15 @@ class GrpcGrid(Grid): # pylint: disable=too-many-instance-attributes
def __init__( # pylint: disable=too-many-arguments
self,
serverappio_service_address: str = SERVERAPPIO_API_DEFAULT_CLIENT_ADDRESS,
insecure: bool = False,
root_certificates: bytes | None = None,
*,
token: str,
) -> None:
if token == "":
raise ValueError("`token` must be a non-empty string")
self._addr = serverappio_service_address
self._insecure = insecure
self._cert = root_certificates
self._token = token
self._run: Run | None = None
Expand All @@ -152,7 +154,7 @@ def _connect(self) -> None:
return
self._channel = create_channel(
server_address=self._addr,
insecure=(self._cert is None),
insecure=self._insecure,
root_certificates=self._cert,
interceptors=[AppIoTokenClientInterceptor(token=self._token)],
)
Expand Down
12 changes: 5 additions & 7 deletions framework/py/flwr/server/serverapp/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
from flwr.supercore.heartbeat import HeartbeatSender, make_app_heartbeat_fn_grpc
from flwr.supercore.superexec.plugin import ServerAppExecPlugin
from flwr.supercore.superexec.run_superexec import run_with_deprecation_warning
from flwr.supercore.tls import load_root_certificates


def flwr_serverapp() -> None:
Expand All @@ -79,12 +80,6 @@ def flwr_serverapp() -> None:

args = _parse_args_run_flwr_serverapp().parse_args()

if not args.insecure:
flwr_exit(
ExitCode.COMMON_TLS_NOT_SUPPORTED,
"`flwr-serverapp` does not support TLS yet.",
)

# Disallow long-running `flwr-serverapp` processes
if args.token is None:
run_with_deprecation_warning(
Expand All @@ -109,7 +104,8 @@ def flwr_serverapp() -> None:
serverappio_api_address=args.serverappio_api_address,
log_queue=log_queue,
token=args.token,
certificates=None,
insecure=args.insecure,
certificates=load_root_certificates(args.root_certificates, args.insecure),
parent_pid=args.parent_pid,
)

Expand All @@ -121,6 +117,7 @@ def run_serverapp( # pylint: disable=R0913, R0914, R0915, R0917, W0212
serverappio_api_address: str,
log_queue: Queue[str | None],
token: str,
insecure: bool,
certificates: bytes | None = None,
parent_pid: int | None = None,
) -> None:
Expand Down Expand Up @@ -177,6 +174,7 @@ def on_exit() -> None:
# Initialize the GrpcGrid
grid = GrpcGrid(
serverappio_service_address=serverappio_api_address,
insecure=insecure,
root_certificates=certificates,
token=token,
)
Expand Down
14 changes: 7 additions & 7 deletions framework/py/flwr/simulation/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
from flwr.supercore.heartbeat import HeartbeatSender, make_app_heartbeat_fn_grpc
from flwr.supercore.superexec.plugin import ServerAppExecPlugin
from flwr.supercore.superexec.run_superexec import run_with_deprecation_warning
from flwr.supercore.tls import load_root_certificates


def _run_simulation_settings(
Expand Down Expand Up @@ -112,12 +113,6 @@ def flwr_simulation() -> None:

args = _parse_args_run_flwr_simulation().parse_args()

if not args.insecure:
flwr_exit(
ExitCode.COMMON_TLS_NOT_SUPPORTED,
"`flwr-simulation` does not support TLS yet.",
)

# Disallow long-running `flwr-simulation` processes
if args.token is None:
run_with_deprecation_warning(
Expand All @@ -131,6 +126,8 @@ def flwr_simulation() -> None:
)
return

certificates = load_root_certificates(args.root_certificates, args.insecure)

log(INFO, "Starting Flower Simulation")
log(
DEBUG,
Expand All @@ -141,7 +138,8 @@ def flwr_simulation() -> None:
serverappio_api_address=args.serverappio_api_address,
log_queue=log_queue,
token=args.token,
certificates=None,
insecure=args.insecure,
certificates=certificates,
parent_pid=args.parent_pid,
)
Comment thread
panh99 marked this conversation as resolved.

Expand All @@ -153,6 +151,7 @@ def run_simulation_process( # pylint: disable=R0913, R0914, R0915, R0917, W0212
serverappio_api_address: str,
log_queue: Queue[str | None],
token: str,
insecure: bool,
certificates: bytes | None = None,
parent_pid: int | None = None,
) -> None:
Expand All @@ -163,6 +162,7 @@ def run_simulation_process( # pylint: disable=R0913, R0914, R0915, R0917, W0212

conn = SimulationIoConnection(
serverappio_api_address=serverappio_api_address,
insecure=insecure,
root_certificates=certificates,
token=token,
)
Expand Down
2 changes: 2 additions & 0 deletions framework/py/flwr/simulation/app_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ def test_run_simulation_process_passes_token_to_connection(
run_simulation_process(
serverappio_api_address="127.0.0.1:9091",
log_queue=Queue(),
insecure=True,
token="test-token",
)

mock_connection_cls.assert_called_once_with(
serverappio_api_address="127.0.0.1:9091",
insecure=True,
root_certificates=None,
token="test-token",
)
Expand Down
4 changes: 3 additions & 1 deletion framework/py/flwr/simulation/simulationio_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,15 @@ class SimulationIoConnection:
def __init__( # pylint: disable=too-many-arguments
self,
serverappio_api_address: str = SERVERAPPIO_API_DEFAULT_CLIENT_ADDRESS,
insecure: bool = False,
root_certificates: bytes | None = None,
*,
token: str,
) -> None:
if token == "":
raise ValueError("`token` must be a non-empty string")
self._addr = serverappio_api_address
self._insecure = insecure
self._cert = root_certificates
self._token = token
self._grpc_stub: ServerAppIoStub | None = None
Expand All @@ -78,7 +80,7 @@ def _connect(self) -> None:
return
self._channel = create_channel(
server_address=self._addr,
insecure=(self._cert is None),
insecure=self._insecure,
root_certificates=self._cert,
interceptors=[AppIoTokenClientInterceptor(token=self._token)],
)
Expand Down
15 changes: 9 additions & 6 deletions framework/py/flwr/supercore/cli/flower_superexec.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,6 @@ def flower_superexec() -> None:
warn_if_flwr_update_available(process_name="flower-superexec")
args = _parse_args().parse_args()

if not args.insecure:
flwr_exit(
ExitCode.COMMON_TLS_NOT_SUPPORTED,
"SuperExec does not support TLS yet.",
)

# Log the first message after parsing arguments in case of `--help`
log(INFO, "Starting Flower SuperExec")

Expand Down Expand Up @@ -125,6 +119,8 @@ def flower_superexec() -> None:
plugin_class=plugin_class,
stub_class=stub_class, # type: ignore
appio_api_address=args.appio_api_address,
insecure=args.insecure,
root_certificates_path=args.root_certificates,
superexec_auth_secret=superexec_auth_secret,
plugin_config=plugin_config,
parent_pid=args.parent_pid,
Expand Down Expand Up @@ -160,6 +156,13 @@ def _parse_args() -> argparse.ArgumentParser:
"Data transmitted between the client and server is not encrypted. "
"Use this flag only if you understand the risks.",
)
parser.add_argument(
"--root-certificates",
metavar="ROOT_CERT",
type=str,
help="Path to the PEM-encoded root certificate file used to establish "
"a secure TLS connection.",
)
parser.add_argument(
"--parent-pid",
type=int,
Expand Down
2 changes: 2 additions & 0 deletions framework/py/flwr/supercore/cli/flower_superexec_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def test_flower_superexec_clientapp_allows_missing_secret(
insecure=True,
plugin_type=ExecPluginType.CLIENT_APP,
plugin_config=None,
root_certificates=None,
superexec_auth_secret_file=None,
appio_api_address="127.0.0.1:9091",
parent_pid=None,
Expand Down Expand Up @@ -131,6 +132,7 @@ def test_flower_superexec_serverapp_allows_missing_secret(
insecure=True,
plugin_type=ExecPluginType.SERVER_APP,
plugin_config=None,
root_certificates=None,
superexec_auth_secret_file=None,
appio_api_address="127.0.0.1:9091",
parent_pid=None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ def select_run_id(self, candidate_run_ids: Sequence[int]) -> int | None:

def launch_app(self, token: str, run_id: int) -> None:
"""Launch the application associated with a given run ID and token."""
cmds = [self.command, "--insecure"]
cmds = [self.command]
if self.insecure:
cmds.append("--insecure")
elif self.root_certificates_path:
cmds += ["--root-certificates", self.root_certificates_path]
cmds += [self.appio_api_address_arg, self.appio_api_address]
cmds += ["--token", token]
cmds += ["--parent-pid", str(os.getpid())]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ def test_clientapp_launch_inherits_default_stdio() -> None:
"""ClientApp launch should use default stdio behavior."""
plugin = ClientAppExecPlugin(
appio_api_address="127.0.0.1:9094",
insecure=True,
root_certificates_path=None,
get_run=_get_run,
)

Expand All @@ -47,6 +49,8 @@ def test_serverapp_launch_isolates_stdio() -> None:
"""ServerApp launch should not inherit parent stdio streams."""
plugin = ServerAppExecPlugin(
appio_api_address="127.0.0.1:9092",
insecure=True,
root_certificates_path=None,
get_run=_get_run,
)

Expand All @@ -55,3 +59,38 @@ def test_serverapp_launch_isolates_stdio() -> None:

assert popen.call_args.kwargs["stdout"] is subprocess.DEVNULL
assert popen.call_args.kwargs["stderr"] is subprocess.DEVNULL


def test_clientapp_launch_forwards_root_certificate() -> None:
"""ClientApp launch should forward the configured root certificate path."""
plugin = ClientAppExecPlugin(
appio_api_address="127.0.0.1:9094",
insecure=False,
root_certificates_path="/tmp/root.pem",
get_run=_get_run,
)

with patch("subprocess.Popen") as mock_popen:
plugin.launch_app(token="token", run_id=7)

assert mock_popen.call_args.args[0][:3] == [
"flwr-clientapp",
"--root-certificates",
"/tmp/root.pem",
]


def test_clientapp_launch_omits_tls_flags_when_using_system_certificates() -> None:
"""ClientApp launch should omit TLS flags when relying on system certificates."""
plugin = ClientAppExecPlugin(
appio_api_address="127.0.0.1:9094",
insecure=False,
root_certificates_path=None,
get_run=_get_run,
)

with patch("subprocess.Popen") as mock_popen:
plugin.launch_app(token="token", run_id=7)

assert "--insecure" not in mock_popen.call_args.args[0]
assert "--root-certificates" not in mock_popen.call_args.args[0]
4 changes: 4 additions & 0 deletions framework/py/flwr/supercore/superexec/plugin/exec_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,13 @@ class ExecPlugin(ABC):
def __init__(
self,
appio_api_address: str,
insecure: bool,
root_certificates_path: str | None,
get_run: Callable[[int], Run],
) -> None:
self.appio_api_address = appio_api_address
self.insecure = insecure
self.root_certificates_path = root_certificates_path
self.get_run = get_run

@abstractmethod
Expand Down
Loading
Loading