Skip to content
Draft
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
3 changes: 3 additions & 0 deletions supervisor/api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@ def _register_os(self, app: web.Application) -> None:
web.get("/os/datadisk/list", api_os.list_data),
web.post("/os/datadisk/wipe", api_os.wipe_data),
web.post("/os/boot-slot", api_os.set_boot_slot),
web.get("/os/ssh/authorized_keys", api_os.ssh_authorized_keys_list),
web.post("/os/ssh/authorized_keys", api_os.ssh_authorized_keys_add),
web.delete("/os/ssh/authorized_keys", api_os.ssh_authorized_keys_clear),
]
)

Expand Down
2 changes: 2 additions & 0 deletions supervisor/api/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
ATTR_IS_ACTIVE = "is_active"
ATTR_IS_OWNER = "is_owner"
ATTR_JOBS = "jobs"
ATTR_KEY = "key"
ATTR_KEYS = "keys"
ATTR_LLMNR = "llmnr"
ATTR_LLMNR_HOSTNAME = "llmnr_hostname"
ATTR_LOCAL_ONLY = "local_only"
Expand Down
4 changes: 2 additions & 2 deletions supervisor/api/middleware/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class _AppSecurityPatterns:
r"|/multicast/.+"
r"|/network/.+"
r"|/observer/.+"
r"|/os/(?!datadisk/wipe).+"
r"|/os/(?!datadisk/wipe|ssh/authorized_keys).+"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we make this a core only endpoint rather then a manager one? Putting it in manager would give many apps the ability to call it. I know its hardly the only attack avenue if you consider the idea of a malicious app but it still just doesn't seem like capability we want to allow apps to do.

The downside would be the SSH app also can't call it which is probably the one app we'd prefer to allow. But as long as we make the proposed UI this seems like an acceptable situation to only allow host ssh key management from HA UI and the host shell itself.

r"|/refresh_updates"
r"|/resolution/.+"
r"|/security/.+"
Expand Down Expand Up @@ -226,7 +226,7 @@ class _AppSecurityPatterns:
r"|/multicast/.+"
r"|/network/.+"
r"|/observer/.+"
r"|/os/(?!datadisk/wipe).+"
r"|/os/(?!datadisk/wipe|ssh/authorized_keys).+"
r"|/reload_updates"
r"|/resolution/.+"
r"|/security/.+"
Expand Down
61 changes: 61 additions & 0 deletions supervisor/api/os.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
ATTR_DEV_PATH,
ATTR_DEVICE,
ATTR_DISKS,
ATTR_KEY,
ATTR_KEYS,
ATTR_MODEL,
ATTR_STATUS,
ATTR_SYSTEM_HEALTH_LED,
Expand All @@ -61,6 +63,10 @@
# .Firmware D-Bus interface first shipped in this OS Agent release.
RPI_FIRMWARE_MIN_OS_AGENT_VERSION: AwesomeVersion = AwesomeVersion("1.9.0")

# Listing SSH authorized keys requires the ListSSHAuthKeys D-Bus method
# first shipped in this OS Agent release.
SSH_KEYS_LIST_MIN_OS_AGENT_VERSION: AwesomeVersion = AwesomeVersion("1.11.0")

# pylint: disable=no-value-for-parameter
SCHEMA_VERSION = vol.Schema({vol.Optional(ATTR_VERSION): version_tag})
SCHEMA_SET_BOOT_SLOT = vol.Schema({vol.Required(ATTR_BOOT_SLOT): vol.Coerce(BootSlot)})
Expand Down Expand Up @@ -89,6 +95,35 @@
vol.Optional(ATTR_SWAPPINESS): vol.All(int, vol.Range(min=0, max=200)),
}
)

# dropbear, which consumes authorized_keys on Home Assistant OS, ignores
# lines longer than 3000 bytes
SSH_AUTH_KEY_MAX_LENGTH = 3000

