diff --git a/.env.example b/.env.example index f4cf1855..7691df50 100644 --- a/.env.example +++ b/.env.example @@ -3,7 +3,7 @@ TC_HOST='root@192.168.1.101' TC_PASSWORD='' -TC_SSH_OPTS='-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedAlgorithms=+ssh-rsa -o KexAlgorithms=+diffie-hellman-group14-sha1 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null' +TC_SSH_OPTS='-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedAlgorithms=+ssh-rsa -o KexAlgorithms=+diffie-hellman-group14-sha1 -o StrictHostKeyChecking=accept-new -o UserKnownHostsFile=~/.tcapsule_known_hosts' TC_INTERNAL_SHARE_USE_DISK_ROOT='false' TC_SMB_BROWSE_COMPATIBILITY='false' TC_ANY_PROTOCOL='false' diff --git a/DETAIL.md b/DETAIL.md index 9daa94a7..6dd5a565 100644 --- a/DETAIL.md +++ b/DETAIL.md @@ -893,7 +893,7 @@ Current defaults: - `TC_INTERNAL_SHARE_USE_DISK_ROOT=false` - `TC_ATA_IDLE_SECONDS=300` - `TC_ATA_STANDBY=` leaves the standby timer unchanged; set `0` to disable standby -- `TC_SSH_OPTS` includes the legacy SSH algorithms required by AirPort firmware +- `TC_SSH_OPTS` includes the legacy SSH algorithms required by AirPort firmware and pins the device host key on first use in `~/.tcapsule_known_hosts` - docs and examples use SMB username `admin` - the managed payload directory is fixed at `.samba4` @@ -902,7 +902,7 @@ Samba NetBIOS, Samba server string, Bonjour instance, and Bonjour host labels ar Current validation behavior: - `TC_HOST`: must be non-empty. - `TC_PASSWORD`: must be present for commands that authenticate to the device. -- `TC_SSH_OPTS`: is written by `configure` with the legacy SSH options needed for AirPort firmware. +- `TC_SSH_OPTS`: is written by `configure` with the legacy SSH options needed for AirPort firmware. It now pins the device host key on first use (`StrictHostKeyChecking=accept-new`) in `~/.tcapsule_known_hosts` instead of disabling host-key checking, so a later key change is detected. `doctor` warns if a saved `.env` still uses `StrictHostKeyChecking=no`. If the device is factory-reset and its key legitimately changes, remove its entry from `~/.tcapsule_known_hosts` (or delete the file) and rerun. - `TC_INTERNAL_SHARE_USE_DISK_ROOT`: hidden boolean; internal disks use `ShareRoot` by default, and external disks always use the disk root. - `TC_ATA_IDLE_SECONDS`: optional non-negative integer; default `300`, and `0` disables the ATA idle timer through `atactl setidle 0`. - `TC_ATA_STANDBY`: optional non-negative integer; blank leaves standby unchanged, and `0` disables standby through `atactl setstandby 0`. diff --git a/src/timecapsulesmb/checks/doctor_steps.py b/src/timecapsulesmb/checks/doctor_steps.py index 1a83e36e..5844e940 100644 --- a/src/timecapsulesmb/checks/doctor_steps.py +++ b/src/timecapsulesmb/checks/doctor_steps.py @@ -61,7 +61,7 @@ parse_xattr_tdb_paths, ) from timecapsulesmb.checks.smb_targets import doctor_smb_servers -from timecapsulesmb.core.config import AppConfig, DEFAULT_SAMBA_AUTH_USER, validate_app_config +from timecapsulesmb.core.config import AppConfig, DEFAULT_SAMBA_AUTH_USER, ssh_opts_disable_host_key_checking, validate_app_config from timecapsulesmb.core.release import CLI_VERSION_CODE, RELEASE_TAG from timecapsulesmb.core.net import endpoint_host from timecapsulesmb.device.compat import is_netbsd4_payload_family, is_netbsd6_payload_family, render_compatibility_message @@ -343,6 +343,13 @@ def _add_config_validation_results( add_result(CheckResult("PASS", f"{config.path} contains all required settings")) + if ssh_opts_disable_host_key_checking(config.get("TC_SSH_OPTS")): + add_result(CheckResult( + "WARN", + "TC_SSH_OPTS disables SSH host-key checking (StrictHostKeyChecking=no / " + "UserKnownHostsFile=/dev/null); rerun `tcapsule configure` to pin the device host key", + )) + for result in check_required_local_tools(): add_result(result) for result in check_required_artifacts(repo_root): diff --git a/src/timecapsulesmb/core/config.py b/src/timecapsulesmb/core/config.py index 4f56bdae..ccdf40c1 100644 --- a/src/timecapsulesmb/core/config.py +++ b/src/timecapsulesmb/core/config.py @@ -60,7 +60,7 @@ class AirportDeviceIdentity: DEFAULTS = { "TC_HOST": DEFAULT_SSH_TARGET_PLACEHOLDER, - "TC_SSH_OPTS": "-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedAlgorithms=+ssh-rsa -o KexAlgorithms=+diffie-hellman-group14-sha1 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null", + "TC_SSH_OPTS": "-o HostKeyAlgorithms=+ssh-rsa -o PubkeyAcceptedAlgorithms=+ssh-rsa -o KexAlgorithms=+diffie-hellman-group14-sha1 -o StrictHostKeyChecking=accept-new -o UserKnownHostsFile=~/.tcapsule_known_hosts", "TC_INTERNAL_SHARE_USE_DISK_ROOT": "false", "TC_SMB_BIND_LAN_ONLY": "false", "TC_SMB_BROWSE_COMPATIBILITY": "false", @@ -648,6 +648,13 @@ def preserved_env_file_values(values: dict[str, str]) -> dict[str, str]: return {key: value for key, value in values.items() if key not in ENV_FILE_OMIT_KEYS} +def ssh_opts_disable_host_key_checking(ssh_opts: str | None) -> bool: + if not ssh_opts: + return False + normalized = "".join(ssh_opts.lower().split()).replace('"', "").replace("'", "") + return "stricthostkeychecking=no" in normalized or "userknownhostsfile=/dev/null" in normalized + + def write_env_file(path: Path, values: dict[str, str]) -> None: text = render_env_text(values) tmp_name: str | None = None diff --git a/tests/test_checks.py b/tests/test_checks.py index 66788497..82f1b3a9 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -41,6 +41,7 @@ DOCTOR_STARTUP_GRACE_SECONDS, STARTUP_GRACE_DETAIL_KEY, STARTUP_GRACE_MASK, + _add_config_validation_results, _apply_startup_grace, ) from timecapsulesmb.checks.local_tools import check_required_local_tools @@ -478,6 +479,16 @@ def test_run_doctor_checks_stops_invalid_config_before_remote_checks(self) -> No bonjour.assert_not_called() smb_listing.assert_not_called() + def test_config_validation_warns_when_ssh_host_key_checking_disabled(self) -> None: + config = self.doctor_config( + self.valid_doctor_values(TC_SSH_OPTS="-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"), + exists=True, + ) + results: list[CheckResult] = [] + _add_config_validation_results(config, repo_root=REPO_ROOT, add_result=results.append) + warnings = [r for r in results if r.status == "WARN" and "host-key checking" in r.message] + self.assertTrue(warnings) + def test_run_doctor_checks_passes_when_deployed_version_matches_current_cli(self) -> None: run = self.run_doctor_with_mocks( ssh_login=mock.Mock(status="PASS", message="ssh ok"), diff --git a/tests/test_config.py b/tests/test_config.py index d1b23277..9c772e06 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -25,6 +25,7 @@ parse_env_file, parse_env_value, preserved_env_file_values, + ssh_opts_disable_host_key_checking, require_valid_app_config, render_env_text, validate_app_config, @@ -151,6 +152,24 @@ def test_render_env_text_contains_config_keys(self) -> None: self.assertIn("TC_ATA_STANDBY=''", rendered) self.assertIn("TC_CONFIGURE_ID=12345678-1234-1234-1234-123456789012", rendered) + def test_default_ssh_opts_pin_host_key(self) -> None: + opts = DEFAULTS["TC_SSH_OPTS"] + self.assertIn("StrictHostKeyChecking=accept-new", opts) + self.assertIn("UserKnownHostsFile=~/.tcapsule_known_hosts", opts) + self.assertNotIn("StrictHostKeyChecking=no", opts) + self.assertNotIn("/dev/null", opts) + + def test_ssh_opts_disable_host_key_checking_detection(self) -> None: + self.assertTrue(ssh_opts_disable_host_key_checking("-o StrictHostKeyChecking=no")) + self.assertTrue(ssh_opts_disable_host_key_checking("-o UserKnownHostsFile=/dev/null")) + self.assertTrue(ssh_opts_disable_host_key_checking("-o StrictHostKeyChecking\t=\tno")) + self.assertTrue(ssh_opts_disable_host_key_checking('-o UserKnownHostsFile="/dev/null"')) + self.assertTrue(ssh_opts_disable_host_key_checking("-o StrictHostKeyChecking='no'")) + self.assertFalse(ssh_opts_disable_host_key_checking(None)) + self.assertFalse(ssh_opts_disable_host_key_checking("")) + self.assertFalse(ssh_opts_disable_host_key_checking(DEFAULTS["TC_SSH_OPTS"])) + self.assertFalse(ssh_opts_disable_host_key_checking("-o ProxyJump=bastion")) + def test_render_env_text_preserves_custom_settings_but_omits_deprecated_keys(self) -> None: values = dict(DEFAULTS) values.update({