Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(framework) Update CLI SSL flags #3512

Merged
merged 14 commits into from
May 28, 2024
Merged
4 changes: 3 additions & 1 deletion doc/source/how-to-authenticate-supernodes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ Use the following terminal command to start a Flower :code:`SuperNode` that has
.. code-block:: bash
flower-superlink
--certificates certificates/ca.crt certificates/server.pem certificates/server.key
--ssl-ca-certfile certificates/ca.crt
--ssl-certfile certificates/server.pem
--ssl-keyfile certificates/server.key
--auth-list-public-keys keys/client_public_keys.csv
--auth-superlink-private-key keys/server_credentials
--auth-superlink-public-key keys/server_credentials.pub
Expand Down
5 changes: 4 additions & 1 deletion doc/source/how-to-enable-ssl-connections.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ Use the following terminal command to start a sever (SuperLink) that uses the pr

.. code-block:: bash
flower-superlink --certificates certificates/ca.crt certificates/server.pem certificates/server.key
flower-superlink
--ssl-ca-certfile certificates/ca.crt
--ssl-certfile certificates/server.pem
--ssl-keyfile certificates/server.key
When providing certificates, the server expects a tuple of three certificates paths: CA certificate, server certificate and server private key.

Expand Down
17 changes: 10 additions & 7 deletions doc/source/how-to-run-flower-using-docker.rst
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,15 @@ PEM-encoded certificate chain.
Assuming all files we need are in the local ``certificates`` directory, we can use the flag
``--volume`` to mount the local directory into the ``/app/`` directory of the container. This allows
the SuperLink to access the files within the container. Finally, we pass the names of the
certificates to the SuperLink with the ``--certificates`` flag.
certificates to the SuperLink with the ``--root-certificates`` flag.

.. code-block:: bash
$ docker run --rm \
-p 9091:9091 -p 9092:9092 --volume ./certificates/:/app/ flwr/superlink:1.8.0 \
--certificates ca.crt server.pem server.key
-p 9091:9091 -p 9092:9092 --volume ./certificates/:/app/ flwr/superlink:1.9.0 \
--ssl-ca-certfile ca.crt \
--ssl-certfile server.pem \
--ssl-keyfile server.key
Flower SuperNode
----------------
Expand Down Expand Up @@ -264,13 +266,13 @@ To enable SSL, we will need to mount a PEM-encoded root certificate into your Su

Assuming the certificate already exists locally, we can use the flag ``--volume`` to mount the local
certificate into the container's ``/app/`` directory. This allows the SuperNode to access the
certificate within the container. Use the ``--certificates`` flag when starting the container.
certificate within the container. Use the ``--root-certificates`` flag when starting the container.

.. code-block:: bash
$ docker run --rm --volume ./ca.crt:/app/ca.crt flwr_supernode:0.0.1 \
--server 192.168.1.100:9092 \
--certificates ca.crt
--root-certificates ca.crt
Flower ServerApp
----------------
Expand Down Expand Up @@ -379,13 +381,14 @@ To enable SSL, we will need to mount a PEM-encoded root certificate into your Se

Assuming the certificate already exists locally, we can use the flag ``--volume`` to mount the local
certificate into the container's ``/app/`` directory. This allows the ServerApp to access the
certificate within the container. Use the ``--certificates`` flag when starting the container.
certificate within the container. Use the ``--ssl-ca-certfile``, ``--ssl-certfile``, and ``--ssl-keyfile``
flags when starting the container.

.. code-block:: bash
$ docker run --rm --volume ./ca.crt:/app/ca.crt flwr_serverapp:0.0.1 \
--server 192.168.1.100:9091 \
--certificates ca.crt
--root-certificates ca.crt
Advanced Docker options
-----------------------
Expand Down
12 changes: 6 additions & 6 deletions doc/source/how-to-upgrade-to-flower-next.rst
Original file line number Diff line number Diff line change
Expand Up @@ -171,16 +171,16 @@ Deployment
# In yet another terminal window, run the ServerApp (this starts the actual training run)
$ flower-server-app server:app --insecure
- Here's another example to start with HTTPS. Use the ``--certificates`` command line
argument to pass paths to (CA certificate, server certificate, and server private key).
- Here's another example to start with HTTPS. Use the ``--ssl-ca-certfile``, ``--ssl-certfile``, and ``--ssl-keyfile`` command line
options to pass paths to (CA certificate, server certificate, and server private key).