RE_SSH_KEY_CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]")
Comment thread
agners marked this conversation as resolved.


def ssh_auth_key(value: Any) -> str:
"""Run a basic sanity check on an SSH authorized key entry.

Proper key validation is done by OS Agent; reject only what could write
more than one authorized_keys line per key (control characters) or
produce a line dropbear ignores (too long).
"""
if not isinstance(value, str):
raise vol.Invalid("SSH public key must be a string")

key = value.strip()
# dropbear and OS Agent limit the line length in bytes, not characters
if not key or len(key.encode()) > SSH_AUTH_KEY_MAX_LENGTH:
raise vol.Invalid("SSH public key is empty or too long")
if RE_SSH_KEY_CONTROL_CHARS.search(key):
raise vol.Invalid("SSH public key contains control characters")

return key


SCHEMA_SSH_AUTHORIZED_KEY = vol.Schema({vol.Required(ATTR_KEY): ssh_auth_key})
# pylint: enable=no-value-for-parameter


Expand Down Expand Up @@ -153,6 +188,32 @@ async def set_boot_slot(self, request: web.Request) -> None:
body = await api_validate(SCHEMA_SET_BOOT_SLOT, request)
await asyncio.shield(self.sys_os.set_boot_slot(body[ATTR_BOOT_SLOT]))

@api_process
async def ssh_authorized_keys_list(self, request: web.Request) -> dict[str, Any]:
"""Return root's SSH authorized keys on the host."""
if (
not self.sys_dbus.agent.is_connected
or self.sys_dbus.agent.version < SSH_KEYS_LIST_MIN_OS_AGENT_VERSION
):
raise APINotFound(
f"OS Agent {SSH_KEYS_LIST_MIN_OS_AGENT_VERSION} or newer required "
"to list SSH authorized keys",
_LOGGER.debug,
)

return {ATTR_KEYS: await self.sys_dbus.agent.system.list_ssh_auth_keys()}

@api_process
async def ssh_authorized_keys_add(self, request: web.Request) -> None:
"""Add an SSH authorized key for root on the host."""
body = await api_validate(SCHEMA_SSH_AUTHORIZED_KEY, request)
await asyncio.shield(self.sys_os.add_ssh_authorized_key(body[ATTR_KEY]))

@api_process
def ssh_authorized_keys_clear(self, request: web.Request) -> Awaitable[None]:
"""Remove all SSH authorized keys of root on the host."""
return asyncio.shield(self.sys_os.clear_ssh_authorized_keys())

@api_process
async def list_data(self, request: web.Request) -> dict[str, Any]:
"""Return possible data targets."""
Expand Down
22 changes: 22 additions & 0 deletions supervisor/dbus/agent/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,25 @@ async def schedule_wipe_device(self) -> bool:
async def migrate_docker_storage_driver(self, backend: str) -> None:
"""Migrate Docker storage driver."""
await self.connected_dbus.System.call("migrate_docker_storage_driver", backend)

@dbus_connected
async def add_ssh_auth_key(self, key: str) -> None:
"""Append a public key to root's SSH authorized keys on the host.

OS Agent validates the key since 1.10.0; older releases write the
string verbatim to the authorized_keys file.
"""
await self.connected_dbus.System.call("add_ssh_auth_key", key)

@dbus_connected
async def clear_ssh_auth_keys(self) -> None:
"""Remove all of root's SSH authorized keys on the host."""
await self.connected_dbus.System.call("clear_ssh_auth_keys")

@dbus_connected
async def list_ssh_auth_keys(self) -> list[str]:
"""Return root's SSH authorized keys on the host.

Requires OS Agent 1.11.0 or newer.
"""
return await self.connected_dbus.System.call("list_ssh_auth_keys")
67 changes: 67 additions & 0 deletions supervisor/os/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
from ..exceptions import (
DBusError,
DBusNotConnectedError,
HassOSError,
HassOSJobError,
HassOSSlotNotFound,
HassOSSlotUpdateError,
HassOSUpdateError,
HostError,
)
from ..jobs.const import JobConcurrency, JobCondition
from ..jobs.decorator import Job
Expand All @@ -27,6 +29,16 @@

