diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 4a5b82ac2..e33db5ec0 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -34,13 +34,33 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} + - name: Short commit sha + id: shortsha + run: echo "value=${GITHUB_SHA::7}" >> "$GITHUB_OUTPUT" + - name: Build and push Docker image uses: docker/build-push-action@v6 with: context: . push: true build-args: ROS_DISTRO=${{ matrix.ros_distro }} - tags: ${{ env.REGISTRY }}/${{ github.repository_owner }}/ros2_medkit-${{ matrix.ros_distro }}:latest + # Two tags, and the second one is the point. `:latest` moves + # on every merge, so a user who pulls it has no way back to + # the behaviour they had yesterday - and the defaults this + # image ships are exactly the kind of thing that changes + # under them. `sha-` gives them something immutable to + # pin. The release workflow publishes the semver tags; this + # one runs on every push to main, where there is no version + # number to use. + # + # Short form, to match the sha- tags the release workflow + # already publishes (docker/metadata-action's + # `type=sha,format=short`). Two spellings of the same idea in + # one registry is how a user ends up unable to find the image + # they pinned. + tags: | + ${{ env.REGISTRY }}/${{ github.repository_owner }}/ros2_medkit-${{ matrix.ros_distro }}:latest + ${{ env.REGISTRY }}/${{ github.repository_owner }}/ros2_medkit-${{ matrix.ros_distro }}:sha-${{ steps.shortsha.outputs.value }} cache-from: type=gha,scope=${{ matrix.ros_distro }} # mode=min, not max: max exports every intermediate build # stage, which for three distros held ~3.1 GB of the repo's diff --git a/README.md b/README.md index 532571891..be906bb26 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ corner - and the only trace is a line buried in a log you would have to SSH in t Start ros2_medkit next to it (no changes to Nav2). The aborted goal becomes a fault: ```bash -curl http://localhost:8080/api/v1/apps/bt_navigator/faults +curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/bt_navigator/faults # → ACTION_NAVIGATE_TO_POSE_ABORTED severity=ERROR source=/bt_navigator status=CONFIRMED # + a black-box rosbag of the seconds around the failure ``` @@ -51,7 +51,7 @@ trajectory. Same story: the same action bridge surfaces the aborted move as a fa `move_group` entity, with the freeze-frame of what the arm was doing, without touching MoveIt. ```bash -curl http://localhost:8080/api/v1/apps/move_group/faults +curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/move_group/faults # → the aborted MoveGroup goal, as a structured fault with its snapshot ``` @@ -87,6 +87,8 @@ docker run --rm --network host --ipc host \ ghcr.io/selfpatch/ros2_medkit-jazzy:latest \ ros2 launch ros2_medkit_gateway bringup.launch.py # → REST API live at http://localhost:8080/api/v1/ +# The container prints a one-time client_secret on startup; use it to get a +# token. Pass -e MEDKIT_AUTH_DISABLED=1 to run without authentication. ``` Swap `jazzy` for `humble`/`lyrical`; the two `-e` flags forward your shell's `ROS_DOMAIN_ID` and @@ -96,7 +98,23 @@ picks them up it is a plain apt install too: ```bash sudo apt install ros-jazzy-ros2-medkit-gateway # or ros-humble- / ros-lyrical- -ros2 launch ros2_medkit_gateway bringup.launch.py + +# The gateway ships closed: it requires a credential and refuses to start +# without a signing secret. Supply one, and either a certificate or +# tls_enabled:=false on a host nothing else can reach. +ros2 launch ros2_medkit_gateway bringup.launch.py \ + tls_enabled:=false \ + jwt_secret:=change-me-to-at-least-32-characters-long \ + auth_clients:=demo:demo-secret:admin +``` + +Every `curl` below then needs a token: + +```bash +TOKEN=$(curl -s http://localhost:8080/api/v1/auth/authorize \ + -H 'Content-Type: application/json' \ + -d '{"grant_type":"client_credentials","client_id":"demo","client_secret":"demo-secret"}' \ + | jq -r .access_token) ``` > [!TIP] diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index ef82bb433..936fbf0ff 100644 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -24,6 +24,94 @@ source "${COLCON_WS}/install/setup.bash" # Default to FastDDS (can be overridden via RMW_IMPLEMENTATION env var) export RMW_IMPLEMENTATION="${RMW_IMPLEMENTATION:-rmw_fastrtps_cpp}" +# Bootstrap credentials so the image can ship closed AND still start. +# +# The packaged params file turns authentication on, matching the gateway's own +# default, and the gateway refuses to start without a signing secret. An image +# that therefore needed a secret before `docker run` would do anything is a +# quickstart nobody completes, and the usual reaction is to turn auth off and +# leave it off. So: if the operator supplied nothing, generate a secret and an +# admin client for this container and print the credential once. +# +# Supply MEDKIT_JWT_SECRET and MEDKIT_CLIENTS to pin your own, or +# MEDKIT_AUTH_DISABLED=1 to run open on a host nothing else can reach. + +# The params file the node will actually read, if any. `docker run` can replace +# the default CMD, so this cannot be assumed: an operator who mounts their own +# file has already decided the auth configuration, and generating a credential +# on top of it would override the one their clients hold. +PARAMS_FILE="" +prev="" +for arg in "$@"; do + if [ "$prev" = "--params-file" ]; then PARAMS_FILE="$arg"; fi + prev="$arg" +done + +# True when that file names its own signing secret, which is the marker that +# the operator brought their own credentials rather than wanting generated ones. +operator_supplied_auth() { + [ -n "$PARAMS_FILE" ] && [ -r "$PARAMS_FILE" ] && grep -qE '^[[:space:]]*jwt_secret[[:space:]]*:' "$PARAMS_FILE" +} + +AUTH_ARGS=() +if [ "${MEDKIT_AUTH_DISABLED:-0}" = "1" ]; then + AUTH_ARGS+=(-p auth.enabled:=false) + echo "ros2_medkit: MEDKIT_AUTH_DISABLED=1 - starting WITHOUT authentication." >&2 + echo " Every route is readable by anyone who can reach this port." >&2 +elif operator_supplied_auth; then + echo "ros2_medkit: ${PARAMS_FILE} carries its own auth.jwt_secret - using it as-is," >&2 + echo " generating nothing and overriding nothing." >&2 +else + if [ -z "${MEDKIT_JWT_SECRET:-}" ]; then + # Per container, and not persisted: a restart issues a new one, which is + # correct for a credential nobody chose and nobody stored. + MEDKIT_JWT_SECRET="$(head -c 32 /dev/urandom | base64 | tr -d '\n' | tr '+/' '-_' | tr -d '=')" + MEDKIT_CLIENT_SECRET="$(head -c 24 /dev/urandom | base64 | tr -d '\n' | tr '+/' '-_' | tr -d '=')" + MEDKIT_CLIENTS="${MEDKIT_CLIENTS:-medkit:${MEDKIT_CLIENT_SECRET}:admin}" + echo "=============================================================" >&2 + echo "ros2_medkit: generated a one-time admin credential for this" >&2 + echo " container. It changes on every restart." >&2 + echo "" >&2 + echo " client_id: medkit" >&2 + echo " client_secret: ${MEDKIT_CLIENT_SECRET}" >&2 + echo "" >&2 + echo " curl -s http://localhost:8080/api/v1/auth/authorize \\" >&2 + echo " -H 'Content-Type: application/json' \\" >&2 + echo " -d '{\"grant_type\":\"client_credentials\",\"client_id\":\"medkit\",\"client_secret\":\"${MEDKIT_CLIENT_SECRET}\"}'" >&2 + echo "" >&2 + echo " Set MEDKIT_JWT_SECRET and MEDKIT_CLIENTS to pin your own." >&2 + echo "=============================================================" >&2 + fi + # Asserted here, not left to the params file. `docker run --ros-args -p + # server.port:=9090` REPLACES the default CMD, so the params file is never + # loaded, and the node's compiled-in default for auth.enabled is false - the + # image would print an admin credential and then serve every route to anyone. + # Saying it on the command line means the image is closed however it is + # started, and it is a no-op when the params file is loaded and agrees. + AUTH_ARGS+=(-p auth.enabled:=true) + if [ -z "$PARAMS_FILE" ]; then + # Same reasoning one level down: with no params file the compiled-in + # require_auth_for is "write", which leaves every read open. A file, even a + # mounted one, is the operator's statement about this and is left alone. + AUTH_ARGS+=(-p auth.require_auth_for:=all) + fi + AUTH_ARGS+=(-p "auth.jwt_secret:=${MEDKIT_JWT_SECRET}") + [ -n "${MEDKIT_CLIENTS:-}" ] && AUTH_ARGS+=(-p "auth.clients:=[${MEDKIT_CLIENTS}]") +fi +# Exported so the other dispatch branch works too: `docker run ros2 launch +# ... bringup.launch.py` execs a command instead of the node, so it never sees +# AUTH_ARGS. gateway.launch.py falls back to these variables. +export MEDKIT_JWT_SECRET MEDKIT_CLIENTS MEDKIT_AUTH_DISABLED +# The image serves plain HTTP; see gateway_docker_params.yaml for why. +# +# Only when the operator has not brought a config of their own. This is the +# image's DEFAULT, not a policy: `docker run ros2 launch ... +# gateway.launch.py config_file:=/mnt/secure.yaml` with TLS on in that file must +# get TLS, and an unconditional export here silently served it plaintext. +if [ -z "$PARAMS_FILE" ] && ! printf '%s\n' "$@" | grep -q '^config_file:='; then + export MEDKIT_TLS_DISABLED=1 +fi + # Dispatch on the first argument: # - empty, or starts with "-" (the default CMD "--ros-args --params-file ..." # or an override like --ros-args -p server.port:=9090): run the gateway node @@ -31,6 +119,6 @@ export RMW_IMPLEMENTATION="${RMW_IMPLEMENTATION:-rmw_fastrtps_cpp}" # - a full command (e.g. `ros2 launch ros2_medkit_gateway bringup.launch.py` # or `bash`): exec it as-is, so the image can launch the whole bringup stack. if [ -z "$1" ] || [ "${1#-}" != "$1" ]; then - exec ros2 run ros2_medkit_gateway gateway_node "$@" + exec ros2 run ros2_medkit_gateway gateway_node "$@" "${AUTH_ARGS[@]}" fi exec "$@" diff --git a/docker/gateway_docker_params.yaml b/docker/gateway_docker_params.yaml index 1d4049a57..2bf90df5e 100644 --- a/docker/gateway_docker_params.yaml +++ b/docker/gateway_docker_params.yaml @@ -6,6 +6,13 @@ ros2_medkit_gateway: server: host: "0.0.0.0" port: 8080 + # TLS stays OFF in the image, unlike config/gateway_params.yaml. A + # container has no certificate of its own and is almost always fronted by + # something that terminates TLS; requiring one here would mean the image + # could not start at all. Terminate TLS at your ingress, or mount a + # certificate and set server.tls.* yourself. + tls: + enabled: false refresh_interval_ms: 2000 # The web UI runs as a separate origin (its own host/port), so the # documented "run the web UI next to the gateway" path needs CORS. Without @@ -17,3 +24,17 @@ ros2_medkit_gateway: allowed_origins: - "http://localhost:3000" - "http://localhost:5173" + + # Authentication. On, matching config/gateway_params.yaml, so the published + # image has the same posture as the source default rather than a quietly + # weaker one. + # + # jwt_secret and clients are NOT set here on purpose: a secret baked into a + # published image is a secret every user of that image shares. The + # entrypoint generates one per container and prints the credential, or + # takes MEDKIT_JWT_SECRET / MEDKIT_CLIENTS from the environment. Run with + # MEDKIT_AUTH_DISABLED=1 to serve without authentication. + auth: + enabled: true + require_auth_for: "all" + issuer: "ros2_medkit_gateway" diff --git a/docs/api/rest.rst b/docs/api/rest.rst index 0321f22fc..2dc630be1 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -944,7 +944,7 @@ Read and publish data from ROS 2 topics. Item ids follow .. code-block:: bash - curl http://localhost:8080/api/v1/components/temp_sensor/data/powertrain%2Fengine%2Ftemperature + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/temp_sensor/data/powertrain%2Fengine%2Ftemperature ``PUT /api/v1/components/{id}/data/{topic_path}`` Publish to a topic. @@ -1719,7 +1719,7 @@ List available bulk-data categories for an entity. Returns the union of rosbag c .. code-block:: bash - curl http://localhost:8080/api/v1/apps/motor_controller/bulk-data + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/motor_controller/bulk-data **Response (200 OK):** @@ -1740,7 +1740,7 @@ List all bulk-data items in a category for the entity. .. code-block:: bash - curl http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags **Response (200 OK):** @@ -3187,7 +3187,7 @@ topic (push-based). .. code-block:: bash - curl http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-topic-beacon + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-topic-beacon **Response (200 OK):** @@ -3242,7 +3242,7 @@ polling ROS 2 node parameters matching a configured prefix (pull-based). .. code-block:: bash - curl http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-param-beacon + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-param-beacon **Response (200 OK):** diff --git a/docs/config/discovery-options.rst b/docs/config/discovery-options.rst index c47613dc0..d8672ac2f 100644 --- a/docs/config/discovery-options.rst +++ b/docs/config/discovery-options.rst @@ -472,7 +472,7 @@ The ``x-medkit-topic-beacon`` vendor endpoint exposes current beacon state: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-topic-beacon + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-topic-beacon **Example Response:** diff --git a/docs/config/server.rst b/docs/config/server.rst index 0171b268a..b0e15d6be 100644 --- a/docs/config/server.rst +++ b/docs/config/server.rst @@ -116,7 +116,7 @@ TLS/HTTPS Configuration - Description * - ``server.tls.enabled`` - bool - - ``false`` + - ``true`` - Enable HTTPS using OpenSSL. * - ``server.tls.cert_file`` - string @@ -857,7 +857,7 @@ default for local development. - Description * - ``auth.enabled`` - bool - - ``false`` + - ``true`` - Enable/disable JWT authentication. * - ``auth.jwt_secret`` - string @@ -881,7 +881,7 @@ default for local development. - Refresh token validity period in seconds (24 hours). Must be >= ``token_expiry_seconds``. * - ``auth.require_auth_for`` - string - - ``"write"`` + - ``"all"`` - When to require authentication: ``"none"`` (auth endpoints still available), ``"write"`` (POST/PUT/DELETE only), or ``"all"`` (every request). * - ``auth.issuer`` - string @@ -891,6 +891,10 @@ default for local development. - string[] - ``[]`` - Pre-configured clients as ``"client_id:client_secret:role"`` strings. + * - ``auth.public_routes`` + - string[] + - ``[]`` + - Routes answered with no credential, each written ``"METHOD /path"``. Layers over ``require_auth_for`` and only ever removes a requirement. Matched exactly, no wildcards. Every entry is logged at ``WARN`` on startup, and a malformed entry stops the gateway. .. note:: @@ -922,6 +926,35 @@ Example: token_expiry_seconds: 3600 clients: ["admin:REPLACE_WITH_STRONG_SECRET:admin", "viewer:REPLACE_WITH_STRONG_SECRET:viewer"] +Opening a route to uncredentialed callers +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Under ``require_auth_for: "all"`` the only routes answered without a credential +are ``/api/v1/auth/*``, because authentication cannot bootstrap through a door +that demands the credential it hands out. Health is not special and is refused +like everything else. + +When something that cannot hold a credential has to reach a route, name it: + +.. code-block:: yaml + + auth: + public_routes: ["GET /api/v1/health"] + +Matching is exact. ``GET /api/v1/health`` opens that method on that path and +nothing else - not ``HEAD``, not ``/api/v1/healthz``, not the subtree. Wildcards +are rejected rather than accepted and matched literally, and a malformed entry +stops the gateway rather than being dropped silently. + +An anonymous caller on such a route gets a reduced body: ``GET /health`` answers +with ``status`` and ``timestamp`` only, plus ``x-medkit-reduced: true`` so a +monitor can tell a withheld answer from a clean one. A credential still returns +the whole document. + +Most liveness probes need no entry at all. A ``401`` already proves the process +is up and answering HTTP, so a probe that accepts ``200``, ``401`` and ``403`` +works against any auth configuration and leaves nothing open. Prefer that. + See :doc:`/tutorials/authentication` for a complete setup tutorial. Plugin Framework diff --git a/docs/getting_started.rst b/docs/getting_started.rst index 7ee1ccd57..ed4bb8f66 100644 --- a/docs/getting_started.rst +++ b/docs/getting_started.rst @@ -40,7 +40,10 @@ Open three terminals. In each, source your workspace: .. code-block:: bash - ros2 launch ros2_medkit_gateway gateway.launch.py + ros2 launch ros2_medkit_gateway gateway.launch.py \ + tls_enabled:=false \ + jwt_secret:=change-me-to-at-least-32-characters-long \ + auth_clients:=demo:demo-secret:admin You should see: @@ -122,6 +125,41 @@ Required if you want to test the Faults API. The ``~/.ros2_medkit/`` directory must exist before starting the fault manager. SQLite will create the database file automatically. +.. important:: + + The gateway ships **closed**: ``auth.enabled`` is true and + ``require_auth_for`` is ``all``, so every route below needs a credential and + the gateway will not start without a signing secret. That is deliberate - a + gateway that booted open is one nobody notices. The two arguments above are + a throwaway development credential; a real deployment injects them from its + own secret store. + + Get a token once and reuse it for every command on this page: + + .. code-block:: bash + + TOKEN=$(curl -s http://localhost:8080/api/v1/auth/authorize \ + -H 'Content-Type: application/json' \ + -d '{"grant_type":"client_credentials","client_id":"demo","client_secret":"demo-secret"}' \ + | jq -r .access_token) + + ``tls_enabled:=false`` is what keeps the rest of this page on ``http://``. + TLS is on in the shipped config and the gateway will not start without a + certificate, so a first run either turns it off, as here, or supplies one: + run ``scripts/generate_dev_certs.sh ./certs`` and pass + ``cert_file:=./certs/cert.pem key_file:=./certs/key.pem``. Turn it off only + on a host nothing else can reach. See :doc:`tutorials/https` for a real + certificate. + + ``POST /api/v1/auth/authorize`` takes the ``client_credentials`` grant; + ``/auth/token`` is the refresh endpoint and takes ``refresh_token``. + + ``GET /api/v1/health`` is the other route that stays open, so a container + supervisor with no credential can still tell the process is alive. + + To run without authentication - only on a host nothing else can reach - + pass ``auth_enabled:=false``. + .. admonition:: ✅ Checkpoint :class: tip @@ -146,7 +184,7 @@ The gateway exposes all endpoints under ``/api/v1``. Let's explore! .. code-block:: bash - curl http://localhost:8080/api/v1/ + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/ Response shows available endpoints and version info. @@ -197,7 +235,7 @@ ros2_medkit organizes ROS 2 nodes into a SOVD-aligned entity hierarchy: .. code-block:: bash - curl http://localhost:8080/api/v1/functions + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions With ``demo_nodes.launch.py``, you'll see Functions like ``powertrain``, ``chassis``, and ``body`` (created from the first namespace segment). @@ -206,7 +244,7 @@ With ``demo_nodes.launch.py``, you'll see Functions like ``powertrain``, ``chass .. code-block:: bash - curl http://localhost:8080/api/v1/components + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components In runtime mode, you'll see a single host-level Component. @@ -214,7 +252,7 @@ In runtime mode, you'll see a single host-level Component. .. code-block:: bash - curl http://localhost:8080/api/v1/areas + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/areas In runtime mode, this returns an empty list. Areas require a manifest definition (see :doc:`tutorials/manifest-discovery`). @@ -228,7 +266,7 @@ The data endpoints let you read topic data from apps. .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/data + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/data Response structure (showing one topic): @@ -282,7 +320,7 @@ Each data item includes: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/data/powertrain%2Fengine%2Ftemperature + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/data/powertrain%2Fengine%2Ftemperature Response with live data: @@ -339,7 +377,7 @@ The operations endpoints let you call ROS 2 services and actions. .. code-block:: bash - curl http://localhost:8080/api/v1/apps/calibration/operations + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/calibration/operations **Call a service (synchronous execution):** @@ -387,7 +425,7 @@ Response (202 Accepted): .. code-block:: bash - curl http://localhost:8080/api/v1/apps/long_calibration/operations/long_calibration/executions/a1b2c3d4-e5f6-7890-abcd-ef1234567890 + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/long_calibration/operations/long_calibration/executions/a1b2c3d4-e5f6-7890-abcd-ef1234567890 **Cancel a running action:** @@ -406,13 +444,13 @@ The configurations endpoints expose ROS 2 parameters. .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/configurations + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/configurations **Get a specific parameter:** .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/configurations/publish_rate + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/configurations/publish_rate **Set a parameter value:** @@ -439,13 +477,13 @@ Step 7: Monitor Faults .. code-block:: bash - curl http://localhost:8080/api/v1/faults + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/faults **List faults for a specific component:** .. code-block:: bash - curl http://localhost:8080/api/v1/apps/lidar_sensor/faults + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/lidar_sensor/faults **Clear a fault:** diff --git a/docs/index.rst b/docs/index.rst index 1adf85ec5..e398425fa 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -45,7 +45,13 @@ Quick Links Quick Reference --------------- -Common commands for quick access: +Common commands for quick access. + +.. note:: + + The gateway ships closed: every command below needs a credential, and + ``GET /api/v1/health`` is the only one that does not. See + :doc:`getting_started` for how to obtain ``$TOKEN``. .. code-block:: bash @@ -53,28 +59,28 @@ Common commands for quick access: curl http://localhost:8080/api/v1/health # List all areas - curl http://localhost:8080/api/v1/areas + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/areas # List all components - curl http://localhost:8080/api/v1/components + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components # List all apps - curl http://localhost:8080/api/v1/apps + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps # List all functions - curl http://localhost:8080/api/v1/functions + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions # Get data from an entity (area, component, app, or function) - curl http://localhost:8080/api/v1/{entity-type}/{entity-id}/data + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/{entity-type}/{entity-id}/data # List operations for an entity (area, component, app, or function) - curl http://localhost:8080/api/v1/{entity-type}/{entity-id}/operations + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/{entity-type}/{entity-id}/operations # Get configurations (parameters) - curl http://localhost:8080/api/v1/{entity-type}/{entity-id}/configurations + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/{entity-type}/{entity-id}/configurations # List faults - curl http://localhost:8080/api/v1/faults + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/faults Community --------- diff --git a/docs/tutorials/authentication.rst b/docs/tutorials/authentication.rst index 301ab65bd..47f40619a 100644 --- a/docs/tutorials/authentication.rst +++ b/docs/tutorials/authentication.rst @@ -11,7 +11,7 @@ Role-Based Access Control (RBAC) in ros2_medkit_gateway. Overview -------- -By default, the gateway runs without authentication for easy development. +The gateway requires authentication by default. For production deployments, you should enable authentication to: - Control who can access the API diff --git a/docs/tutorials/beacon-discovery.rst b/docs/tutorials/beacon-discovery.rst index 43e0ad13d..b181a40e8 100644 --- a/docs/tutorials/beacon-discovery.rst +++ b/docs/tutorials/beacon-discovery.rst @@ -259,7 +259,7 @@ The plugin registers a vendor extension endpoint on all apps and components: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-topic-beacon + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-topic-beacon Example response: @@ -460,7 +460,7 @@ The plugin registers its own vendor extension endpoint: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-param-beacon + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/engine_temp_sensor/x-medkit-param-beacon The response format is identical to the topic beacon endpoint. diff --git a/docs/tutorials/demos/demo-sensor.rst b/docs/tutorials/demos/demo-sensor.rst index b2cf6087b..a0bf1343f 100644 --- a/docs/tutorials/demos/demo-sensor.rst +++ b/docs/tutorials/demos/demo-sensor.rst @@ -107,16 +107,16 @@ Query sensor data via REST API: .. code-block:: bash # Get LiDAR scan - curl http://localhost:8080/api/v1/apps/lidar-sim/data/scan | jq '.ranges[:5]' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/lidar-sim/data/scan | jq '.ranges[:5]' # Get IMU data - curl http://localhost:8080/api/v1/apps/imu-sim/data/imu | jq '.linear_acceleration' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/imu-sim/data/imu | jq '.linear_acceleration' # Get GPS fix - curl http://localhost:8080/api/v1/apps/gps-sim/data/fix | jq '{lat: .latitude, lon: .longitude}' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/gps-sim/data/fix | jq '{lat: .latitude, lon: .longitude}' # Get camera image info - curl http://localhost:8080/api/v1/apps/camera-sim/data/image | jq '{width, height, encoding}' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/camera-sim/data/image | jq '{width, height, encoding}' Managing Configurations ----------------------- @@ -128,10 +128,10 @@ View and modify sensor parameters: .. code-block:: bash # List all LiDAR configurations - curl http://localhost:8080/api/v1/apps/lidar-sim/configurations | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/lidar-sim/configurations | jq # Get specific parameter - curl http://localhost:8080/api/v1/apps/lidar-sim/configurations/noise_stddev | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/lidar-sim/configurations/noise_stddev | jq # Change scan rate curl -X PUT http://localhost:8080/api/v1/apps/lidar-sim/configurations/scan_rate \ @@ -176,10 +176,10 @@ faults at runtime using provided scripts: .. code-block:: bash # List all system faults - curl http://localhost:8080/api/v1/faults + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/faults # Get faults for specific sensor - curl http://localhost:8080/api/v1/apps/lidar-sim/faults + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/lidar-sim/faults **Manual fault injection via API:** diff --git a/docs/tutorials/demos/demo-turtlebot3.rst b/docs/tutorials/demos/demo-turtlebot3.rst index 24c609b08..ff3340e36 100644 --- a/docs/tutorials/demos/demo-turtlebot3.rst +++ b/docs/tutorials/demos/demo-turtlebot3.rst @@ -90,7 +90,7 @@ Querying via API: .. code-block:: bash - curl http://localhost:8080/api/v1/areas | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/areas | jq .. figure:: /_static/images/13_curl_areas_turtlebot3.png :alt: Areas response @@ -116,13 +116,13 @@ Query data via REST API: .. code-block:: bash # List all apps - curl http://localhost:8080/api/v1/apps | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps | jq # Get specific topic from AMCL localization - curl http://localhost:8080/api/v1/apps/amcl/data | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/amcl/data | jq # Get specific topic from controller server - curl http://localhost:8080/api/v1/apps/controller-server/data | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/controller-server/data | jq .. figure:: /_static/images/06_topic_data_view.png :alt: Topic data view @@ -148,10 +148,10 @@ You can also interact with the navigation stack via API: .. code-block:: bash # List operations on BT Navigator - curl http://localhost:8080/api/v1/apps/bt-navigator/operations | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/bt-navigator/operations | jq # List operations on Controller Server - curl http://localhost:8080/api/v1/apps/controller-server/operations | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/controller-server/operations | jq Managing Parameters ------------------- @@ -163,10 +163,10 @@ View and modify parameters: .. code-block:: bash # List all configurations for AMCL - curl http://localhost:8080/api/v1/apps/amcl/configurations | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/amcl/configurations | jq # Get specific parameter - curl http://localhost:8080/api/v1/apps/amcl/configurations/use_sim_time | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/amcl/configurations/use_sim_time | jq # Change parameter value curl -X PUT http://localhost:8080/api/v1/apps/amcl/configurations/use_sim_time \ @@ -199,7 +199,7 @@ The demo includes fault injection scripts to test diagnostic capabilities: ./check-faults.sh # Or query via API - curl http://localhost:8080/api/v1/faults + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/faults .. figure:: /_static/images/18_faults_injected_dashboard.png :alt: Faults dashboard diff --git a/docs/tutorials/docker.rst b/docs/tutorials/docker.rst index 84c042e67..92abb4382 100644 --- a/docs/tutorials/docker.rst +++ b/docs/tutorials/docker.rst @@ -58,12 +58,19 @@ Test the gateway: .. code-block:: bash - curl http://localhost:8080/api/v1/health - # {"status":"healthy","timestamp":...} + curl -i http://localhost:8080/api/v1/health + # HTTP/1.1 401 Unauthorized + # The image ships closed: every route needs a credential, health included. + # A 401 here is the gateway working - it received the request, routed it and + # refused it. - curl http://localhost:8080/api/v1/version-info + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/version-info # {"items":[{"version":"","vendor_info":{"name":"ros2_medkit",...}}]} +The container prints a generated ``client_id`` and ``client_secret`` on its +first line of output; exchange them for ``$TOKEN`` at ``/api/v1/auth/authorize``. +See :doc:`authentication`. + Custom Configuration -------------------- @@ -156,7 +163,15 @@ Example ``docker-compose.yml`` with the gateway and web UI: environment: - ROS_DOMAIN_ID=42 healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:8080/api/v1/health"] + # Any HTTP answer proves the process is up, 401 included. `curl -f` + # would exit non-zero on the refusal an authenticated gateway gives + # an uncredentialed probe, and report a healthy container as sick. + test: + - CMD-SHELL + - >- + code=$$(curl -s -o /dev/null -w '%{http_code}' + http://localhost:8080/api/v1/health) && case "$$code" in + 200|401|403) exit 0 ;; *) exit 1 ;; esac interval: 10s timeout: 5s retries: 3 @@ -230,17 +245,37 @@ writes. Add your own UI origin(s): Health Checks ------------- -The gateway exposes a health endpoint at ``/api/v1/health``: +The gateway exposes a health endpoint at ``/api/v1/health``. It needs a +credential like every other route, so a probe should read the status code +rather than insist on success: .. code-block:: yaml healthcheck: - test: ["CMD", "curl", "-f", "http://localhost:8080/api/v1/health"] + # 401 means the gateway is up and refused an uncredentialed probe, which + # is exactly what a liveness check wants to know. + test: + - CMD-SHELL + - >- + code=$$(curl -s -o /dev/null -w '%{http_code}' + http://localhost:8080/api/v1/health) && case "$$code" in + 200|401|403) exit 0 ;; *) exit 1 ;; esac interval: 10s timeout: 5s retries: 3 start_period: 15s +If a probe cannot be changed - a load balancer that only accepts 200, say - +open the route explicitly instead: + +.. code-block:: yaml + + auth: + public_routes: ["GET /api/v1/health"] + +An anonymous caller then gets liveness only, marked ``x-medkit-reduced``. See +:doc:`/config/server` for what that setting does and does not open. + Production Considerations ------------------------- diff --git a/docs/tutorials/fault-correlation.rst b/docs/tutorials/fault-correlation.rst index 07357780c..ca3d6cc4d 100644 --- a/docs/tutorials/fault-correlation.rst +++ b/docs/tutorials/fault-correlation.rst @@ -310,7 +310,7 @@ Querying Correlation Data .. code-block:: bash - curl http://localhost:8080/api/v1/faults + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/faults Response always includes: diff --git a/docs/tutorials/graph-provider.rst b/docs/tutorials/graph-provider.rst index 17b46c9b0..1082cd69b 100644 --- a/docs/tutorials/graph-provider.rst +++ b/docs/tutorials/graph-provider.rst @@ -211,7 +211,7 @@ The Discovery Path .. code-block:: bash - curl http://localhost:8080/api/v1/functions | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions | jq 2. **Read the Function's detail** and follow its capability href. A Function detail response carries an ``"x-medkit-graph"`` link exactly while this @@ -220,7 +220,7 @@ The Discovery Path .. code-block:: bash - curl http://localhost:8080/api/v1/functions/engine-monitoring | jq '."x-medkit-graph"' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions/engine-monitoring | jq '."x-medkit-graph"' # "/api/v1/functions/engine-monitoring/x-medkit-graph" 3. **GET the graph** at that href. @@ -235,7 +235,7 @@ Function ``engine-monitoring`` hosting an ``engine-temp-sensor`` App (publishes .. code-block:: bash - curl http://localhost:8080/api/v1/functions/engine-monitoring/x-medkit-graph | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions/engine-monitoring/x-medkit-graph | jq .. code-block:: json diff --git a/docs/tutorials/heuristic-apps.rst b/docs/tutorials/heuristic-apps.rst index 8d4b74638..638d98d5e 100644 --- a/docs/tutorials/heuristic-apps.rst +++ b/docs/tutorials/heuristic-apps.rst @@ -55,7 +55,7 @@ Query available Apps: .. code-block:: bash - curl http://localhost:8080/api/v1/apps | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps | jq Example response: @@ -120,16 +120,16 @@ In runtime mode, the gateway maps the ROS 2 graph as follows: .. code-block:: bash - curl http://localhost:8080/api/v1/apps + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps # Returns: [{"id": "lidar_driver"}, {"id": "camera_node"}] - curl http://localhost:8080/api/v1/components + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components # Returns: [{"id": "my-hostname", "source": "runtime", ...}] - curl http://localhost:8080/api/v1/functions + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions # Returns: [{"id": "perception"}, {"id": "navigation"}] - curl http://localhost:8080/api/v1/areas + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/areas # Returns: {"items": []} (empty - Areas come from manifest only) API Endpoints @@ -159,7 +159,7 @@ Component derived from system information: .. code-block:: bash - curl http://localhost:8080/api/v1/components | jq '.items[] | {id, source}' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components | jq '.items[] | {id, source}' .. code-block:: json @@ -177,7 +177,7 @@ In runtime mode, Functions are created from the first namespace segment: .. code-block:: bash - curl http://localhost:8080/api/v1/functions | jq '.items[] | {id}' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions | jq '.items[] | {id}' .. code-block:: json diff --git a/docs/tutorials/https.rst b/docs/tutorials/https.rst index 5604fbd8a..016f9380b 100644 --- a/docs/tutorials/https.rst +++ b/docs/tutorials/https.rst @@ -11,7 +11,7 @@ encrypted HTTPS communication with the gateway. Overview -------- -By default, the gateway uses plain HTTP. For production deployments, +The gateway serves HTTPS by default. For production deployments, you should enable TLS to: - Encrypt all traffic between clients and the gateway @@ -100,10 +100,69 @@ Configuration Options - Path to PEM-encoded private key * - ``server.tls.ca_file`` - ``""`` - - CA certificate (for future mutual TLS) + - CA that signs client certificates. Setting it turns on mutual TLS and + makes a client certificate **required**; leave empty for server-only TLS * - ``server.tls.min_version`` - ``"1.2"`` - - Minimum TLS version: ``"1.2"`` or ``"1.3"`` + - Minimum TLS version: ``"1.2"`` or ``"1.3"``. Enforced on the server's own + SSL context, so it is the floor regardless of what the local OpenSSL + policy would otherwise allow. Any other value is rejected at startup + +Defaults +-------- + +TLS is **on** in the shipped ``gateway_params.yaml``, and ``cert_file`` and +``key_file`` are empty. A gateway with TLS enabled and no certificate refuses +to start rather than fall back to plaintext, so a first run has to supply one +of the two: + +.. code-block:: bash + + # a certificate, for a real deployment or a self-signed pair for a first run + ros2 launch ros2_medkit_gateway gateway.launch.py \ + cert_file:=/path/to/cert.pem key_file:=/path/to/key.pem + + # or no TLS at all, only on a host nothing else can reach + ros2 launch ros2_medkit_gateway gateway.launch.py tls_enabled:=false + +For a first run on a developer machine, ``scripts/generate_dev_certs.sh`` +writes a self-signed certificate and key. Browsers and ``curl`` will refuse it +until you pass the CA explicitly, which is the correct behaviour for a +certificate nothing has vouched for, not a problem to work around in +production. + +Mutual TLS +---------- + +Set ``ca_file`` to the CA that signs your client certificates and the gateway +requires one from **every** client: + +.. code-block:: yaml + + server: + tls: + enabled: true + cert_file: "/etc/ros2_medkit/certs/server.pem" + key_file: "/etc/ros2_medkit/certs/server-key.pem" + ca_file: "/etc/ros2_medkit/certs/client-ca.pem" + +This is all or nothing per gateway. A client that presents no certificate is +rejected during the handshake, before any request is read, and there is no +"verify it only if offered" setting. A client whose certificate is signed by +any other CA is rejected the same way. + +.. code-block:: bash + + # without a client certificate: no response, the handshake never completes + curl --cacert ca.pem https://localhost:8443/api/v1/areas + + # with one signed by ca_file + curl --cacert ca.pem --cert client.pem --key client-key.pem \ + https://localhost:8443/api/v1/areas + +Mutual TLS is transport-level and sits alongside token authentication rather +than replacing it. SOVD authenticates with bearer tokens, so leave ``ca_file`` +empty unless every client on that network can be issued a certificate. Using with curl --------------- diff --git a/docs/tutorials/linux-introspection.rst b/docs/tutorials/linux-introspection.rst index 50d8938cd..6e9c340f2 100644 --- a/docs/tutorials/linux-introspection.rst +++ b/docs/tutorials/linux-introspection.rst @@ -112,7 +112,7 @@ Returns process-level metrics for a single ROS 2 node: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/x-medkit-procfs | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/x-medkit-procfs | jq .. code-block:: json @@ -136,7 +136,7 @@ Each entry includes a ``node_ids`` array listing the Apps that share the process .. code-block:: bash - curl http://localhost:8080/api/v1/components/sensor_suite/x-medkit-procfs | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/sensor_suite/x-medkit-procfs | jq .. code-block:: json @@ -167,7 +167,7 @@ Returns the systemd unit managing the node's process: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/x-medkit-systemd | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/x-medkit-systemd | jq .. code-block:: json @@ -186,7 +186,7 @@ Returns aggregated unit info for all child Apps, deduplicated by unit name: .. code-block:: bash - curl http://localhost:8080/api/v1/components/sensor_suite/x-medkit-systemd | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/sensor_suite/x-medkit-systemd | jq .. code-block:: json @@ -213,7 +213,7 @@ Returns container metadata for a node running inside a container: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/x-medkit-container | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/x-medkit-container | jq .. code-block:: json @@ -249,7 +249,7 @@ Returns aggregated container info for all child Apps, deduplicated by container .. code-block:: bash - curl http://localhost:8080/api/v1/components/sensor_suite/x-medkit-container | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/sensor_suite/x-medkit-container | jq .. code-block:: json diff --git a/docs/tutorials/locking.rst b/docs/tutorials/locking.rst index acfb2639c..00901525f 100644 --- a/docs/tutorials/locking.rst +++ b/docs/tutorials/locking.rst @@ -117,7 +117,7 @@ Check what locks exist on an entity: .. code-block:: bash - curl http://localhost:8080/api/v1/components/motor_controller/locks \ + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/motor_controller/locks \ -H "X-Client-Id: $CLIENT_ID" The ``owned`` field in each lock item indicates whether the requesting client diff --git a/docs/tutorials/manifest-discovery.rst b/docs/tutorials/manifest-discovery.rst index 83e0e9f35..14295f1c4 100644 --- a/docs/tutorials/manifest-discovery.rst +++ b/docs/tutorials/manifest-discovery.rst @@ -173,7 +173,7 @@ Check manifest status: .. code-block:: bash - curl http://localhost:8080/api/v1/manifest/status + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/manifest/status Expected response: @@ -195,13 +195,13 @@ List apps: .. code-block:: bash - curl http://localhost:8080/api/v1/apps + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps List functions: .. code-block:: bash - curl http://localhost:8080/api/v1/functions + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/functions Understanding Hybrid Mode ------------------------- @@ -336,7 +336,7 @@ Check which apps are online: .. code-block:: bash - curl http://localhost:8080/api/v1/apps | jq '.items[] | {id, name, is_online}' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps | jq '.items[] | {id, name, is_online}' Example response: diff --git a/docs/tutorials/migration-to-manifest.rst b/docs/tutorials/migration-to-manifest.rst index ea7024657..9f8752b65 100644 --- a/docs/tutorials/migration-to-manifest.rst +++ b/docs/tutorials/migration-to-manifest.rst @@ -323,13 +323,13 @@ Step 7: Test in Hybrid Mode .. code-block:: bash - curl http://localhost:8080/api/v1/manifest/status | jq + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/manifest/status | jq 4. **Verify apps are linked**: .. code-block:: bash - curl http://localhost:8080/api/v1/apps | jq '.items[] | {id, name, is_online}' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps | jq '.items[] | {id, name, is_online}' 5. **Check for orphan nodes** (warnings in gateway logs): diff --git a/docs/tutorials/openapi.rst b/docs/tutorials/openapi.rst index 053c9f892..6e71db8c5 100644 --- a/docs/tutorials/openapi.rst +++ b/docs/tutorials/openapi.rst @@ -15,16 +15,16 @@ Append ``/docs`` to any valid API path: .. code-block:: bash # Full gateway spec (all endpoints) - curl http://localhost:8080/api/v1/docs | jq . + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/docs | jq . # Spec scoped to the components collection - curl http://localhost:8080/api/v1/components/docs + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/docs # Spec for a specific component and its resource collections - curl http://localhost:8080/api/v1/components/my_sensor/docs + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/my_sensor/docs # Spec for one resource collection (e.g. data) - curl http://localhost:8080/api/v1/components/my_sensor/data/docs + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/my_sensor/data/docs Entity-level specs reflect the actual capabilities of each entity at runtime. Plugin-registered vendor routes also appear when the requested diff --git a/docs/tutorials/scripts.rst b/docs/tutorials/scripts.rst index e395ebf33..48b0c8b2d 100644 --- a/docs/tutorials/scripts.rst +++ b/docs/tutorials/scripts.rst @@ -82,7 +82,7 @@ Quick Example .. code-block:: bash - curl http://localhost:8080/api/v1/components/main-computer/scripts/script_1717123456_0/executions/exec_1717123500_0 + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/main-computer/scripts/script_1717123456_0/executions/exec_1717123500_0 Response when finished: diff --git a/docs/tutorials/snapshots.rst b/docs/tutorials/snapshots.rst index f17d55d7e..cd3aad85c 100644 --- a/docs/tutorials/snapshots.rst +++ b/docs/tutorials/snapshots.rst @@ -56,7 +56,7 @@ Quick Start .. code-block:: bash - curl http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/faults/MOTOR_OVERHEAT/snapshots Configuration Options --------------------- @@ -241,7 +241,7 @@ Snapshots are included inline in the fault response as ``environment_data``: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/motor_controller/faults/MOTOR_OVERHEAT + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/motor_controller/faults/MOTOR_OVERHEAT **Response:** @@ -307,7 +307,7 @@ Snapshots are included inline in the fault response as ``environment_data``: .. code-block:: bash - curl http://localhost:8080/api/v1/apps/motor_controller/faults/MOTOR_OVERHEAT | \ + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/motor_controller/faults/MOTOR_OVERHEAT | \ jq '.environment_data.snapshots' Example Workflow @@ -342,7 +342,7 @@ This example demonstrates the complete snapshot capture workflow. .. code-block:: bash - curl http://localhost:8080/api/v1/apps/nav_node/faults/NAV_ERROR | \ + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/nav_node/faults/NAV_ERROR | \ jq '.environment_data.snapshots' The response will contain the odometry data that was captured at the @@ -737,7 +737,7 @@ Rosbag files are downloaded via SOVD bulk-data endpoints. .. code-block:: bash - curl http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/motor_controller/bulk-data/rosbags One item per **recording**, not per fault. A burst of correlated faults shares a single recording and appears once, with every fault it covers listed in diff --git a/docs/tutorials/triggers-use-cases.rst b/docs/tutorials/triggers-use-cases.rst index fb68119b6..63b1040fb 100644 --- a/docs/tutorials/triggers-use-cases.rst +++ b/docs/tutorials/triggers-use-cases.rst @@ -119,7 +119,7 @@ Step 4: List all triggers .. code-block:: bash - curl http://localhost:8080/api/v1/apps/temp_sensor/triggers + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/triggers **Response:** @@ -281,7 +281,7 @@ component. .. code-block:: bash - curl http://localhost:8080/api/v1/components | jq '.items[].id' + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components | jq '.items[].id' If ``engine`` does not appear, the demo nodes may not be running or the namespace grouping may differ. Adjust ``engine`` to match the actual @@ -306,10 +306,10 @@ Step 4: Verify all triggers .. code-block:: bash # App-level triggers - curl http://localhost:8080/api/v1/apps/temp_sensor/triggers + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/temp_sensor/triggers # Component-level triggers - curl http://localhost:8080/api/v1/components/engine/triggers + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/components/engine/triggers Step 5: Connect SSE streams and observe cascade ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -424,10 +424,10 @@ Step 4: Verify triggers on different entity types .. code-block:: bash # Area-level triggers - curl http://localhost:8080/api/v1/areas/powertrain/triggers + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/areas/powertrain/triggers # App-level triggers - curl http://localhost:8080/api/v1/apps/engine-temp-sensor/triggers + curl -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/v1/apps/engine-temp-sensor/triggers Step 5: Connect SSE streams ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/postman/README.md b/postman/README.md index fa55023bf..8a2f1fe73 100644 --- a/postman/README.md +++ b/postman/README.md @@ -92,7 +92,7 @@ ros2 launch ros2_medkit_gateway gateway.launch.py 4. Tokens are automatically saved to environment variables 5. Use `{{access_token}}` in Authorization header for protected endpoints -> **Note:** Auth endpoints are always accessible. By default (`require_auth_for: write`), only write operations (POST, PUT, DELETE) require authentication. GET requests work without a token. +> **Note:** Auth endpoints are always accessible. By default (`require_auth_for: all`), every request needs a token, GET included. **Discovery:** 1. Expand **"Discovery"** folder diff --git a/postman/collections/ros2-medkit-gateway.postman_collection.json b/postman/collections/ros2-medkit-gateway.postman_collection.json index 5a138a0da..db08042f6 100644 --- a/postman/collections/ros2-medkit-gateway.postman_collection.json +++ b/postman/collections/ros2-medkit-gateway.postman_collection.json @@ -2101,5 +2101,15 @@ } ] } - ] + ], + "auth": { + "type": "bearer", + "bearer": [ + { + "key": "token", + "value": "{{access_token}}", + "type": "string" + } + ] + } } diff --git a/scripts/pixi-smoke-test.sh b/scripts/pixi-smoke-test.sh index 077baddc9..6a8203a03 100755 --- a/scripts/pixi-smoke-test.sh +++ b/scripts/pixi-smoke-test.sh @@ -29,7 +29,12 @@ fi PORT="${GATEWAY_SMOKE_PORT:-8080}" TIMEOUT=30 -ros2 launch ros2_medkit_gateway gateway.launch.py server_port:="$PORT" & +# The shipped config has TLS and authentication on, so a bare launch refuses to +# start. This is a smoke test for "does the gateway come up and answer", not for +# the security posture, so it runs the way a developer on a local machine would: +# plain HTTP, no credential. test_shipped_defaults covers the shipped posture. +ros2 launch ros2_medkit_gateway gateway.launch.py \ + server_port:="$PORT" tls_enabled:=false auth_enabled:=false & GW_PID=$! # shellcheck disable=SC2317 # cleanup is invoked indirectly via trap diff --git a/src/ros2_medkit_gateway/README.md b/src/ros2_medkit_gateway/README.md index 0cff2b0d3..4b4ee543d 100644 --- a/src/ros2_medkit_gateway/README.md +++ b/src/ros2_medkit_gateway/README.md @@ -1485,32 +1485,32 @@ Cross-Origin Resource Sharing (CORS) settings for browser-based clients. CORS is #### Authentication Configuration -JWT-based authentication with Role-Based Access Control (RBAC). Authentication is **disabled by default** for backward compatibility. +JWT-based authentication with Role-Based Access Control (RBAC). Authentication is **enabled by default** for backward compatibility. | Parameter | Type | Default | Description | | ----------------------------------- | -------- | --------------------- | --------------------------------------------------------------------------- | -| `auth.enabled` | bool | `false` | Enable/disable authentication. Set to `true` to require auth. | +| `auth.enabled` | bool | `true` | Enable/disable authentication. Set to `true` to require auth. | | `auth.jwt_secret` | string | (required if enabled) | Secret key for HS256 signing. Must be at least 32 characters. | | `auth.jwt_algorithm` | string | `HS256` | JWT signing algorithm: `HS256` (symmetric) or `RS256` (asymmetric). | | `auth.token_expiry_seconds` | int | `3600` | Access token lifetime in seconds (range: 60-86400). | | `auth.refresh_token_expiry_seconds` | int | `86400` | Refresh token lifetime in seconds (range: 300-604800). | -| `auth.require_auth_for` | string | `write` | Auth requirement: `none`, `write` (POST/PUT/DELETE only), or `all`. | +| `auth.require_auth_for` | string | `all` | Auth requirement: `none`, `write` (POST/PUT/DELETE only), or `all`. | | `auth.issuer` | string | `ros2_medkit_gateway` | JWT issuer claim for token validation. | | `auth.clients` | string[] | `[]` | Client credentials in format `client_id:client_secret:role`. | #### TLS/HTTPS Configuration -TLS (Transport Layer Security) enables encrypted HTTPS communication. TLS is **disabled by default** for backward compatibility. +TLS (Transport Layer Security) enables encrypted HTTPS communication. TLS is **enabled by default**; the gateway refuses to start without a certificate. | Parameter | Type | Default | Description | | ---------------------------- | ------ | ------- | --------------------------------------------------------------------------- | -| `server.tls.enabled` | bool | `false` | Enable/disable TLS. When enabled, server uses HTTPS instead of HTTP. | +| `server.tls.enabled` | bool | `true` | Enable/disable TLS. When enabled, server uses HTTPS instead of HTTP. | | `server.tls.cert_file` | string | (required if enabled) | Path to PEM-encoded certificate file. | | `server.tls.key_file` | string | (required if enabled) | Path to PEM-encoded private key file. | -| `server.tls.ca_file` | string | `""` | Optional CA certificate (reserved for future mutual TLS support). | +| `server.tls.ca_file` | string | `""` | CA that signs CLIENT certificates. Setting it enables mutual TLS and REQUIRES a client certificate from every caller. | | `server.tls.min_version` | string | `"1.2"` | Minimum TLS version: `"1.2"` (compatible) or `"1.3"` (more secure). | -> **Note:** Mutual TLS (client certificate verification) is planned for a future release. +> **Note:** Mutual TLS is available: set `server.tls.ca_file`. **Roles and Permissions:** @@ -1581,7 +1581,7 @@ auth: jwt_algorithm: "HS256" token_expiry_seconds: 3600 refresh_token_expiry_seconds: 86400 - require_auth_for: "write" # GET requests work without auth + require_auth_for: "all" # every request needs a token, GET included issuer: "ros2_medkit_gateway" clients: - "admin:admin_secret:admin" diff --git a/src/ros2_medkit_gateway/config/gateway_params.yaml b/src/ros2_medkit_gateway/config/gateway_params.yaml index d6a4dce4d..8c30d4674 100644 --- a/src/ros2_medkit_gateway/config/gateway_params.yaml +++ b/src/ros2_medkit_gateway/config/gateway_params.yaml @@ -80,8 +80,14 @@ ros2_medkit_gateway: # TLS/HTTPS Configuration # Enables encrypted communication using OpenSSL tls: - # Enable/disable TLS (default: false for backward compatibility) - enabled: false + # On, so a gateway reachable from anything but loopback is encrypted + # without a deployment having to remember to turn it on. cert_file and + # key_file below must be filled in; the gateway refuses to start with + # TLS on and no certificate, which is the intended failure - an + # unencrypted gateway that started anyway is the worse outcome. + # Turn OFF only for a gateway bound to 127.0.0.1 behind a TLS + # terminator that is itself doing this job. + enabled: true # Path to PEM-encoded certificate file (required when TLS enabled) # Example: "/etc/ros2_medkit/certs/cert.pem" @@ -101,8 +107,11 @@ ros2_medkit_gateway: # Options: "1.2" (default, widely compatible), "1.3" (more secure) min_version: "1.2" - # TODO: Mutual TLS (client certificate verification) is not yet implemented - # See: https://github.com/selfpatch/ros2_medkit/issues/XXX + # Mutual TLS. Set ca_file above to the CA that signs your client + # certificates and the gateway REQUIRES one from every client: a + # caller with no certificate is rejected during the handshake, before + # any request is read. Leave ca_file empty for ordinary server-only + # TLS, which is what bearer-token clients expect. # Safety-backstop refresh interval in milliseconds. # @@ -360,11 +369,22 @@ ros2_medkit_gateway: # Authentication Configuration (REQ_INTEROP_086, REQ_INTEROP_087) # JWT-based authentication with Role-Based Access Control (RBAC) auth: - # Enable/disable authentication - # Default: false (disabled for local development) - enabled: false + # On. This file is the default a deployment gets when it brings no + # profile of its own, so it has to be the safe one: an unauthenticated + # SOVD gateway exposes the entity tree, the fault history and every + # operation the plugins register. + # + # The gateway REFUSES TO START while this is true and jwt_secret is + # empty (auth_config.cpp: "JWT secret is required when authentication is + # enabled"). That is deliberate. A gateway that will not boot is a + # deployment problem someone fixes in a minute; a gateway that booted + # open is one nobody notices. + enabled: true - # JWT signing secret (required when enabled) + # JWT signing secret. REQUIRED - the gateway will not start without it + # while auth.enabled is true. At least 32 characters for HS256. + # Inject it from a secret store or the deployment's own configuration; + # do not commit a real secret here. # For HS256: The shared secret string # For RS256: Path to the private key file (PEM format) jwt_secret: "" @@ -390,8 +410,40 @@ ros2_medkit_gateway: # - "none": No authentication required (auth endpoints still available) # - "write": Auth required for write operations (POST, PUT, DELETE) # - "all": Auth required for all operations - # Default: "write" - require_auth_for: "write" + # + # "all", not "write". Under "write" every read stays open even with + # authentication switched on, and the reads are where the disclosure is: + # the entity tree names the machines, the fault history is the + # maintenance record. Both are readable by anyone who can reach the port. + # Only /auth/* stays public under "all", because authentication cannot + # bootstrap through a door that demands the credential it hands out. + require_auth_for: "all" + + # Routes answered with no credential at all, on top of whatever + # require_auth_for decides. Each entry is "METHOD /path", matched + # exactly: no wildcards, so this list is the whole public surface and a + # reviewer can read it as such. + # + # Empty as shipped. Add an entry when something that cannot hold a + # credential has to reach a route - a container supervisor or a load + # balancer probing health is the case this exists for: + # + # public_routes: ["GET /api/v1/health"] + # + # A liveness probe usually needs no entry: a 401 already proves the + # process is up and answering HTTP. Prefer teaching the probe to accept + # it over opening the route. When the route is opened, the body an + # anonymous caller gets is narrowed to liveness - no entity names, no + # counts - so the probe works and the disclosure does not follow. + # + # Every entry is logged at WARN on startup, once per route. + # + # Left absent rather than written as `public_routes: []`. An empty YAML + # sequence carries no type, so rclcpp cannot tell a string array from any + # other and the node dies at startup with "No parameter value set". The + # declared default is already empty, so absence and `[]` mean the same + # thing - one of them just starts. + # public_routes: ["GET /api/v1/health"] # JWT issuer claim # Default: "ros2_medkit_gateway" diff --git a/src/ros2_medkit_gateway/design/hardening.rst b/src/ros2_medkit_gateway/design/hardening.rst index 16311c0f3..4f4630980 100644 --- a/src/ros2_medkit_gateway/design/hardening.rst +++ b/src/ros2_medkit_gateway/design/hardening.rst @@ -1,15 +1,28 @@ Gateway hardening (secure field profile) ======================================== -The gateway ships every transport and access control needed for a hardened -deployment - JWT authentication with RBAC, TLS/HTTPS, restricted CORS, and -token-bucket rate limiting - but they are **disabled by default** so local -development works out of the box. A gateway exposed on a plant network with the -defaults is wide open: unauthenticated reads and writes over cleartext HTTP. - -For any deployment reachable from an untrusted network, start from the secure -field profile preset ``config/gateway_params.secure.yaml`` instead of -``config/gateway_params.yaml``: +The gateway ships **closed**. ``config/gateway_params.yaml`` sets +``auth.enabled: true``, ``auth.require_auth_for: "all"`` and +``server.tls.enabled: true``, so out of the box every route needs a credential +and the transport is encrypted. + +Two consequences worth stating plainly: + +* **The gateway refuses to start without a signing secret.** With auth enabled + and ``auth.jwt_secret`` empty it exits with "JWT secret is required when + authentication is enabled" (and HS256 additionally requires at least 32 + characters). This is intended. A gateway that will not boot is a deployment + problem someone fixes in a minute; a gateway that booted open is one nobody + notices. +* **``GET /api/v1/health`` and ``/api/v1/auth/*`` stay public.** Health so a + container supervisor or load balancer with no credential can tell the + process is alive - it carries a fixed status document and no topology. Auth + because authentication cannot bootstrap through a door that already demands + the credential it exists to hand out. Nothing else is exempt. + +For a deployment reachable from an untrusted network, the remaining controls - +restricted CORS, rate limiting, locking, a reduced surface - are collected in +the secure field profile preset ``config/gateway_params.secure.yaml``: .. code-block:: bash @@ -24,9 +37,10 @@ What the secure profile turns on ================================ ============== =========================================== Control Default Secure profile ================================ ============== =========================================== -``auth.enabled`` false true -``auth.require_auth_for`` write all (auth on reads + writes) -``server.tls.enabled`` false true (HTTPS, min TLS 1.3) +``auth.enabled`` true true +``auth.require_auth_for`` all all (auth on reads + writes) +``server.tls.enabled`` true true (HTTPS, min TLS 1.3) +``auth.jwt_secret`` *unset* *unset* - both REQUIRE one at deploy time ``cors.allowed_origins`` ``[]`` explicit origin list (no wildcard) ``rate_limiting.enabled`` false true (global + per-client + per-endpoint) ``scripts.allow_uploads`` true false (manifest-defined scripts only) @@ -35,6 +49,20 @@ Control Default Secure profile ``locking`` on operations none lock required before mutation ================================ ============== =========================================== +The access-control rows now match: the difference between the two files is the +surface reduction below them, not whether the door is locked. + +Running without authentication +------------------------------ + +On a host nothing else can reach - a laptop, a CI job, a single-container demo +- pass ``auth_enabled:=false`` to ``gateway.launch.py``, or set +``auth.enabled: false`` in your own params file. Do this deliberately and never +on a machine reachable from a plant or office network: with authentication off +the entity tree names the machines, the fault history is the maintenance record +of the line, and every registered operation is callable by anyone who can reach +the port. + Credential and certificate provisioning ---------------------------------------- diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_config.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_config.hpp index b77acb99a..3970a1aa0 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_config.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_config.hpp @@ -91,6 +91,15 @@ struct AuthConfig { // Pre-configured clients (for development/testing) std::vector clients; + /// Routes answered with no credential, each written "METHOD /path". + /// + /// Empty by default, so `require_auth_for` alone decides and the gateway is + /// closed as shipped. An operator adds an entry when something that cannot + /// hold a credential has to reach a route - a container supervisor probing + /// `GET /api/v1/health` is the case this exists for. Matching is exact and + /// there are no wildcards, so the list reads as the whole public surface. + std::vector public_routes; + /// Permission entries for the routes the `RouteRegistry` does not hold. /// /// The gateway's own routes derive their entries from their registration @@ -119,6 +128,7 @@ class AuthConfigBuilder { AuthConfigBuilder & with_refresh_token_expiry(int seconds); AuthConfigBuilder & with_require_auth_for(AuthRequirement requirement); AuthConfigBuilder & with_issuer(const std::string & issuer); + AuthConfigBuilder & with_public_routes(const std::vector & public_routes); AuthConfigBuilder & add_client(const std::string & client_id, const std::string & client_secret, UserRole role); AuthConfig build(); diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_manager.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_manager.hpp index 578a927cb..da0570c8e 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_manager.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_manager.hpp @@ -186,6 +186,13 @@ class AuthManager { */ bool enable_client(const std::string & client_id); + /// How many refresh records are currently held. + /// + /// Public so a test can observe that the sweep actually runs. The count is + /// the thing the unbounded-growth claim is about, and asserting on it is the + /// only way to tell a sweep that works from one that is never called. + size_t refresh_token_count() const; + private: /** * @brief Generate a JWT token @@ -242,6 +249,10 @@ class AuthManager { mutable std::mutex clients_mutex_; std::unordered_map clients_; + /// Drop every expired record. The caller must already hold + /// refresh_tokens_mutex_; cleanup_expired_tokens() is the locking wrapper. + size_t cleanup_expired_locked(); + // Refresh token storage (thread-safe) mutable std::mutex refresh_tokens_mutex_; std::unordered_map refresh_tokens_; diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_requirement_policy.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_requirement_policy.hpp index 6e25328eb..2a6960696 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_requirement_policy.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/core/auth/auth_requirement_policy.hpp @@ -14,6 +14,8 @@ #pragma once +#include + #include #include #include @@ -69,13 +71,17 @@ class NoAuthRequirementPolicy : public IAuthRequirementPolicy { /** * @brief Policy that always requires authentication * - * Except for public endpoints (auth endpoints, health check) + * The only exception is anything under `/api/v1/auth/`. Authentication cannot + * bootstrap through a door that demands the credential it exists to hand out. + * + * Nothing else is public here, health probes included. An operator who needs + * a route answered without a credential names it in `auth.public_routes`, + * which layers over this policy - see PublicRouteExemptionPolicy. */ class AllAuthRequirementPolicy : public IAuthRequirementPolicy { public: bool requires_authentication(const std::string & method, const std::string & path) const override { (void)method; - // Auth endpoints are always public (to allow login) return path.find("/api/v1/auth/") != 0; } @@ -155,6 +161,62 @@ class ConfigurableAuthRequirementPolicy : public IAuthRequirementPolicy { bool use_requirements_map_; }; +/// One entry of `auth.public_routes`: a method and a path this gateway answers +/// with no credential at all. +struct PublicRoute { + std::string method; ///< Upper-case HTTP method, e.g. "GET" + std::string path; ///< Full request path, e.g. "/api/v1/health" + + bool operator==(const PublicRoute & other) const { + return method == other.method && path == other.path; + } +}; + +/// Parses one `auth.public_routes` entry, written "METHOD /path". +/// +/// Matching is exact and there are no wildcards, so an operator cannot open a +/// subtree by accident: every route that stops requiring a credential is a +/// line somebody wrote and a reviewer can read. `GET /api/v1/health` opens the +/// health probe and nothing else, where `GET /api/v1/*` would have opened the +/// whole read surface with one character. +/// +/// @return the parsed route, or a message naming what is wrong with the entry. +tl::expected parse_public_route(const std::string & entry); + +/// Parses a whole `auth.public_routes` list, dropping entries that do not +/// parse. Dropping keeps the route protected, which is the safe reading of a +/// malformed entry; GatewayNode validates the list first and refuses to start +/// rather than let a typo silently protect a route the operator wanted open. +std::vector parse_public_routes(const std::vector & entries); + +/** + * @brief Layers an operator's `auth.public_routes` over another policy + * + * `require_auth_for` stays the primary axis; this only ever *removes* the + * credential requirement, never adds one, so wrapping cannot make a gateway + * stricter than the policy underneath and cannot be used to shadow it. + * + * The gateway ships with an empty list, which makes this a no-op: closed by + * default, and open exactly where somebody said so. + */ +class PublicRouteExemptionPolicy : public IAuthRequirementPolicy { + public: + PublicRouteExemptionPolicy(std::unique_ptr inner, std::vector public_routes); + + bool requires_authentication(const std::string & method, const std::string & path) const override; + + std::string description() const override; + + /// The routes this layer exempts, in the order they were configured. + const std::vector & public_routes() const { + return public_routes_; + } + + private: + std::unique_ptr inner_; + std::vector public_routes_; +}; + /** * @brief Factory to create auth requirement policies from configuration */ @@ -173,6 +235,15 @@ class AuthRequirementPolicyFactory { * @return Policy implementation based on config.enabled and config.auth_requirements */ static std::unique_ptr create(const AuthConfig & config); + + /** + * @brief Create policy for a requirement level, exempting configured routes + * @param requirement The auth requirement level + * @param public_routes Entries of `auth.public_routes`, already parsed + * @return The requirement policy, wrapped only when the list is non-empty + */ + static std::unique_ptr create(AuthRequirement requirement, + const std::vector & public_routes); }; } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/health.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/health.hpp index 7cbcb7cd5..32f31bf99 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/health.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/dto/health.hpp @@ -172,10 +172,21 @@ struct Health { /// why a peer refused their stream with 503. std::optional x_medkit_sse; std::optional peers; // free-form array of peer status objects + /// Present and true only when this answer was cut down for an anonymous + /// caller. Wire key: "x-medkit-reduced". + /// + /// `warnings` below promises that an empty array means "nothing flagged". + /// That promise cannot hold on a route an operator put in + /// `auth.public_routes`, where the sections are skipped before they are + /// built, so a monitor would read a withheld body as a clean bill of health. + /// This field is what tells it apart: when it is present, `warnings` says + /// nothing about the gateway and the caller needs a credential to learn more. + std::optional x_medkit_reduced; // wire key: "x-medkit-reduced" // NOT optional: these two are emitted in every mode, aggregation or not, so // the generated schema must list them in `required` and let a typed client // read them without a presence check. An empty `warnings` array is the - // "nothing flagged" answer - absence is not a value this endpoint has. + // "nothing flagged" answer - absence is not a value this endpoint has, + // except as qualified by `x_medkit_reduced` above. int64_t warning_schema_version{kWarningSchemaVersion}; std::vector warnings; }; @@ -190,8 +201,12 @@ inline constexpr auto dto_fields = std::make_tuple( field("discovery", &Health::discovery), field("x-medkit-data-provider", &Health::x_medkit_data_provider), field("x-medkit-subscription-executor", &Health::x_medkit_subscription_executor), field("x-medkit-entity-cache", &Health::x_medkit_entity_cache), field("x-medkit-sse", &Health::x_medkit_sse), - field("peers", &Health::peers), field("warning_schema_version", &Health::warning_schema_version), - field("warnings", &Health::warnings)); + field("peers", &Health::peers), + field("x-medkit-reduced", &Health::x_medkit_reduced, + "Present and true only when the caller presented no credential and the operator opened this route in " + "auth.public_routes. The answer then carries liveness only, and `warnings` says nothing about the " + "gateway - authenticate to read the full document."), + field("warning_schema_version", &Health::warning_schema_version), field("warnings", &Health::warnings)); template <> inline constexpr std::string_view dto_name = "HealthStatus"; diff --git a/src/ros2_medkit_gateway/launch/bringup.launch.py b/src/ros2_medkit_gateway/launch/bringup.launch.py index 4fb776651..442b91668 100644 --- a/src/ros2_medkit_gateway/launch/bringup.launch.py +++ b/src/ros2_medkit_gateway/launch/bringup.launch.py @@ -52,6 +52,12 @@ def generate_launch_description(): server_host = LaunchConfiguration('server_host') server_port = LaunchConfiguration('server_port') cors_allowed_origins = LaunchConfiguration('cors_allowed_origins') + tls_enabled = LaunchConfiguration('tls_enabled') + cert_file = LaunchConfiguration('cert_file') + key_file = LaunchConfiguration('key_file') + auth_enabled = LaunchConfiguration('auth_enabled') + jwt_secret = LaunchConfiguration('jwt_secret') + auth_clients = LaunchConfiguration('auth_clients') args = [ DeclareLaunchArgument( @@ -70,6 +76,34 @@ def generate_launch_description(): default_value='http://localhost:3000,http://localhost:5173', description='Comma-separated CORS origins allowed to call the gateway from a browser, ' 'so the web UI works out of the box. Empty disables CORS.'), + # Forwarded to gateway.launch.py. Without these the gateway's own + # defaults apply and bringup cannot start at all: the shipped config + # turns TLS and authentication on, and the gateway refuses to run + # without a certificate and a signing secret. Passing them here is what + # makes `ros2 launch ... bringup.launch.py tls_enabled:=false ...` a + # complete command rather than a dead end. + DeclareLaunchArgument( + 'tls_enabled', default_value='', + description='Serve HTTPS. Empty means leave it to the config file, which ' + 'has it on. Pass false to serve plain HTTP on a host nothing ' + 'else can reach; needs cert_file and key_file when on.'), + DeclareLaunchArgument( + 'cert_file', default_value='', + description='PEM certificate for HTTPS. Required while tls_enabled is true.'), + DeclareLaunchArgument( + 'key_file', default_value='', + description='PEM private key matching cert_file.'), + DeclareLaunchArgument( + 'auth_enabled', default_value='', + description='Require a credential on every request. Empty means leave it ' + 'to the config file, which has it on.'), + DeclareLaunchArgument( + 'jwt_secret', default_value='', + description='HS256 signing secret, at least 32 characters. Required while ' + 'auth_enabled is true.'), + DeclareLaunchArgument( + 'auth_clients', default_value='', + description='Comma-separated "client_id:client_secret:role" triples.'), DeclareLaunchArgument( 'enable_fault_manager', default_value='true', description='Start the fault_manager node.'), @@ -90,7 +124,10 @@ def generate_launch_description(): gateway = _include( 'ros2_medkit_gateway', 'gateway.launch.py', launch_arguments={'server_host': server_host, 'server_port': server_port, - 'cors_allowed_origins': cors_allowed_origins}) + 'cors_allowed_origins': cors_allowed_origins, + 'tls_enabled': tls_enabled, 'cert_file': cert_file, + 'key_file': key_file, 'auth_enabled': auth_enabled, + 'jwt_secret': jwt_secret, 'auth_clients': auth_clients}) fault_manager = _include( 'ros2_medkit_fault_manager', 'fault_manager.launch.py', enable_arg='enable_fault_manager', diff --git a/src/ros2_medkit_gateway/launch/gateway.launch.py b/src/ros2_medkit_gateway/launch/gateway.launch.py index 0da999c7f..9b6353ea5 100644 --- a/src/ros2_medkit_gateway/launch/gateway.launch.py +++ b/src/ros2_medkit_gateway/launch/gateway.launch.py @@ -27,6 +27,27 @@ CORS_DEFAULT = 'http://localhost:3000,http://localhost:5173' +def parse_security_flag(name, raw): + """ + Return True/False for a security launch argument, or raise on anything else. + + An allowlist for true with everything else meaning false is the wrong shape + here: ``tls_enabled:=on`` and ``tls_enabled:=ture`` would both mean "serve + plain HTTP", and the override is still written, so the typo beats a config + file that had TLS on. For a flag whose two values are "encrypted" and "not", + an unrecognised spelling has to stop the launch rather than pick one. + """ + value = raw.strip().lower() + if value in ('true', '1', 'yes', 'on'): + return True + if value in ('false', '0', 'no', 'off'): + return False + raise RuntimeError( + f'{name}:={raw!r} is not a boolean. Use true or false. ' + f'Leaving {name} unset lets the config file decide.' + ) + + def cors_override(cors_arg, config_file, default_config): """ Return the ``cors.allowed_origins`` entry for the final overrides, or {}. @@ -94,6 +115,53 @@ def generate_launch_description(): 'controls the periodic forced refresh. Must match the default ' 'in config/gateway_params.yaml.')) + declare_jwt_secret_arg = DeclareLaunchArgument( + 'jwt_secret', default_value='', + description=( + 'HS256 signing secret, at least 32 characters. REQUIRED: the ' + 'shipped config has auth.enabled true, and the gateway refuses to ' + 'start without a secret. Pass one here, or point config_file at a ' + 'file that sets auth.jwt_secret and auth.clients. To run without ' + 'authentication - only ever on a host nothing else can reach - ' + 'pass auth_enabled:=false explicitly.')) + + declare_auth_enabled_arg = DeclareLaunchArgument( + 'auth_enabled', default_value='', + description=( + 'Require a credential. On by default, matching the shipped ' + 'config. Turning it off makes the entity tree, the fault history ' + 'and every operation readable by anyone who can reach the port.')) + + declare_clients_arg = DeclareLaunchArgument( + 'auth_clients', default_value='', + description=( + 'Comma-separated "client_id:client_secret:role" triples ' + '(roles: viewer, operator, configurator, admin). Needed to obtain ' + 'a token from /auth/token.')) + + declare_tls_enabled_arg = DeclareLaunchArgument( + 'tls_enabled', default_value='', + description=( + 'Serve HTTPS. On by default, matching the shipped config. Needs ' + 'cert_file and key_file; the gateway refuses to start with TLS on ' + 'and no certificate rather than fall back to plaintext. Pass ' + 'tls_enabled:=false to serve plain HTTP on a host nothing else ' + 'can reach.')) + + declare_cert_file_arg = DeclareLaunchArgument( + 'cert_file', default_value='', + description=( + 'PEM certificate (or full chain) for HTTPS. REQUIRED while ' + 'tls_enabled is true. For a first run, generate a self-signed ' + 'pair with scripts/generate_dev_certs.sh - browsers will warn, ' + 'which is correct for a certificate nothing has vouched for.')) + + declare_key_file_arg = DeclareLaunchArgument( + 'key_file', default_value='', + description=( + 'PEM private key matching cert_file. REQUIRED while tls_enabled ' + 'is true. Keep it chmod 600 and owned by the gateway user.')) + declare_cors_arg = DeclareLaunchArgument( 'cors_allowed_origins', default_value=CORS_DEFAULT, @@ -120,6 +188,73 @@ def _launch_setup(context, *_args, **_kwargs): param_overrides.update(cors_override( LaunchConfiguration('cors_allowed_origins').perform(context), LaunchConfiguration('config_file').perform(context), default_config)) + + # Precedence: an explicit launch argument, then the environment, then + # whatever the config file says. Unset means "do not touch it", which + # matters because this launch file is included by others and is used + # with config_file: re-asserting a default here would silently override + # a value someone put in their own file on purpose. + tls_arg = LaunchConfiguration('tls_enabled').perform(context).strip() + if tls_arg: + tls_enabled = parse_security_flag('tls_enabled', tls_arg) + param_overrides['server.tls.enabled'] = tls_enabled + elif os.environ.get('MEDKIT_TLS_DISABLED') == '1': + # The container image serves plain HTTP behind whatever terminates + # TLS for it, and has no certificate of its own. An explicit + # tls_enabled:= above still wins over this. + tls_enabled = False + param_overrides['server.tls.enabled'] = False + else: + # Nothing said otherwise, so the shipped config decides. It has TLS + # on, and the warning below still applies to that case. + tls_enabled = True + cert_file = (LaunchConfiguration('cert_file').perform(context) + or os.environ.get('MEDKIT_TLS_CERT_FILE', '')) + key_file = (LaunchConfiguration('key_file').perform(context) + or os.environ.get('MEDKIT_TLS_KEY_FILE', '')) + if cert_file: + param_overrides['server.tls.cert_file'] = cert_file + if key_file: + param_overrides['server.tls.key_file'] = key_file + if tls_enabled and not (cert_file and key_file): + # The gateway would refuse to start a moment from now, naming the + # config file. Name the launch arguments instead, here, where they + # are the thing the reader can actually change. + print('[gateway.launch.py] TLS is enabled and cert_file/key_file were not both ' + 'given. Pass cert_file:= key_file:=, set them in a config_file, ' + 'or pass tls_enabled:=false to serve plain HTTP. ' + 'scripts/generate_dev_certs.sh makes a self-signed pair for a first run.') + + # Same precedence as TLS above: explicit argument, then environment, + # then the config file. The environment path is what makes the container + # image work - its entrypoint generates a per-container credential and + # exports it, and `ros2 launch` inside that container has no other way + # to receive it. + auth_arg = LaunchConfiguration('auth_enabled').perform(context).strip() + if auth_arg: + auth_enabled = parse_security_flag('auth_enabled', auth_arg) + param_overrides['auth.enabled'] = auth_enabled + elif os.environ.get('MEDKIT_AUTH_DISABLED') == '1': + auth_enabled = False + param_overrides['auth.enabled'] = False + else: + auth_enabled = True + jwt_secret = (LaunchConfiguration('jwt_secret').perform(context) + or os.environ.get('MEDKIT_JWT_SECRET', '')) + clients = (LaunchConfiguration('auth_clients').perform(context) + or os.environ.get('MEDKIT_CLIENTS', '')) + if jwt_secret: + param_overrides['auth.jwt_secret'] = jwt_secret + if clients: + param_overrides['auth.clients'] = [c for c in clients.split(',') if c] + if auth_enabled and not jwt_secret: + # The gateway would refuse to start a moment from now with a + # message about the config file. Say the actionable thing instead, + # here, where the launch argument that fixes it is in scope. + print('[gateway.launch.py] auth is enabled and no jwt_secret was given. ' + 'Pass jwt_secret:= and ' + 'auth_clients:=::admin, set them in a config_file, ' + 'or pass auth_enabled:=false to run without authentication.') return [Node( package='ros2_medkit_gateway', executable='gateway_node', @@ -133,6 +268,12 @@ def _launch_setup(context, *_args, **_kwargs): declare_host_arg, declare_port_arg, declare_refresh_arg, + declare_auth_enabled_arg, + declare_jwt_secret_arg, + declare_clients_arg, + declare_tls_enabled_arg, + declare_cert_file_arg, + declare_key_file_arg, declare_cors_arg, OpaqueFunction(function=_launch_setup), ]) diff --git a/src/ros2_medkit_gateway/launch/gateway_https.launch.py b/src/ros2_medkit_gateway/launch/gateway_https.launch.py index b7a269100..3bc70649b 100644 --- a/src/ros2_medkit_gateway/launch/gateway_https.launch.py +++ b/src/ros2_medkit_gateway/launch/gateway_https.launch.py @@ -90,7 +90,13 @@ def generate_certificates(cert_dir: str) -> dict: return { 'cert_file': cert_file, 'key_file': key_file, - 'ca_file': ca_file if os.path.exists(ca_file) else '', + # Deliberately NOT passed as server.tls.ca_file. This CA signs the + # SERVER certificate so a client can verify the gateway; setting it + # as the gateway's ca_file turns on mutual TLS and rejects every + # client that has no certificate of its own, including the curl + # this launch file prints. Kept here only so the hint below can + # tell the user which CA to pass with --cacert. + 'ca_file_for_client': ca_file if os.path.exists(ca_file) else '', } os.makedirs(cert_dir, exist_ok=True) @@ -153,7 +159,9 @@ def generate_certificates(cert_dir: str) -> dict: return { 'cert_file': cert_file, 'key_file': key_file, - 'ca_file': ca_file, + # See the note above: this is the CA a CLIENT verifies the server with, + # not a client-certificate authority for the gateway to demand. + 'ca_file_for_client': ca_file, } @@ -196,7 +204,7 @@ def launch_setup(context): LogInfo(msg=[f' curl -k https://{server_host}:{server_port}/api/v1/health']), LogInfo(msg=['']), LogInfo(msg=['Test with CA verification:']), - LogInfo(msg=[f' curl --cacert {cert_paths["ca_file"]} ' + LogInfo(msg=[f' curl --cacert {cert_paths["ca_file_for_client"]} ' f'https://{server_host}:{server_port}/api/v1/health']), LogInfo(msg=['='*60]), diff --git a/src/ros2_medkit_gateway/scripts/generate_dev_certs.sh b/src/ros2_medkit_gateway/scripts/generate_dev_certs.sh old mode 100644 new mode 100755 index 16fd0c216..e14b8551e --- a/src/ros2_medkit_gateway/scripts/generate_dev_certs.sh +++ b/src/ros2_medkit_gateway/scripts/generate_dev_certs.sh @@ -122,7 +122,11 @@ echo " tls:" echo " enabled: true" echo " cert_file: \"$OUTPUT_DIR/cert.pem\"" echo " key_file: \"$OUTPUT_DIR/key.pem\"" -echo " ca_file: \"$OUTPUT_DIR/ca.pem\"" +echo "" +echo " Do NOT add ca_file here. It is not the CA a client verifies the server" +echo " with - setting it makes the gateway REQUIRE a client certificate from" +echo " every caller, and the curl below would then be refused. Pass ca.pem to" +echo " the client with --cacert instead, as shown." echo "" echo "Test with curl:" echo " curl -k https://localhost:8080/api/v1/health" diff --git a/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp b/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp index 93de53553..6d6daf9d9 100644 --- a/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp +++ b/src/ros2_medkit_gateway/src/core/auth/auth_config.cpp @@ -87,6 +87,11 @@ AuthConfigBuilder & AuthConfigBuilder::with_issuer(const std::string & issuer) { return *this; } +AuthConfigBuilder & AuthConfigBuilder::with_public_routes(const std::vector & public_routes) { + config_.public_routes = public_routes; + return *this; +} + AuthConfigBuilder & AuthConfigBuilder::add_client(const std::string & client_id, const std::string & client_secret, UserRole role) { ClientCredentials creds; diff --git a/src/ros2_medkit_gateway/src/core/auth/auth_manager.cpp b/src/ros2_medkit_gateway/src/core/auth/auth_manager.cpp index 51ac0f626..3e92fcd85 100644 --- a/src/ros2_medkit_gateway/src/core/auth/auth_manager.cpp +++ b/src/ros2_medkit_gateway/src/core/auth/auth_manager.cpp @@ -26,6 +26,30 @@ namespace ros2_medkit_gateway { +namespace { + +/// Compare two secrets without returning early on the first differing byte. +/// +/// The lengths are compared too, and a length mismatch is reported. That does +/// leak the length, which is acceptable: secrets here are operator-chosen and +/// their length is not the secret. What must not leak is WHICH bytes matched, +/// and the loop below always visits every byte of the expected value. +bool constant_time_equals(const std::string & expected, const std::string & presented) { + // Fold the length difference into the result rather than returning, so both + // branches cost the same. + unsigned char diff = static_cast(expected.size() != presented.size()); + const std::size_t n = expected.size(); + for (std::size_t i = 0; i < n; ++i) { + // Index the presented value modulo its own size so a shorter input cannot + // read out of bounds; the length check above already forced a mismatch. + const unsigned char p = presented.empty() ? 0U : static_cast(presented[i % presented.size()]); + diff |= static_cast(static_cast(expected[i]) ^ p); + } + return diff == 0; +} + +} // namespace + // Helper to read file contents static std::string read_file_contents(const std::string & path) { std::ifstream file(path); @@ -65,8 +89,11 @@ AuthManager::AuthManager(const AuthConfig & config) : config_(config) { clients_[client.client_id] = client; } - // Create auth requirement policy from config - auth_policy_ = AuthRequirementPolicyFactory::create(config_.require_auth_for); + // Create auth requirement policy from config. `require_auth_for` decides the + // baseline; `public_routes` then lifts the credential requirement from the + // routes an operator named, and from nothing else. + auth_policy_ = + AuthRequirementPolicyFactory::create(config_.require_auth_for, parse_public_routes(config_.public_routes)); } tl::expected AuthManager::authenticate(const std::string & client_id, @@ -85,8 +112,15 @@ tl::expected AuthManager::authenticate(const s return tl::unexpected(AuthErrorResponse::invalid_client("Client is disabled")); } - // Verify secret - if (client.client_secret != client_secret) { + // Verify secret in constant time. A plain std::string comparison returns as + // soon as two bytes differ, so the time it takes to refuse leaks how many + // leading bytes were right, and a caller who can measure it can recover the + // secret one byte at a time. Every default deployment now needs a configured + // client, so this path is on the critical path for all of them. + // + // Secrets are still stored in plaintext in the configuration; making this + // comparison constant-time does not change that and is not meant to. + if (!constant_time_equals(client.client_secret, client_secret)) { return tl::unexpected(AuthErrorResponse::invalid_client("Invalid client_secret")); } @@ -248,10 +282,27 @@ TokenValidationResult AuthManager::validate_token(const std::string & token, Tok return result; } - // Check if associated refresh token is revoked (for access tokens) + // An access token that names a refresh record is only valid while that + // record is present and not revoked. + // + // Absent counts as invalid, not as "nothing to check". Records live in + // memory, so after a restart the map is empty; treating absence as fine + // would make every revoked token work again until it expired on its own, + // which on the default one-hour expiry is a long time to keep honouring a + // credential somebody explicitly withdrew. + // + // The cost is deliberate and worth stating: a restart invalidates every + // access token, so clients re-authenticate after one. That is visible + // behaviour, and it is the trade this gateway makes elsewhere too - refusing + // is better than quietly allowing. if (claims.refresh_token_id.has_value()) { auto record = get_refresh_token(claims.refresh_token_id.value()); - if (record.has_value() && record->revoked) { + if (!record.has_value()) { + result.valid = false; + result.error = "Associated refresh token is no longer known to this gateway"; + return result; + } + if (record->revoked) { result.valid = false; result.error = "Associated refresh token has been revoked"; return result; @@ -350,25 +401,44 @@ bool AuthManager::revoke_refresh_token(const std::string & refresh_token) { return true; } -size_t AuthManager::cleanup_expired_tokens() { +size_t AuthManager::cleanup_expired_locked() { auto now = std::chrono::system_clock::now(); auto now_ts = std::chrono::duration_cast(now.time_since_epoch()).count(); - std::lock_guard lock(refresh_tokens_mutex_); - size_t count = 0; + // A record outlives its own expiry by one access-token lifetime. + // + // validate_token() rejects an access token whose refresh record is gone, so + // dropping the record the instant it expires cuts short every access token + // minted from it. The last one can be issued a second before the refresh + // token expires and is then promised a full token_expiry_seconds; sweeping + // on expires_at alone would refuse it about a minute later, with most of its + // life left. Keeping the record until nothing issued from it can still be + // valid costs one extra lifetime of memory per client and removes the whole + // race. + const int64_t grace = static_cast(config_.token_expiry_seconds); + size_t count = 0; for (auto it = refresh_tokens_.begin(); it != refresh_tokens_.end();) { - if (it->second.expires_at < now_ts) { + if (it->second.expires_at + grace < now_ts) { it = refresh_tokens_.erase(it); ++count; } else { ++it; } } - return count; } +size_t AuthManager::refresh_token_count() const { + std::lock_guard lock(refresh_tokens_mutex_); + return refresh_tokens_.size(); +} + +size_t AuthManager::cleanup_expired_tokens() { + std::lock_guard lock(refresh_tokens_mutex_); + return cleanup_expired_locked(); +} + bool AuthManager::register_client(const std::string & client_id, const std::string & client_secret, UserRole role) { std::lock_guard lock(clients_mutex_); @@ -609,6 +679,19 @@ bool AuthManager::matches_path(const std::string & pattern, const std::string & void AuthManager::store_refresh_token(const RefreshTokenRecord & record) { std::lock_guard lock(refresh_tokens_mutex_); + + // Sweep before inserting. Nothing else calls the sweep, so without this the + // map keeps one record per successful authorisation for the life of the + // process, and validate_token looks that map up on every authenticated + // request. Doing it here rather than on a timer keeps the bound a property + // of the data structure instead of a property of a thread that might not be + // running, and makes it observable in a test without waiting on wall clock. + // + // The cost is a scan per authorisation. The map only ever holds unexpired + // records, so it is sized by how many tokens are live at once, not by how + // many have ever been issued. + cleanup_expired_locked(); + refresh_tokens_[record.token_id] = record; } diff --git a/src/ros2_medkit_gateway/src/core/auth/auth_requirement_policy.cpp b/src/ros2_medkit_gateway/src/core/auth/auth_requirement_policy.cpp index 16393f3b8..d48210071 100644 --- a/src/ros2_medkit_gateway/src/core/auth/auth_requirement_policy.cpp +++ b/src/ros2_medkit_gateway/src/core/auth/auth_requirement_policy.cpp @@ -15,7 +15,12 @@ #include "ros2_medkit_gateway/core/auth/auth_requirement_policy.hpp" #include +#include +#include #include +#include +#include +#include namespace ros2_medkit_gateway { @@ -168,8 +173,101 @@ std::unique_ptr AuthRequirementPolicyFactory::create(con return std::make_unique(); } - // Use the require_auth_for setting from config - return create(config.require_auth_for); + return create(config.require_auth_for, parse_public_routes(config.public_routes)); +} + +std::vector parse_public_routes(const std::vector & entries) { + std::vector routes; + routes.reserve(entries.size()); + for (const auto & entry : entries) { + auto parsed = parse_public_route(entry); + if (parsed.has_value()) { + routes.push_back(*parsed); + } + } + return routes; +} + +std::unique_ptr +AuthRequirementPolicyFactory::create(AuthRequirement requirement, const std::vector & public_routes) { + auto policy = create(requirement); + if (public_routes.empty()) { + return policy; + } + return std::make_unique(std::move(policy), public_routes); +} + +tl::expected parse_public_route(const std::string & entry) { + auto is_space = [](unsigned char c) { + return std::isspace(c) != 0; + }; + auto begin = std::find_if_not(entry.begin(), entry.end(), is_space); + auto end = std::find_if_not(entry.rbegin(), entry.rend(), is_space).base(); + const std::string trimmed = (begin < end) ? std::string(begin, end) : std::string(); + + if (trimmed.empty()) { + return tl::unexpected("entry is empty"); + } + + const size_t space = trimmed.find(' '); + if (space == std::string::npos) { + return tl::unexpected("expected \"METHOD /path\", e.g. \"GET /api/v1/health\""); + } + + PublicRoute route; + route.method = trimmed.substr(0, space); + route.path = trimmed.substr(space + 1); + + // The path is compared against what cpp-httplib hands the middleware, which + // is a single token. A second space means two paths or a stray argument, and + // either way the entry does not describe one route. + if (route.path.find(' ') != std::string::npos) { + return tl::unexpected("path contains a space: \"" + route.path + "\""); + } + + std::transform(route.method.begin(), route.method.end(), route.method.begin(), [](unsigned char c) { + return static_cast(std::toupper(c)); + }); + + static const std::vector kMethods = {"GET", "HEAD", "POST", "PUT", "DELETE", "PATCH", "OPTIONS"}; + if (std::find(kMethods.begin(), kMethods.end(), route.method) == kMethods.end()) { + return tl::unexpected("unknown HTTP method \"" + route.method + "\""); + } + + if (route.path.empty() || route.path.front() != '/') { + return tl::unexpected("path must start with \"/\": \"" + route.path + "\""); + } + + // Wildcards are refused rather than ignored. Accepting the character and + // then matching it literally would read as "this opens the subtree" and + // silently open nothing; refusing says so while the operator is watching. + if (route.path.find('*') != std::string::npos) { + return tl::unexpected("wildcards are not supported; name each route exactly: \"" + route.path + "\""); + } + + return route; +} + +PublicRouteExemptionPolicy::PublicRouteExemptionPolicy(std::unique_ptr inner, + std::vector public_routes) + : inner_(std::move(inner)), public_routes_(std::move(public_routes)) { +} + +bool PublicRouteExemptionPolicy::requires_authentication(const std::string & method, const std::string & path) const { + for (const auto & route : public_routes_) { + if (route.method == method && route.path == path) { + return false; + } + } + return inner_->requires_authentication(method, path); +} + +std::string PublicRouteExemptionPolicy::description() const { + std::string desc = inner_->description() + "; public_routes:"; + for (const auto & route : public_routes_) { + desc += " " + route.method + " " + route.path; + } + return desc; } } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/core/config.cpp b/src/ros2_medkit_gateway/src/core/config.cpp index b140bd110..37cb72fc9 100644 --- a/src/ros2_medkit_gateway/src/core/config.cpp +++ b/src/ros2_medkit_gateway/src/core/config.cpp @@ -58,11 +58,6 @@ std::string TlsConfig::validate() const { return "TLS: ca_file does not exist or is not readable: " + ca_file; } - // TODO(future): Add mutual TLS validation when implemented - // if (mutual_tls && ca_file.empty()) { - // return "TLS: ca_file is required when mutual_tls is enabled"; - // } - // Validate minimum TLS version if (min_version != "1.2" && min_version != "1.3") { return "TLS: min_version must be '1.2' or '1.3', got: " + min_version; diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index a8f32986e..b194fe404 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -30,6 +30,7 @@ #include #include "ros2_medkit_gateway/core/aggregation/network_utils.hpp" +#include "ros2_medkit_gateway/core/auth/auth_requirement_policy.hpp" #include "ros2_medkit_gateway/core/data/topic_data_provider.hpp" #include "ros2_medkit_gateway/core/discovery/refresh_debounce.hpp" #include "ros2_medkit_gateway/core/entity_validation.hpp" @@ -173,6 +174,7 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki declare_parameter("auth.require_auth_for", "write"); declare_parameter("auth.issuer", "ros2_medkit_gateway"); declare_parameter("auth.clients", std::vector{}); + declare_parameter("auth.public_routes", std::vector{}); // OpenAPI documentation endpoints declare_parameter("docs.enabled", true); @@ -435,7 +437,6 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki .with_key_file(get_parameter("server.tls.key_file").as_string()) .with_ca_file(get_parameter("server.tls.ca_file").as_string()) .with_min_version(get_parameter("server.tls.min_version").as_string()) - // TODO(future): Add .with_mutual_tls() when implemented .build(); // Note: HttpServerManager will log TLS configuration details } catch (const std::exception & e) { @@ -496,10 +497,30 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki } } + // Routes the operator has taken outside authentication. Validated here + // rather than where the policy is built, because a typo must stop the + // gateway while somebody is watching: silently dropping the entry would + // leave a route protected that the operator believes is reachable, and + // silently widening it would be worse. + auto public_routes = get_parameter("auth.public_routes").as_string_array(); + for (const auto & entry : public_routes) { + auto parsed = parse_public_route(entry); + if (!parsed) { + throw std::invalid_argument("auth.public_routes entry \"" + entry + "\" is invalid: " + parsed.error()); + } + } + auth_builder.with_public_routes(public_routes); + auth_config_ = auth_builder.build(); RCLCPP_INFO(get_logger(), "Authentication enabled - algorithm: %s, require_auth_for: %s", algorithm_to_string(auth_config_.jwt_algorithm).c_str(), get_parameter("auth.require_auth_for").as_string().c_str()); + for (const auto & entry : public_routes) { + // One line per route, at WARN: every entry here is a hole somebody + // opened on purpose, and an operator reading the startup log should see + // the whole public surface without going to look for the config file. + RCLCPP_WARN(get_logger(), "auth.public_routes: %s is answered WITHOUT a credential", entry.c_str()); + } } catch (const std::exception & e) { // Fail closed: authentication was explicitly requested but could not be // built (e.g. empty jwt_secret). Refuse to start rather than silently diff --git a/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp index fcc0948d0..fef043f13 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/health_handlers.cpp @@ -17,6 +17,7 @@ #include #include "ros2_medkit_gateway/aggregation/aggregation_manager.hpp" +#include "ros2_medkit_gateway/core/auth/auth_middleware.hpp" #include "ros2_medkit_gateway/core/auth/auth_models.hpp" #include "ros2_medkit_gateway/core/data/topic_data_provider.hpp" #include "ros2_medkit_gateway/core/discovery/discovery_enums.hpp" @@ -50,13 +51,55 @@ ErrorInfo make_internal_error(const char * where, const std::exception & e) { } // namespace +namespace { + +/// True when authentication is on and this request did not present a token +/// this gateway accepts. +/// +/// Reachable two ways: under `require_auth_for: write`, where every GET is +/// open, and on a route an operator listed in `auth.public_routes`. In both an +/// anonymous caller reaches this handler, and the full body is more than the +/// probe asked for: the linking warnings name entities and ROS node FQNs, and +/// the entity cache reports how many apps, areas and components this gateway +/// sees. So an anonymous caller gets liveness and nothing else, flagged as cut +/// down, and an authenticated one gets the whole document. +bool is_anonymous(const HandlerContext & ctx, const http::TypedRequest & req) { + if (!ctx.auth_config().enabled) { + return false; // Nothing is anonymous when nothing is authenticated. + } + auto * manager = ctx.auth_manager(); + if (manager == nullptr) { + return true; // Fail closed: cannot verify, so do not disclose. + } + auto header = req.header("Authorization"); + if (!header) { + return true; + } + auto token = AuthMiddleware::extract_bearer_token(*header); + if (!token) { + return true; + } + return !manager->validate_token(*token).valid; +} + +} // namespace + http::Result HealthHandlers::get_health(const http::TypedRequest & req) { - (void)req; // Unused parameter try { dto::Health response; response.status = "healthy"; response.timestamp = std::chrono::system_clock::now().time_since_epoch().count(); + // Liveness and nothing else for an anonymous caller. Returned before any + // of the sections below are built, so a section added later is private by + // default rather than public until someone remembers to think about it. + // The flag is what keeps the empty `warnings` below from reading as + // "nothing is wrong here" to a monitor that never presented a credential. + if (is_anonymous(ctx_, req)) { + response.x_medkit_reduced = true; + return response; + } + // Operator-actionable warnings the gateway flags without taking itself // offline. Collected across every subsystem that can produce one, so the // array and its schema version are part of the /health contract whether or diff --git a/src/ros2_medkit_gateway/src/http/http_server.cpp b/src/ros2_medkit_gateway/src/http/http_server.cpp index 3979bd477..2aaefe186 100644 --- a/src/ros2_medkit_gateway/src/http/http_server.cpp +++ b/src/ros2_medkit_gateway/src/http/http_server.cpp @@ -17,6 +17,11 @@ #include #include +#ifdef CPPHTTPLIB_OPENSSL_SUPPORT +// TLS1_2_VERSION / TLS1_3_VERSION and SSL_CTX_set_min_proto_version. +#include +#endif + namespace ros2_medkit_gateway { HttpServerManager::HttpServerManager(const TlsConfig & tls_config, std::size_t thread_pool_size, @@ -24,8 +29,19 @@ HttpServerManager::HttpServerManager(const TlsConfig & tls_config, std::size_t t : tls_config_(tls_config), thread_pool_size_(thread_pool_size), keep_alive_timeout_sec_(keep_alive_timeout_sec) { #ifdef CPPHTTPLIB_OPENSSL_SUPPORT if (tls_config_.enabled) { - // Create SSL server with certificate and key - ssl_server_ = std::make_unique(tls_config_.cert_file.c_str(), tls_config_.key_file.c_str()); + // A non-empty ca_file turns on mutual TLS. The SSLServer constructor does + // the work itself: given a client CA path it calls + // SSL_CTX_load_verify_locations and then + // SSL_CTX_set_verify(SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT). + // + // That pairing is why this is all-or-nothing per gateway: with a CA set, + // a client that presents NO certificate is rejected at the handshake. + // There is no "verify it if offered" middle setting without patching the + // vendored header. Leaving ca_file empty keeps ordinary server-only TLS, + // which is what the SOVD bearer-token flow expects. + ssl_server_ = + std::make_unique(tls_config_.cert_file.c_str(), tls_config_.key_file.c_str(), + tls_config_.ca_file.empty() ? nullptr : tls_config_.ca_file.c_str()); if (!ssl_server_->is_valid()) { throw std::runtime_error( @@ -33,13 +49,29 @@ HttpServerManager::HttpServerManager(const TlsConfig & tls_config, std::size_t t " (key configured: " + (tls_config_.key_file.empty() ? "no" : "yes") + ")"); } + // The constructor above ignores what SSL_CTX_load_verify_locations returned, + // so `is_valid()` is true for a ca_file that exists but is unreadable or is + // not PEM. The gateway would then start, log "REQUIRED (mutual TLS)", and + // reject every client at the handshake with no trust store to check them + // against - an outage that reads as a client problem. Load it again and + // look at the answer this time; loading the same file twice is a no-op + // beyond re-adding the same certificates to the store. + if (!tls_config_.ca_file.empty() && + SSL_CTX_load_verify_locations(ssl_server_->ssl_context(), tls_config_.ca_file.c_str(), nullptr) != 1) { + throw std::runtime_error( + "Mutual TLS is configured but the client CA bundle could not be loaded: " + tls_config_.ca_file + + ". The file must be readable by the gateway and contain PEM certificates."); + } + // Configure additional TLS settings configure_tls(); apply_thread_pool(*ssl_server_); apply_keep_alive(*ssl_server_); - RCLCPP_INFO(rclcpp::get_logger("http_server"), "TLS/HTTPS enabled - cert: %s, min_version: %s", - tls_config_.cert_file.c_str(), tls_config_.min_version.c_str()); + RCLCPP_INFO(rclcpp::get_logger("http_server"), + "TLS/HTTPS enabled - cert: %s, min_version: %s, client certificates: %s", tls_config_.cert_file.c_str(), + tls_config_.min_version.c_str(), + tls_config_.ca_file.empty() ? "not required" : "REQUIRED (mutual TLS)"); // Note: key_file path intentionally not logged for security reasons } else { server_ = std::make_unique(); @@ -138,26 +170,25 @@ void HttpServerManager::configure_tls() { return; } - // YAGNI Decision: min_version field exists in TlsConfig for future extensibility - // but is not fully implemented. - // - // Rationale: - // - cpp-httplib's SSLServer doesn't expose SSL_CTX for min_version configuration - // - Modern OpenSSL (1.1.1+) defaults to TLS 1.2+ which is secure + // Set the protocol floor on our own context rather than inheriting one. // - // Future implementation options: - // 1. Fork cpp-httplib to expose SSL_CTX for SSL_CTX_set_min_proto_version() - // 2. Use OpenSSL system-wide configuration (/etc/ssl/openssl.cnf) - // 3. Replace cpp-httplib with Boost.Beast or another library with full SSL control + // Two reasons it has to be us. The SSLServer constructor calls + // SSL_CTX_set_min_proto_version(ctx_, TLS1_1_VERSION), so the library asks + // for a floor of TLS 1.1. What a deployment actually gets on top of that is + // whatever the local OpenSSL policy allows, which differs between the + // distributions we ship for. Neither of those is a decision this project + // made, and SOVD requires TLS 1.2 as the minimum, so the value is set here + // where it can be read and tested. // - // TODO(future): Add mutual TLS support - requires cpp-httplib modifications - // to expose SSL_CTX for SSL_CTX_set_verify() with SSL_VERIFY_PEER - - if (tls_config_.min_version != "1.2") { - RCLCPP_WARN(rclcpp::get_logger("http_server"), - "min_version='%s' requested but cpp-httplib uses OpenSSL defaults (TLS 1.2+). " - "Custom min_version not enforced.", - tls_config_.min_version.c_str()); + // The string is validated in TlsConfig::validate(), which rejects anything + // other than "1.2" or "1.3" before a server is ever constructed. + const int min_proto = (tls_config_.min_version == "1.3") ? TLS1_3_VERSION : TLS1_2_VERSION; + SSL_CTX * ctx = ssl_server_->ssl_context(); + if (ctx == nullptr || SSL_CTX_set_min_proto_version(ctx, min_proto) != 1) { + // Refuse to serve rather than fall back to the library floor: a caller + // that asked for 1.3 and silently got 1.1 is worse off than one that got + // an error, because nothing downstream can tell the difference. + throw std::runtime_error("Failed to set the minimum TLS version to " + tls_config_.min_version); } // Log TLS handshake failures for debugging diff --git a/src/ros2_medkit_gateway/src/http/rest_server.cpp b/src/ros2_medkit_gateway/src/http/rest_server.cpp index 5b070aece..c5d3806ec 100644 --- a/src/ros2_medkit_gateway/src/http/rest_server.cpp +++ b/src/ros2_medkit_gateway/src/http/rest_server.cpp @@ -317,8 +317,25 @@ void RESTServer::setup_pre_routing_handler() { } } - // 2. Handle preflight OPTIONS requests - if (req.method == "OPTIONS") { + // 2. Handle preflight OPTIONS requests. + // + // This is answered WITHOUT a credential, and it has to be. A browser + // never puts Authorization on a preflight - asking permission before + // sending the real request, headers included, is the entire purpose of + // the mechanism - so requiring one here does not harden the gateway, it + // makes every browser client impossible. + // + // It is safe because a preflight discloses nothing about the system: the + // response is the CORS policy for an origin the operator configured, with + // no body, and the real request that follows is authenticated normally. + // Treat this as a named exemption alongside /auth/, not as an oversight. + // A real preflight carries Access-Control-Request-Method; the browser + // sends it to ask whether the method it is about to use is allowed. + // Requiring it keeps the exemption to the case that genuinely cannot + // authenticate. Without that check a plain anonymous OPTIONS with an + // allowed Origin took the same early return, which widens a narrow, + // necessary exemption into "OPTIONS is public". + if (req.method == "OPTIONS" && req.has_header("Access-Control-Request-Method")) { if (origin_allowed) { res.set_header("Access-Control-Max-Age", std::to_string(cors_config_.max_age_seconds)); res.status = 204; @@ -329,21 +346,39 @@ void RESTServer::setup_pre_routing_handler() { } } - // 3. Rate limiting check. If rejected, return Handled (CORS headers already set) + // 3. Rate limiting is METERED here and ANSWERED after authentication. + // + // Both halves matter and they pull in opposite directions. Metering after + // authentication means a request refused for a bad credential never spends + // any allowance, so an anonymous caller can hammer a protected route for + // free and pay only for the signature verification each attempt costs the + // gateway - which under RS256 is not cheap. Answering before + // authentication means an anonymous caller who exhausted the allowance + // gets 429 from a protected route instead of 401: an answer, and a small + // disclosure of limiter state, without any credential. + // + // So: consume the allowance for every request, and decide what to say + // about it once we know whether the caller had a credential. + bool rate_limited = false; + RateLimitResult rl_result; if (rate_limiter_ && rate_limiter_->is_enabled() && req.method != "OPTIONS") { - auto rl_result = rate_limiter_->check(req.remote_addr, req.path); + rl_result = rate_limiter_->check(req.remote_addr, req.path); RateLimiter::apply_headers(rl_result, res); - if (!rl_result.allowed) { - RateLimiter::apply_rejection(rl_result, res); - return handled(req, res); - } + rate_limited = !rl_result.allowed; } - // 1. Handle CORS (existing logic) - - // Handle Authentication if enabled + // 4. Authentication. if (auth_middleware_ && auth_middleware_->is_enabled()) { - // Use AuthMiddleware to process the request + // With the allowance gone, a request that presents no credential at all + // on a route that needs one is refused without verifying anything. This + // is the shape an anonymous flood takes, and it is the case where the + // verification is pure waste - the answer is 401 either way. + const bool needs_credential = auth_manager_ && auth_manager_->requires_authentication(req.method, req.path); + if (rate_limited && needs_credential && !req.has_header("Authorization")) { + res.status = 401; + return handled(req, res); + } + auto auth_request = AuthMiddleware::from_httplib_request(req); auto result = auth_middleware_->process(auth_request); @@ -353,6 +388,14 @@ void RESTServer::setup_pre_routing_handler() { } } + // 5. Now the limiter may speak. The caller reached this line with a + // credential this gateway accepts, or on a route that needs none, so 429 + // tells them something they are entitled to know. + if (rate_limited) { + RateLimiter::apply_rejection(rl_result, res); + return handled(req, res); + } + return httplib::Server::HandlerResponse::Unhandled; }); } diff --git a/src/ros2_medkit_gateway/test/test_auth_manager.cpp b/src/ros2_medkit_gateway/test/test_auth_manager.cpp index aad73dd5d..4ffdb8721 100644 --- a/src/ros2_medkit_gateway/test/test_auth_manager.cpp +++ b/src/ros2_medkit_gateway/test/test_auth_manager.cpp @@ -529,6 +529,62 @@ TEST(AuthManagerRequirementTest, RequireAuthForAll) { EXPECT_FALSE(manager.requires_authentication("POST", "/api/v1/auth/authorize")); } +// An access token keeps its promised lifetime after its refresh token expires. +// +// validate_token() refuses an access token whose refresh record is gone, which +// is what makes a revocation survive. The cost is that the sweep decides how +// long an access token really lives: sweeping on the refresh token's own +// expiry would cut short the last access token minted from it, which was +// promised a full token_expiry_seconds a moment earlier. This is the test that +// fails if the grace period is removed. +// @verifies REQ_INTEROP_086 +TEST(AuthManagerRequirementTest, AccessTokenOutlivesItsExpiredRefreshRecord) { + // The two expiries are equal, which is the tightest the builder allows + // (refresh must be >= access). That is also the worst case: refresh_access_token + // reuses the refresh token's jti rather than rotating it, so the access token + // it mints is promised token_expiry_seconds from NOW while the record still + // dies at its original expiry. Every refresh therefore produces an access + // token that outlives its own record. + // Three seconds, not one. Two constraints set these numbers. + // + // Expiries are whole seconds, so the sweep's comparison only moves at second + // boundaries: with a one-second expiry and a 1.3 s wait, `expires_at < now` + // is still false through integer truncation, and the test would pass with or + // without the grace period - measuring nothing. + // + // And the waits are wall-clock on a machine running the rest of the suite, so + // each one has to sit well clear of the boundary it is about rather than just + // past it. The record expires at t+3 and is swept after t+6; the checks are + // at t+4 and t+9, leaving 2 s and 3 s of slack for a late wake-up. + AuthConfig config = AuthConfigBuilder() + .with_enabled(true) + .with_jwt_secret("test_secret_key_min_32_chars_life_") + .with_token_expiry(3) + .with_refresh_token_expiry(3) + .with_require_auth_for(AuthRequirement::ALL) + .build(); + config.clients.push_back({"c", "s", UserRole::ADMIN, true}); + + AuthManager manager(config); + ASSERT_TRUE(manager.authenticate("c", "s").has_value()); + ASSERT_EQ(manager.refresh_token_count(), 1u); + + // t+4s: past the record's own expiry (t+3), inside the one access-token + // lifetime it is held for (to t+6). Sweeping here is what cut an access token + // short. + std::this_thread::sleep_for(std::chrono::milliseconds(4000)); + manager.cleanup_expired_tokens(); + EXPECT_EQ(manager.refresh_token_count(), 1u) + << "the record was dropped at its own expiry, so any access token minted " + "from it in its last moments is refused with most of its life left"; + + // t+9s: past the grace too. It does not live forever, or a revocation would + // be honoured out of a map that only ever grows. + std::this_thread::sleep_for(std::chrono::milliseconds(5000)); + manager.cleanup_expired_tokens(); + EXPECT_EQ(manager.refresh_token_count(), 0u) << "the record outlived even its grace period"; +} + // Test none auth requirement mode TEST(AuthManagerRequirementTest, RequireAuthForNone) { AuthConfig config = AuthConfigBuilder() @@ -677,6 +733,136 @@ TEST_F(AuthManagerTest, CleanupExpiredTokens) { EXPECT_GE(cleaned, 1); } +// --------------------------------------------------------------------------- +// Refresh-record growth, constant-time secret comparison, and revocation. +// --------------------------------------------------------------------------- + +namespace { + +/// A manager with a single admin client, parameterised on the two expiry +/// values, so a test can put them at their endpoints rather than at one +/// comfortable middle value. +AuthManager make_manager(int access_expiry, int refresh_expiry) { + auto config = AuthConfigBuilder() + .with_enabled(true) + .with_jwt_secret("expiry_sweep_secret_key_at_least_32_chars_long") + .with_require_auth_for(AuthRequirement::ALL) + .with_token_expiry(access_expiry) + .with_refresh_token_expiry(refresh_expiry) + .add_client("svc", "svc_secret", UserRole::ADMIN) + .build(); + return AuthManager(config); +} + +} // namespace + +// The sweep exists but had no production caller, so the map grew by one record +// per successful authorisation for the life of the process. What this asserts +// is the COUNT, because a sweep that is never invoked returns the right answer +// when a test calls it directly and still leaks in production. +// @verifies REQ_INTEROP_086 +TEST(AuthManagerTokenLifetimeTest, RepeatedLoginsDoNotGrowTheStoreWithoutBound) { + // Refresh expiry at its minimum legal value: validate() requires + // refresh >= access, so this is the endpoint, not a convenient number. + auto manager = make_manager(1, 1); + + for (int i = 0; i < 5; ++i) { + ASSERT_TRUE(manager.authenticate("svc", "svc_secret").has_value()); + } + EXPECT_EQ(manager.refresh_token_count(), 5U) << "records should accumulate while they are live"; + + // Past the refresh expiry AND the access-token lifetime a record is held for + // beyond it, the next authorisation must clear them out. Records expire at + // t+1 and are swept after t+2, so this waits to t+4: far enough clear of the + // boundary that a late wake-up on a loaded machine cannot land short of it. + std::this_thread::sleep_for(std::chrono::milliseconds(4000)); + ASSERT_TRUE(manager.authenticate("svc", "svc_secret").has_value()); + + EXPECT_EQ(manager.refresh_token_count(), 1U) + << "the five expired records survived a later authorisation - the sweep is not running"; +} + +// The other endpoint. A long-lived refresh token must NOT be swept: an +// over-eager sweep would log clients out mid-session, which is the opposite +// failure and just as real. +// @verifies REQ_INTEROP_086 +TEST(AuthManagerTokenLifetimeTest, LongLivedRecordsAreNotSweptEarly) { + auto manager = make_manager(1, 86400); + + for (int i = 0; i < 4; ++i) { + ASSERT_TRUE(manager.authenticate("svc", "svc_secret").has_value()); + } + std::this_thread::sleep_for(std::chrono::seconds(2)); + ASSERT_TRUE(manager.authenticate("svc", "svc_secret").has_value()); + + EXPECT_EQ(manager.refresh_token_count(), 5U) << "records well inside their expiry were discarded"; +} + +// Degenerate case: access and refresh expiry equal and both large. +// @verifies REQ_INTEROP_086 +TEST(AuthManagerTokenLifetimeTest, EqualAccessAndRefreshExpiryKeepsRecords) { + auto manager = make_manager(3600, 3600); + ASSERT_TRUE(manager.authenticate("svc", "svc_secret").has_value()); + ASSERT_TRUE(manager.authenticate("svc", "svc_secret").has_value()); + EXPECT_EQ(manager.refresh_token_count(), 2U); +} + +// A wrong secret must be refused whatever its shape. The interesting inputs +// are the ones a short-circuiting comparison treats differently from a +// constant-time one: a correct prefix, and a value that extends the real one. +// @verifies REQ_INTEROP_086 +TEST(AuthManagerSecretComparisonTest, OnlyTheExactSecretAuthenticates) { + auto manager = make_manager(3600, 3600); + + EXPECT_TRUE(manager.authenticate("svc", "svc_secret").has_value()) << "the real secret must work"; + + // The last two are the ones that matter. Everything before them differs in + // length, or in the first byte, so a comparison that checked the length and + // then only a prefix would satisfy the whole list. "svc_secreT" differs only + // in the FINAL byte: shorten the comparison loop by one and it is accepted + // while every other case here still fails correctly. + for (const auto & wrong : + {"", "s", "svc_secre", "svc_secret_", "svc_secretX", "SVC_SECRET", "xxxxxxxxxx", "svc_secreT", "Svc_secret"}) { + EXPECT_FALSE(manager.authenticate("svc", wrong).has_value()) << "secret \"" << wrong << "\" was accepted"; + } +} + +// An access token whose refresh record is gone is invalid, not "unchecked". +// Records are in memory, so this is also what a gateway restart looks like to +// a token issued before it: the deliberate consequence is that a restart makes +// clients re-authenticate. +// @verifies REQ_INTEROP_086 +TEST(AuthManagerRevocationTest, AnAccessTokenWithNoSurvivingRecordIsRejected) { + auto manager = make_manager(3600, 3600); + auto issued = manager.authenticate("svc", "svc_secret"); + ASSERT_TRUE(issued.has_value()); + + EXPECT_TRUE(manager.validate_token(issued->access_token).valid) + << "the token must be valid while its record is present"; + + // Revoking drops or marks the record; either way the access token that names + // it must stop being accepted. + ASSERT_TRUE(issued->refresh_token.has_value()); + ASSERT_TRUE(manager.revoke_refresh_token(issued->refresh_token.value())); + EXPECT_FALSE(manager.validate_token(issued->access_token).valid) + << "an access token whose refresh record was revoked was still accepted"; +} + +// A second manager standing in for the same gateway after a restart: same +// secret and issuer, so the signature still verifies, but no records. +// @verifies REQ_INTEROP_086 +TEST(AuthManagerRevocationTest, ARestartInvalidatesAccessTokensRatherThanTrustingThem) { + auto before = make_manager(3600, 3600); + auto issued = before.authenticate("svc", "svc_secret"); + ASSERT_TRUE(issued.has_value()); + ASSERT_TRUE(before.validate_token(issued->access_token).valid); + + auto after_restart = make_manager(3600, 3600); + EXPECT_FALSE(after_restart.validate_token(issued->access_token).valid) + << "a token from before the restart was accepted although the gateway has no record of it - " + "a revoked token would come back to life this way"; +} + // Test JwtClaims TEST(JwtClaimsTest, ToJson) { JwtClaims claims; @@ -1130,6 +1316,125 @@ TEST_F(AuthRequirementPolicyTest, AllAuthPolicyAlwaysRequiresAuth) { EXPECT_TRUE(policy.requires_authentication("DELETE", "/api/v1/admin/users")); } +// Health is NOT special to the ALL policy. It is closed like everything else +// until an operator names it in auth.public_routes, and this is the test that +// fails if somebody hardcodes the exemption back in. +// @verifies REQ_INTEROP_086 +TEST_F(AuthRequirementPolicyTest, AllAuthPolicyDoesNotExemptHealth) { + AllAuthRequirementPolicy policy; + + EXPECT_TRUE(policy.requires_authentication("GET", "/api/v1/health")); + EXPECT_TRUE(policy.requires_authentication("HEAD", "/api/v1/health")); +} + +// An entry of auth.public_routes opens the route it names and nothing beside +// it. Widening the comparison to a prefix, or dropping the method, is the +// natural next edit and would open a hole, so the boundary is pinned here. +// @verifies REQ_INTEROP_086 +TEST_F(AuthRequirementPolicyTest, PublicRouteExemptionOpensOnlyWhatItNames) { + auto policy = AuthRequirementPolicyFactory::create(AuthRequirement::ALL, {{"GET", "/api/v1/health"}}); + + // The route the operator named. + EXPECT_FALSE(policy->requires_authentication("GET", "/api/v1/health")); + + // Only GET. A write to the health path is not a liveness probe, and + // cpp-httplib dispatches HEAD into the GET handler table, so dropping the + // method check would hand the status document to an anonymous HEAD. + EXPECT_TRUE(policy->requires_authentication("POST", "/api/v1/health")); + EXPECT_TRUE(policy->requires_authentication("PUT", "/api/v1/health")); + EXPECT_TRUE(policy->requires_authentication("DELETE", "/api/v1/health")); + EXPECT_TRUE(policy->requires_authentication("PATCH", "/api/v1/health")); + EXPECT_TRUE(policy->requires_authentication("HEAD", "/api/v1/health")); + + // Only that exact path. A prefix or suffix match would hand an attacker a + // trivial bypass: append or prepend the magic word and walk in. + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/health/detail")); + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/healthz")); + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/components/health")); + EXPECT_TRUE(policy->requires_authentication("GET", "/health")); + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/health?x=1")); + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v2/health")); + + // And the rest of the surface is untouched by the entry. + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/areas")); + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/")); +} + +// The layer only ever removes a requirement. Wrapping must not make a gateway +// stricter than the policy underneath, or an operator who adds a probe route +// would silently close the reads that `write` leaves open. +// @verifies REQ_INTEROP_086 +TEST_F(AuthRequirementPolicyTest, PublicRouteExemptionNeverAddsARequirement) { + auto policy = AuthRequirementPolicyFactory::create(AuthRequirement::WRITE, {{"POST", "/api/v1/health"}}); + + EXPECT_FALSE(policy->requires_authentication("GET", "/api/v1/areas")); + EXPECT_FALSE(policy->requires_authentication("POST", "/api/v1/health")); + EXPECT_TRUE(policy->requires_authentication("POST", "/api/v1/areas")); +} + +// An empty list must leave the policy exactly as it was, or "closed by +// default" would depend on the wrapper behaving itself. +// @verifies REQ_INTEROP_086 +TEST_F(AuthRequirementPolicyTest, EmptyPublicRoutesChangesNothing) { + auto policy = AuthRequirementPolicyFactory::create(AuthRequirement::ALL, {}); + + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/health")); + EXPECT_TRUE(policy->requires_authentication("GET", "/api/v1/areas")); + EXPECT_FALSE(policy->requires_authentication("POST", "/api/v1/auth/authorize")); +} + +// @verifies REQ_INTEROP_086 +TEST_F(AuthRequirementPolicyTest, PublicRouteEntryParsing) { + auto ok = parse_public_route("GET /api/v1/health"); + ASSERT_TRUE(ok.has_value()); + EXPECT_EQ(ok->method, "GET"); + EXPECT_EQ(ok->path, "/api/v1/health"); + + // Case and surrounding whitespace are the operator's typing, not a decision. + auto lower = parse_public_route(" get /api/v1/health "); + ASSERT_TRUE(lower.has_value()); + EXPECT_EQ(lower->method, "GET"); + EXPECT_EQ(lower->path, "/api/v1/health"); + + // Everything below must be refused rather than half-understood. A wildcard + // accepted and then matched literally would read as "this opens the subtree" + // and open nothing, which is the worst of both. + EXPECT_FALSE(parse_public_route("/api/v1/health").has_value()); + EXPECT_FALSE(parse_public_route("GET").has_value()); + EXPECT_FALSE(parse_public_route("").has_value()); + EXPECT_FALSE(parse_public_route(" ").has_value()); + EXPECT_FALSE(parse_public_route("FETCH /api/v1/health").has_value()); + EXPECT_FALSE(parse_public_route("GET api/v1/health").has_value()); + EXPECT_FALSE(parse_public_route("GET /api/v1/*").has_value()); + EXPECT_FALSE(parse_public_route("GET /api/v1/health extra").has_value()); +} + +// A malformed entry must not quietly open something. Dropping it keeps the +// route protected; GatewayNode refuses to start so the typo is not silent. +// @verifies REQ_INTEROP_086 +TEST_F(AuthRequirementPolicyTest, MalformedPublicRoutesAreDropped) { + auto routes = parse_public_routes({"GET /api/v1/health", "nonsense", "GET /api/v1/*"}); + + ASSERT_EQ(routes.size(), 1u); + EXPECT_EQ(routes[0].path, "/api/v1/health"); +} + +// @verifies REQ_INTEROP_086 +TEST_F(AuthRequirementPolicyTest, AllAuthPolicyExemptsAuthEndpoints) { + AllAuthRequirementPolicy policy; + + // Authentication cannot bootstrap through a door that already demands the + // credential it exists to hand out. + EXPECT_FALSE(policy.requires_authentication("POST", "/api/v1/auth/authorize")); + EXPECT_FALSE(policy.requires_authentication("POST", "/api/v1/auth/token")); + EXPECT_FALSE(policy.requires_authentication("POST", "/api/v1/auth/revoke")); + + // The prefix must be anchored: a path that merely mentions auth later is + // not an auth endpoint. + EXPECT_TRUE(policy.requires_authentication("GET", "/api/v1/components/auth/data")); + EXPECT_TRUE(policy.requires_authentication("GET", "/api/v1/authorization")); +} + // @verifies REQ_INTEROP_086 TEST_F(AuthRequirementPolicyTest, WriteOnlyPolicyForGetRequests) { WriteOnlyAuthRequirementPolicy policy; diff --git a/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/gateway_test_case.py b/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/gateway_test_case.py index 22b9cb57c..33d9248b1 100644 --- a/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/gateway_test_case.py +++ b/src/ros2_medkit_integration_tests/ros2_medkit_test_utils/gateway_test_case.py @@ -93,7 +93,15 @@ def setUpClass(cls): @classmethod def _wait_for_gateway_health(cls): - """Poll GET /health until the gateway responds with 200. + """Poll GET /health until the gateway answers at all. + + A refusal counts as up. What this waits for is a process that is + listening and speaking HTTP, and a 401 proves both: the request was + received, routed and decided on. Requiring 200 would instead make this + wait for a specific auth configuration, and under the shipped defaults + - ``require_auth_for: all`` with an empty ``auth.public_routes`` - + that 200 never arrives, so every test class using this helper would + time out against a perfectly healthy gateway. Uses ``time.monotonic()`` for a reliable, monotonic clock. @@ -108,7 +116,7 @@ def _wait_for_gateway_health(cls): while time.monotonic() < deadline: try: response = requests.get(f'{cls.BASE_URL}/health', timeout=2) - if response.status_code == 200: + if response.status_code in (200, 401, 403): return except requests.exceptions.RequestException: pass diff --git a/src/ros2_medkit_integration_tests/test/features/test_closed_by_default.test.py b/src/ros2_medkit_integration_tests/test/features/test_closed_by_default.test.py new file mode 100644 index 000000000..dc2f7f48a --- /dev/null +++ b/src/ros2_medkit_integration_tests/test/features/test_closed_by_default.test.py @@ -0,0 +1,587 @@ +#!/usr/bin/env python3 +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Every route refuses an uncredentialed request under the shipped defaults. + +The gateway's own half of the closed-door acceptance. It does not check +configuration values: it asks the RUNNING gateway for its route table and then +probes every route in it. A test that asserted `require_auth_for == "all"` +would keep passing the day a route is registered outside the policy, which is +the failure this is here to catch. + +The route table comes from RouteRegistry via `GET /api/v1/`, so a route added +next year is swept the day it is registered, with nothing here to update. + +Two exemptions, and both are named with their reason in EXEMPT below. + +@verifies REQ_INTEROP_086, REQ_INTEROP_087 +""" + +import unittest + +import launch +import launch_testing +import launch_testing.actions +import pytest +import requests + +from ros2_medkit_test_utils.constants import ( + ALLOWED_EXIT_CODES, + API_BASE_PATH, + get_test_port, +) +from ros2_medkit_test_utils.gateway_test_case import GatewayTestCase +from ros2_medkit_test_utils.launch_helpers import create_gateway_node + +CLOSED_PORT = get_test_port() +CORS_PORT = get_test_port(1) +CLOSED_BASE_URL = f'http://127.0.0.1:{CLOSED_PORT}{API_BASE_PATH}' +CLOSED_ROOT = f'http://127.0.0.1:{CLOSED_PORT}' +CORS_BASE_URL = f'http://127.0.0.1:{CORS_PORT}{API_BASE_PATH}' +RL_PORT = get_test_port(2) +RL_BASE_URL = f'http://127.0.0.1:{RL_PORT}{API_BASE_PATH}' +PUBLIC_PORT = get_test_port(3) +PUBLIC_BASE_URL = f'http://127.0.0.1:{PUBLIC_PORT}{API_BASE_PATH}' +ALLOWED_ORIGIN = 'https://ui.example' + +# At least 32 characters, or the gateway refuses to start under HS256. +JWT_SECRET = 'closed_by_default_integration_secret_key_0123456789' +CLIENT_ID = 'diagbox' +CLIENT_SECRET = 'diagbox_client_secret' + +# A path parameter is filled with an id that exists on no gateway. A route that +# refuses for a nonexistent id refuses for a real one, and a probe that turns +# out to reach an OPEN route cannot mutate anything real. +PROBE_ID = 'closed-by-default-probe' + + +@pytest.mark.launch_test +def generate_test_description(): + """Gateway with the posture the shipped default config carries.""" + gateway_node = create_gateway_node( + port=CLOSED_PORT, + extra_params={ + 'server.host': '127.0.0.1', + # These three are what config/gateway_params.yaml now ships. The + # secret and client cannot come from that file (a committed secret + # is a secret every deployment shares), so they are supplied here + # the way a deployment supplies them. + 'auth.enabled': True, + 'auth.require_auth_for': 'all', + 'auth.issuer': 'ros2_medkit_gateway', + 'auth.jwt_secret': JWT_SECRET, + 'auth.clients': [f'{CLIENT_ID}:{CLIENT_SECRET}:admin'], + }, + ) + + cors_gateway = create_gateway_node( + port=CORS_PORT, + name='gateway_with_cors', + extra_params={ + 'server.host': '127.0.0.1', + 'auth.enabled': True, + 'auth.require_auth_for': 'all', + 'auth.issuer': 'ros2_medkit_gateway', + 'auth.jwt_secret': JWT_SECRET, + 'auth.clients': [f'{CLIENT_ID}:{CLIENT_SECRET}:admin'], + # The configuration where the hole lived. + 'cors.allowed_origins': [ALLOWED_ORIGIN], + }, + ) + + # Rate limiting on, and tight, so the limiter can actually be exhausted + # inside a test. Without a gateway in this state the ordering between the + # limiter and authentication is unobservable, which is how it went + # unnoticed in the first place. + rl_gateway = create_gateway_node( + port=RL_PORT, + name='gateway_with_rate_limit', + extra_params={ + 'server.host': '127.0.0.1', + 'auth.enabled': True, + 'auth.require_auth_for': 'all', + 'auth.issuer': 'ros2_medkit_gateway', + 'auth.jwt_secret': JWT_SECRET, + 'auth.clients': [f'{CLIENT_ID}:{CLIENT_SECRET}:admin'], + 'rate_limiting.enabled': True, + 'rate_limiting.global_requests_per_minute': 2, + 'rate_limiting.client_requests_per_minute': 2, + }, + ) + + # The opt-in half. `auth.public_routes` is empty on every gateway above, so + # without this one nothing here would exercise the knob an operator uses to + # take a route outside authentication, and "empty by default" would be + # indistinguishable from "the setting does nothing". + public_route_gateway = create_gateway_node( + port=PUBLIC_PORT, + name='gateway_with_public_route', + extra_params={ + 'server.host': '127.0.0.1', + 'auth.enabled': True, + 'auth.require_auth_for': 'all', + 'auth.issuer': 'ros2_medkit_gateway', + 'auth.jwt_secret': JWT_SECRET, + 'auth.clients': [f'{CLIENT_ID}:{CLIENT_SECRET}:admin'], + 'auth.public_routes': ['GET /api/v1/health'], + }, + ) + + return launch.LaunchDescription([ + gateway_node, + cors_gateway, + rl_gateway, + public_route_gateway, + launch_testing.actions.ReadyToTest(), + ]), {'gateway_node': gateway_node, 'cors_gateway': cors_gateway, + 'rl_gateway': rl_gateway, 'public_route_gateway': public_route_gateway} + + +def _is_exempt(method, path): + """Routes that are deliberately reachable without a credential. + + /auth/* alone, and only because authentication cannot bootstrap through a + door that already demands the credential it exists to hand out. + + Health is NOT here. `auth.public_routes` is empty as shipped, so a probe + that wants an uncredentialed answer is a decision an operator makes and + writes down; TestConfiguredPublicRoute below covers that path. + """ + del method # the one exemption is path-shaped: every method under /auth/ + return path.startswith(f'{API_BASE_PATH}/auth/') + + +class TestClosedByDefault(GatewayTestCase): + """The gateway refuses every route it serves, bar the named exemptions.""" + + BASE_URL = CLOSED_BASE_URL + + @classmethod + def setUpClass(cls): + super().setUpClass() + resp = requests.post( + f'{CLOSED_BASE_URL}/auth/authorize', + json={ + 'grant_type': 'client_credentials', + 'client_id': CLIENT_ID, + 'client_secret': CLIENT_SECRET, + }, + timeout=10, + ) + assert resp.status_code == 200, f'could not obtain a token: {resp.status_code} {resp.text}' + cls.token = resp.json()['access_token'] + cls.auth = {'Authorization': f'Bearer {cls.token}'} + + # The route table, read from the gateway itself. A hardened gateway + # does not list its routes anonymously, so this read authenticates. + root = requests.get(f'{CLOSED_BASE_URL}/', headers=cls.auth, timeout=10) + assert root.status_code == 200, f'route table unreadable: {root.status_code}' + cls.endpoints = root.json().get('endpoints', []) + assert cls.endpoints, 'gateway reported no endpoints - nothing would be proven' + + @staticmethod + def _fill(path): + out, depth = [], 0 + for ch in path: + if ch == '{': + depth += 1 + if depth == 1: + out.append(PROBE_ID) + elif ch == '}': + depth -= 1 + elif depth == 0: + out.append(ch) + return ''.join(out) + + def test_01_route_table_is_substantial(self): + """A sweep over three routes would prove almost nothing.""" + self.assertGreater( + len(self.endpoints), 50, + f'expected the full gateway surface, got {len(self.endpoints)} routes' + ) + + def test_02_no_route_answers_without_a_credential(self): + """Sweep EVERY registered route. This is the acceptance.""" + answered = [] + for entry in self.endpoints: + method, _, raw = entry.partition(' ') + if not raw: + continue + if _is_exempt(method, raw): + continue + path = self._fill(raw) + # A write method needs a body: without Content-Length the server + # waits for one that never arrives and the probe times out with no + # status, measuring nothing at all. + kwargs = {'timeout': 15} + if method in ('POST', 'PUT', 'PATCH'): + kwargs['json'] = {} + try: + resp = requests.request(method, f'{CLOSED_ROOT}{path}', **kwargs) + except requests.RequestException as exc: + answered.append(f'{method} {path} -> transport error {exc}') + continue + # 401/403 only. A 404 is an ANSWER: the gateway parsed the request + # and told an anonymous caller what does not exist here. + if resp.status_code not in (401, 403): + answered.append(f'{method} {path} -> {resp.status_code}') + + self.assertEqual( + [], answered, + 'these routes answered an uncredentialed request:\n ' + + '\n '.join(answered) + ) + + def test_03_a_wrong_credential_is_refused_everywhere(self): + """A token this gateway never issued gets no further than none at all.""" + bad = {'Authorization': 'Bearer not.a.real.token'} + answered = [] + for entry in self.endpoints: + method, _, raw = entry.partition(' ') + if not raw or _is_exempt(method, raw): + continue + path = self._fill(raw) + kwargs = {'timeout': 15, 'headers': bad} + if method in ('POST', 'PUT', 'PATCH'): + kwargs['json'] = {} + try: + resp = requests.request(method, f'{CLOSED_ROOT}{path}', **kwargs) + except requests.RequestException as exc: + answered.append(f'{method} {path} -> transport error {exc}') + continue + if resp.status_code not in (401, 403): + answered.append(f'{method} {path} -> {resp.status_code}') + + self.assertEqual( + [], answered, + 'these routes accepted a forged credential:\n ' + '\n '.join(answered) + ) + + def test_04_reads_are_refused_not_just_writes(self): + """The require_auth_for="write" hole, pinned directly. + + Under "write" every one of these answers 200 to an anonymous caller, + and they are the disclosure: the entity tree names the machines. + """ + for path in ('/', '/areas', '/components', '/apps', '/functions', '/version-info'): + with self.subTest(path=path): + resp = requests.get(f'{CLOSED_BASE_URL}{path}', timeout=15) + self.assertIn(resp.status_code, (401, 403)) + + def test_05_health_refuses_like_everything_else(self): + """Health is not special. It is closed until somebody opens it. + + The route a hardening change is most tempted to leave open, pinned so + the temptation shows up as a red test. `auth.public_routes` is the way + to open it, and TestConfiguredPublicRoute holds that end. + """ + resp = requests.get(f'{CLOSED_BASE_URL}/health', timeout=15) + self.assertIn( + resp.status_code, (401, 403), + f'GET /health answered {resp.status_code} with no credential and an ' + 'empty auth.public_routes' + ) + + def test_06_the_full_health_document_names_entities(self): + """Why the anonymous body has to be cut down when a route is opened. + + With a credential the same route returns discovery state and entity + cache counts. That is a legitimate operator surface, and it is exactly + what an anonymous caller must not receive - so if this ever stops being + true, the narrowing in TestConfiguredPublicRoute has become pointless + and should be revisited rather than left as dead weight. + """ + body = requests.get( + f'{CLOSED_BASE_URL}/health', headers=self.auth, timeout=15 + ).json() + self.assertIn('discovery', body) + self.assertIn('x-medkit-entity-cache', body) + self.assertNotIn( + 'x-medkit-reduced', body, + 'an authenticated caller was served the cut-down body' + ) + + def test_07_a_valid_credential_gets_through(self): + """Otherwise the sweeps above would pass on a gateway that serves nobody.""" + resp = requests.get(f'{CLOSED_BASE_URL}/areas', headers=self.auth, timeout=15) + self.assertEqual(resp.status_code, 200) + + +class TestConfiguredPublicRoute(GatewayTestCase): + """`auth.public_routes` opens exactly what it names, and nothing near it. + + The gateway under this class runs `require_auth_for: all` with one entry, + `GET /api/v1/health`. Everything here is about the edge of that entry: a + setting that opened the route it names AND its neighbours would pass a test + that only checked the route it names. + """ + + BASE_URL = PUBLIC_BASE_URL + + @classmethod + def setUpClass(cls): + super().setUpClass() + token = requests.post( + f'{PUBLIC_BASE_URL}/auth/authorize', + json={ + 'grant_type': 'client_credentials', + 'client_id': CLIENT_ID, + 'client_secret': CLIENT_SECRET, + }, + timeout=15, + ).json()['access_token'] + cls.auth = {'Authorization': f'Bearer {token}'} + + def test_01_the_named_route_answers_without_a_credential(self): + """The knob does something. Without this the rest proves only refusal.""" + resp = requests.get(f'{PUBLIC_BASE_URL}/health', timeout=15) + self.assertEqual( + resp.status_code, 200, + 'auth.public_routes named GET /api/v1/health and it still refused' + ) + + def test_02_the_route_next_door_is_untouched(self): + """An entry opens one route, not the surface around it. + + The failure this catches is a prefix or wildcard match creeping into + the comparison: `/health` opening `/healthz`, or worse, one entry + opening every GET. + """ + for path in ('/', '/areas', '/components', '/apps', '/version-info'): + with self.subTest(path=path): + resp = requests.get(f'{PUBLIC_BASE_URL}{path}', timeout=15) + self.assertIn( + resp.status_code, (401, 403), + f'{path} answered {resp.status_code} on a gateway whose only ' + 'public route is GET /api/v1/health' + ) + + def test_03_the_method_is_part_of_the_entry(self): + """An entry names a method, and the method is part of the match. + + cpp-httplib dispatches HEAD into the GET handler table, so a comparison + that dropped the method would hand the status document to an anonymous + HEAD. The write methods have no handler here and answer the same either + way, so HEAD is the one that can show the difference. + """ + head = requests.head(f'{PUBLIC_BASE_URL}/health', timeout=15) + self.assertIn( + head.status_code, (401, 403), + f'HEAD /health answered {head.status_code} for an entry that named GET' + ) + + def test_04_the_anonymous_body_is_liveness_and_says_so(self): + """Opening the route must not publish the entity inventory. + + An allowlist, not a denylist: listing the fields known to leak today + would pass the day a new section is added, and a probe needs no more + than "am I alive". + """ + body = requests.get(f'{PUBLIC_BASE_URL}/health', timeout=15).json() + self.assertEqual( + set(body), + {'status', 'timestamp', 'warnings', 'warning_schema_version', + 'x-medkit-reduced'}, + f'an anonymous /health returned more than liveness: {body}' + ) + self.assertEqual(body['status'], 'healthy') + # The array is the leak vector: a linking warning reads like + # "App 'engine_ecu' cannot bind to '/nav/controller'", naming an entity + # and a ROS node FQN. + self.assertEqual(body['warnings'], []) + # And the empty array must not read as "nothing is wrong". A monitor + # that cannot tell withheld from clean would clear a real warning. + self.assertIs( + body['x-medkit-reduced'], True, + 'the cut-down body did not say it was cut down, so an empty ' + 'warnings array reads as a clean bill of health' + ) + + def test_05_a_credential_still_gets_the_whole_document(self): + """Opening a route for probes must not cost the operator surface.""" + body = requests.get( + f'{PUBLIC_BASE_URL}/health', headers=self.auth, timeout=15 + ).json() + self.assertIn('discovery', body) + self.assertNotIn('x-medkit-reduced', body) + + def test_06_a_forged_credential_is_an_anonymous_caller(self): + """A token this gateway never issued must not unlock the full body.""" + body = requests.get( + f'{PUBLIC_BASE_URL}/health', + headers={'Authorization': 'Bearer not.a.real.token'}, + timeout=15, + ).json() + self.assertIs(body.get('x-medkit-reduced'), True, body) + self.assertNotIn('discovery', body) + + +class TestNothingAnswersBeforeAuth(GatewayTestCase): + """What the CORS preflight may and may not do without a credential. + + Preflight is answered anonymously on purpose, and it is the second named + exemption after /auth/*. A browser never puts Authorization on a preflight + - asking permission before sending the real request is the whole point of + the mechanism - so demanding one would not harden anything, it would make + browser clients impossible. The control below is what pins that. + + What must hold instead: the preflight discloses only CORS policy, and the + REAL request that follows is still refused without a credential. + + This gateway enables CORS for a real origin, which the rest of the file + deliberately does not, because that is the configuration in which any of + this is reachable at all. + """ + + BASE_URL = CORS_BASE_URL + + def _preflight(self, extra=None): + headers = {'Origin': ALLOWED_ORIGIN, 'Access-Control-Request-Method': 'GET'} + headers.update(extra or {}) + return requests.options(f'{CORS_BASE_URL}/apps', headers=headers, timeout=15) + + def test_01_an_anonymous_preflight_is_answered(self): + """The exemption, stated as a test rather than left implicit. + + This is the control that failed when the branch briefly required a + credential here: a browser cannot send one, so a 401 or 403 means no + browser client can reach this gateway at all. + """ + resp = self._preflight() + self.assertEqual( + resp.status_code, 204, + f'an anonymous preflight got {resp.status_code}; a browser cannot ' + 'authenticate a preflight, so this makes browser clients impossible' + ) + self.assertEqual(resp.headers.get('Access-Control-Allow-Origin'), ALLOWED_ORIGIN) + + def test_02_the_preflight_discloses_only_cors_policy(self): + """Why the exemption is safe: there is nothing in the response. + + If a preflight ever grew a body, the exemption would start leaking and + this fails rather than letting it pass unnoticed. + """ + resp = self._preflight() + self.assertEqual( + resp.content, b'', + f'the preflight returned a body: {resp.content[:200]!r}' + ) + + def test_03_a_preflight_from_an_unknown_origin_is_refused(self): + """The exemption is scoped to origins the operator configured.""" + resp = requests.options( + f'{CORS_BASE_URL}/apps', + headers={'Origin': 'https://not-configured.example', + 'Access-Control-Request-Method': 'GET'}, + timeout=15, + ) + self.assertEqual(resp.status_code, 403) + + def test_04_the_real_request_after_a_preflight_still_needs_a_credential(self): + """The property that actually matters. + + A preflight being answered must not carry any implication for the GET + that follows it, which is where the data is. + """ + resp = requests.get( + f'{CORS_BASE_URL}/apps', headers={'Origin': ALLOWED_ORIGIN}, timeout=15 + ) + self.assertIn( + resp.status_code, (401, 403), + f'a cross-origin GET got {resp.status_code} with no credential' + ) + + def test_04b_a_plain_options_without_the_preflight_header_is_refused(self): + """The exemption is for preflights, not for the OPTIONS method. + + A browser preflight always carries Access-Control-Request-Method. An + OPTIONS without it is an ordinary request that any client could send, + and it has no reason to skip the credential check. The helper above + always sends both headers, so this boundary needs its own case. + """ + resp = requests.options( + f'{CORS_BASE_URL}/apps', + headers={'Origin': ALLOWED_ORIGIN}, + timeout=15, + ) + self.assertIn( + resp.status_code, (401, 403), + f'a plain OPTIONS with no Access-Control-Request-Method got ' + f'{resp.status_code}; the preflight exemption is too wide' + ) + + def test_05_an_authenticated_cross_origin_request_works(self): + """The mirror: CORS is live and a credentialed browser call succeeds.""" + headers = {'Origin': ALLOWED_ORIGIN} + headers.update(self.cors_auth) + resp = requests.get(f'{CORS_BASE_URL}/apps', headers=headers, timeout=15) + self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.headers.get('Access-Control-Allow-Origin'), ALLOWED_ORIGIN) + + @classmethod + def setUpClass(cls): + super().setUpClass() + resp = requests.post( + f'{CORS_BASE_URL}/auth/authorize', + json={ + 'grant_type': 'client_credentials', + 'client_id': CLIENT_ID, + 'client_secret': CLIENT_SECRET, + }, + timeout=30, + ) + assert resp.status_code == 200, f'token request failed: {resp.status_code}' + cls.cors_auth = {'Authorization': f'Bearer {resp.json()["access_token"]}'} + + +class TestRateLimiterDoesNotAnswerBeforeAuth(GatewayTestCase): + """An anonymous caller gets 401, never 429. + + The rate limiter runs in the same pre-routing handler and also returns + "handled". With it ordered before authentication, a caller with no + credential who exhausted the allowance received 429 from a protected route: + an answer, plus a small disclosure of limiter state, without ever presenting + anything. The limit here is deliberately tiny so the exhausted state is + reachable in a test at all. + """ + + BASE_URL = RL_BASE_URL + + def test_01_an_exhausted_anonymous_caller_still_gets_401(self): + seen = [] + # Comfortably past a limit of 2/minute. + for _ in range(8): + seen.append(requests.get(f'{RL_BASE_URL}/apps', timeout=15).status_code) + + self.assertNotIn( + 429, seen, + f'an anonymous caller was rate-limited instead of refused: {seen}' + ) + self.assertTrue( + all(code in (401, 403) for code in seen), + f'expected only 401/403 for an uncredentialed caller, got {seen}' + ) + + +@launch_testing.post_shutdown_test() +class TestClosedByDefaultShutdown(unittest.TestCase): + """Gateway exits cleanly.""" + + def test_exit_codes(self, proc_info, gateway_node, cors_gateway, rl_gateway): + for proc in (gateway_node, cors_gateway, rl_gateway): + launch_testing.asserts.assertExitCodes( + proc_info, allowable_exit_codes=ALLOWED_EXIT_CODES, process=proc + ) diff --git a/src/ros2_medkit_integration_tests/test/features/test_peer_recovery.test.py b/src/ros2_medkit_integration_tests/test/features/test_peer_recovery.test.py index 7aedd42e2..ee029d0d1 100644 --- a/src/ros2_medkit_integration_tests/test/features/test_peer_recovery.test.py +++ b/src/ros2_medkit_integration_tests/test/features/test_peer_recovery.test.py @@ -825,9 +825,20 @@ def test_07_a_peer_owned_read_succeeds_again_and_the_peer_answered_it(self): 'test_04 must watch this URL fail before test_07 can claim it recovered', ) + # Poll for the condition this test actually asserts, not merely for a + # 200. Recovery has two steps that finish at different times: the route + # comes back, and then a sample arrives on the re-created subscription. + # Between them the read answers 200 with status "metadata_only" and an + # empty body, so a poll that stops at the status code hands the + # assertions below a response taken from that window - and the wider + # the machine's load, the wider the window. def served(): answer = self._aggregate_read_of_peer_topic() - return answer if answer.status_code == 200 else None + if answer.status_code != 200: + return None + if answer.json().get('x-medkit', {}).get('status') != 'data': + return None + return answer response = _poll(served, timeout=RECOVERY_TIMEOUT) self.assertIsNotNone( diff --git a/src/ros2_medkit_integration_tests/test/features/test_shipped_defaults.test.py b/src/ros2_medkit_integration_tests/test/features/test_shipped_defaults.test.py new file mode 100644 index 000000000..e0c97a191 --- /dev/null +++ b/src/ros2_medkit_integration_tests/test/features/test_shipped_defaults.test.py @@ -0,0 +1,215 @@ +#!/usr/bin/env python3 +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Boot the gateway from the SHIPPED config file and check the posture. + +Every other test in this suite builds its parameters inline, which is fine for +testing behaviour but means nothing here ever loaded +``config/gateway_params.yaml``. That left the branch in an odd state: the file +could be reverted to ``auth.enabled: false`` and the whole suite would stay +green, because each test supplies the values it needs itself. + +This file closes that gap. It launches with ``--params-file`` pointing at the +installed copy of the shipped config, overriding only the port, the signing +secret and the client - the three things a real deployment must supply and the +file deliberately leaves empty - and then checks that what ships is closed. + +TLS is turned off here and only here. The shipped file has it on, which is +correct, but a certificate is a deployment artefact and generating one would +test the certificate rather than the posture. ``test_tls_protocol_floor`` +covers TLS itself against real handshakes. + +@verifies REQ_INTEROP_086, REQ_INTEROP_087 +""" + +import os +import socket +import time +import unittest + +from ament_index_python.packages import get_package_share_directory +import launch +import launch_ros.actions +import launch_testing +import launch_testing.actions +import pytest +import requests + +from ros2_medkit_test_utils.constants import ( + ALLOWED_EXIT_CODES, + API_BASE_PATH, + get_test_port, +) +from ros2_medkit_test_utils.coverage import get_coverage_env + +PORT = get_test_port() +BASE_URL = f'http://127.0.0.1:{PORT}{API_BASE_PATH}' + +SHIPPED_PARAMS = os.path.join( + get_package_share_directory('ros2_medkit_gateway'), 'config', 'gateway_params.yaml' +) + +JWT_SECRET = 'shipped_defaults_integration_secret_key_0123456789' +CLIENT_ID = 'shipped' +CLIENT_SECRET = 'shipped_client_secret' + + +@pytest.mark.launch_test +def generate_test_description(): + """Launch the gateway with the shipped params file, plus the required secrets.""" + gateway_node = launch_ros.actions.Node( + package='ros2_medkit_gateway', + executable='gateway_node', + name='ros2_medkit_gateway', + output='screen', + parameters=[ + SHIPPED_PARAMS, + { + 'server.host': '127.0.0.1', + 'server.port': PORT, + 'refresh_interval_ms': 1000, + # A certificate is a deployment artefact, not part of the + # posture under test here. + 'server.tls.enabled': False, + # What the shipped file leaves empty on purpose. + 'auth.jwt_secret': JWT_SECRET, + 'auth.clients': [f'{CLIENT_ID}:{CLIENT_SECRET}:admin'], + }, + ], + additional_env=dict(get_coverage_env()), + ) + + return launch.LaunchDescription([ + gateway_node, + launch_testing.actions.ReadyToTest(), + ]), {'gateway_node': gateway_node} + + +def _wait_listening(port, timeout=90.0): + """Block until the gateway accepts a connection. + + launch_testing starts the tests when the process is spawned, not when it is + serving. Without this the first request is refused by a gateway that simply + has not opened its socket yet, which looks nothing like the posture this + file is about. + + The timeout is generous because this gateway loads the full shipped config, + which does more work at startup than the inline parameter sets the rest of + the suite uses. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + try: + with socket.create_connection(('127.0.0.1', port), timeout=2): + return + except OSError: + time.sleep(0.25) + raise AssertionError(f'gateway on port {port} never started listening within {timeout}s') + + +class TestShippedDefaults(unittest.TestCase): + """What config/gateway_params.yaml actually produces.""" + + @classmethod + def setUpClass(cls): + _wait_listening(PORT) + resp = requests.post( + f'{BASE_URL}/auth/authorize', + json={ + 'grant_type': 'client_credentials', + 'client_id': CLIENT_ID, + 'client_secret': CLIENT_SECRET, + }, + timeout=30, + ) + assert resp.status_code == 200, f'token request failed: {resp.status_code} {resp.text}' + cls.auth = {'Authorization': f'Bearer {resp.json()["access_token"]}'} + + def test_01_the_shipped_file_turns_authentication_on(self): + """Reverting auth.enabled in the shipped file must fail here. + + No other test would notice: they all pass auth.enabled themselves. + """ + resp = requests.get(f'{BASE_URL}/areas', timeout=15) + self.assertIn( + resp.status_code, (401, 403), + 'the shipped config served /areas to an anonymous caller' + ) + + def test_02_the_shipped_file_covers_reads_not_just_writes(self): + """Pins require_auth_for: "all" as the shipped value. + + Under "write" every one of these answers 200 without a credential. + """ + for path in ('/', '/areas', '/components', '/apps', '/functions', '/version-info'): + with self.subTest(path=path): + resp = requests.get(f'{BASE_URL}{path}', timeout=15) + self.assertIn(resp.status_code, (401, 403)) + + def test_03_health_refuses_in_the_shipped_file_too(self): + """The shipped file opens nothing, health included. + + `auth.public_routes` is absent from the shipped config, so the file + that goes out in the package and the image leaves no route reachable + without a credential. An operator who wants a probe route adds the + entry themselves - that path is covered in test_closed_by_default. + + Pinned here separately from the sweep above because health is the route + a hardening change is most tempted to leave open, and a shipped file + that quietly did so would still pass every other test in this class. + """ + resp = requests.get(f'{BASE_URL}/health', timeout=15) + self.assertIn( + resp.status_code, (401, 403), + f'the shipped config answered GET /health with {resp.status_code} ' + 'to a caller holding no credential' + ) + + def test_04_a_configured_client_still_works(self): + """The mirror: a gateway that refused everyone would pass the rest.""" + resp = requests.get(f'{BASE_URL}/areas', headers=self.auth, timeout=15) + self.assertEqual(resp.status_code, 200) + + def test_05_the_shipped_file_is_the_one_under_test(self): + """Guard against this test silently drifting off the real file. + + If the installed config stops declaring the values this file exists to + check, the assertions above would still pass for the wrong reason. + """ + with open(SHIPPED_PARAMS, encoding='utf-8') as handle: + text = handle.read() + self.assertIn('require_auth_for: "all"', text) + self.assertIn('enabled: true', text) + # And no route is opened by the file itself. An uncommented entry here + # would be a public route shipped to every deployment, which is exactly + # what this branch exists to stop. The commented example does not count. + live_public_routes = [ + line for line in text.splitlines() + if line.strip().startswith('public_routes:') + ] + self.assertEqual( + live_public_routes, [], + f'the shipped config declares public routes: {live_public_routes}' + ) + + +@launch_testing.post_shutdown_test() +class TestShippedDefaultsShutdown(unittest.TestCase): + """Gateway exits cleanly.""" + + def test_exit_codes(self, proc_info, gateway_node): + launch_testing.asserts.assertExitCodes( + proc_info, allowable_exit_codes=ALLOWED_EXIT_CODES, process=gateway_node + ) diff --git a/src/ros2_medkit_integration_tests/test/features/test_tls_protocol_floor.test.py b/src/ros2_medkit_integration_tests/test/features/test_tls_protocol_floor.test.py new file mode 100644 index 000000000..6a629cce2 --- /dev/null +++ b/src/ros2_medkit_integration_tests/test/features/test_tls_protocol_floor.test.py @@ -0,0 +1,335 @@ +#!/usr/bin/env python3 +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Check the TLS protocol floor and client-certificate verification. + +Both are driven by a real client against a real gateway. + +Both properties are about what happens during the TLS handshake, before any +HTTP request exists, so they cannot be observed from Python's requests or from +a unit test that checks a setter was called. Every assertion here comes from +``openssl s_client`` completing or failing a handshake at a pinned version. + +The client is run with ``-cipher ALL:@SECLEVEL=0``. Without it a modern +OpenSSL client refuses to OFFER TLS 1.0/1.1 on its own, and the test would pass +while proving nothing about the server: it has to be the server that says no. + +Two gateways run side by side, one with min_version 1.2 and one with 1.3, so +the floor is shown to MOVE with the setting rather than happening to sit where +OpenSSL's own default put it. + +@verifies REQ_INTEROP_086 +""" + +import os +import re +import shutil +import socket +import subprocess +import tempfile +import time +import unittest + +import launch +import launch_testing +import launch_testing.actions +import pytest + +from ros2_medkit_test_utils.constants import ALLOWED_EXIT_CODES, get_test_port +from ros2_medkit_test_utils.launch_helpers import create_gateway_node + +PORT_TLS12 = get_test_port(0) +PORT_TLS13 = get_test_port(1) +PORT_MTLS = get_test_port(2) + +_CERT_DIR = tempfile.mkdtemp(prefix='medkit_tls_floor_') + + +def _run(*args): + subprocess.run(args, check=True, capture_output=True) + + +def _make_ca(name): + """Build a CA key plus its self-signed certificate.""" + key = os.path.join(_CERT_DIR, f'{name}-ca-key.pem') + crt = os.path.join(_CERT_DIR, f'{name}-ca.pem') + _run('openssl', 'req', '-x509', '-newkey', 'rsa:2048', '-nodes', + '-keyout', key, '-out', crt, '-days', '1', + '-subj', f'/CN=medkit-test-{name}-ca') + return key, crt + + +def _make_leaf(name, ca_key, ca_crt, cn): + """Build a leaf key and certificate signed by the given CA.""" + key = os.path.join(_CERT_DIR, f'{name}-key.pem') + csr = os.path.join(_CERT_DIR, f'{name}.csr') + crt = os.path.join(_CERT_DIR, f'{name}.pem') + _run('openssl', 'req', '-newkey', 'rsa:2048', '-nodes', + '-keyout', key, '-out', csr, '-subj', f'/CN={cn}') + _run('openssl', 'x509', '-req', '-in', csr, '-CA', ca_crt, '-CAkey', ca_key, + '-CAcreateserial', '-out', crt, '-days', '1') + return key, crt + + +# The CA that signs the server certificate and the legitimate client. +CA_KEY, CA_CRT = _make_ca('trusted') +SRV_KEY, SRV_CRT = _make_leaf('server', CA_KEY, CA_CRT, 'localhost') +CLI_KEY, CLI_CRT = _make_leaf('client', CA_KEY, CA_CRT, 'medkit-test-client') + +# A second CA the gateway was never told about, for the certificate that is +# well-formed and correctly signed but by the wrong authority. +ROGUE_KEY, ROGUE_CRT = _make_ca('rogue') +ROGUE_CLI_KEY, ROGUE_CLI_CRT = _make_leaf('rogue-client', ROGUE_KEY, ROGUE_CRT, 'rogue') + + +def _tls_params(port, min_version, ca_file=''): + params = { + 'server.host': '127.0.0.1', + 'server.tls.enabled': True, + 'server.tls.cert_file': SRV_CRT, + 'server.tls.key_file': SRV_KEY, + 'server.tls.min_version': min_version, + # Auth off: this file is about the handshake, and a 401 would arrive + # long after the point under test has already been decided. + 'auth.enabled': False, + } + if ca_file: + params['server.tls.ca_file'] = ca_file + return params + + +@pytest.mark.launch_test +def generate_test_description(): + """Three gateways: floor at 1.2, floor at 1.3, and one demanding a client cert.""" + nodes = [ + create_gateway_node(port=PORT_TLS12, name='gateway_tls12', + extra_params=_tls_params(PORT_TLS12, '1.2')), + create_gateway_node(port=PORT_TLS13, name='gateway_tls13', + extra_params=_tls_params(PORT_TLS13, '1.3')), + create_gateway_node(port=PORT_MTLS, name='gateway_mtls', + extra_params=_tls_params(PORT_MTLS, '1.2', ca_file=CA_CRT)), + ] + return launch.LaunchDescription(nodes + [launch_testing.actions.ReadyToTest()]), { + 'gateway_tls12': nodes[0], + 'gateway_tls13': nodes[1], + 'gateway_mtls': nodes[2], + } + + +def _handshake(port, version, client_cert=None, client_key=None, timeout=20): + """Attempt one handshake. True only when a cipher was actually agreed. + + `openssl s_client` exits 0 in cases where no session was established, and + it prints the protocol it ATTEMPTED whether or not the server accepted it. + "Cipher is (NONE)" is the reliable tell for a handshake that did not + complete, so that is what is read here rather than the exit status. + """ + cmd = ['openssl', 's_client', f'-{version}', + '-cipher', 'ALL:@SECLEVEL=0', + '-connect', f'127.0.0.1:{port}'] + if client_cert: + cmd += ['-cert', client_cert, '-key', client_key] + try: + proc = subprocess.run(cmd, input=b'', capture_output=True, timeout=timeout) + except subprocess.TimeoutExpired: + return False + out = (proc.stdout + proc.stderr).decode(errors='replace') + + # "Cipher is " is NOT proof that the handshake completed, and reading + # it that way is how an earlier version of this file reported mutual TLS as + # broken when it was working. Under TLS 1.2 the cipher suite is agreed + # before the client certificate is examined, so a server that then rejects + # the certificate still leaves a cipher name in the output, followed by a + # fatal alert. Verified by hand against this gateway: a client with no + # certificate printed "Cipher is ECDHE-RSA-AES256-GCM-SHA384" AND + # "sslv3 alert handshake failure", while curl against the same endpoint got + # no HTTP response at all. + # + # So a fatal alert is the signal, and "Cipher is (NONE)" covers the case + # where the version itself was refused before any suite was picked. + if 'Cipher is (NONE)' in out: + return False + if re.search(r'alert (handshake failure|protocol version|certificate|unknown ca)', out): + return False + return 'Cipher is ' in out + + +def _free_port(): + """Return a port nothing is listening on, for the control server above.""" + with socket.socket() as sock: + sock.bind(('127.0.0.1', 0)) + return sock.getsockname()[1] + + +def _wait_listening(port, timeout=60.0): + """Block until the port accepts a TCP connection. + + launch_testing starts the tests as soon as the processes are spawned, not + when they are serving, and a gateway that is not listening yet refuses + every connection. That looks identical to "the server rejected this + handshake", so without this gate the refusal assertions pass for the wrong + reason and the acceptance assertions fail at random. Observed directly: + the same file reported two failures, then two, then one, across three runs. + + TCP only, deliberately. A TLS handshake cannot be the readiness probe here + because on the mutual-TLS gateway a probe without a client certificate is + supposed to fail. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + try: + with socket.create_connection(('127.0.0.1', port), timeout=2): + return + except OSError: + time.sleep(0.25) + raise AssertionError(f'gateway on port {port} never started listening within {timeout}s') + + +class TestTlsProtocolFloor(unittest.TestCase): + """The floor moves with min_version, and it is the server that enforces it.""" + + @classmethod + def setUpClass(cls): + _wait_listening(PORT_TLS12) + _wait_listening(PORT_TLS13) + + def test_01_floor_12_accepts_12_and_13(self): + """The mirror of the refusals below. + + Without this, a gateway that refused every version would pass the + whole file while serving nobody. + """ + self.assertTrue(_handshake(PORT_TLS12, 'tls1_2'), 'TLS 1.2 must be accepted at floor 1.2') + self.assertTrue(_handshake(PORT_TLS12, 'tls1_3'), 'TLS 1.3 must be accepted at floor 1.2') + + def test_01b_the_client_actually_offers_the_old_versions(self): + """Guard the negative assertions below against becoming vacuous. + + `_handshake` returns False both when the SERVER refuses and when the + client never put a ClientHello on the wire. A modern OpenSSL will not + offer TLS 1.0/1.1 unless `-cipher ALL:@SECLEVEL=0` persuades it, and on + a distro built `no-tls1 no-tls1_1`, or under a crypto policy pinning + MinProtocol, it cannot offer them at all. In either case test_02 below + would pass against a gateway happily serving TLS 1.0. + + So: stand up a plain `openssl s_server` that accepts everything, and + require the client to reach 1.0 and 1.1 against it. If it cannot, the + refusals in test_02 prove nothing and this fails instead of lying. + """ + for version in ('tls1', 'tls1_1'): + with self.subTest(version=version): + port = _free_port() + server = subprocess.Popen( + ['openssl', 's_server', '-accept', str(port), '-quiet', + '-cert', SRV_CRT, '-key', SRV_KEY, + '-cipher', 'ALL:@SECLEVEL=0', f'-{version}'], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + ) + try: + _wait_listening(port, timeout=15) + self.assertTrue( + _handshake(port, version), + f'this client cannot offer {version} at all, so the ' + f'{version} refusals in test_02 would pass against a ' + 'gateway that accepts it' + ) + finally: + server.terminate() + server.wait(timeout=10) + + def test_02_floor_12_refuses_11_and_10(self): + """SOVD requires TLS 1.2 as the minimum, so 1.1 and 1.0 must not connect. + + The vendored cpp-httplib asks OpenSSL for a floor of TLS 1.1 + (SSL_CTX_set_min_proto_version(ctx_, TLS1_1_VERSION)), so without the + gateway setting its own floor this is the version that decides whether + we comply, and it is not a value this project chose. + """ + self.assertFalse(_handshake(PORT_TLS12, 'tls1_1'), 'TLS 1.1 must be refused at floor 1.2') + self.assertFalse(_handshake(PORT_TLS12, 'tls1'), 'TLS 1.0 must be refused at floor 1.2') + + def test_03_floor_13_refuses_12(self): + """The test that fails if min_version is inert. + + A gateway configured for 1.3 that still completes a 1.2 handshake is + exactly the state this branch shipped before: the value was read, + logged, and then ignored. TLS 1.2 is accepted by the OTHER gateway in + this same launch, so a failure here cannot be blamed on the client or + on the certificate. + """ + self.assertFalse(_handshake(PORT_TLS13, 'tls1_2'), 'TLS 1.2 must be refused at floor 1.3') + self.assertFalse(_handshake(PORT_TLS13, 'tls1_1'), 'TLS 1.1 must be refused at floor 1.3') + + def test_04_floor_13_accepts_13(self): + self.assertTrue(_handshake(PORT_TLS13, 'tls1_3'), 'TLS 1.3 must be accepted at floor 1.3') + + +class TestMutualTls(unittest.TestCase): + """With ca_file set, a client certificate is required and verified.""" + + @classmethod + def setUpClass(cls): + _wait_listening(PORT_MTLS) + _wait_listening(PORT_TLS12) + + def test_05_no_client_certificate_is_refused(self): + """ca_file set means SSL_VERIFY_FAIL_IF_NO_PEER_CERT: no cert, no session.""" + self.assertFalse( + _handshake(PORT_MTLS, 'tls1_2'), + 'a client presenting no certificate must not complete the handshake' + ) + + def test_06_a_certificate_from_the_configured_ca_is_accepted(self): + self.assertTrue( + _handshake(PORT_MTLS, 'tls1_2', client_cert=CLI_CRT, client_key=CLI_KEY), + 'a client certificate signed by the configured CA must be accepted' + ) + + def test_07_a_certificate_from_another_ca_is_refused(self): + """Well-formed and correctly signed, but by an authority we never trusted. + + This separates "verification is on" from "any certificate will do", + which test_05 alone cannot. + """ + self.assertFalse( + _handshake(PORT_MTLS, 'tls1_2', client_cert=ROGUE_CLI_CRT, client_key=ROGUE_CLI_KEY), + 'a client certificate from an unconfigured CA must be refused' + ) + + def test_08_a_gateway_without_ca_file_does_not_demand_one(self): + """The default stays server-only TLS. + + SOVD authenticates with bearer tokens, so requiring a client + certificate by default would put us outside the spec. mTLS is opt-in + and this pins that it is. + """ + self.assertTrue( + _handshake(PORT_TLS12, 'tls1_2'), + 'a gateway with no ca_file must still serve a client that has no certificate' + ) + + +@launch_testing.post_shutdown_test() +class TestTlsFloorShutdown(unittest.TestCase): + """All three gateways exit cleanly.""" + + def test_exit_codes(self, proc_info, gateway_tls12, gateway_tls13, gateway_mtls): + for proc in (gateway_tls12, gateway_tls13, gateway_mtls): + launch_testing.asserts.assertExitCodes( + proc_info, allowable_exit_codes=ALLOWED_EXIT_CODES, process=proc) + + @classmethod + def tearDownClass(cls): + shutil.rmtree(_CERT_DIR, ignore_errors=True)