From a893520885ebff8e3466a57f93c7f5e9778e6dc3 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 14 Oct 2026 11:51:58 +0200 Subject: [PATCH] Validate build inputs and isolate BuildKit workers on 0.9.0 --- deploy/buildkit/buildkitd.toml | 4 +- openserverless/impl/builder/build_service.py | 43 +++++++++-- openserverless/rest/build.py | 8 +-- tests/test_build_api_security.py | 52 ++++++++++++++ tests/test_build_target_security.py | 76 ++++++++++++++++++++ 5 files changed, 171 insertions(+), 12 deletions(-) create mode 100644 tests/test_build_api_security.py create mode 100644 tests/test_build_target_security.py diff --git a/deploy/buildkit/buildkitd.toml b/deploy/buildkit/buildkitd.toml index 0a3c3ce..6a3ebe6 100644 --- a/deploy/buildkit/buildkitd.toml +++ b/deploy/buildkit/buildkitd.toml @@ -21,7 +21,7 @@ [worker.oci] enabled = true rootless = true - no-process-sandbox = true + noProcessSandbox = false snapshotter = "overlayfs" # usa overlayfs se il kernel lo consente [worker.containerd] @@ -38,4 +38,4 @@ # Logging # ========================= [log] - level = "trace" \ No newline at end of file + level = "trace" diff --git a/openserverless/impl/builder/build_service.py b/openserverless/impl/builder/build_service.py index 44dc85d..1990d77 100644 --- a/openserverless/impl/builder/build_service.py +++ b/openserverless/impl/builder/build_service.py @@ -25,6 +25,7 @@ import random import string import binascii +import re JOB_NAME = "build" CM_NAME = "cm" @@ -36,6 +37,19 @@ class BuildService: based on the provided build configuration. """ + @staticmethod + def valid_image_target(target, expected_user=None): + """Accept one OPS image name and a Docker-compatible tag.""" + if not isinstance(target, str): + return False + match = re.fullmatch(r"([a-z0-9](?:[a-z0-9._-]*[a-z0-9])?):([A-Za-z0-9_][A-Za-z0-9_.-]{0,127})", target) + return bool(match and (expected_user is None or match.group(1) == expected_user)) + + @staticmethod + def valid_source_image(source): + """Keep FROM limited to a single image reference, never Dockerfile text.""" + return isinstance(source, str) and bool(re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._:/@-]*", source)) + def __init__(self, user_env=None): # A super userful Kube Api Client self.kube_client = KubeApiClient() @@ -206,6 +220,11 @@ def build(self, image_name: str) -> tuple[bool, str]: import tempfile import base64 + if not self.valid_image_target(image_name): + return (False, "Invalid target for the build") + if not self.valid_source_image(self.build_config.get("source")): + return (False, "Invalid source image for the build") + # define registry host self.registry_host = self.get_registry_host() if self.registry_host is None: @@ -402,11 +421,16 @@ def check_build_dir(self, unzip_dir: str) -> bool: def create_build_job(self, image_name: str) -> dict: """Create a Kubernetes job manifest for building the Docker image.""" + if not self.valid_image_target(image_name): + raise ValueError("Invalid target for the build") if not self.custom_registry_auth: registry_image_name = f"{self.registry_host}/{image_name}" else: registry_image_name = f"{image_name}" + # Require a private Kubernetes user namespace; keep BuildKit's process + # sandbox enabled. Subordinate IDs exclude Pod root and the worker UID. + # The resulting nested map supports build IDs 0..65534. # --- MANIFEST DEL JOB --- job_manifest = { "apiVersion": "batch/v1", @@ -417,6 +441,8 @@ def create_build_job(self, image_name: str) -> dict: "template": { "spec": { "restartPolicy": "Never", + "automountServiceAccountToken": False, + "hostUsers": False, "volumes": [ { "name": "openserverless-buildkitd-conf", @@ -449,7 +475,7 @@ def create_build_job(self, image_name: str) -> dict: "name": "copy-build-context", "image": "busybox:1.36", "command": [ - "sh", "-c", "cp -rvL /configmap/* /workspace/ && cat /workspace/Dockerfile", + "sh", "-c", "cp -rL /configmap/* /workspace/ && printf 'user:1:999\\nuser:1001:64535\\n' > /workspace/subuid && cp /workspace/subuid /workspace/subgid", ], "volumeMounts": [ { "name": "build-context-vol", "mountPath": "/configmap", }, @@ -460,25 +486,30 @@ def create_build_job(self, image_name: str) -> dict: "containers": [ { "name": "buildkit", - "image": "moby/buildkit:master-rootless", + "image": "moby/buildkit@sha256:e6102bb86f8a2c79c8ae8aa0de3d5edef500dd33b619414413e06138e13fb1a1", "command": ["sh", "-c"], "args": [ "rootlesskit buildkitd --config /config/buildkitd.toml & sleep 3 && " - f"buildctl build --progress=plain --frontend=dockerfile.v0 --local context=/workspace --local dockerfile=/workspace --output=type=image,name={registry_image_name},push=true" + 'buildctl build --progress=plain --frontend=dockerfile.v0 --local context=/workspace --local dockerfile=/workspace --output="type=image,name=${REGISTRY_IMAGE_NAME},push=true"' ], "securityContext": { "runAsUser": 1000, "runAsGroup": 1000, "allowPrivilegeEscalation": True, - "privileged": True, - "seccompProfile": { "type": "Unconfined" } + "privileged": False, + "procMount": "Unmasked", + "seccompProfile": { "type": "Unconfined" }, + "appArmorProfile": { "type": "Unconfined" }, }, "env": [ - { "name": "BUILDKIT_ROOTLESS", "value": "1" } + { "name": "BUILDKIT_ROOTLESS", "value": "1" }, + { "name": "REGISTRY_IMAGE_NAME", "value": registry_image_name } ], "volumeMounts": [ { "name": "openserverless-buildkitd-conf", "mountPath": "/config" }, { "name": "workspace", "mountPath": "/workspace" }, + { "name": "workspace", "mountPath": "/etc/subuid", "subPath": "subuid", "readOnly": True }, + { "name": "workspace", "mountPath": "/etc/subgid", "subPath": "subgid", "readOnly": True }, { "name": "docker-config", "mountPath": "/home/user/.docker" }, { "name": "img-cache", "mountPath": "/tmp" }, { "name": "cdi-etc", "mountPath": "/etc/cdi" }, diff --git a/openserverless/rest/build.py b/openserverless/rest/build.py index 066ab60..c74ed37 100644 --- a/openserverless/rest/build.py +++ b/openserverless/rest/build.py @@ -145,12 +145,12 @@ def build(): # validate the target wsk_user_name = auth_result.get('login','').lower() target = json_data.get('target') - target_user = str(target).split(':')[0] - # Strict user check is enabled by default for security strict_user_check = os.environ.get("STRICT_USER_CHECK", "true").lower() not in ("false", "0", "no", "off") - if strict_user_check and (wsk_user_name != target_user): + if not BuildService.valid_image_target(target, wsk_user_name if strict_user_check else None): return res_builder.build_error_message("Invalid target for the build.", status_code=HTTPStatus.BAD_REQUEST) + if not BuildService.valid_source_image(json_data.get('source')): + return res_builder.build_error_message("Invalid source image for the build.", status_code=HTTPStatus.BAD_REQUEST) env['wsk_user_name'] = wsk_user_name build_service = BuildService(user_env=env) @@ -251,4 +251,4 @@ def clean(): if clean_result == -1: return res_builder.build_error_message("Failed to clean up old build jobs.", status_code=HTTPStatus.INTERNAL_SERVER_ERROR) - return res_builder.build_response_message(f"Cleaned up {clean_result} jobs successfully.", status_code=HTTPStatus.OK) \ No newline at end of file + return res_builder.build_response_message(f"Cleaned up {clean_result} jobs successfully.", status_code=HTTPStatus.OK) diff --git a/tests/test_build_api_security.py b/tests/test_build_api_security.py new file mode 100644 index 0000000..d546e98 --- /dev/null +++ b/tests/test_build_api_security.py @@ -0,0 +1,52 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import unittest +from unittest.mock import patch, Mock +from openserverless import app +from openserverless.rest import build +from openserverless.impl.builder.build_service import BuildService + +class BuildApiSecurityTest(unittest.TestCase): + def setUp(self): + self.client = app.test_client() + self.payload = {"source":"runtime:python", "target":"alpha:release", "kind":"python"} + + @patch.object(build, "env_to_dict", return_value={"SYNTHETIC":"value"}) + @patch.object(build, "authorize", return_value={"login":"alpha"}) + def test_invalid_inputs_rejected_before_worker_creation(self, authorize, env): + with patch.dict(build.os.environ, {"STRICT_USER_CHECK":"true"}), patch.object(BuildService, "__init__", return_value=None) as initialize: + for key, value in (("target","alpha:ok;id"), ("target","beta:release"), + ("target","alpha:ok$(id)"), ("source","runtime:python\nRUN id")): + response = self.client.post("/system/api/v1/build/start",json={**self.payload,key:value}) + self.assertEqual(400,response.status_code) + initialize.assert_not_called() + + @patch.object(build, "env_to_dict", return_value={"SYNTHETIC":"value"}) + @patch.object(build, "authorize", return_value={"login":"alpha"}) + def test_valid_request_keeps_response_contract(self, authorize, env): + service = Mock(id="synthetic-id", job_name="build-alpha-test") + service.build.return_value = (True,"build-alpha-test") + with patch.object(build,"BuildService") as factory: + factory.valid_image_target.side_effect = BuildService.valid_image_target + factory.valid_source_image.side_effect = BuildService.valid_source_image + factory.return_value = service + response = self.client.post("/system/api/v1/build/start",json=self.payload) + self.assertEqual(200,response.status_code) + self.assertEqual("build-alpha-test",response.json["job_name"]) + service.init.assert_called_once_with(build_config=self.payload) + service.build.assert_called_once_with("alpha:release") diff --git a/tests/test_build_target_security.py b/tests/test_build_target_security.py new file mode 100644 index 0000000..152d583 --- /dev/null +++ b/tests/test_build_target_security.py @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import unittest +from unittest.mock import Mock + +from openserverless.impl.builder.build_service import BuildService + +class BuildTargetSecurityTest(unittest.TestCase): + def test_rejects_shell_syntax_before_kubernetes_calls(self): + service = BuildService.__new__(BuildService) + service.user = "alpha" + service.kube_client = Mock() + for target in ("alpha:ok;id", "alpha:ok$(id)", "alpha:ok\nRUN id", "alpha:ok,foo=bar"): + self.assertEqual((False, "Invalid target for the build"), service.build(target)) + service.kube_client.assert_not_called() + self.assertEqual([], service.kube_client.mock_calls) + + def test_valid_target_is_passed_as_data_to_buildctl(self): + service = BuildService.__new__(BuildService) + service.user = "alpha" + service.registry_host = "registry.local:5000" + service.custom_registry_auth = False + service.job_name = "build-alpha-test" + service.cm = "cm-alpha-test" + service.registry_auth = "registry-pull-secret" + job = service.create_build_job("alpha:release_1.2") + buildkit = job["spec"]["template"]["spec"]["containers"][0] + self.assertEqual("registry.local:5000/alpha:release_1.2", next(x["value"] for x in buildkit["env"] if x["name"] == "REGISTRY_IMAGE_NAME")) + self.assertIn('${REGISTRY_IMAGE_NAME}', buildkit["args"][0]) + self.assertNotIn("alpha:release_1.2", buildkit["args"][0]) + pod = job["spec"]["template"]["spec"] + self.assertFalse(pod["automountServiceAccountToken"]) + self.assertFalse(pod["hostUsers"]) + self.assertEqual("Unmasked", buildkit["securityContext"]["procMount"]) + self.assertIn("@sha256:", buildkit["image"]) + self.assertEqual({"/etc/subuid", "/etc/subgid"}, { + mount["mountPath"] for mount in buildkit["volumeMounts"] if mount.get("subPath") + }) + self.assertFalse(buildkit["securityContext"]["privileged"]) + self.assertEqual("Unconfined", buildkit["securityContext"]["appArmorProfile"]["type"]) + self.assertEqual("Unconfined", buildkit["securityContext"]["seccompProfile"]["type"]) + + def test_source_cannot_add_dockerfile_instructions(self): + self.assertTrue(BuildService.valid_source_image("docker.io/apache/runtime:1.2")) + self.assertFalse(BuildService.valid_source_image("docker.io/apache/runtime:1.2\nRUN id")) + self.assertFalse(BuildService.valid_image_target("beta:release", "alpha")) + + def test_dependency_build_keeps_existing_extension_contract(self): + service = BuildService.__new__(BuildService) + service.build_config = {"source": "runtime:python", "kind": "python", "file": "encoded"} + dockerfile = service.create_docker_file("requirements.txt") + self.assertIn("RUN /bin/extend\n", dockerfile) + self.assertTrue(dockerfile.endswith("USER nobody\n")) + + def test_base_image_only_build_does_not_install_dependencies(self): + service = BuildService.__new__(BuildService) + service.build_config = {"source": "runtime:python", "kind": "python"} + self.assertEqual("FROM runtime:python\n\n", service.create_docker_file()) + +if __name__ == "__main__": + unittest.main()