_LOGGER: logging.Logger = logging.getLogger(__name__)

# SSH service on Home Assistant OS consuming /root/.ssh/authorized_keys
DROPBEAR_SERVICE = "dropbear.service"

# OS Agent releases before this return the os.Remove error when clearing an
# already absent authorized_keys file (inverted error check)
CLEAR_SSH_AUTH_KEYS_FIXED_VERSION = AwesomeVersion("1.10.0")
CLEAR_SSH_AUTH_KEYS_MISSING_FILE_ERROR = (
"remove /root/.ssh/authorized_keys: no such file or directory"
)


@dataclass(slots=True, frozen=True)
class SlotStatus:
Expand Down Expand Up @@ -501,3 +513,58 @@ async def set_boot_slot(self, boot_name: str) -> None:

_LOGGER.info("Rebooting into new boot slot now")
await self.sys_host.control.reboot()

@Job(
name="os_manager_add_ssh_authorized_key",
conditions=[JobCondition.HAOS],
on_condition=HassOSJobError,
internal=True,
)
async def add_ssh_authorized_key(self, key: str) -> None:
"""Add an SSH authorized key for root on the host and start dropbear.

OS Agent validates the key since 1.10.0; older releases append it to
the authorized_keys file as submitted.
"""
_LOGGER.info("Adding SSH authorized key on host")
try:
await self.sys_dbus.agent.system.add_ssh_auth_key(key)
except DBusError as err:
raise HassOSError(
f"Can't add SSH authorized key: {err!s}", _LOGGER.error
) from err

# dropbear on Home Assistant OS is gated by
# ConditionFileNotEmpty=/root/.ssh/authorized_keys, which systemd only
# evaluates when the unit starts. A running dropbear re-reads the file
# on every authentication attempt and starting an active unit is a
# no-op, so only the stopped service needs this.
try:
await self.sys_host.services.start(DROPBEAR_SERVICE)
except (HostError, DBusError) as err:
raise HassOSError(
f"SSH authorized key written, but can't start dropbear: {err!s}",
_LOGGER.error,
) from err

@Job(
name="os_manager_clear_ssh_authorized_keys",
conditions=[JobCondition.HAOS],
on_condition=HassOSJobError,
internal=True,
)
async def clear_ssh_authorized_keys(self) -> None:
"""Remove all SSH authorized keys of root on the host."""
_LOGGER.info("Clearing SSH authorized keys on host")
try:
await self.sys_dbus.agent.system.clear_ssh_auth_keys()
except DBusError as err:
# On affected OS Agent releases the missing-file error is the
# empty state clearing aims for, so treat it as success there.
if (
self.sys_dbus.agent.version >= CLEAR_SSH_AUTH_KEYS_FIXED_VERSION
or CLEAR_SSH_AUTH_KEYS_MISSING_FILE_ERROR not in str(err)
):
raise HassOSError(
f"Can't clear SSH authorized keys: {err!s}", _LOGGER.error
) from err

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this stop dropbear service after a successful key clear? Since we've reset to initial state where the authorized key file is empty.

3 changes: 3 additions & 0 deletions tests/api/middleware/test_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ def _versioned_path(prefix: str, path: str) -> str:
("post", "/addons/abc123/restart", {"admin", "manager"}),
("post", "/addons/abc123/security", {"admin"}),
("post", "/os/datadisk/wipe", {"admin"}),
("get", "/os/ssh/authorized_keys", {"admin"}),
("post", "/os/ssh/authorized_keys", {"admin"}),
("delete", "/os/ssh/authorized_keys", {"admin"}),
("post", "/addons/self/sys_options", set()),
("post", "/addons/abc123/sys_options", set()),
],
Expand Down
Loading