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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions doc/getting_started.rst
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,26 @@ Follow these instructions to install the systemd files on your machine(s):

# usermod -a -G labgrid <user>

Enabling gRPC connection security
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It is encouraged to use TLS for gRPC channels in a production environment.

This can be enabled on the ``labgrid-coordinator`` by adding the ``--tls``,
``--cert`` and ``--key`` options.
Refer to the ``labgrid-coordinator`` man page for details.

When you are connecting with ``labgrid-client`` or ``labgrid-exporter`` to a
``labgrid-coordinator`` that has TLS gRPC channels enabled you need to pass
the ``--tls`` option. If ``--cert`` is not set, labgrid uses the host CA
certificates to verify the coordinator certificate. Use ``--cert`` to provide
a specific CA certificate instead.
Refer to the ``labgrid-client`` and ``labgrid-exporter`` man pages for details.
For ``RemotePlace`` connections from an environment config, set the
``coordinator_tls`` option or ``LG_COORDINATOR_TLS``. If ``coordinator_cert``
is not set, labgrid uses the host CA certificates. Set ``coordinator_cert``
to provide a specific CA certificate instead.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explain roots.pem and GRPC_DEFAULT_SSL_ROOTS_FILE_PATH precedence here, please

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Using a Strategy
----------------

Expand Down
11 changes: 11 additions & 0 deletions doc/man/client.rst
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ LG_COORDINATOR
This variable can be used to set the default coordinator in the format
``HOST[:PORT]`` (instead of using the ``-x`` option).

LG_COORDINATOR_TLS
~~~~~~~~~~~~~~~~~~
This variable can be set to enable TLS gRPC channels without using the
``--tls`` option.

LG_PROXY
~~~~~~~~
This variable can be used to specify a SSH proxy hostname which should be used
Expand Down Expand Up @@ -128,6 +133,12 @@ Add all resources with the group "example-group" to the place example-place:

