diff --git a/charts/qiskit-serverless/charts/gateway/templates/deployment.yaml b/charts/qiskit-serverless/charts/gateway/templates/deployment.yaml index 6730776321..460a2dc156 100644 --- a/charts/qiskit-serverless/charts/gateway/templates/deployment.yaml +++ b/charts/qiskit-serverless/charts/gateway/templates/deployment.yaml @@ -330,7 +330,7 @@ spec: volumeMounts: - mountPath: "/usr/src/app/media/" name: gateway-pv-storage - - mountPath: "/tmp/templates/" + - mountPath: "/etc/gateway/templates/" name: ray-cluster-template - mountPath: "/tmp/gpujobs/" name: gpu-jobs diff --git a/client/qiskit_serverless/core/files.py b/client/qiskit_serverless/core/files.py index 68d52c6818..a6dda4ea35 100644 --- a/client/qiskit_serverless/core/files.py +++ b/client/qiskit_serverless/core/files.py @@ -98,7 +98,9 @@ def _download_with_url( # pylint: disable=too-many-positional-arguments total_size_in_bytes = int(req.headers.get("content-length", 0)) chunk_size = 8192 progress_bar = tqdm(total=total_size_in_bytes, unit="iB", unit_scale=True) - file_name = target_name or f"downloaded_{str(uuid.uuid4())[:8]}_{file}" + # Strip any directory components so a crafted target_name/file + # (e.g. "../../etc/passwd") cannot escape download_location. + file_name = os.path.basename(target_name or f"downloaded_{str(uuid.uuid4())[:8]}_{file}") with open(os.path.join(download_location, file_name), "wb") as f: for chunk in req.iter_content(chunk_size=chunk_size): progress_bar.update(len(chunk)) diff --git a/client/qiskit_serverless/serializers/program_serializers.py b/client/qiskit_serverless/serializers/program_serializers.py index 6296b50f52..a5814d499e 100644 --- a/client/qiskit_serverless/serializers/program_serializers.py +++ b/client/qiskit_serverless/serializers/program_serializers.py @@ -67,12 +67,23 @@ def object_hook(self, obj: Any) -> Any: if "__type__" in obj: obj_type = obj["__type__"] + # Only reconstruct known types, and only from a well-formed mapping + # of string keys. This prevents a malicious/compromised gateway from + # splatting an arbitrarily-shaped "__value__" into these + # constructors (which would otherwise be attacker-controlled kwargs). + value = obj.get("__value__") + + def _safe_kwargs() -> dict: + if not isinstance(value, dict) or not all(isinstance(k, str) for k in value): + raise ValueError(f"Invalid '__value__' payload for type '{obj_type}'.") + return value + if obj_type == "QiskitRuntimeService": - return QiskitRuntimeService(**obj["__value__"]) + return QiskitRuntimeService(**_safe_kwargs()) if obj_type == "SamplerResult": - return SamplerResult(**obj["__value__"]) + return SamplerResult(**_safe_kwargs()) if obj_type == "EstimatorResult": - return EstimatorResult(**obj["__value__"]) + return EstimatorResult(**_safe_kwargs()) return super().object_hook(obj) return obj diff --git a/docker-compose.yaml b/docker-compose.yaml index 4d6cac4fd1..5be5fdc5ce 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -47,6 +47,9 @@ services: user: "root" # root user is needed to write on volumes environment: - DEBUG=0 + # Local-only dev secret. With DEBUG=0 the app fails closed without this. + - DJANGO_SECRET_KEY=django-insecure-local-compose-only-change-me + - ALLOWED_HOSTS=* - RAY_HOST=http://ray-head:8265 - RAY_CLUSTER_MODE_LOCAL=true - DJANGO_SUPERUSER_USERNAME=admin @@ -76,6 +79,9 @@ services: entrypoint: "./entrypoint-scheduler.sh" environment: - DEBUG=0 + # Local-only dev secret. With DEBUG=0 the app fails closed without this. + - DJANGO_SECRET_KEY=django-insecure-local-compose-only-change-me + - ALLOWED_HOSTS=* - DATABASE_HOST=postgres - DATABASE_PORT=5432 - DATABASE_NAME=serverlessdb diff --git a/gateway/api/serializers.py b/gateway/api/serializers.py index f4f065c88b..86dee13593 100644 --- a/gateway/api/serializers.py +++ b/gateway/api/serializers.py @@ -194,6 +194,22 @@ class ProgramSerializer(serializers.ModelSerializer): class Meta: model = Program + # Never serialize all fields implicitly: `env_vars` holds encrypted + # tokens. An explicit allowlist prevents credential leakage if this base + # serializer is ever used directly (subclasses override `fields`). + fields = [ + "id", + "title", + "entrypoint", + "artifact", + "dependencies", + "provider", + "description", + "documentation_url", + "type", + "version", + "runner", + ] class JobSerializer(serializers.ModelSerializer): @@ -203,6 +219,18 @@ class JobSerializer(serializers.ModelSerializer): class Meta: model = Job + # Explicit allowlist: never expose `env_vars` (encrypted token + args), + # `account_id` or `instance_crn` implicitly. Subclasses override. + fields = [ + "id", + "result", + "status", + "program", + "created", + "sub_status", + "fleet_id", + "compute_profile", + ] class JobSerializerWithoutResult(serializers.ModelSerializer): @@ -212,6 +240,15 @@ class JobSerializerWithoutResult(serializers.ModelSerializer): class Meta: model = Job + fields = [ + "id", + "status", + "program", + "created", + "sub_status", + "fleet_id", + "compute_profile", + ] class RunProgramSerializer(serializers.Serializer): diff --git a/gateway/api/v1/serializers.py b/gateway/api/v1/serializers.py index 10439b1d0e..63edf7f814 100644 --- a/gateway/api/v1/serializers.py +++ b/gateway/api/v1/serializers.py @@ -4,6 +4,7 @@ import json import logging +import re from typing import Any from packaging.requirements import Requirement, InvalidRequirement @@ -16,6 +17,30 @@ logger = logging.getLogger("api.api.v1.serializers") +# Strict OCI/Docker image reference grammar: +# [registry[:port]/]repository[:tag][@digest] +# This rejects shell metacharacters, whitespace and otherwise malformed +# references before they ever reach the runner / orchestration objects. +IMAGE_REFERENCE_RE = re.compile( + r"^" + r"(?:(?P[a-z0-9]+(?:[.-][a-z0-9]+)*(?::[0-9]+)?)/)?" # optional registry host[:port]/ + r"(?P[a-z0-9]+(?:[._-][a-z0-9]+)*(?:/[a-z0-9]+(?:[._-][a-z0-9]+)*)*)" # repository path + r"(?::(?P[A-Za-z0-9_][A-Za-z0-9._-]{0,127}))?" # optional tag + r"(?:@(?P[A-Za-z0-9]+:[A-Fa-f0-9]{32,}))?" # optional digest + r"$" +) + + +def _image_in_registry(image: str, registry: str) -> bool: + """Return True only when ``image`` is hosted under ``registry``. + + Enforces a path boundary so that a registry of ``docker.io`` is NOT + satisfied by ``docker.io.attacker.com/evil`` (the bug a bare + ``str.startswith`` introduces). + """ + registry = registry.rstrip("/") + return image == registry or image.startswith(registry + "/") + class ProgramSerializer(serializers.ProgramSerializer): """ @@ -52,9 +77,38 @@ class UploadProgramSerializer(serializers.UploadProgramSerializer): UploadProgramSerializer is used by the /upload end-point """ + def validate_entrypoint(self, value): + """Validate the entrypoint is a safe, relative ``.py`` file path. + + The entrypoint is interpolated into the runner's execution command + (e.g. ``python {entrypoint}`` for Ray) and into COS/PDS object paths, so + it must not contain shell metacharacters, be absolute, or traverse + outside the function directory via ``..``. + """ + if value is None: + return value + segments = value.split("/") + if ( + not isinstance(value, str) + or not re.fullmatch(r"[A-Za-z0-9_./-]+\.py", value) + or value.startswith("/") + or ".." in segments + ): + raise ValidationError( + "Invalid entrypoint. It must be a relative path to a .py file " + "without '..' segments or shell characters." + ) + return value + def validate_image(self, value): - """Validates image.""" - # place to add image validation + """Validate that the image is a well-formed container reference.""" + if value is None: + return value + if not isinstance(value, str) or not IMAGE_REFERENCE_RE.match(value): + raise ValidationError( + "Invalid image reference. Expected format: " + "[registry[:port]/]repository[:tag][@digest]." + ) return value def _parse_dependency(self, dep: Any): @@ -139,7 +193,7 @@ def validate(self, attrs): # pylint: disable=too-many-branches provider_instance = Provider.objects.filter(name=provider).first() if provider_instance is None: raise ValidationError(f"{provider} is not valid provider.") - if provider_instance.registry and not image.startswith(provider_instance.registry): + if provider_instance.registry and not _image_in_registry(image, provider_instance.registry): raise ValidationError(f"Custom images must be in {provider_instance.registry}.") # Validate `version` using packaging.version (PEP 440 compatible) diff --git a/gateway/api/v1/views/programs.py b/gateway/api/v1/views/programs.py index 1280c2222e..97c03b41a2 100644 --- a/gateway/api/v1/views/programs.py +++ b/gateway/api/v1/views/programs.py @@ -40,6 +40,10 @@ def get_serializer_run_job(*args, **kwargs): def get_serializer_job(*args, **kwargs): return v1_serializers.JobSerializer(*args, **kwargs) + @staticmethod + def get_serializer_job_without_result(*args, **kwargs): + return v1_serializers.JobSerializerWithoutResult(*args, **kwargs) + @swagger_auto_schema( operation_description="List author Qiskit Functions", manual_parameters=[ diff --git a/gateway/api/views/programs.py b/gateway/api/views/programs.py index f629078d23..4b1ba11a21 100644 --- a/gateway/api/views/programs.py +++ b/gateway/api/views/programs.py @@ -24,6 +24,7 @@ JobConfigSerializer, RunJobSerializer, JobSerializer, + JobSerializerWithoutResult, RunProgramSerializer, UploadProgramSerializer, ) @@ -93,6 +94,15 @@ def get_serializer_job(*args, **kwargs): return JobSerializer(*args, **kwargs) + @staticmethod + def get_serializer_job_without_result(*args, **kwargs): + """ + This method returns the job serializer that omits the (author-private) + result field, for listings that a provider can read across authors. + """ + + return JobSerializerWithoutResult(*args, **kwargs) + def get_serializer_class(self): return self.serializer_class @@ -425,10 +435,14 @@ def get_jobs(self, request, pk=None): # pylint: disable=invalid-name,unused-arg ) if user_is_provider: + # A provider admin may list every author's jobs for the function, + # but results are author-private (gated by JobAccessPolicies + # everywhere else), so never serialize the `result` field here. jobs = Job.objects.filter(program=program) + serializer = self.get_serializer_job_without_result(jobs, many=True) else: jobs = Job.objects.filter(program=program, author=request.user) - serializer = self.get_serializer_job(jobs, many=True) + serializer = self.get_serializer_job(jobs, many=True) logger.info( "[programs-get-jobs] user_id=%s program_id=%s program=%s | Jobs listed ok", request.user.id, diff --git a/gateway/core/ibm_cloud/code_engine/fleets/utils.py b/gateway/core/ibm_cloud/code_engine/fleets/utils.py index 924f8e9fb2..90bb9f673e 100644 --- a/gateway/core/ibm_cloud/code_engine/fleets/utils.py +++ b/gateway/core/ibm_cloud/code_engine/fleets/utils.py @@ -169,6 +169,12 @@ def build_run_env_variables( """ env = dict(stored_env_vars) + # The full JSON job arguments are delivered to the container via COS at + # ARGUMENTS_PATH (mounted file), so the duplicate ENV_JOB_ARGUMENTS literal + # env var only widens plaintext exposure of (potentially sensitive) user + # arguments in the Code Engine control plane. Drop it for fleets. + env.pop("ENV_JOB_ARGUMENTS", None) + gateway_host = getattr(settings, "FLEETS_GATEWAY_HOST", None) if gateway_host: env["ENV_JOB_GATEWAY_HOST"] = gateway_host diff --git a/gateway/core/services/runners/fleets_runner.py b/gateway/core/services/runners/fleets_runner.py index 41fbd975e5..ba714419e1 100644 --- a/gateway/core/services/runners/fleets_runner.py +++ b/gateway/core/services/runners/fleets_runner.py @@ -16,6 +16,7 @@ import json import logging +import os import re import tarfile import time @@ -493,17 +494,31 @@ def _upload_artifact_to_cos(self, paths: FleetJobPaths) -> None: for member in tar.getmembers(): if not member.isfile(): continue + + # Defend against path traversal: a crafted archive could use + # member names like "../../other-user/results.json" to write + # COS keys outside this job's prefix. Reject absolute paths + # and any name that escapes via "..". + normalized_name = os.path.normpath(member.name) + if os.path.isabs(normalized_name) or normalized_name.startswith(".."): + logger.warning( + "Skipping unsafe artifact member [%s] for job_id=%s", + member.name, + self.job.id, + ) + continue + extracted = tar.extractfile(member) if extracted is None: continue - if member.name == entrypoint_name: + if normalized_name == entrypoint_name: bucket_name = provider_bucket if is_provider else user_bucket prefix = paths.cos_provider_function_prefix if is_provider else paths.cos_user_function_prefix - key = f"{prefix}/{member.name}" + key = f"{prefix}/{normalized_name}" else: bucket_name = user_bucket - key = f"{paths.cos_user_job_prefix}/{member.name}" + key = f"{paths.cos_user_job_prefix}/{normalized_name}" self._get_cos().upload_fileobj(fileobj=extracted, bucket_name=bucket_name, key=key) logger.debug("Uploaded [%s] for job_id=%s to %s/%s", member.name, self.job.id, bucket_name, key) diff --git a/gateway/core/utils.py b/gateway/core/utils.py index 906aa84697..4f9e55d345 100644 --- a/gateway/core/utils.py +++ b/gateway/core/utils.py @@ -27,6 +27,7 @@ """ import base64 +import hashlib import json import logging import os @@ -109,6 +110,20 @@ def retry_function( # pylint: disable=too-many-positional-arguments return None +def _build_fernet() -> Fernet: + """Build a Fernet instance from a dedicated encryption secret. + + The key is derived with SHA-256 (a full-entropy 32-byte digest) instead of + naively space-padding/truncating the raw secret, which produced a low + entropy key when the secret was short. A dedicated ``TOKENS_ENCRYPTION_KEY`` + setting is preferred; we fall back to ``SECRET_KEY`` for backwards + compatibility. + """ + secret = getattr(settings, "TOKENS_ENCRYPTION_KEY", None) or settings.SECRET_KEY + digest = hashlib.sha256(secret.encode("utf-8")).digest() + return Fernet(base64.urlsafe_b64encode(digest)) + + def encrypt_string(string: str) -> str: """Encrypts string using symmetrical encryption. @@ -118,9 +133,7 @@ def encrypt_string(string: str) -> str: Returns: encrypted string """ - code_bytes = settings.SECRET_KEY.encode("utf-8") - fernet = Fernet(base64.urlsafe_b64encode(code_bytes.ljust(32)[:32])) - return fernet.encrypt(string.encode("utf-8")).decode("utf-8") + return _build_fernet().encrypt(string.encode("utf-8")).decode("utf-8") def decrypt_string(string: str) -> str: @@ -132,9 +145,7 @@ def decrypt_string(string: str) -> str: Returns: decrypted string """ - code_bytes = settings.SECRET_KEY.encode("utf-8") - fernet = Fernet(base64.urlsafe_b64encode(code_bytes.ljust(32)[:32])) - return fernet.decrypt(string.encode("utf-8")).decode("utf-8") + return _build_fernet().decrypt(string.encode("utf-8")).decode("utf-8") def encrypt_env_vars(env_vars: Dict[str, str]) -> Dict[str, str]: @@ -161,12 +172,16 @@ def decrypt_env_vars(env_vars: Dict[str, str]) -> Dict[str, str]: Returns: decrypted env vars dict """ - for key, value in env_vars.items(): + for key, value in list(env_vars.items()): if "token" in key.lower(): try: env_vars[key] = decrypt_string(value) except Exception: # pylint: disable=broad-exception-caught - logger.error("Cannot decrypt %s.", key) + # Fail closed: never inject the still-encrypted ciphertext into + # the runtime environment (it would mask a key-rotation issue + # and leak unusable secret material). Drop the variable instead. + logger.error("Cannot decrypt %s; dropping it from env vars.", key) + env_vars.pop(key, None) return env_vars diff --git a/gateway/main/settings.py b/gateway/main/settings.py index 1cf26b0b44..80fe199100 100644 --- a/gateway/main/settings.py +++ b/gateway/main/settings.py @@ -16,6 +16,9 @@ import os.path import sys from pathlib import Path + +from django.core.exceptions import ImproperlyConfigured + from core.utils import sanitize_file_path RELEASE_VERSION = os.environ.get("VERSION", "UNKNOWN") @@ -36,21 +39,34 @@ # Quick-start development settings - unsuitable for production # See https://docs.djangoproject.com/en/4.1/howto/deployment/checklist/ -# SECURITY WARNING: keep the secret key used in production secret! -SECRET_KEY = os.environ.get( - "DJANGO_SECRET_KEY", - "django-insecure-&)i3b5aue*#-i6k9i-03qm(d!0h&662lbhj12on_*gimn3x8p7", -) - # SECURITY WARNING: don't run with debug turned on in production! -DEBUG = int(os.environ.get("DEBUG", 1)) +# Defaults to off (0); set DEBUG=1 explicitly for local development only. +DEBUG = int(os.environ.get("DEBUG", 0)) -# SECURITY WARNING: don't run with debug turned on in production! -LOG_LEVEL = "DEBUG" if int(os.environ.get("DEBUG", 1)) else "INFO" +# SECURITY WARNING: keep the secret key used in production secret! +# A hardcoded fallback is only allowed in development/tests. When DEBUG is off +# (i.e. production) the process must fail closed if no key is provided, so we +# never silently sign sessions/tokens with a publicly known key. +SECRET_KEY = os.environ.get("DJANGO_SECRET_KEY") +if not SECRET_KEY: + if DEBUG or IS_TEST: + SECRET_KEY = "django-insecure-&)i3b5aue*#-i6k9i-03qm(d!0h&662lbhj12on_*gimn3x8p7" + else: + raise ImproperlyConfigured("DJANGO_SECRET_KEY environment variable must be set when DEBUG is disabled.") + +# Dedicated secret used to encrypt user tokens at rest. Falls back to +# SECRET_KEY when unset; provision a distinct value to decouple token +# encryption from session/CSRF signing. Rotating this value invalidates +# previously encrypted env vars (they will be re-supplied on the next run). +TOKENS_ENCRYPTION_KEY = os.environ.get("TOKENS_ENCRYPTION_KEY") + +LOG_LEVEL = "DEBUG" if DEBUG else "INFO" LOG_FORMAT = "json" if os.environ.get("LOG_FORMAT", "simple") == "json" else "simple" # It must be a full url without protocol: mydomain.com -ALLOWED_HOSTS = os.environ.get("ALLOWED_HOSTS", "*").split(",") +# In production (DEBUG off) require an explicit allowlist instead of "*", which +# otherwise enables Host header attacks (cache poisoning, password-reset, etc.). +ALLOWED_HOSTS = os.environ.get("ALLOWED_HOSTS", "*" if DEBUG else "localhost").split(",") # It must be a full url: https://mydomain.com CSRF_TRUSTED_ORIGINS = os.environ.get("CSRF_TRUSTED_ORIGINS", "http://localhost").split(",") @@ -109,10 +125,16 @@ ROOT_URLCONF = "main.urls" +# Extra template directory (e.g. the ray cluster template delivered via a +# configmap mount). Defaults to a non-world-writable path; do NOT point this at +# a shared/world-writable location such as /tmp where a co-resident process +# could drop a malicious template into Django's search path. +GATEWAY_TEMPLATES_DIR = os.environ.get("GATEWAY_TEMPLATES_DIR", "/etc/gateway/templates") + TEMPLATES = [ { "BACKEND": "django.template.backends.django.DjangoTemplates", - "DIRS": [BASE_DIR / "templates", "/tmp/templates"], + "DIRS": [BASE_DIR / "templates", GATEWAY_TEMPLATES_DIR], "APP_DIRS": True, "OPTIONS": { "context_processors": [ @@ -227,9 +249,15 @@ "api.authentication.MockTokenBackend", ], } -DJR_DEFAULT_AUTHENTICATION_CLASSES = ALL_AUTH_CLASSES_CONFIGURATION.get( - SETTINGS_AUTH_MECHANISM, ALL_AUTH_CLASSES_CONFIGURATION["mock_token"] -) +# Fail closed on an unknown mechanism instead of silently falling back to the +# insecure mock_token backend (which authenticates anyone presenting a static +# shared token). +if SETTINGS_AUTH_MECHANISM not in ALL_AUTH_CLASSES_CONFIGURATION: + raise ImproperlyConfigured( + f"Unknown SETTINGS_AUTH_MECHANISM '{SETTINGS_AUTH_MECHANISM}'. " + f"Valid values are: {list(ALL_AUTH_CLASSES_CONFIGURATION)}." + ) +DJR_DEFAULT_AUTHENTICATION_CLASSES = ALL_AUTH_CLASSES_CONFIGURATION[SETTINGS_AUTH_MECHANISM] # mock token value SETTINGS_AUTH_MOCK_TOKEN = os.environ.get("SETTINGS_AUTH_MOCK_TOKEN", "awesome_token") SETTINGS_AUTH_MOCKPROVIDER_REGISTRY = os.environ.get("SETTINGS_AUTH_MOCKPROVIDER_REGISTRY", None)