Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: allow tmate ip addresses to firewall. #209

Merged
merged 18 commits into from
Jan 29, 2024
Merged
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
6 changes: 6 additions & 0 deletions docs/explanation/ssh-debug.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,9 @@ by default on [tmate-ssh-server charm](https://charmhub.io/tmate-ssh-server/).
Authorized keys are registered via [action-tmate](https://github.com/canonical/action-tmate/)'s
`limit-access-to-actor` feature. This feature uses GitHub users's SSH key to launch an instance
of tmate session with `-a` option, which adds the user's SSH key to `~/.ssh/authorized_keys`.

### Firewall rules

By default, if there are any overlapping IPs within the `denylist` config option with the IP
assigned to `tmate-ssh-server`, an exception to that IP will be made so that the `debug-ssh`
relation can be setup correctly.
6 changes: 3 additions & 3 deletions src-docs/charm_state.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ Supported storage as runner disk.

---

## <kbd>class</kbd> `SSHDebugInfo`
## <kbd>class</kbd> `SSHDebugConnection`
SSH connection information for debug workflow.


Expand All @@ -183,7 +183,7 @@ SSH connection information for debug workflow.
### <kbd>classmethod</kbd> `from_charm`

```python
from_charm(charm: CharmBase) → list['SSHDebugInfo']
from_charm(charm: CharmBase) → list['SSHDebugConnection']
```

Initialize the SSHDebugInfo from charm relation data.
Expand All @@ -208,7 +208,7 @@ The charm state.
- <b>`proxy_config`</b>: Proxy-related configuration.
- <b>`charm_config`</b>: Configuration of the juju charm.
- <b>`arch`</b>: The underlying compute architecture, i.e. x86_64, amd64, arm64/aarch64.
- <b>`ssh_debug_infos`</b>: SSH debug connections configuration information.
- <b>`ssh_debug_connections`</b>: SSH debug connections configuration information.



Expand Down
13 changes: 8 additions & 5 deletions src-docs/firewall.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The runner firewall manager.
## <kbd>class</kbd> `Firewall`
Represent a firewall and provides methods to refresh its configuration.

<a href="../src/firewall.py#L50"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/firewall.py#L52"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `__init__`

Expand All @@ -33,7 +33,7 @@ Initialize a new Firewall instance.

---

<a href="../src/firewall.py#L58"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/firewall.py#L60"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `get_host_ip`

Expand All @@ -50,12 +50,15 @@ Get the host IP address for the corresponding LXD network.

---

<a href="../src/firewall.py#L69"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/firewall.py#L100"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>function</kbd> `refresh_firewall`

```python
refresh_firewall(denylist: List[FirewallEntry])
refresh_firewall(
denylist: Iterable[FirewallEntry],
allowlist: Optional[Iterable[FirewallEntry]] = None
)
```

Refresh the firewall configuration.
Expand Down Expand Up @@ -83,7 +86,7 @@ Represent an entry in the firewall.

---

<a href="../src/firewall.py#L25"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>
<a href="../src/firewall.py#L27"><img align="right" style="float:right;" src="https://img.shields.io/badge/-source-cccccc?style=flat-square"></a>

### <kbd>classmethod</kbd> `decode`

Expand Down
2 changes: 1 addition & 1 deletion src-docs/runner_type.py.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ Configuration for runner.
- <b>`path`</b>: GitHub repository path in the format '<owner>/<repo>', or the GitHub organization name.
- <b>`proxies`</b>: HTTP(S) proxy settings.
- <b>`dockerhub_mirror`</b>: URL of dockerhub mirror to use.
- <b>`ssh_debug_infos`</b>: The SSH debug server connections metadata.
- <b>`ssh_debug_connections`</b>: The SSH debug server connections metadata.



Expand Down
6 changes: 5 additions & 1 deletion src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -892,8 +892,11 @@ def _refresh_firewall(self):
FirewallEntry.decode(entry.strip())
for entry in firewall_denylist_config.split(",")
]
allowlist = [
FirewallEntry.decode(str(entry.host)) for entry in self._state.ssh_debug_connections
]
firewall = Firewall("lxdbr0")
firewall.refresh_firewall(denylist)
firewall.refresh_firewall(denylist=denylist, allowlist=allowlist)
logger.debug(
"firewall update, current firewall: %s",
execute_command(["/usr/sbin/nft", "list", "ruleset"]),
Expand All @@ -912,6 +915,7 @@ def _apt_install(self, packages: Sequence[str]) -> None:

def _on_debug_ssh_relation_changed(self, _: ops.RelationChangedEvent) -> None:
"""Handle debug ssh relation changed event."""
self._refresh_firewall()
runner_manager = self._get_runner_manager()
runner_manager.flush(flush_busy=False)
self._reconcile_runners(runner_manager)
Expand Down
20 changes: 10 additions & 10 deletions src/charm_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def _get_supported_arch() -> ARCH:
raise UnsupportedArchitectureError(arch=arch)


class SSHDebugInfo(BaseModel):
class SSHDebugConnection(BaseModel):
"""SSH connection information for debug workflow.

Attributes:
Expand All @@ -200,13 +200,13 @@ class SSHDebugInfo(BaseModel):
ed25519_fingerprint: str = Field(pattern="^SHA256:.*")

@classmethod
def from_charm(cls, charm: CharmBase) -> list["SSHDebugInfo"]:
def from_charm(cls, charm: CharmBase) -> list["SSHDebugConnection"]:
"""Initialize the SSHDebugInfo from charm relation data.

Args:
charm: The charm instance.
"""
ssh_debug_connections: list[SSHDebugInfo] = []
ssh_debug_connections: list[SSHDebugConnection] = []
relations = charm.model.relations[DEBUG_SSH_INTEGRATION_NAME]
if not relations or not (relation := relations[0]).units:
return ssh_debug_connections
Expand All @@ -223,7 +223,7 @@ def from_charm(cls, charm: CharmBase) -> list["SSHDebugInfo"]:
)
continue
ssh_debug_connections.append(
SSHDebugInfo(
SSHDebugConnection(
host=host,
port=port,
rsa_fingerprint=rsa_fingerprint,
Expand All @@ -242,14 +242,14 @@ class State:
proxy_config: Proxy-related configuration.
charm_config: Configuration of the juju charm.
arch: The underlying compute architecture, i.e. x86_64, amd64, arm64/aarch64.
ssh_debug_infos: SSH debug connections configuration information.
ssh_debug_connections: SSH debug connections configuration information.
"""

is_metrics_logging_available: bool
proxy_config: ProxyConfig
charm_config: CharmConfig
arch: ARCH
ssh_debug_infos: list[SSHDebugInfo]
ssh_debug_connections: list[SSHDebugConnection]

@classmethod
def from_charm(cls, charm: CharmBase) -> "State":
Expand Down Expand Up @@ -298,7 +298,7 @@ def from_charm(cls, charm: CharmBase) -> "State":
raise CharmConfigInvalidError(f"Unsupported architecture {exc.arch}") from exc

try:
ssh_debug_infos = SSHDebugInfo.from_charm(charm)
ssh_debug_connections = SSHDebugConnection.from_charm(charm)
except ValidationError as exc:
logger.error("Invalid SSH debug info: %s.", exc)
raise CharmConfigInvalidError("Invalid SSH Debug info") from exc
Expand All @@ -308,15 +308,15 @@ def from_charm(cls, charm: CharmBase) -> "State":
proxy_config=proxy_config,
charm_config=charm_config,
arch=arch,
ssh_debug_infos=ssh_debug_infos,
ssh_debug_connections=ssh_debug_connections,
)

state_dict = dataclasses.asdict(state)
# Convert pydantic object to python object serializable by json module.
state_dict["proxy_config"] = json.loads(state_dict["proxy_config"].json())
state_dict["charm_config"] = json.loads(state_dict["charm_config"].json())
state_dict["ssh_debug_infos"] = [
debug_info.json() for debug_info in state_dict["ssh_debug_infos"]
state_dict["ssh_debug_connections"] = [
debug_info.json() for debug_info in state_dict["ssh_debug_connections"]
]
json_data = json.dumps(state_dict, ensure_ascii=False)
CHARM_STATE_PATH.write_text(json_data, encoding="utf-8")
Expand Down
71 changes: 53 additions & 18 deletions src/firewall.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

from utilities import execute_command

NetworkT = typing.TypeVar("NetworkT", ipaddress.IPv4Network, ipaddress.IPv6Network)


@dataclasses.dataclass
class FirewallEntry:
Expand Down Expand Up @@ -66,7 +68,40 @@ def get_host_ip(self) -> str:
)
return str(ipaddress.IPv4Interface(address.strip()).ip)

def refresh_firewall(self, denylist: typing.List[FirewallEntry]):
def _exclude_network(
self,
networks: list[NetworkT],
exclude: list[NetworkT],
) -> list[NetworkT]:
"""Excludes the network segment from a pool of networks.

Args:
networks: The network pool to apply.
exclude: The networks to exclude from the pool.

Returns:
The network pool without the network segments in excludes.
"""
total_networks_without_excluded = networks

for exclude_net in exclude:
scoped_excluded_networks: list[NetworkT] = []
for net in total_networks_without_excluded:
if net == exclude_net or net.subnet_of(exclude_net):
continue
if net.overlaps(exclude_net):
scoped_excluded_networks.extend(net.address_exclude(exclude_net))
else:
scoped_excluded_networks.append(net)
total_networks_without_excluded = scoped_excluded_networks

return total_networks_without_excluded

def refresh_firewall(
self,
denylist: typing.Iterable[FirewallEntry],
allowlist: typing.Iterable[FirewallEntry] | None = None,
):
"""Refresh the firewall configuration.

Args:
Expand Down Expand Up @@ -128,23 +163,23 @@ def refresh_firewall(self, denylist: typing.List[FirewallEntry]):
"state": "enabled",
},
]
host_network = ipaddress.IPv4Network(host_ip)
for entry in denylist:
entry_network = ipaddress.IPv4Network(entry.ip_range)
try:
excluded = list(entry_network.address_exclude(host_network))
except ValueError:
excluded = [entry_network]
egress_rules.extend(
[
{
"action": "reject",
"destination": str(ip),
"state": "enabled",
}
for ip in excluded
]
)

allowed_ips = [
ipaddress.IPv4Network(host_ip),
*[ipaddress.IPv4Network(entry.ip_range) for entry in (allowlist or [])],
]
ips_to_deny = [ipaddress.IPv4Network(entry.ip_range) for entry in denylist]
denied_ips = self._exclude_network(networks=ips_to_deny, exclude=allowed_ips)
egress_rules.extend(
[
{
"action": "reject",
"destination": str(ip),
"state": "enabled",
}
for ip in denied_ips
]
)
acl_config["egress"] = egress_rules
execute_command(
["lxc", "network", "acl", "edit", self._ACL_RULESET_NAME],
Expand Down
8 changes: 5 additions & 3 deletions src/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import yaml

import shared_fs
from charm_state import ARCH, SSHDebugInfo
from charm_state import ARCH, SSHDebugConnection
from errors import (
CreateSharedFilesystemError,
LxdError,
Expand Down Expand Up @@ -672,8 +672,10 @@ def _configure_runner(self) -> None:
# As the user already has sudo access, this does not give the user any additional access.
self.instance.execute(["/usr/bin/sudo", "chmod", "777", "/usr/local/bin"])

selected_ssh_connection: SSHDebugInfo | None = (
secrets.choice(self.config.ssh_debug_infos) if self.config.ssh_debug_infos else None
selected_ssh_connection: SSHDebugConnection | None = (
secrets.choice(self.config.ssh_debug_connections)
if self.config.ssh_debug_connections
else None
)
logger.info("SSH Debug info: %s", selected_ssh_connection)
# Load `/etc/environment` file.
Expand Down
4 changes: 2 additions & 2 deletions src/runner_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ def _spawn_new_runners(self, count: int, resources: VirtualMachineResources):
path=self.config.path,
proxies=self.proxies,
name=self._generate_runner_name(),
ssh_debug_infos=self.config.charm_state.ssh_debug_infos,
ssh_debug_connections=self.config.charm_state.ssh_debug_connections,
)
runner = Runner(self._clients, config, RunnerStatus())
try:
Expand Down Expand Up @@ -611,7 +611,7 @@ def create_runner_info(
name=name,
path=self.config.path,
proxies=self.proxies,
ssh_debug_infos=self.config.charm_state.ssh_debug_infos,
ssh_debug_connections=self.config.charm_state.ssh_debug_connections,
)
return Runner(
self._clients,
Expand Down
6 changes: 3 additions & 3 deletions src/runner_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from pathlib import Path
from typing import NamedTuple, Optional, TypedDict, Union

from charm_state import SSHDebugInfo
from charm_state import SSHDebugConnection


@dataclass
Expand Down Expand Up @@ -77,7 +77,7 @@ class RunnerConfig: # pylint: disable=too-many-instance-attributes
name.
proxies: HTTP(S) proxy settings.
dockerhub_mirror: URL of dockerhub mirror to use.
ssh_debug_infos: The SSH debug server connections metadata.
ssh_debug_connections: The SSH debug server connections metadata.
"""

app_name: str
Expand All @@ -87,7 +87,7 @@ class RunnerConfig: # pylint: disable=too-many-instance-attributes
path: GithubPath
proxies: ProxySetting
dockerhub_mirror: str | None = None
ssh_debug_infos: SSHDebugInfo | None = None
ssh_debug_connections: SSHDebugConnection | None = None


@dataclass
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ async def tmate_ssh_server_app_fixture(
"""tmate-ssh-server charm application related to GitHub-Runner app charm."""
tmate_app: Application = await model.deploy("tmate-ssh-server", channel="edge")
await app_no_wait.relate("debug-ssh", f"{tmate_app.name}:debug-ssh")
await model.wait_for_idle(status=ACTIVE, timeout=60 * 30)
await model.wait_for_idle(apps=[tmate_app.name], status=ACTIVE, timeout=60 * 30)

return tmate_app

Expand Down
Loading
Loading