.. code-block:: bash
# Start a secure Superlink
$ flower-superlink --certificates \
<your-ca-cert-filepath> \
<your-server-cert-filepath> \
<your-privatekey-filepath>
$ flower-superlink \
--ssl-ca-certfile <your-ca-cert-filepath> \
--ssl-certfile <your-server-cert-filepath> \
--ssl-keyfile <your-privatekey-filepath>
# In a new terminal window, start a long-running secure SuperNode
$ flower-client-app client:app \
Expand Down
4 changes: 2 additions & 2 deletions e2e/test_driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ case "$1" in
;;
bare-https)
./generate.sh
server_arg="--certificates certificates/ca.crt certificates/server.pem certificates/server.key"
server_arg="--ssl-ca-certfile certificates/ca.crt --ssl-certfile certificates/server.pem --ssl-keyfile certificates/server.key"
client_arg="--root-certificates certificates/ca.crt"
server_dir="./"
;;
Expand Down Expand Up @@ -45,7 +45,7 @@ case "$2" in
server_address="127.0.0.1:9092"
server_app_address="127.0.0.1:9091"
db_arg="--database :flwr-in-memory-state:"
server_arg="--certificates certificates/ca.crt certificates/server.pem certificates/server.key"
server_arg="--ssl-ca-certfile certificates/ca.crt --ssl-certfile certificates/server.pem --ssl-keyfile certificates/server.key"
client_arg="--root-certificates certificates/ca.crt"
server_auth="--auth-list-public-keys keys/client_public_keys.csv --auth-superlink-private-key keys/server_credentials --auth-superlink-public-key keys/server_credentials.pub"
client_auth_1="--auth-supernode-private-key keys/client_credentials_1 --auth-supernode-public-key keys/client_credentials_1.pub"
Expand Down
4 changes: 3 additions & 1 deletion examples/flower-authentication/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ containing file path to the SuperLink's private key `server_credentials`, and `-