$ labgrid-client -p example-place add-match */example-group/*/*

Retrieve a list of places on a ``labgrid-coordinator`` that uses TLS gRPC channels:

.. code-block:: bash

$ labgrid-client --tls [--cert PATH] places

See Also
--------

Expand Down
32 changes: 32 additions & 0 deletions doc/man/coordinator.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,41 @@ OPTIONS
display command line help
-l ADDRESS, --listen ADDRESS
make coordinator listen on host and port
--tls
enable TLS gRPC channel
--cert
path to TLS certificate (in PEM format)
--key
path to TLS key (in PEM format)
-d, --debug
enable debug mode

TLS WITH A REVERSE PROXY
------------------------

Instead of enabling TLS in ``labgrid-coordinator`` directly, a reverse proxy can
terminate TLS and forward cleartext gRPC to the coordinator. For example, with
``nginx``:

.. code-block:: nginx
server {
listen 20407 ssl http2;
server_name labgrid.example.com;
ssl_certificate /etc/ssl/labgrid-coordinator.crt;
ssl_certificate_key /etc/ssl/labgrid-coordinator.key;
location / {
grpc_pass grpc://127.0.0.1:20408;
}
}
In this setup, start ``labgrid-coordinator`` without ``--tls`` and point
``labgrid-client`` and ``labgrid-exporter`` at the reverse proxy using
``--tls``.


SEE ALSO
--------

Expand Down
9 changes: 9 additions & 0 deletions doc/man/device-config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,15 @@ OPTIONS KEYS
takes as parameter the coordinator ``HOST[:PORT]`` to connect to.
Defaults to ``127.0.0.1:20408``.

``coordinator_tls``
enables a TLS gRPC channel for ``RemotePlace`` connections.
Defaults to whether ``LG_COORDINATOR_TLS`` is set.

``coordinator_cert``
path to the coordinator TLS certificate in PEM format for ``RemotePlace``
connections. If unset, the host CA certificates are used. Relative paths are
resolved relative to the configuration file.

.. _labgrid-device-config-images:

IMAGES
Expand Down
10 changes: 10 additions & 0 deletions doc/man/exporter.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ OPTIONS
display command line help
-x, --coordinator
coordinator ``HOST[:PORT]`` to connect to, defaults to ``127.0.0.1:20408``
--tls
enable TLS gRPC channel
--cert
path to TLS certificate (in PEM format)
-i, --isolated
enable isolated mode (always request SSH forwards)
-n, --name
Expand Down Expand Up @@ -99,6 +103,12 @@ Same as above, but with name ``myname``:

$ labgrid-exporter -n myname my-config.yaml

Same as above, but connecting to a ``labgrid-coordinator`` that uses TLS gRPC channels:

.. code-block:: bash

$ labgrid-exporter --tls [--cert PATH] -n myname my-config.yaml

SEE ALSO
--------

Expand Down
38 changes: 31 additions & 7 deletions labgrid/remote/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
TAG_KEY,
TAG_VAL,
queue_as_aiter,
get_client_credentials,
)
from .. import Environment, Target, target_factory
from ..exceptions import NoDriverFoundError, NoResourceFoundError, InvalidConfigError
Expand Down Expand Up @@ -92,6 +93,7 @@ class ClientSession:
the coordinator."""

address = attr.ib(validator=attr.validators.instance_of(str))
credentials = attr.ib(validator=attr.validators.optional(attr.validators.instance_of(grpc.ChannelCredentials)))
loop = attr.ib(validator=attr.validators.instance_of(asyncio.BaseEventLoop))
env = attr.ib(default=None, validator=attr.validators.optional(attr.validators.instance_of(Environment)))
role = attr.ib(default=None, validator=attr.validators.optional(attr.validators.instance_of(str)))
Expand Down Expand Up @@ -120,10 +122,18 @@ def __attrs_post_init__(self):
("grpc.http2.max_pings_without_data", 0), # no limit
]

self.channel = grpc.aio.insecure_channel(
target=self.address,
options=channel_options,
)
if self.credentials:
self.channel = grpc.aio.secure_channel(
target=self.address,
credentials=self.credentials,
options=channel_options,
)
else:
self.channel = grpc.aio.insecure_channel(
target=self.address,
options=channel_options,
)

self.stub = labgrid_coordinator_pb2_grpc.CoordinatorStub(self.channel)

self.out_queue = asyncio.Queue()
Expand Down Expand Up @@ -1698,7 +1708,12 @@ def ensure_event_loop(external_loop=None):


def start_session(
address: str, *, extra: Dict[str, Any] = None, debug: bool = False, loop: "asyncio.AbstractEventLoop | None" = None
address: str,
*,
extra: Dict[str, Any] = None,
credentials: grpc.ChannelCredentials = None,
debug: bool = False,
loop: "asyncio.AbstractEventLoop | None" = None,
):
"""
Starts a ClientSession.
Expand All @@ -1720,7 +1735,7 @@ def start_session(

address = proxymanager.get_grpc_address(address, default_port=20408)

session = ClientSession(address, loop, **extra)
session = ClientSession(address, credentials, loop, **extra)
loop.run_until_complete(session.start())
return session

Expand Down Expand Up @@ -1848,6 +1863,13 @@ def get_parser(auto_doc_mode=False) -> "argparse.ArgumentParser | AutoProgramArg
type=str,
help="coordinator HOST[:PORT] (default: value from env variable LG_COORDINATOR, otherwise 127.0.0.1:20408)",
)
parser.add_argument(
"--tls",
action="store_true",
default=os.environ.get("LG_COORDINATOR_TLS") is not None,
help="enable TLS gRPC channel",
)
parser.add_argument("--cert", type=pathlib.PurePath, help="path to the server's TLS certificate (in PEM format)")
parser.add_argument(
"-c",
"--config",
Expand Down Expand Up @@ -2346,7 +2368,9 @@ def main():
logging.debug('Starting session with "%s"', coordinator_address)
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
session = start_session(coordinator_address, extra=extra, debug=args.debug, loop=loop)
session = start_session(
coordinator_address, extra=extra, credentials=get_client_credentials(args), debug=args.debug, loop=loop
)
logging.debug("Started session")

try:
Expand Down
59 changes: 58 additions & 1 deletion labgrid/remote/common.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
import asyncio
import platform
import subprocess
import time
import enum
import random
import re
import string
import logging
from argparse import Namespace
from datetime import datetime
from fnmatch import fnmatchcase

from typing import Optional
import attr
import grpc

from .generated import labgrid_coordinator_pb2

Expand Down Expand Up @@ -58,6 +62,59 @@ def build_dict_from_map(m):
return d


def _fetch_root_certificates_darwin():
try:
p = subprocess.run(
["security", "find-certificate", "-a", "-p"],
capture_output=True,
timeout=10,
)
if p.returncode != 0 or not p.stdout:
return None
return p.stdout
except Exception:
logging.exception("unexpected error when fetching certificates from macOS Keychain")

return None


def _fetch_root_certificates_linux():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On why this is needed.

the certificate precedence is (src/core/credentials/transport/tls/ssl_utils.cc @ ComputePemRootCerts)

GRPC_DEFAULT_SSL_ROOTS_FILE_PATH
UseSystemRootsOverLanguageCallback()
ssl_roots_override_cb <= never reached
LoadSystemRootCerts()
installed_roots_path fallback

the cb is set at src/python/grpcio/grpc/_cython/cygrpc.pyx
added at github.com/grpc/grpc/commit/fa6cad701c7993aa6e5746824931efbfca84ca24

The only options are indeed GRPC_DEFAULT_SSL_ROOTS_FILE_PATH or as the argument value.

the grpc defaults for linux are (https://github.com/grpc/grpc/blob/master/src/core/credentials/transport/tls/load_system_roots_supported.cc#L48-L62)

    "/etc/ssl/certs/ca-certificates.crt", "/etc/pki/tls/certs/ca-bundle.crt",
    "/etc/ssl/ca-bundle.pem", "/etc/pki/tls/cacert.pem",
    "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem"

I wouldn't have one of the default here, but just document that it will default to the bundled roots.pem if not explicitly set through the env or as the argument value.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gastmaier I think the important Python bit here is that grpcio installs a roots override callback which loads its bundled roots.pem, so on Python the normal default is not the Linux system trust store. That means if we do not pass root_certificates, --tls may ignore CAs installed on the host.

So I think the correct ordering for Labgrid should be:

  • explicit --cert
  • system trust store loaded by Labgrid
  • gRPC/Python default only as fallback if we cannot load system roots

I'll update the getting_started docs to represent this

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--tls may ignore CAs installed on the host.
Due to how the gprc source code is structured, it will ignore CAs installed on the host (never read) if roots.pem is present.
and roots.pem is always present with the pip package install:

find . -name 'roots.pem'
./lib/python3.14/site-packages/grpc/_cython/_credentials/roots.pem

Hacking rm ./lib/python3.14/site-packages/grpc/_cython/_credentials/roots.pem as a post-install step for grpcio is not an option.

ca_bundle_path = "/etc/ssl/certs/ca-certificates.crt"
try:
# TODO: Current supports Debian/Ubuntu. Extend to support other distributions.
with open(ca_bundle_path, "rb") as f:
certs = f.read()
if certs:
return certs
except OSError as e:
logging.warning("failed to read CA bundle at %s: %s", ca_bundle_path, e)
except Exception:
logging.exception("unexpected error while reading ca certificates")

return None


def _fetch_root_certificates():
if platform.system() == "Darwin":
return _fetch_root_certificates_darwin()

if platform.system() == "Linux":
return _fetch_root_certificates_linux()

return None


def get_client_credentials(args: Namespace) -> Optional[grpc.ChannelCredentials]:
if not args.tls:
return None

if not args.cert:
return grpc.ssl_channel_credentials(root_certificates=_fetch_root_certificates())

with open(args.cert, "rb") as fc:
return grpc.ssl_channel_credentials(root_certificates=fc.read())


@attr.s(eq=False)
class ResourceEntry:
data = attr.ib() # cls, params
Expand Down
29 changes: 25 additions & 4 deletions labgrid/remote/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
import copy
import random
import signal

import pathlib
from typing import Optional
import attr
import grpc
from grpc_reflection.v1alpha import reflection
Expand Down Expand Up @@ -1109,7 +1110,7 @@ async def GetReservations(self, request: labgrid_coordinator_pb2.GetReservations
return labgrid_coordinator_pb2.GetReservationsResponse(reservations=reservations)


async def serve(listen, cleanup) -> None:
async def serve(listen, cleanup, server_credentials=None) -> None:
asyncio.current_task().set_name("coordinator-serve")
# It seems since https://github.com/grpc/grpc/pull/34647, the
# ping_timeout_ms default of 60 seconds overrides keepalive_timeout_ms,
Expand Down Expand Up @@ -1146,7 +1147,11 @@ async def serve(listen, cleanup) -> None:
except ImportError:
logging.info("Module grpcio-channelz not available")

bound = server.add_insecure_port(listen)
if server_credentials:
bound = server.add_secure_port(listen, server_credentials)
else:
bound = server.add_insecure_port(listen)

logging.debug("Starting server")
await server.start()

Expand Down Expand Up @@ -1175,6 +1180,18 @@ def callback():
await server.wait_for_termination()


def get_server_credentials(args: argparse.Namespace) -> Optional[grpc.ServerCredentials]:
if not args.tls:
return None

if not args.cert or not args.key:
raise RuntimeError("--cert and --key must be provided when --tls is provided")

with open(args.key, "rb") as fk:
with open(args.cert, "rb") as fc:
return grpc.ssl_server_credentials([(fk.read(), fc.read())])


def main():
parser = argparse.ArgumentParser()
parser.add_argument(
Expand All @@ -1185,6 +1202,9 @@ def main():
default="[::]:20408",
help="coordinator listening host and port",
)
parser.add_argument("--tls", action="store_true", default=False, help="enable TLS gRPC channel")
parser.add_argument("--cert", type=pathlib.PurePath, help="path to TLS certificate (in PEM format)")
parser.add_argument("--key", type=pathlib.PurePath, help="path to TLS key (in PEM format)")
parser.add_argument("-d", "--debug", action="store_true", default=False, help="enable debug mode")
parser.add_argument("--pystuck", action="store_true", help="enable pystuck")
parser.add_argument(
Expand Down Expand Up @@ -1214,7 +1234,8 @@ def main():
cleanup = []
loop.set_debug(True)
try:
loop.run_until_complete(serve(args.listen, cleanup))
server_credentials = get_server_credentials(args)
loop.run_until_complete(serve(args.listen, cleanup, server_credentials))
finally:
if cleanup:
loop.run_until_complete(*cleanup)
Expand Down
Loading
Loading