From 8a9c3f1d52d2f4adf831401f1ea89633370d72ef Mon Sep 17 00:00:00 2001 From: Willi Mann Date: Fri, 10 Jul 2026 09:56:35 +0200 Subject: [PATCH 1/2] feat: support remote compilation over SSH tunnels Add SSH as a first-class connection type alongside TCP. SSH hosts (@HOST / USER@HOST) are served by tunneling to an already running homccd: a multiplexed OpenSSH master (ControlMaster/ControlPersist) local-forwards a port to the daemon's loopback port, and the existing protocol runs over that port. All jobs reach the same daemon, so its global connection limit covers TCP and SSH alike, and per-job latency stays close to a plain TCP connect. - Extract transport-agnostic RemoteCompilationClient base from TCPClient; add SSHClient and SSHTunnel (client/ssh.py) - Dispatch on host.type via create_remote_client factory - Parse optional remote daemon port for SSH hosts (@host:port, user@host:port, @[ipv6]:port) - Add SSHError(ConnectionError) so tunnel failures fall through to the next host / local fallback - Wire ssh_executable / ssh_control_persist / ssh_options config - Serialize per-host master setup with an flock so concurrent homcc processes converge on one tunnel - Tests: tunnel logic, dispatch, SSH port parsing, config, and an opt-in e2e test behind --runssh; document in README and CLAUDE.md Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 97 +++++++++++++++++ README.md | 20 +++- homcc/client/client.py | 71 ++++++++---- homcc/client/compilation.py | 33 ++++-- homcc/client/config.py | 47 ++++++++ homcc/client/ssh.py | 205 +++++++++++++++++++++++++++++++++++ homcc/common/errors.py | 8 ++ homcc/common/host.py | 14 ++- tests/client/parsing_test.py | 28 +++++ tests/client/ssh_test.py | 145 +++++++++++++++++++++++++ tests/conftest.py | 14 +++ tests/e2e/e2e_test.py | 13 ++- 12 files changed, 659 insertions(+), 36 deletions(-) create mode 100644 CLAUDE.md create mode 100644 homcc/client/ssh.py create mode 100644 tests/client/ssh_test.py diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..85e526d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,97 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Overview + +`HOMCC` is a work-from-home friendly `distcc` replacement: it distributes `C`/`C++` compilation (`gcc`/`clang`) to remote servers while guaranteeing the same result as a local build. Its distinguishing feature over `distcc` is aggressive optimization for thin network uplinks: dependencies are compressed and **server-side cached by SHA1 hash**, so a warmed-up cache only re-transmits missing dependencies. + +The project ships two executables plus an optional GUI: +- `homcc` — the client (entry point `homcc.client.main:main`) +- `homccd` — the server daemon (entry point `homcc.server.main:main`) +- `homcc-monitor` — a PySide2 GUI that watches client state files (entry point `homcc.monitor.main:main`) + +## Commands + +All linting/testing runs against `*.py homcc tests` from the repo root. + +```sh +# Install dev dependencies (requires liblzo2-dev liblzma-dev apt packages) +python -m pip install -r requirements.txt + +# Run all tests with coverage +pytest -v -rfEs --cov=homcc + +# Run a single test file / test +pytest -v tests/client/client_test.py +pytest -v tests/client/client_test.py::TestClass::test_name + +# Tests that exercise sandboxed compilation (opt-in, need a configured environment) +pytest -v -rfEs --cov=homcc --runschroot=jammy +pytest -v -rfEs --cov=homcc --rundocker=jammy + +# Lint + static typing (both run in CI and must pass) +pylint -v --rcfile=.pylintrc *.py homcc tests +mypy --pretty *.py homcc tests + +# Format (line length 120, black skips string normalization) +black --check --color --diff --verbose *.py homcc tests +isort --check --color --diff --gitignore --verbose *.py homcc tests + +# Build Debian packages (needs stdeb toolchain; run as root) +sudo make homcc # -> target/homcc.deb +sudo make homccd # -> target/homccd.deb +``` + +CI (`.github/workflows/`) runs three jobs: linters (pylint + mypy), format check (black + isort), and tests (pytest). Match all three locally before pushing. + +## Architecture + +The codebase is three packages sharing a common protocol layer: + +- **`homcc/common/`** — shared by client and server. This is the contract between them; changes here usually require touching both sides. +- **`homcc/client/`** — invoked once per compilation (as `CCACHE_PREFIX=homcc`), selects a remote host, drives the protocol, falls back to local compilation. +- **`homcc/server/`** — long-lived `socketserver`-based daemon that receives requests, resolves dependencies against its cache, and compiles inside a sandbox. +- **`homcc/monitor/`** — read-only GUI; independent of the compile path. + +### The wire protocol (`common/messages.py`) + +All client/server communication is a stream of length-prefixed `Message` subclasses (JSON header + optional binary payload), serialized via `Message.to_bytes()` / `Message.from_bytes()`. The compilation handshake: + +1. Client sends `ArgumentMessage` (compiler args, cwd, target, sandbox profile, compression, and a `{path: sha1sum}` map of all dependencies). +2. Server checks each hash against its `Cache`; for cache misses it sends `DependencyRequestMessage`, and the client replies with `DependencyReplyMessage` (compressed file bytes) until all dependencies are present. +3. Server compiles and returns a `CompilationResultMessage` (object files + stdout/stderr/return code), or a `ConnectionRefusedMessage` if it is at capacity. + +When adding a message type, register it in `MessageType` and `Message._parse_message_json`. + +### Client compilation flow (`client/compilation.py`) + +`compile_remotely` is the orchestrator. Key invariants: +- **Local fallback is the norm, not an error**: on most failures the client compiles locally so builds never break (unless `--no-local-compilation` is set). Preprocessing (`_preprocess`) and final linking (`execute_linking`) always happen locally; only the compile step is distributed. +- **Recursion guard**: because homcc invokes a real compiler that might itself be homcc-wrapped, `main.py` sets the `_HOMCC_SAFEGUARD` env var and children detect it (`RECURSIVE_ERROR_MESSAGE`) to abort. +- **Host selection** (`client/client.py`): `RemoteHostSelector` picks hosts randomly weighted by their `LIMIT`. Concurrency is bounded by **SysV semaphores** (`sysv_ipc`) — `RemoteHostSemaphore`, `LocalHostCompilationSemaphore`, `LocalHostPreprocessingSemaphore`. Debug open semaphores with `ipcs -s`. +- Hosts come from `$HOMCC_HOSTS` or a `hosts` file. Parsing lives in `common/host.py` (`_parse_host`); formats are `HOST[:PORT][/LIMIT][,COMPRESSION]` for TCP and `@HOST[:PORT]` / `USER@HOST[:PORT]` (`[IPv6]` bracketed) for SSH. + +### Transports (`client/client.py`, `client/ssh.py`) + +The protocol is transport-agnostic: `RemoteCompilationClient` (abstract, in `client/client.py`) holds all message framing/send/receive logic and only defers `_open_connection` to subclasses. `TCPClient` opens a direct connection to `host:port`; `SSHClient` (`client/ssh.py`) connects to `127.0.0.1:`. `create_remote_client` in `compilation.py` dispatches on `host.type`. When adding a transport, subclass `RemoteCompilationClient` and set `connection_target`. + +SSH is a **tunnel to a running `homccd`**, not a per-job remote process: `SSHTunnel` establishes a multiplexed OpenSSH master (`ControlMaster`/`ControlPersist`) that local-forwards a port to the daemon's loopback port, so all jobs (TCP and SSH alike) hit the same daemon and its global connection limit, and per-job latency is ~a local TCP connect. A per-host `flock` under `$HOMCC_DIR/ssh/` serializes master setup across the many concurrent `homcc` processes a build spawns. `SSHError` subclasses `ConnectionError` so tunnel failures fall through to the next host / local fallback like any lost connection. **No server changes** are needed for SSH. + +### `Arguments` (`common/arguments.py`) + +The central abstraction for a compiler command line. It parses/normalizes args, distinguishes source files, dependencies, output paths, and compiler capabilities. Both client (to build the request) and server (to reconstruct the command in the sandbox) depend on it, so its behavior must stay symmetric across the wire. + +### Server sandboxing (`server/environment.py`, `docker.py`, `schroot.py`) + +Each request runs in a fresh `Environment` under a temp dir in `/tmp/`, with client paths remapped into a server-local `mapped_cwd`. Compilation executes through a `ShellEnvironment` strategy: `HostShellEnvironment` (bare), `DockerShellEnvironment` (`--docker-container`), or `SchrootShellEnvironment` (`--schroot-profile`). The `Cache` (`server/cache.py`) is a hash-addressed, LRU-evicted dependency store bounded by `--max-cache-size`. + +### State files & monitoring (`common/statefile.py`) + +Each in-flight client compilation writes a binary `StateFile` (phases: STARTUP → CONNECT → PREPROCESSING → COMPILE, adapted from distcc's format for tooling compatibility) into `HOMCC_STATE_DIR`. The monitor GUI uses `watchdog` to observe this directory. This is purely observational and decoupled from the compile path. + +## Configuration & packaging notes + +- `setup.py` is **generated** at package-build time by `make` copying `setup_client.py` or `setup_server.py` over it — do not hand-edit `setup.py`; edit the `setup_client.py` / `setup_server.py` sources. Version is read from `homcc/{client,server}/__init__.py` `__version__`. +- Config resolution order and CLI/env/file precedence live in the `parsing.py` modules (`client/parsing.py`, `server/parsing.py`, `common/parsing.py`). Client and server each have a config dataclass (`ClientConfig`, `ServerConfig`). +- Compression (`common/compression.py`) supports `lzo` (python-lzo, needs `liblzo2-dev`) and `lzma`; default is none. diff --git a/README.md b/README.md index a68d9b6..ace4415 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,11 @@ Additionally, `HOMCC` provides sandboxed compiler execution for remote compilati - `HOST` format: - `HOST`: TCP connection to specified `HOST` with default port `3126` - `HOST:PORT`: TCP connection to specified `HOST` with specified `PORT` + - `@HOST` / `USER@HOST` format: + - Connect to `HOST` through an SSH tunnel instead of a plain TCP connection, optionally authenticating as `USER` + - `homcc` establishes a multiplexed SSH master connection to `HOST` and local-forwards a port to the `homccd` running there; the daemon only needs to listen on the remote loopback interface (e.g. `--listen=127.0.0.1`). Subsequent compilations reuse the master connection (OpenSSH `ControlMaster`/`ControlPersist`), so the per-compilation cost stays close to a plain TCP connection + - The daemon's port on the remote defaults to `3126` and can be overridden with `@HOST:PORT` (use `@[IPv6]:PORT` for IPv6 addresses) + - Authentication relies on your existing SSH setup (keys/agent); `homcc` does not manage SSH credentials itself - `HOST/LIMIT` format: - Define any of the above `HOST` formats with an additional `LIMIT` parameter that specifies the maximum connection limit to the corresponding `HOST` - It is advised to always specify your `LIMIT`s as they will otherwise default to 2 and only enable minor levels of concurrency @@ -93,6 +98,8 @@ Additionally, `HOMCC` provides sandboxed compiler execution for remote compilati remotehost/12 192.168.0.1:3126/21 [FC00::1]:3126/42,lzo + @buildhost/24,lzo + user@buildhost:3126/24 
     # Comment