```bash
flower-superlink \
--certificates certificates/ca.crt certificates/server.pem certificates/server.key \
--ssl-ca-certfile certificates/ca.crt \
--ssl-certfile certificates/server.pem \
--ssl-keyfile certificates/server.key \
--auth-list-public-keys keys/client_public_keys.csv \
--auth-superlink-private-key keys/server_credentials \
--auth-superlink-public-key keys/server_credentials.pub
Expand Down
Empty file modified examples/flower-authentication/generate.sh
100644 → 100755
Empty file.
142 changes: 70 additions & 72 deletions src/py/flwr/server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
import importlib.util
import sys
import threading
from logging import ERROR, INFO, WARN
from logging import INFO, WARN
from os.path import isfile
from pathlib import Path
from typing import List, Optional, Sequence, Set, Tuple
from typing import Optional, Sequence, Set, Tuple

import grpc
from cryptography.exceptions import UnsupportedAlgorithm
Expand Down Expand Up @@ -346,6 +346,9 @@ def run_superlink() -> None:
sys.exit(MISSING_EXTRA_REST)
address_arg = args.rest_fleet_api_address
parsed_address = parse_address(address_arg)
_, ssl_certfile, ssl_keyfile = (
certificates if certificates is not None else (None, None, None)
)
if not parsed_address:
sys.exit(f"Fleet IP address ({address_arg}) cannot be parsed.")
host, port, _ = parsed_address
Expand All @@ -354,8 +357,8 @@ def run_superlink() -> None:
args=(
host,
port,
args.ssl_keyfile,
args.ssl_certfile,
ssl_keyfile,
ssl_certfile,
state_factory,
args.rest_fleet_api_workers,
),
Expand Down Expand Up @@ -442,8 +445,8 @@ def _try_setup_client_authentication(
if certificates is None:
sys.exit(
"Authentication requires secure connections. "
"Please provide certificate paths using '--certificates' and "
"try again."
"Please provide certificate paths to `--ssl-certfile`, "
"`--ssl-keyfile`, and `—-ssl-ca-certfile` and try again."
)

client_keys_file_path = Path(args.auth_list_public_keys)
Expand Down Expand Up @@ -512,21 +515,52 @@ def _try_obtain_certificates(
# Obtain certificates
if args.insecure:
log(WARN, "Option `--insecure` was set. Starting insecure HTTP server.")
certificates = None
return None
# Check if certificates are provided
elif args.certificates:
certificates = (
Path(args.certificates[0]).read_bytes(), # CA certificate
Path(args.certificates[1]).read_bytes(), # server certificate
Path(args.certificates[2]).read_bytes(), # server private key
)
else:
sys.exit(
"Certificates are required unless running in insecure mode. "
"Please provide certificate paths with '--certificates' or run the server "
"in insecure mode using '--insecure' if you understand the risks."
)
return certificates
if args.fleet_api_type == TRANSPORT_TYPE_GRPC_RERE:
if args.ssl_certfile and args.ssl_keyfile and args.ssl_ca_certfile:
if not isfile(args.ssl_ca_certfile):
sys.exit("Path argument `--ssl-ca-certfile` does not point to a file.")
if not isfile(args.ssl_certfile):
sys.exit("Path argument `--ssl-certfile` does not point to a file.")
if not isfile(args.ssl_keyfile):
sys.exit("Path argument `--ssl-keyfile` does not point to a file.")
certificates = (
Path(args.ssl_ca_certfile).read_bytes(), # CA certificate
Path(args.ssl_certfile).read_bytes(), # server certificate
Path(args.ssl_keyfile).read_bytes(), # server private key
)
return certificates
if args.ssl_certfile or args.ssl_keyfile or args.ssl_ca_certfile:
sys.exit(
"You need to provide valid file paths to `--ssl-certfile`, "
"`--ssl-keyfile`, and `—-ssl-ca-certfile` to create a secure "
"connection in Fleet API server (gRPC-rere)."
)
if args.fleet_api_type == TRANSPORT_TYPE_REST:
if args.ssl_certfile and args.ssl_keyfile:
if not isfile(args.ssl_certfile):
sys.exit("Path argument `--ssl-certfile` does not point to a file.")
if not isfile(args.ssl_keyfile):
sys.exit("Path argument `--ssl-keyfile` does not point to a file.")
certificates = (
b"",
Path(args.ssl_certfile).read_bytes(), # server certificate
Path(args.ssl_keyfile).read_bytes(), # server private key
)
return certificates
if args.ssl_certfile or args.ssl_keyfile:
sys.exit(
"You need to provide valid file paths to `--ssl-certfile` "
"and `--ssl-keyfile` to create a secure connection "
"in Fleet API server (REST, experimental)."
)
sys.exit(
"Certificates are required unless running in insecure mode. "
"Please provide certificate paths to `--ssl-certfile`, "
"`--ssl-keyfile`, and `—-ssl-ca-certfile` or run the server "
"in insecure mode using '--insecure' if you understand the risks."
)


def _run_fleet_api_grpc_rere(
Expand Down Expand Up @@ -582,14 +616,6 @@ def _run_fleet_api_rest(
# See: https://www.starlette.io/applications/#accessing-the-app-instance
fast_api_app.state.STATE_FACTORY = state_factory

validation_exceptions = _validate_ssl_files(
ssl_certfile=ssl_certfile, ssl_keyfile=ssl_keyfile
)
if any(validation_exceptions):
# Starting with 3.11 we can use ExceptionGroup but for now
# this seems to be the reasonable approach.
raise ValueError(validation_exceptions)

uvicorn.run(
app="flwr.server.superlink.fleet.rest_rere.rest_api:app",
port=port,
Expand All @@ -602,32 +628,6 @@ def _run_fleet_api_rest(
)


def _validate_ssl_files(
ssl_keyfile: Optional[str], ssl_certfile: Optional[str]
) -> List[ValueError]:
validation_exceptions = []

if ssl_keyfile is not None and not isfile(ssl_keyfile):
msg = "Path argument `--ssl-keyfile` does not point to a file."
log(ERROR, msg)
validation_exceptions.append(ValueError(msg))

if ssl_certfile is not None and not isfile(ssl_certfile):
msg = "Path argument `--ssl-certfile` does not point to a file."
log(ERROR, msg)
validation_exceptions.append(ValueError(msg))

if not bool(ssl_keyfile) == bool(ssl_certfile):
msg = (
"When setting one of `--ssl-keyfile` and "
"`--ssl-certfile`, both have to be used."
)
log(ERROR, msg)
validation_exceptions.append(ValueError(msg))

return validation_exceptions


def _parse_args_run_driver_api() -> argparse.ArgumentParser:
"""Parse command line arguments for Driver API."""
parser = argparse.ArgumentParser(
Expand Down Expand Up @@ -684,13 +684,23 @@ def _add_args_common(parser: argparse.ArgumentParser) -> None:
"Use this flag only if you understand the risks.",
)
parser.add_argument(
"--certificates",
nargs=3,
metavar=("CA_CERT", "SERVER_CERT", "PRIVATE_KEY"),
"--ssl-certfile",
help="Fleet API server SSL certificate file (as a path str) "
"to create a secure connection.",
type=str,
default=None,
)
parser.add_argument(
"--ssl-keyfile",
help="Fleet API server SSL private key file (as a path str) "
"to create a secure connection.",
type=str,
)
parser.add_argument(
"--ssl-ca-certfile",
help="Fleet API server SSL CA certificate file (as a path str) "
"to create a secure connection.",
type=str,
help="Paths to the CA certificate, server certificate, and server private "
"key, in that order. Note: The server can only be started without "
"certificates by enabling the `--insecure` flag.",
)
parser.add_argument(
"--database",
Expand Down Expand Up @@ -763,18 +773,6 @@ def _add_args_fleet_api(parser: argparse.ArgumentParser) -> None:
help="Fleet API (REST) server address (IPv4, IPv6, or a domain name)",
default=ADDRESS_FLEET_API_REST,
)
rest_group.add_argument(
"--ssl-certfile",
help="Fleet API (REST) server SSL certificate file (as a path str), "
"needed for using 'https'.",
default=None,
)
rest_group.add_argument(
"--ssl-keyfile",
help="Fleet API (REST) server SSL private key file (as a path str), "
"needed for using 'https'.",
default=None,
)
rest_group.add_argument(
"--rest-fleet-api-workers",
help="Set the number of concurrent workers for the Fleet API REST server.",
Expand Down