diff --git a/sdcm/keystore.py b/sdcm/keystore.py index 34384a3fec2..64fc8cc6bcb 100644 --- a/sdcm/keystore.py +++ b/sdcm/keystore.py @@ -42,6 +42,10 @@ # client must always be pinned explicitly. KEYSTORE_SM_REGION = "us-east-1" +# Namespace the mirrored keystore entries live under in Secrets Manager, so a +# keystore file `foo.json` maps to the secret `sct/foo.json`. +KEYSTORE_SM_PREFIX = "sct/" + SSHKey = namedtuple("SSHKey", ["name", "public_key", "private_key"]) BOTO3_CLIENT_CREATION_LOCK = threading.Lock() @@ -98,7 +102,7 @@ def __init__(self, backend: str | None = None): self._cache: dict[str, bytes] = {} self._cache_lock = threading.Lock() self._backend = backend or os.environ.get("SCT_KEYSTORE_BACKEND") or "secretsmanager" - self._sm_prefix = os.environ.get("SCT_KEYSTORE_SM_PREFIX") or "sct/" + self._sm_prefix = os.environ.get("SCT_KEYSTORE_SM_PREFIX") or KEYSTORE_SM_PREFIX self._sm_region = os.environ.get("SCT_KEYSTORE_SM_REGION") or KEYSTORE_SM_REGION @property @@ -350,9 +354,18 @@ def get_obj_if_needed(self, key, local_path, permissions): self.download_file(filename=key, dest_filename=path) os.chmod(path=path, mode=permissions) if self._backend == "secretsmanager": - with open(f"{path}.version", "w", encoding="utf-8") as vf: + with open(version_path, "w", encoding="utf-8") as vf: vf.write(remote_version) + if self._backend == "secretsmanager": + # Match the key's permissions: the sidecar lands in the same + # directory (often ~/.ssh, where ssh rejects group/world readable + # content) and would otherwise inherit the umask. Enforced on every + # sync rather than only after a download, because a sidecar whose + # version is still current is never rewritten -- one left at 0o664 + # by an earlier sync would keep those permissions forever. + os.chmod(path=version_path, mode=permissions) + def sync(self, keys, local_path, permissions=0o777): """Syncs the local and remote copies from the configured backend.""" if not keys: diff --git a/test_sync.py b/test_sync.py index e0aeef4a10b..8ab25200a16 100644 --- a/test_sync.py +++ b/test_sync.py @@ -23,7 +23,7 @@ def is_using_aws_mock(): if not is_using_aws_mock(): key_store = KeyStore() key_store.sync( - keys=["scylla-qa-ec2", "scylla-test", "scylla_test_id_ed25519"] * 20, + keys=["scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"] * 30, local_path=Path("~/.ssh/").expanduser(), permissions=0o0600, ) diff --git a/unit_tests/test_aws_services.py b/unit_tests/test_aws_services.py index 3e4ead49fd2..03ee7e414ab 100644 --- a/unit_tests/test_aws_services.py +++ b/unit_tests/test_aws_services.py @@ -12,7 +12,7 @@ os.environ["MOTO_AMIS_PATH"] = str(Path(__file__).parent / "test_data" / "mocked_ami_data.json") from moto.server import ThreadedMotoServer -from sdcm.keystore import KeyStore +from sdcm.keystore import KEYSTORE_S3_BUCKET, KEYSTORE_SM_PREFIX, KEYSTORE_SM_REGION, KeyStore from sdcm.utils.aws_region import AwsRegion from sdcm.sct_provision.common.layout import SCTProvisionLayout, create_sct_configuration from sdcm.utils.common import get_scylla_ami_versions @@ -53,28 +53,27 @@ def moto_server(): @pytest.fixture(scope="module", autouse=True) def keystore_configure(moto_server): - s3 = boto3.resource(service_name="s3", region_name=AWS_REGION, endpoint_url=moto_server) + entries = { + "gcp-sct-project-1.json": b"{}", + "aws_images_role.json": ( + b'{"role_arn": "arn:aws:iam::123456789012:role/role-name", "role_session_name": "role-session-name"}' + ), + } + for file in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"): + entries[file] = (Path("~/.ssh").expanduser() / file).read_bytes() - bucket = s3.Bucket("scylla-qa-keystore") + s3 = boto3.resource(service_name="s3", region_name=AWS_REGION, endpoint_url=moto_server) + bucket = s3.Bucket(KEYSTORE_S3_BUCKET) bucket.create() - bucket.put_object(Key="gcp-sct-project-1.json", Body=b"{}") - bucket.put_object( - Key="aws_images_role.json", - Body=b'{"role_arn": "arn:aws:iam::123456789012:role/role-name", "role_session_name": "role-session-name"}', - ) - for file in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"): - bucket.upload_file(Filename=str(Path("~/.ssh").expanduser() / file), Key=file) - - # The Secrets Manager backend (the default) reads `sct/` secrets, - # so mirror the same entries into moto's Secrets Manager as well. - sm = boto3.client("secretsmanager", region_name=AWS_REGION, endpoint_url=moto_server) - sm.create_secret(Name="sct/gcp-sct-project-1.json", SecretString="{}") - sm.create_secret( - Name="sct/aws_images_role.json", - SecretString='{"role_arn": "arn:aws:iam::123456789012:role/role-name", "role_session_name": "role-session-name"}', - ) - for file in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.pub"): - sm.create_secret(Name=f"sct/{file}", SecretBinary=(Path("~/.ssh").expanduser() / file).read_bytes()) + for key, body in entries.items(): + bucket.put_object(Key=key, Body=body) + + # KeyStore reads from Secrets Manager by default, so mirror the same entries + # there under the `sct/` prefix - seeding only the bucket would leave every + # lookup failing with ResourceNotFoundException. + sm = boto3.client("secretsmanager", region_name=KEYSTORE_SM_REGION, endpoint_url=moto_server) + for key, body in entries.items(): + sm.create_secret(Name=f"{KEYSTORE_SM_PREFIX}{key}", SecretBinary=body) @pytest.fixture(scope="module") diff --git a/unit_tests/unit/test_keystore.py b/unit_tests/unit/test_keystore.py index b833fd23247..a2907b43aa1 100644 --- a/unit_tests/unit/test_keystore.py +++ b/unit_tests/unit/test_keystore.py @@ -22,6 +22,7 @@ import json import logging import os +import stat import time from concurrent.futures.thread import ThreadPoolExecutor from unittest.mock import MagicMock, PropertyMock, patch @@ -884,6 +885,24 @@ def test_sync_from_sm(self, sm_ks, tmp_path): # Version sidecar written alongside the key assert (tmp_path / "scylla_test_id_ed25519.version").exists() + def test_sync_from_sm_applies_permissions_to_sidecar(self, sm_ks, tmp_path): + """The sidecar must not inherit the umask - ssh rejects a group/world readable ~/.ssh.""" + sm_ks.sync(["scylla_test_id_ed25519"], str(tmp_path), 0o600) + for name in ("scylla_test_id_ed25519", "scylla_test_id_ed25519.version"): + assert stat.S_IMODE((tmp_path / name).stat().st_mode) == 0o600, f"{name} has wrong permissions" + + def test_sync_from_sm_repairs_sidecar_permissions_on_cache_hit(self, sm_ks, tmp_path): + """A sidecar left too permissive by an earlier sync is repaired without a re-download.""" + sm_ks.get_obj_if_needed("scylla_test_id_ed25519", str(tmp_path), 0o600) + sidecar = tmp_path / "scylla_test_id_ed25519.version" + sidecar.chmod(0o664) + + # Version is unchanged, so this must fix the permissions without downloading again + with patch.object(sm_ks, "download_file") as mock_dl: + sm_ks.get_obj_if_needed("scylla_test_id_ed25519", str(tmp_path), 0o600) + mock_dl.assert_not_called() + assert stat.S_IMODE(sidecar.stat().st_mode) == 0o600 + def test_get_sm_version_id_returns_awscurrent(self, sm_ks): version_id = sm_ks.get_sm_version_id("scylla_test_id_ed25519") assert version_id # non-empty UUID-like string