@@ -100,6 +107,8 @@ Additionally, `HOMCC` provides sandboxed compiler execution for remote compilati
     Named "remotehost" TCP host with limit of 12 at default port 3126
     IPv4 "192.168.0.1" TCP host at port 3126 with limit of 21
     IPv6 "FC00::1" TCP host at port 3126 with limit of 42 and lzo compression
+    "buildhost" via SSH tunnel with limit of 24 and lzo compression
+    "buildhost" via SSH tunnel as user "user", daemon port 3126, limit of 24
@@ -158,6 +167,9 @@ Additionally, `HOMCC` provides sandboxed compiler execution for remote compilati HOMCC_LOG_LEVEL HOMCC_VERBOSE HOMCC_NO_LOCAL_COMPILATION + HOMCC_SSH_EXECUTABLE + HOMCC_SSH_CONTROL_PERSIST + HOMCC_SSH_OPTIONS   # homccd HOMCCD_LIMIT @@ -179,6 +191,9 @@ Additionally, `HOMCC` provides sandboxed compiler execution for remote compilati log_level=DEBUG verbose=True no_local_compilation=True + ssh_executable=ssh + ssh_control_persist=600 + ssh_options=-o BatchMode=yes   [homccd] limit=64 @@ -199,6 +214,9 @@ Additionally, `HOMCC` provides sandboxed compiler execution for remote compilati Detail level for log messages: {DEBUG, INFO, WARNING, ERROR, CRITICAL} Enable verbosity mode which implies detailed and colored logging Enforce that even on recoverable failures no local compilation is executed + Executable used to establish SSH tunnels for '@HOST'/'USER@HOST' hosts + Seconds an idle multiplexed SSH master connection is kept alive for reuse + Additional options passed to the SSH executable, e.g. '-o' flags   # Server configuration Maximum limit of concurrent compilations @@ -213,7 +231,7 @@ Additionally, `HOMCC` provides sandboxed compiler execution for remote compilati ## Deployment hints Things to keep in mind when deploying `homccd`: -- `homcc` currently does not support any transport encryption such as TLS, so source files would get transmitted over the internet in plain text if not using a VPN. +- `homcc` does not support built-in transport encryption such as TLS: plain TCP hosts transmit source files unencrypted, so a VPN is required over untrusted networks. Alternatively, use an SSH host (`@HOST`/`USER@HOST`) to tunnel the connection through an encrypted, authenticated SSH channel to a `homccd` bound to the remote loopback interface. - `homccd` does not limit simultaneous connections of a single client. A malicious client could therefore block the service by always opening up connections until no server slots are available any more. - `homccd` does not limit access to docker containers or chroot environments. A client can choose any docker container or chroot environment available on the server to execute the compilation in. diff --git a/homcc/client/client.py b/homcc/client/client.py index f68e2ec..464b007 100644 --- a/homcc/client/client.py +++ b/homcc/client/client.py @@ -17,11 +17,12 @@ import types from abc import ABC, abstractmethod from pathlib import Path -from typing import ClassVar, Dict, Iterator, List, Optional +from typing import ClassVar, Dict, Iterator, List, Optional, Tuple import sysv_ipc from homcc.common.arguments import Arguments +from homcc.common.compression import Compression from homcc.common.constants import TCP_BUFFER_SIZE from homcc.common.errors import ( ClientParsingError, @@ -264,17 +265,25 @@ def __init__(self, host: Host, expected_preprocessing_time: float = DEFAULT_EXPE super().__init__(host, expected_preprocessing_time) -class TCPClient: - """Wrapper class to exchange homcc protocol messages via TCP""" +class RemoteCompilationClient(ABC): + """ + Transport-agnostic base class to exchange homcc protocol messages with a remote server. - def __init__(self, host: Host, timeout: float, state: StateFile): - connection_type: ConnectionType = host.type + All protocol logic (message framing, sending arguments and dependencies, receiving replies) lives here and operates + on an asyncio stream reader/writer pair. Concrete subclasses only have to establish the transport-specific + connection by implementing `_open_connection`, e.g. a plain TCP connection (`TCPClient`) or a connection tunneled + through SSH (`SSHClient`). + """ - if connection_type != ConnectionType.TCP: - raise ValueError(f"TCPClient cannot be initialized with {connection_type}!") + host: str + """Name of the remote host, used for logging and host name resolution errors.""" + connection_target: str + """Human-readable connection target for logging and error messages, e.g. 'host:port' or 'user@host'.""" + compression: Compression + """Compression used for data transfer.""" - self.host: str = host.name - self.port: int = host.port + def __init__(self, host: Host, timeout: float, state: StateFile): + self.host = host.name self.compression = host.compression self.timeout: float = timeout @@ -285,16 +294,17 @@ def __init__(self, host: Host, timeout: float, state: StateFile): state.set_connect() - async def __aenter__(self) -> TCPClient: - """connect to specified server at host:port""" - logger.debug("Connecting to '%s:%i'.", self.host, self.port) + @abstractmethod + async def _open_connection(self) -> Tuple[asyncio.StreamReader, asyncio.StreamWriter]: + """Establish the transport-specific asyncio connection to the remote server.""" + + async def __aenter__(self) -> RemoteCompilationClient: + """connect to the specified server""" + logger.debug("Connecting to '%s'.", self.connection_target) try: - self._reader, self._writer = await asyncio.wait_for( - asyncio.open_connection(host=self.host, port=self.port, limit=TCP_BUFFER_SIZE), - timeout=self.timeout, - ) + self._reader, self._writer = await asyncio.wait_for(self._open_connection(), timeout=self.timeout) except asyncio.TimeoutError as error: - logger.warning("Connection establishment to '%s:%s' timed out.", self.host, self.port) + logger.warning("Connection establishment to '%s' timed out.", self.connection_target) raise error from None except socket.gaierror as error: raise FailedHostNameResolutionError(f"Host {self.host} could not be resolved.") from error @@ -302,7 +312,7 @@ async def __aenter__(self) -> TCPClient: async def __aexit__(self, *_): """disconnect from server and close client socket""" - logger.debug("Disconnecting from '%s:%i'.", self.host, self.port) + logger.debug("Disconnecting from '%s'.", self.connection_target) self._writer.close() try: @@ -313,7 +323,7 @@ async def __aexit__(self, *_): async def _send(self, message: Message): """send a message to homcc server""" - logger.debug("Sending %s to '%s:%i':\n%s", message.message_type, self.host, self.port, message.get_json_str()) + logger.debug("Sending %s to '%s':\n%s", message.message_type, self.connection_target, message.get_json_str()) self._writer.write(message.to_bytes()) await self._writer.drain() @@ -350,7 +360,7 @@ async def send_argument_message( error, ) raise HostRefusedConnectionError( - f"Host {self.host}:{self.port} closed the connection, probably due to " + f"Host {self.connection_target} closed the connection, probably due to " "reaching the compilation limit." ) from error @@ -390,10 +400,25 @@ async def receive(self) -> Message: raise ClientParsingError("Received data could not be parsed to a message!") logger.debug( - "Received %s message from '%s:%i':\n%s", + "Received %s message from '%s':\n%s", parsed_message.message_type, - self.host, - self.port, + self.connection_target, parsed_message.get_json_str(), ) return parsed_message + + +class TCPClient(RemoteCompilationClient): + """Client to exchange homcc protocol messages with a remote server via a direct TCP connection.""" + + def __init__(self, host: Host, timeout: float, state: StateFile): + if host.type != ConnectionType.TCP: + raise ValueError(f"TCPClient cannot be initialized with {host.type}!") + + super().__init__(host, timeout, state) + + self.port: int = host.port + self.connection_target = f"{host.name}:{host.port}" + + async def _open_connection(self) -> Tuple[asyncio.StreamReader, asyncio.StreamWriter]: + return await asyncio.open_connection(host=self.host, port=self.port, limit=TCP_BUFFER_SIZE) diff --git a/homcc/client/compilation.py b/homcc/client/compilation.py index dd6bb3d..846183c 100644 --- a/homcc/client/compilation.py +++ b/homcc/client/compilation.py @@ -15,11 +15,13 @@ from homcc.client.client import ( LocalHostCompilationSemaphore, LocalHostPreprocessingSemaphore, + RemoteCompilationClient, RemoteHostSelector, RemoteHostSemaphore, TCPClient, ) from homcc.client.config import ClientConfig +from homcc.client.ssh import SSHClient, SSHTunnel from homcc.common.arguments import Arguments, ArgumentsExecutionResult, Compiler from homcc.common.constants import ENCODING, EXCLUDED_DEPENDENCY_PREFIXES from homcc.common.errors import ( @@ -34,7 +36,7 @@ UnexpectedMessageTypeError, ) from homcc.common.hashing import hash_file_with_path -from homcc.common.host import Host +from homcc.common.host import ConnectionType, Host from homcc.common.messages import ( CompilationResultMessage, ConnectionRefusedMessage, @@ -82,9 +84,7 @@ async def compile_remotely(arguments: Arguments, hosts: List[Host], localhost: H arguments=arguments, dependency_dict=dependency_dict, host=host, - timeout=config.establish_connection_timeout, - schroot_profile=config.schroot_profile, - docker_container=config.docker_container, + config=config, state=state, ), timeout=config.compilation_request_timeout, @@ -119,18 +119,33 @@ async def compile_remotely(arguments: Arguments, hosts: List[Host], localhost: H ) +def create_remote_client(host: Host, timeout: float, state: StateFile, config: ClientConfig) -> RemoteCompilationClient: + """Create the transport-specific client for the given host: a direct TCP client or an SSH-tunneled client.""" + if host.type == ConnectionType.SSH: + tunnel = SSHTunnel( + host, + ssh_executable=config.ssh_executable, + control_persist=config.ssh_control_persist, + ssh_options=config.ssh_options, + ) + return SSHClient(host, timeout=timeout, state=state, tunnel=tunnel) + + return TCPClient(host, timeout=timeout, state=state) + + async def compile_remotely_at( arguments: Arguments, dependency_dict: Dict[str, str], host: Host, - timeout: float, - schroot_profile: Optional[str], - docker_container: Optional[str], + config: ClientConfig, state: StateFile, ) -> int: """main function for the communication between client and a remote compilation host""" - async with TCPClient(host, timeout=timeout, state=state) as client: + schroot_profile: Optional[str] = config.schroot_profile + docker_container: Optional[str] = config.docker_container + + async with create_remote_client(host, config.establish_connection_timeout, state, config) as client: remote_arguments: Arguments = arguments.copy().remove_local_args() target: Optional[str] = None @@ -159,7 +174,7 @@ async def compile_remotely_at( host_response: Message = await client.receive() if isinstance(host_response, ConnectionRefusedMessage): raise HostRefusedConnectionError( - f"Host {client.host}:{client.port} refused the connection:\n{host_response.info}!" + f"Host {client.connection_target} refused the connection:\n{host_response.info}!" ) # invert dependency dictionary to access dependencies via hash diff --git a/homcc/client/config.py b/homcc/client/config.py index e7d73e6..1ee0ab6 100644 --- a/homcc/client/config.py +++ b/homcc/client/config.py @@ -15,6 +15,7 @@ from pathlib import Path from typing import ClassVar, Iterator, List, Optional +from homcc.client.ssh import DEFAULT_SSH_CONTROL_PERSIST, DEFAULT_SSH_EXECUTABLE from homcc.common.compression import Compression from homcc.common.logging import LogLevel from homcc.common.parsing import HOMCC_CONFIG_FILENAME, default_locations, parse_configs @@ -38,6 +39,9 @@ class ClientEnvironmentVariables: HOMCC_LOG_LEVEL_ENV_VAR: ClassVar[str] = "HOMCC_LOG_LEVEL" HOMCC_VERBOSE_ENV_VAR: ClassVar[str] = "HOMCC_VERBOSE" HOMCC_NO_LOCAL_COMPILATION_ENV_VAR: ClassVar[str] = "HOMCC_NO_LOCAL_COMPILATION" + HOMCC_SSH_EXECUTABLE_ENV_VAR: ClassVar[str] = "HOMCC_SSH_EXECUTABLE" + HOMCC_SSH_CONTROL_PERSIST_ENV_VAR: ClassVar[str] = "HOMCC_SSH_CONTROL_PERSIST" + HOMCC_SSH_OPTIONS_ENV_VAR: ClassVar[str] = "HOMCC_SSH_OPTIONS" @classmethod def __iter__(cls) -> Iterator[str]: @@ -51,6 +55,9 @@ def __iter__(cls) -> Iterator[str]: cls.HOMCC_LOG_LEVEL_ENV_VAR, cls.HOMCC_VERBOSE_ENV_VAR, cls.HOMCC_NO_LOCAL_COMPILATION_ENV_VAR, + cls.HOMCC_SSH_EXECUTABLE_ENV_VAR, + cls.HOMCC_SSH_CONTROL_PERSIST_ENV_VAR, + cls.HOMCC_SSH_OPTIONS_ENV_VAR, ) @staticmethod @@ -104,6 +111,22 @@ def get_no_local_compilation(cls) -> Optional[bool]: return cls.parse_bool_str(no_local_compilation) return None + @classmethod + def get_ssh_executable(cls) -> Optional[str]: + return os.getenv(cls.HOMCC_SSH_EXECUTABLE_ENV_VAR) + + @classmethod + def get_ssh_control_persist(cls) -> Optional[int]: + if ssh_control_persist := os.getenv(cls.HOMCC_SSH_CONTROL_PERSIST_ENV_VAR): + return int(ssh_control_persist) + return None + + @classmethod + def get_ssh_options(cls) -> Optional[List[str]]: + if (ssh_options := os.getenv(cls.HOMCC_SSH_OPTIONS_ENV_VAR)) is not None: + return ssh_options.split() + return None + @dataclass class ClientConfig: @@ -119,6 +142,9 @@ class ClientConfig: log_level: Optional[LogLevel] verbose: bool local_compilation_enabled: bool + ssh_executable: str + ssh_control_persist: int + ssh_options: List[str] def __init__( self, @@ -133,6 +159,9 @@ def __init__( log_level: Optional[str] = None, verbose: Optional[bool] = None, no_local_compilation: Optional[bool] = None, + ssh_executable: Optional[str] = None, + ssh_control_persist: Optional[int] = None, + ssh_options: Optional[List[str]] = None, ): self.files = files @@ -164,6 +193,14 @@ def __init__( ClientEnvironmentVariables.get_no_local_compilation() or no_local_compilation ) + self.ssh_executable = ( + ClientEnvironmentVariables.get_ssh_executable() or ssh_executable or DEFAULT_SSH_EXECUTABLE + ) + self.ssh_control_persist = ( + ClientEnvironmentVariables.get_ssh_control_persist() or ssh_control_persist or DEFAULT_SSH_CONTROL_PERSIST + ) + self.ssh_options = ClientEnvironmentVariables.get_ssh_options() or ssh_options or [] + @classmethod def empty(cls): return cls(files=[]) @@ -179,6 +216,10 @@ def from_config_section(cls, files: List[str], homcc_config: configparser.Sectio log_level: Optional[str] = homcc_config.get("log_level") verbose: Optional[bool] = homcc_config.getboolean("verbose") no_local_compilation: Optional[bool] = homcc_config.getboolean("no_local_compilation") + ssh_executable: Optional[str] = homcc_config.get("ssh_executable") + ssh_control_persist: Optional[int] = homcc_config.getint("ssh_control_persist") + ssh_options_str: Optional[str] = homcc_config.get("ssh_options") + ssh_options: Optional[List[str]] = ssh_options_str.split() if ssh_options_str is not None else None return ClientConfig( files=files, @@ -191,6 +232,9 @@ def from_config_section(cls, files: List[str], homcc_config: configparser.Sectio log_level=log_level, verbose=verbose, no_local_compilation=no_local_compilation, + ssh_executable=ssh_executable, + ssh_control_persist=ssh_control_persist, + ssh_options=ssh_options, ) def __str__(self): @@ -205,6 +249,9 @@ def __str__(self): f"\tlog_level:\t\t\t{self.log_level}\n" f"\tverbose:\t\t\t{self.verbose}\n" f"\tlocal_compilation_enabled:\t{self.local_compilation_enabled}\n" + f"\tssh_executable:\t\t\t{self.ssh_executable}\n" + f"\tssh_control_persist:\t\t{self.ssh_control_persist}\n" + f"\tssh_options:\t\t\t{' '.join(self.ssh_options)}\n" ) def set_verbose(self): diff --git a/homcc/client/ssh.py b/homcc/client/ssh.py new file mode 100644 index 0000000..aa54430 --- /dev/null +++ b/homcc/client/ssh.py @@ -0,0 +1,205 @@ +# Copyright (c) 2023 Celonis SE +# Covered under the included MIT License: +# https://github.com/celonis/homcc/blob/main/LICENSE + +""" +SSH transport for the homcc client. + +Remote compilation over SSH is implemented as a tunnel to an already running `homccd`: an OpenSSH master connection is +established once per remote host and local-forwards a port to the daemon's loopback port on the remote. Subsequent +compilations reuse that master via OpenSSH connection multiplexing (ControlMaster/ControlPersist), so the per-job cost +is reduced to a local TCP connection to the forwarded port instead of a full SSH handshake. Because every tunneled +connection reaches the same daemon as direct TCP clients, the server keeps enforcing its global connection/compilation +limit across both transports. +""" +from __future__ import annotations + +import asyncio +import fcntl +import hashlib +import logging +import os +import socket +import subprocess +from pathlib import Path +from typing import List, Optional, Tuple + +from homcc.client.client import RemoteCompilationClient +from homcc.common.constants import ENCODING, TCP_BUFFER_SIZE +from homcc.common.errors import SSHError +from homcc.common.host import Host +from homcc.common.parsing import HOMCC_DIR_ENV_VAR +from homcc.common.statefile import StateFile + +logger = logging.getLogger(__name__) + +DEFAULT_SSH_EXECUTABLE: str = "ssh" +DEFAULT_SSH_CONTROL_PERSIST: int = 600 +"""Seconds an idle multiplexed SSH master connection is kept alive for reuse.""" + + +def _ssh_base_dir() -> Path: + """Directory holding the SSH control sockets and forwarded-port state files.""" + homcc_dir_env_var: Optional[str] = os.getenv(HOMCC_DIR_ENV_VAR) + base: Path = Path(homcc_dir_env_var) if homcc_dir_env_var else Path.home() / ".homcc" + return base / "ssh" + + +def _find_free_local_port() -> int: + """Ask the OS for a currently free local TCP port to forward through the SSH tunnel.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe: + probe.bind(("127.0.0.1", 0)) + return probe.getsockname()[1] + + +class SSHTunnel: + """ + Manages a multiplexed OpenSSH master connection with a local port-forward to a remote `homccd`. + + A single master connection is shared across all concurrent homcc processes that target the same remote host via a + control socket in `$HOMCC_DIR/ssh/` (falling back to `~/.homcc/ssh/`). Setting up the master is guarded by a + per-host file lock so that the many homcc invocations a build system spawns converge on one tunnel instead of + racing to create their own. + """ + + def __init__( + self, + host: Host, + *, + ssh_executable: str = DEFAULT_SSH_EXECUTABLE, + control_persist: int = DEFAULT_SSH_CONTROL_PERSIST, + ssh_options: Optional[List[str]] = None, + ): + self.host = host + self.remote_port: int = host.port + self.ssh_executable = ssh_executable + self.control_persist = control_persist + self.ssh_options: List[str] = ssh_options or [] + + # unique but filesystem-path-length safe identifier for the (user, host, remote port) triple + user: str = host.user or "" + key: str = f"{user}@{host.name}:{host.port}" + digest: str = hashlib.sha1(key.encode(ENCODING)).hexdigest()[:16] + + base_dir: Path = _ssh_base_dir() + self.control_path: Path = base_dir / f"{digest}.sock" + self._port_file: Path = base_dir / f"{digest}.port" + self._lock_file: Path = base_dir / f"{digest}.lock" + + @property + def target(self) -> str: + """SSH target argument, i.e. 'user@host' or 'host'.""" + return f"{self.host.user}@{self.host.name}" if self.host.user else self.host.name + + def _control_args(self) -> List[str]: + """Common OpenSSH arguments enabling connection multiplexing via the shared control socket.""" + return [ + self.ssh_executable, + "-o", + "ControlMaster=auto", + "-o", + f"ControlPath={self.control_path}", + *self.ssh_options, + ] + + def _is_master_alive(self) -> bool: + """Return whether a reusable multiplexed master connection already exists for this host.""" + check = subprocess.run( # noqa: PLW1510 + [*self._control_args(), "-O", "check", self.target], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + return check.returncode == 0 + + def _start_master(self, timeout: float) -> int: + """Start the multiplexed master with a local port-forward and return the chosen local port.""" + local_port: int = _find_free_local_port() + + # -M/-S: act as multiplexing master over the control socket + # -f -N: background after authentication without executing a remote command + # -L: forward local_port to the daemon on the remote loopback interface + # ExitOnForwardFailure: fail fast if the forward can not be set up rather than silently continuing + command: List[str] = [ + *self._control_args(), + "-M", + "-S", + str(self.control_path), + "-o", + f"ControlPersist={self.control_persist}", + "-o", + "ExitOnForwardFailure=yes", + "-f", + "-N", + "-L", + f"{local_port}:localhost:{self.remote_port}", + self.target, + ] + + logger.debug("Establishing SSH master connection to '%s' (local port %i).", self.target, local_port) + try: + result = subprocess.run( + command, + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + timeout=timeout, + check=False, + ) + except subprocess.TimeoutExpired as error: + raise SSHError(f"Establishing the SSH tunnel to '{self.target}' timed out.") from error + + if result.returncode != 0: + stderr: str = result.stderr.decode(ENCODING, errors="replace").strip() + raise SSHError(f"Could not establish the SSH tunnel to '{self.target}': {stderr}") + + self._port_file.write_text(str(local_port), encoding=ENCODING) + return local_port + + def ensure(self, timeout: float) -> int: + """ + Ensure a live multiplexed tunnel exists and return the forwarded local port. + + Reuses an existing master connection when possible; otherwise sets one up while holding a per-host file lock so + that concurrent homcc processes do not each spawn their own tunnel. + """ + self.control_path.parent.mkdir(parents=True, exist_ok=True) + + if self._is_master_alive() and self._port_file.exists(): + return int(self._port_file.read_text(encoding=ENCODING)) + + with self._lock_file.open("w", encoding=ENCODING) as lock: + fcntl.flock(lock, fcntl.LOCK_EX) + try: + # re-check under the lock: another process may have set up the master while we waited + if self._is_master_alive() and self._port_file.exists(): + return int(self._port_file.read_text(encoding=ENCODING)) + return self._start_master(timeout) + finally: + fcntl.flock(lock, fcntl.LOCK_UN) + + def close(self): + """Tear down the multiplexed master connection. Idle masters are otherwise reaped via ControlPersist.""" + subprocess.run( # noqa: PLW1510 + [*self._control_args(), "-O", "exit", self.target], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + ) + self._port_file.unlink(missing_ok=True) + + +class SSHClient(RemoteCompilationClient): + """Client to exchange homcc protocol messages with a remote server through an SSH tunnel to a running `homccd`.""" + + def __init__(self, host: Host, timeout: float, state: StateFile, tunnel: SSHTunnel): + super().__init__(host, timeout, state) + + self._tunnel = tunnel + self.connection_target = tunnel.target + + async def _open_connection(self) -> Tuple[asyncio.StreamReader, asyncio.StreamWriter]: + # establishing/reusing the SSH master may block on subprocess calls, so run it off the event loop; the overall + # setup and connect is still bounded by the connection timeout applied by the base class + loop = asyncio.get_event_loop() + local_port: int = await loop.run_in_executor(None, self._tunnel.ensure, self.timeout) + return await asyncio.open_connection(host="127.0.0.1", port=local_port, limit=TCP_BUFFER_SIZE) diff --git a/homcc/common/errors.py b/homcc/common/errors.py index 6bb16ad..46060b9 100644 --- a/homcc/common/errors.py +++ b/homcc/common/errors.py @@ -58,6 +58,14 @@ class HostRefusedConnectionError(Exception): """Error class to indicate that the host refused establishing the connection""" +class SSHError(ConnectionError): + """ + Error class to indicate that establishing or reusing the SSH tunnel to a remote host failed. Subclasses + ConnectionError so that the remote compilation loop treats it like any other lost connection and falls back to + the next host or to local compilation. + """ + + @dataclass class RemoteCompilationError(Exception): """ diff --git a/homcc/common/host.py b/homcc/common/host.py index 1baee16..dbdd33b 100644 --- a/homcc/common/host.py +++ b/homcc/common/host.py @@ -146,13 +146,13 @@ def _parse_host(host: str) -> Host: return Host(type=connection_type, name=host, **host_dict) # USER@HOST_FORMAT - elif (user_at_host_match := re.match(r"^(\w+)@([\w.:/]+)$", host)) is not None: + elif (user_at_host_match := re.match(r"^(\w+)@([\w.:/\[\]]+)$", host)) is not None: user, host = user_at_host_match.groups() connection_type = ConnectionType.SSH host_dict["user"] = user # @HOST_FORMAT - elif (at_host_match := re.match(r"^@([\w.:/]+)$", host)) is not None: + elif (at_host_match := re.match(r"^@([\w.:/\[\]]+)$", host)) is not None: host = at_host_match.group(1) connection_type = ConnectionType.SSH @@ -168,4 +168,14 @@ def _parse_host(host: str) -> Host: host, limit = host_limit_match.groups() host_dict["limit"] = limit + # for SSH hosts, extract the optional remote daemon port to forward the tunnel to: NAME:PORT or [IPv6]:PORT; + # unbracketed IPv6 addresses (e.g. '::1') are left untouched and use the default port + if connection_type == ConnectionType.SSH: + if (ssh_ipv6_port_match := re.match(r"^\[(\S+)]:(\d+)$", host)) is not None: + host, host_dict["port"] = ssh_ipv6_port_match.groups() + elif (ssh_name_port_match := re.match(r"^([\w.]+):(\d+)$", host)) is not None: + host, host_dict["port"] = ssh_name_port_match.groups() + elif (ssh_ipv6_match := re.match(r"^\[(\S+)]$", host)) is not None: + host = ssh_ipv6_match.group(1) + return Host(type=connection_type, name=host, **host_dict) diff --git a/tests/client/parsing_test.py b/tests/client/parsing_test.py index 38ecfaa..ef22285 100644 --- a/tests/client/parsing_test.py +++ b/tests/client/parsing_test.py @@ -227,6 +227,25 @@ def test_user_at_host(self): ) assert Host.from_str("user@::1/64,lzo") == Host(type=ssh, name="::1", limit=64, compression="lzo", user="user") + def test_ssh_host_remote_port(self): + ssh: ConnectionType = ConnectionType.SSH + + # the port designates the remote homccd port the SSH tunnel forwards to + assert Host.from_str("@buildhost:3126") == Host(type=ssh, name="buildhost", port=3126) + assert Host.from_str("user@buildhost:3127") == Host(type=ssh, name="buildhost", port=3127, user="user") + assert Host.from_str("@127.0.0.1:3126/64") == Host(type=ssh, name="127.0.0.1", port=3126, limit=64) + assert Host.from_str("user@127.0.0.1:3126/64,lzo") == Host( + type=ssh, name="127.0.0.1", port=3126, limit=64, compression="lzo", user="user" + ) + + # bracketed IPv6 with a port, and bracket stripping without a port + assert Host.from_str("@[::1]:3126") == Host(type=ssh, name="::1", port=3126) + assert Host.from_str("user@[::1]:3127/8") == Host(type=ssh, name="::1", port=3127, limit=8, user="user") + assert Host.from_str("@[::1]") == Host(type=ssh, name="::1") + + # unbracketed IPv6 without a port keeps using the default port + assert Host.from_str("@::1").port == Host.from_str("@buildhost").port + def test_load_hosts(self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path): hosts = ["localhost", "localhost:3126 ", "localhost:3126,lzo\t", " ", ""] hosts_no_whitespace = ["localhost", "localhost:3126", "localhost:3126,lzo"] @@ -257,6 +276,9 @@ class TestParsingConfig: "docker_container=some_container", "log_level=INFO", "verbose=TRUE", + "ssh_executable=/usr/bin/ssh", + "ssh_control_persist=300", + "ssh_options=-o StrictHostKeyChecking=no", # the following configs should be ignored "[homccd]", "LOG_LEVEL=DEBUG", @@ -280,6 +302,9 @@ def test_parse_config_file(self, tmp_path: Path): verbose=True, schroot_profile="foobar", docker_container="some_container", + ssh_executable="/usr/bin/ssh", + ssh_control_persist=300, + ssh_options=["-o", "StrictHostKeyChecking=no"], ) def test_parse_multiple_config_files(self, tmp_path: Path): @@ -297,6 +322,9 @@ def test_parse_multiple_config_files(self, tmp_path: Path): docker_container="some_container", log_level="INFO", verbose=False, + ssh_executable="/usr/bin/ssh", + ssh_control_persist=300, + ssh_options=["-o", "StrictHostKeyChecking=no"], ) diff --git a/tests/client/ssh_test.py b/tests/client/ssh_test.py new file mode 100644 index 0000000..480f7a0 --- /dev/null +++ b/tests/client/ssh_test.py @@ -0,0 +1,145 @@ +# Copyright (c) 2023 Celonis SE +# Covered under the included MIT License: +# https://github.com/celonis/homcc/blob/main/LICENSE + +"""Tests for the SSH transport of the homcc client.""" +import subprocess +from pathlib import Path + +import pytest +from pytest_mock import MockerFixture + +from homcc.client.client import TCPClient +from homcc.client.compilation import create_remote_client +from homcc.client.config import ClientConfig +from homcc.client.ssh import DEFAULT_SSH_CONTROL_PERSIST, SSHClient, SSHTunnel +from homcc.common.arguments import Arguments +from homcc.common.errors import SSHError +from homcc.common.host import ConnectionType, Host +from homcc.common.statefile import StateFile + + +class TestSSHTunnel: + """Tests for the SSHTunnel connection multiplexing manager.""" + + # deliberately inspect internals to verify the multiplexing/forwarding behavior + # pylint: disable=protected-access + + def test_target(self): + assert SSHTunnel(Host.from_str("user@buildhost")).target == "user@buildhost" + assert SSHTunnel(Host.from_str("@buildhost")).target == "buildhost" + + def test_control_path_is_stable_and_unique(self, tmp_path: Path, mocker: MockerFixture): + mocker.patch("homcc.client.ssh._ssh_base_dir", return_value=tmp_path) + + # identical hosts share the same control socket, so concurrent processes converge on one tunnel + assert SSHTunnel(Host.from_str("user@buildhost")).control_path == ( + SSHTunnel(Host.from_str("user@buildhost")).control_path + ) + # different user / host / remote port must not collide + assert SSHTunnel(Host.from_str("user@buildhost")).control_path != ( + SSHTunnel(Host.from_str("other@buildhost")).control_path + ) + assert SSHTunnel(Host.from_str("user@buildhost:3126")).control_path != ( + SSHTunnel(Host.from_str("user@buildhost:3127")).control_path + ) + + def test_control_args_enable_multiplexing(self): + tunnel = SSHTunnel( + Host.from_str("user@buildhost"), ssh_executable="/usr/bin/ssh", ssh_options=["-o", "BatchMode=yes"] + ) + args = tunnel._control_args() + + assert args[0] == "/usr/bin/ssh" + assert "ControlMaster=auto" in args + assert f"ControlPath={tunnel.control_path}" in args + assert args[-2:] == ["-o", "BatchMode=yes"] + + def test_ensure_reuses_alive_master(self, tmp_path: Path, mocker: MockerFixture): + mocker.patch("homcc.client.ssh._ssh_base_dir", return_value=tmp_path) + tunnel = SSHTunnel(Host.from_str("user@buildhost")) + + mocker.patch.object(tunnel, "_is_master_alive", return_value=True) + tunnel._port_file.parent.mkdir(parents=True, exist_ok=True) + tunnel._port_file.write_text("54321") + start_master = mocker.patch.object(tunnel, "_start_master") + + assert tunnel.ensure(timeout=10) == 54321 + start_master.assert_not_called() # no new SSH handshake when a master already exists + + def test_ensure_starts_master_when_absent(self, tmp_path: Path, mocker: MockerFixture): + mocker.patch("homcc.client.ssh._ssh_base_dir", return_value=tmp_path) + tunnel = SSHTunnel(Host.from_str("user@buildhost")) + + mocker.patch.object(tunnel, "_is_master_alive", return_value=False) + mocker.patch("homcc.client.ssh._find_free_local_port", return_value=45678) + run = mocker.patch("homcc.client.ssh.subprocess.run", return_value=subprocess.CompletedProcess([], 0)) + + assert tunnel.ensure(timeout=10) == 45678 + assert tunnel._port_file.read_text() == "45678" + + command = run.call_args.args[0] + assert "-M" in command and "-N" in command + assert "45678:localhost:3126" in command # local port forwarded to the daemon on the remote loopback + assert command[-1] == "user@buildhost" + + def test_ensure_raises_ssh_error_on_failure(self, tmp_path: Path, mocker: MockerFixture): + mocker.patch("homcc.client.ssh._ssh_base_dir", return_value=tmp_path) + tunnel = SSHTunnel(Host.from_str("user@buildhost")) + + mocker.patch.object(tunnel, "_is_master_alive", return_value=False) + mocker.patch("homcc.client.ssh._find_free_local_port", return_value=45678) + mocker.patch( + "homcc.client.ssh.subprocess.run", + return_value=subprocess.CompletedProcess([], 255, stderr=b"Permission denied"), + ) + + with pytest.raises(SSHError, match="Permission denied"): + tunnel.ensure(timeout=10) + + def test_ensure_raises_ssh_error_on_timeout(self, tmp_path: Path, mocker: MockerFixture): + mocker.patch("homcc.client.ssh._ssh_base_dir", return_value=tmp_path) + tunnel = SSHTunnel(Host.from_str("user@buildhost")) + + mocker.patch.object(tunnel, "_is_master_alive", return_value=False) + mocker.patch("homcc.client.ssh._find_free_local_port", return_value=45678) + mocker.patch("homcc.client.ssh.subprocess.run", side_effect=subprocess.TimeoutExpired(cmd="ssh", timeout=1)) + + with pytest.raises(SSHError, match="timed out"): + tunnel.ensure(timeout=1) + + +class TestClientFactory: + """Tests that the remote client factory dispatches on the host connection type.""" + + # deliberately inspect the tunnel wired into the SSH client + # pylint: disable=protected-access + + @staticmethod + def _state(host: Host, tmp_path: Path) -> StateFile: + return StateFile(Arguments.from_vargs("gcc", "foo.cpp"), host, state_dir=tmp_path) + + def test_tcp_host_creates_tcp_client(self, tmp_path: Path): + host = Host.from_str("buildhost:3126") + client = create_remote_client(host, timeout=10, state=self._state(host, tmp_path), config=ClientConfig.empty()) + + assert isinstance(client, TCPClient) + assert client.connection_target == "buildhost:3126" + + def test_ssh_host_creates_ssh_client(self, tmp_path: Path): + host = Host.from_str("user@buildhost") + config = ClientConfig(files=[], ssh_control_persist=42, ssh_options=["-o", "BatchMode=yes"]) + client = create_remote_client(host, timeout=10, state=self._state(host, tmp_path), config=config) + + assert isinstance(client, SSHClient) + assert host.type == ConnectionType.SSH + assert client.connection_target == "user@buildhost" + assert client._tunnel.control_persist == 42 + assert client._tunnel.ssh_options == ["-o", "BatchMode=yes"] + + def test_ssh_tunnel_default_control_persist(self, tmp_path: Path): + host = Host.from_str("@buildhost") + client = create_remote_client(host, timeout=10, state=self._state(host, tmp_path), config=ClientConfig.empty()) + + assert isinstance(client, SSHClient) + assert client._tunnel.control_persist == DEFAULT_SSH_CONTROL_PERSIST diff --git a/tests/conftest.py b/tests/conftest.py index 4dcf2dc..240397d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -35,6 +35,12 @@ def pytest_addoption(parser: pytest.Parser): metavar="DOCKER_CONTAINER", help="run e2e docker tests with specified DOCKER_CONTAINER", ) + parser.addoption( + "--runssh", + action="store_true", + default=False, + help="run e2e SSH tunnel tests (requires passwordless 'ssh 127.0.0.1' access)", + ) @pytest.fixture @@ -52,6 +58,7 @@ def pytest_configure(config: pytest.Config): config.addinivalue_line("markers", "clangplusplus: mark tests that execute the clang++ compiler") config.addinivalue_line("markers", "schroot: mark tests that are only run with a set up chroot environment") config.addinivalue_line("markers", "docker: mark tests that are only run with a set up docker environment") + config.addinivalue_line("markers", "ssh: mark tests that require a reachable SSH server on the local host") def pytest_collection_modifyitems(config: pytest.Config, items: List[pytest.Item]): @@ -81,3 +88,10 @@ def add_marker(keyword, marker): elif config.getoption("--rundocker") is None: rundocker_profile_marker = pytest.mark.skip(reason="specify --rundocker=CONTAINER_NAME to execute") add_marker("docker", rundocker_profile_marker) + + if shutil.which("ssh") is None: + ssh_marker = pytest.mark.skip(reason="ssh is not installed") + add_marker("ssh", ssh_marker) + elif not config.getoption("--runssh"): + runssh_marker = pytest.mark.skip(reason="specify --runssh to execute (requires passwordless ssh to 127.0.0.1)") + add_marker("ssh", runssh_marker) diff --git a/tests/e2e/e2e_test.py b/tests/e2e/e2e_test.py index 04991ec..7f81aeb 100644 --- a/tests/e2e/e2e_test.py +++ b/tests/e2e/e2e_test.py @@ -46,6 +46,7 @@ class BasicClientArguments: compression: Compression = NoCompression() schroot_profile: Optional[str] = None docker_container: Optional[str] = None + ssh: bool = False # tunnel the connection through SSH instead of connecting via plain TCP def __post_init__(self): if self.schroot_profile is not None and self.docker_container is not None: @@ -55,7 +56,9 @@ def __iter__(self) -> Iterator[str]: compression = ( f",{self.compression}" if self.compression is not isinstance(self.compression, NoCompression) else "" ) - host_arg = f"--host={TestEndToEnd.ADDRESS}:{self.tcp_port}/1{compression}" # explicit host limit: 1 + # explicit host limit: 1; the '@' prefix selects the SSH transport, forwarding a tunnel to the daemon port + ssh_prefix: str = "@" if self.ssh else "" + host_arg = f"--host={ssh_prefix}{TestEndToEnd.ADDRESS}:{self.tcp_port}/1{compression}" sandbox_arg: str = "--no-sandbox" @@ -421,6 +424,14 @@ def test_cpp_end_to_end_gplusplus_preprocessor_side_effects(self, unused_tcp_por def test_end_to_end_gplusplus_linking_only(self, unused_tcp_port: int): self.cpp_end_to_end_linking_only(self.BasicClientArguments("g++", unused_tcp_port)) + @pytest.mark.gplusplus + @pytest.mark.ssh + @pytest.mark.timeout(TIMEOUT) + def test_end_to_end_ssh_gplusplus(self, unused_tcp_port: int, monkeypatch: pytest.MonkeyPatch): + # keep the SSH tunnel non-interactive so the test never blocks on a host-key or password prompt + monkeypatch.setenv("HOMCC_SSH_OPTIONS", "-o BatchMode=yes -o StrictHostKeyChecking=accept-new") + self.cpp_end_to_end(self.BasicClientArguments("g++", unused_tcp_port, ssh=True)) + @pytest.mark.gplusplus @pytest.mark.schroot @pytest.mark.timeout(TIMEOUT) From 9102cd4243f4bddf0b75637412c0d7ddf56c224d Mon Sep 17 00:00:00 2001 From: Willi Mann Date: Thu, 16 Jul 2026 18:18:24 +0200 Subject: [PATCH 2/2] fix: address PR #103 review comments on SSH transport - ssh.py: catch OSError/CalledProcessError separately in _start_master instead of a manual returncode check, so a missing ssh executable is reported distinctly; reuse common.parsing.default_locations for the ssh control-socket directory; make control_args a public property; switch to asyncio.to_thread; drop the unused SSHTunnel.close() - config.py: parse HOMCC_SSH_OPTIONS/ssh_options with shlex.split so quoted values with spaces work - README.md: clarify that SSH tunneling is also a mitigation for the no-built-in-encryption deployment hint - add AGENTS.md as a symlink to CLAUDE.md Co-Authored-By: Claude Sonnet 5 --- AGENTS.md | 1 + README.md | 2 +- homcc/client/config.py | 5 ++-- homcc/client/ssh.py | 55 ++++++++++++++++++++-------------------- tests/client/ssh_test.py | 15 +++++++++-- 5 files changed, 45 insertions(+), 33 deletions(-) create mode 120000 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 120000 index 0000000..681311e --- /dev/null +++ b/AGENTS.md @@ -0,0 +1 @@ +CLAUDE.md \ No newline at end of file diff --git a/README.md b/README.md index ace4415..2d66d98 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ Things to keep in mind when deploying `homccd`: - `homccd` does not limit simultaneous connections of a single client. A malicious client could therefore block the service by always opening up connections until no server slots are available any more. - `homccd` does not limit access to docker containers or chroot environments. A client can choose any docker container or chroot environment available on the server to execute the compilation in. -:exclamation: The key takeaway of the previous points is to **not expose** `homccd` publicly. You should make sure only internal users (e.g. developers) have access to the service, for example through using a VPN. +:exclamation: The key takeaway of the previous points is to **not expose** `homccd` publicly. Make sure only internal users (e.g. developers) have access to the service, for example through a VPN or by only accepting SSH-tunneled connections. ## Development diff --git a/homcc/client/config.py b/homcc/client/config.py index 1ee0ab6..561e27e 100644 --- a/homcc/client/config.py +++ b/homcc/client/config.py @@ -10,6 +10,7 @@ import configparser import os import re +import shlex import sys from dataclasses import dataclass from pathlib import Path @@ -124,7 +125,7 @@ def get_ssh_control_persist(cls) -> Optional[int]: @classmethod def get_ssh_options(cls) -> Optional[List[str]]: if (ssh_options := os.getenv(cls.HOMCC_SSH_OPTIONS_ENV_VAR)) is not None: - return ssh_options.split() + return shlex.split(ssh_options) return None @@ -219,7 +220,7 @@ def from_config_section(cls, files: List[str], homcc_config: configparser.Sectio ssh_executable: Optional[str] = homcc_config.get("ssh_executable") ssh_control_persist: Optional[int] = homcc_config.getint("ssh_control_persist") ssh_options_str: Optional[str] = homcc_config.get("ssh_options") - ssh_options: Optional[List[str]] = ssh_options_str.split() if ssh_options_str is not None else None + ssh_options: Optional[List[str]] = shlex.split(ssh_options_str) if ssh_options_str is not None else None return ClientConfig( files=files, diff --git a/homcc/client/ssh.py b/homcc/client/ssh.py index aa54430..cd4f739 100644 --- a/homcc/client/ssh.py +++ b/homcc/client/ssh.py @@ -28,7 +28,11 @@ from homcc.common.constants import ENCODING, TCP_BUFFER_SIZE from homcc.common.errors import SSHError from homcc.common.host import Host -from homcc.common.parsing import HOMCC_DIR_ENV_VAR +from homcc.common.parsing import ( + HOMCC_CONFIG_FILENAME, + HOMCC_DIR_ENV_VAR, + default_locations, +) from homcc.common.statefile import StateFile logger = logging.getLogger(__name__) @@ -41,8 +45,13 @@ def _ssh_base_dir() -> Path: """Directory holding the SSH control sockets and forwarded-port state files.""" homcc_dir_env_var: Optional[str] = os.getenv(HOMCC_DIR_ENV_VAR) - base: Path = Path(homcc_dir_env_var) if homcc_dir_env_var else Path.home() / ".homcc" - return base / "ssh" + if homcc_dir_env_var: + return Path(homcc_dir_env_var) / "ssh" + + for config_location in default_locations(HOMCC_CONFIG_FILENAME): + return config_location.parent / "ssh" + + return Path.home() / ".homcc" / "ssh" def _find_free_local_port() -> int: @@ -56,10 +65,9 @@ class SSHTunnel: """ Manages a multiplexed OpenSSH master connection with a local port-forward to a remote `homccd`. - A single master connection is shared across all concurrent homcc processes that target the same remote host via a - control socket in `$HOMCC_DIR/ssh/` (falling back to `~/.homcc/ssh/`). Setting up the master is guarded by a - per-host file lock so that the many homcc invocations a build system spawns converge on one tunnel instead of - racing to create their own. + A single master connection is shared across all concurrent homcc processes that target the same remote host via + the ssh control socket. Setting up the master is guarded by a per-host file lock so that the many homcc + invocations a build system spawns converge on one tunnel instead of racing to create their own. """ def __init__( @@ -91,7 +99,8 @@ def target(self) -> str: """SSH target argument, i.e. 'user@host' or 'host'.""" return f"{self.host.user}@{self.host.name}" if self.host.user else self.host.name - def _control_args(self) -> List[str]: + @property + def control_args(self) -> List[str]: """Common OpenSSH arguments enabling connection multiplexing via the shared control socket.""" return [ self.ssh_executable, @@ -105,7 +114,7 @@ def _control_args(self) -> List[str]: def _is_master_alive(self) -> bool: """Return whether a reusable multiplexed master connection already exists for this host.""" check = subprocess.run( # noqa: PLW1510 - [*self._control_args(), "-O", "check", self.target], + [*self.control_args, "-O", "check", self.target], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, check=False, @@ -121,7 +130,7 @@ def _start_master(self, timeout: float) -> int: # -L: forward local_port to the daemon on the remote loopback interface # ExitOnForwardFailure: fail fast if the forward can not be set up rather than silently continuing command: List[str] = [ - *self._control_args(), + *self.control_args, "-M", "-S", str(self.control_path), @@ -138,19 +147,20 @@ def _start_master(self, timeout: float) -> int: logger.debug("Establishing SSH master connection to '%s' (local port %i).", self.target, local_port) try: - result = subprocess.run( + subprocess.run( command, stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, timeout=timeout, - check=False, + check=True, ) except subprocess.TimeoutExpired as error: raise SSHError(f"Establishing the SSH tunnel to '{self.target}' timed out.") from error - - if result.returncode != 0: - stderr: str = result.stderr.decode(ENCODING, errors="replace").strip() - raise SSHError(f"Could not establish the SSH tunnel to '{self.target}': {stderr}") + except subprocess.CalledProcessError as error: + stderr: str = error.stderr.decode(ENCODING, errors="replace").strip() + raise SSHError(f"Could not establish the SSH tunnel to '{self.target}': {stderr}") from error + except OSError as error: + raise SSHError(f"Could not execute '{self.ssh_executable}': {error}") from error self._port_file.write_text(str(local_port), encoding=ENCODING) return local_port @@ -177,16 +187,6 @@ def ensure(self, timeout: float) -> int: finally: fcntl.flock(lock, fcntl.LOCK_UN) - def close(self): - """Tear down the multiplexed master connection. Idle masters are otherwise reaped via ControlPersist.""" - subprocess.run( # noqa: PLW1510 - [*self._control_args(), "-O", "exit", self.target], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - check=False, - ) - self._port_file.unlink(missing_ok=True) - class SSHClient(RemoteCompilationClient): """Client to exchange homcc protocol messages with a remote server through an SSH tunnel to a running `homccd`.""" @@ -200,6 +200,5 @@ def __init__(self, host: Host, timeout: float, state: StateFile, tunnel: SSHTunn async def _open_connection(self) -> Tuple[asyncio.StreamReader, asyncio.StreamWriter]: # establishing/reusing the SSH master may block on subprocess calls, so run it off the event loop; the overall # setup and connect is still bounded by the connection timeout applied by the base class - loop = asyncio.get_event_loop() - local_port: int = await loop.run_in_executor(None, self._tunnel.ensure, self.timeout) + local_port: int = await asyncio.to_thread(self._tunnel.ensure, self.timeout) return await asyncio.open_connection(host="127.0.0.1", port=local_port, limit=TCP_BUFFER_SIZE) diff --git a/tests/client/ssh_test.py b/tests/client/ssh_test.py index 480f7a0..1e9135e 100644 --- a/tests/client/ssh_test.py +++ b/tests/client/ssh_test.py @@ -48,7 +48,7 @@ def test_control_args_enable_multiplexing(self): tunnel = SSHTunnel( Host.from_str("user@buildhost"), ssh_executable="/usr/bin/ssh", ssh_options=["-o", "BatchMode=yes"] ) - args = tunnel._control_args() + args = tunnel.control_args assert args[0] == "/usr/bin/ssh" assert "ControlMaster=auto" in args @@ -91,12 +91,23 @@ def test_ensure_raises_ssh_error_on_failure(self, tmp_path: Path, mocker: Mocker mocker.patch("homcc.client.ssh._find_free_local_port", return_value=45678) mocker.patch( "homcc.client.ssh.subprocess.run", - return_value=subprocess.CompletedProcess([], 255, stderr=b"Permission denied"), + side_effect=subprocess.CalledProcessError(255, cmd=[], stderr=b"Permission denied"), ) with pytest.raises(SSHError, match="Permission denied"): tunnel.ensure(timeout=10) + def test_ensure_raises_ssh_error_when_executable_missing(self, tmp_path: Path, mocker: MockerFixture): + mocker.patch("homcc.client.ssh._ssh_base_dir", return_value=tmp_path) + tunnel = SSHTunnel(Host.from_str("user@buildhost")) + + mocker.patch.object(tunnel, "_is_master_alive", return_value=False) + mocker.patch("homcc.client.ssh._find_free_local_port", return_value=45678) + mocker.patch("homcc.client.ssh.subprocess.run", side_effect=FileNotFoundError("ssh: No such file")) + + with pytest.raises(SSHError, match="Could not execute"): + tunnel.ensure(timeout=10) + def test_ensure_raises_ssh_error_on_timeout(self, tmp_path: Path, mocker: MockerFixture): mocker.patch("homcc.client.ssh._ssh_base_dir", return_value=tmp_path) tunnel = SSHTunnel(Host.from_str("user@buildhost"))