diff --git a/dockerspawner/dockerspawner.py b/dockerspawner/dockerspawner.py index 541c300f..ea377693 100644 --- a/dockerspawner/dockerspawner.py +++ b/dockerspawner/dockerspawner.py @@ -261,43 +261,56 @@ def _ip_default(self): If a callable, will be called with the Spawner instance as its only argument. The user is accessible as spawner.user. - The callable should return a dict or list as above. + The callable should return a dict or list or None as above. + + If empty (default), the value from ``image`` is used and + any attempt to specify the image via user_options will result in an error. + + .. versionchanged:: 13 + Empty allowed_images means no user-specified images are allowed. + This is the default. + Prior to 13, restricting to single image required a length-1 list, + e.g. ``allowed_images = [image]``. + + .. versionadded:: 13 + To allow any image, specify ``allowed_images = "*"``. .. versionchanged:: 12.0 ``DockerSpawner.image_whitelist`` renamed to ``allowed_images`` - """, ) @validate('allowed_images') - def _allowed_images_dict(self, proposal): + def _validate_allowed_images(self, proposal): """cast allowed_images to a dict If passing a list, cast it to a {item:item} dict where the keys and values are the same. """ - allowed_images = proposal.value - if isinstance(allowed_images, list): + allowed_images = proposal["value"] + if isinstance(allowed_images, str): + if allowed_images != "*": + raise ValueError( + f"'*' (all images) is the only accepted string value for allowed_images, got {allowed_images!r}. Use a list: `[{allowed_images!r}]` if you want to allow just one image." + ) + elif isinstance(allowed_images, list): allowed_images = {item: item for item in allowed_images} return allowed_images def _get_allowed_images(self): """Evaluate allowed_images callable - Or return the list as-is if it's already a dict + Always returns a dict or None """ if callable(self.allowed_images): allowed_images = self.allowed_images(self) - if not isinstance(allowed_images, dict): - # always return a dict - allowed_images = {item: item for item in allowed_images} - return allowed_images + return self._validate_allowed_images({"value": allowed_images}) return self.allowed_images @default('options_form') def _default_options_form(self): allowed_images = self._get_allowed_images() - if len(allowed_images) <= 1: + if allowed_images == "*" or len(allowed_images) <= 1: # default form only when there are images to choose from return '' # form derived from wrapspawner.ProfileSpawner @@ -1065,8 +1078,10 @@ async def remove_object(self): async def check_allowed(self, image): allowed_images = self._get_allowed_images() - if not allowed_images: + if allowed_images == "*": return image + elif not allowed_images: + raise web.HTTPError(400, "Specifying image to launch is not allowed") if image not in allowed_images: raise web.HTTPError( 400, diff --git a/docs/source/changelog.md b/docs/source/changelog.md index 685ce379..0ccb0dcc 100644 --- a/docs/source/changelog.md +++ b/docs/source/changelog.md @@ -12,8 +12,12 @@ command line for details. ([full changelog](https://github.com/jupyterhub/dockerspawner/compare/12.1.0...13.0.0)) +13.0 Fixes security vulnerability GHSA-hfgr-h3vc-p6c2, which allowed authenticated users to spawn arbitrary images +unless `DockerSpawner.allowed_images` was specified. + #### API and Breaking Changes +- Add and require `DockerSpawner.allowed_images='*'` to allow any image to be spawned via `user_options`. (GHSA-hfgr-h3vc-p6c2) - Remove deprecated, broken hub_ip_connect [#499](https://github.com/jupyterhub/dockerspawner/pull/499) ([@minrk](https://github.com/minrk)) - Require python 3.8+ and jupyterhub 2.3.1+ [#488](https://github.com/jupyterhub/dockerspawner/pull/488) ([@consideRatio](https://github.com/consideRatio), [@minrk](https://github.com/minrk)) diff --git a/examples/image_form/jupyterhub_config.py b/examples/image_form/jupyterhub_config.py index 34447adc..207818fc 100644 --- a/examples/image_form/jupyterhub_config.py +++ b/examples/image_form/jupyterhub_config.py @@ -13,26 +13,11 @@ def get_options_form(spawner): c.DockerSpawner.options_form = get_options_form -from dockerspawner import DockerSpawner +# specify that DockerSpawner should accept any image from user input +c.DockerSpawner.allowed_images = "*" - -class CustomDockerSpawner(DockerSpawner): - def options_from_form(self, formdata): - options = {} - image_form_list = formdata.get("image", []) - if image_form_list and image_form_list[0]: - options["image"] = image_form_list[0].strip() - self.log.info(f"User selected image: {options['image']}") - return options - - def load_user_options(self, options): - image = options.get("image") - if image: - self.log.info(f"Loading image {image}") - self.image = image - - -c.JupyterHub.spawner_class = CustomDockerSpawner +# tell JupyterHub to use DockerSpawner +c.JupyterHub.spawner_class = "docker" # the rest of the config is testing boilerplate # to make the Hub connectable from the containers diff --git a/tests/conftest.py b/tests/conftest.py index 0db1f925..b191eb77 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -15,6 +15,7 @@ from jupyterhub.tests.conftest import event_loop # noqa: F401 from jupyterhub.tests.conftest import io_loop # noqa: F401 from jupyterhub.tests.conftest import ssl_tmpdir # noqa: F401 +from jupyterhub.tests.conftest import user # noqa: F401 from jupyterhub.tests.mocking import MockHub from dockerspawner import DockerSpawner, SwarmSpawner, SystemUserSpawner diff --git a/tests/test_dockerspawner.py b/tests/test_dockerspawner.py index 9c970c5f..51eca880 100644 --- a/tests/test_dockerspawner.py +++ b/tests/test_dockerspawner.py @@ -84,32 +84,89 @@ async def test_start_stop(dockerspawner_configured_app, remove): def allowed_images_callable(*_): - return ["jupyterhub/singleuser:1.0", "jupyterhub/singleuser:1.1"] + return ["quay.io/jupyterhub/singleuser:4.0", "quay.io/jupyterhub/singleuser:4"] @pytest.mark.parametrize( - "allowed_images, image", + "allowed_images, image, ok", [ ( { - "1.0": "jupyterhub/singleuser:1.0", - "1.1": "jupyterhub/singleuser:1.1", + "4.0": "quay.io/jupyterhub/singleuser:4.0", + "4": "quay.io/jupyterhub/singleuser:4", }, - "1.0", + "4.0", + True, + ), + ( + { + "4.0": "quay.io/jupyterhub/singleuser:4.0", + "4": "quay.io/jupyterhub/singleuser:4", + }, + None, + True, + ), + ( + [ + "quay.io/jupyterhub/singleuser:4", + "quay.io/jupyterhub/singleuser:4.0", + ], + "not-in-list", + False, + ), + ( + [ + "quay.io/jupyterhub/singleuser:4.0", + "quay.io/jupyterhub/singleuser:4", + ], + "quay.io/jupyterhub/singleuser:4", + True, + ), + ( + allowed_images_callable, + "quay.io/jupyterhub/singleuser:4.0", + True, + ), + ( + allowed_images_callable, + "quay.io/jupyterhub/singleuser:3.0", + False, + ), + ( + None, + "DEFAULT", + False, + ), + ( + None, + None, + True, + ), + ( + # explicitly allow all + "*", + "quay.io/jupyterhub/singleuser:4", + True, ), - (["jupyterhub/singleuser:1.0", "jupyterhub/singleuser:1.1.0"], "1.1.0"), - (allowed_images_callable, "1.0"), ], ) -async def test_allowed_image(dockerspawner_configured_app, allowed_images, image): +async def test_allowed_image( + user, dockerspawner_configured_app, allowed_images, image, ok +): app = dockerspawner_configured_app - name = "checker" - add_user(app.db, app, name=name) - user = app.users[name] + name = user.name assert isinstance(user.spawner, DockerSpawner) + default_image = user.spawner.image # default value + if image == "DEFAULT": + image = default_image user.spawner.remove_containers = True - user.spawner.allowed_images = allowed_images - token = user.new_api_token() + if allowed_images is not None: + user.spawner.allowed_images = allowed_images + + if image: + request_body = json.dumps({"image": image}) + else: + request_body = b"" # start the server r = await api_request( app, @@ -117,29 +174,34 @@ async def test_allowed_image(dockerspawner_configured_app, allowed_images, image name, "server", method="post", - data=json.dumps({"image": image}), + data=request_body, ) - if image not in user.spawner._get_allowed_images(): - with pytest.raises(Exception): - r.raise_for_status() + if not ok: + assert r.status_code == 400 return + else: + r.raise_for_status() + pending = r.status_code == 202 while pending: # request again - await asyncio.sleep(2) + await asyncio.sleep(1) r = await api_request(app, "users", name) user_info = r.json() pending = user_info["servers"][""]["pending"] - url = url_path_join(public_url(app, user), "api/status") - resp = await AsyncHTTPClient().fetch( - url, headers={"Authorization": "token %s" % token} - ) - assert resp.effective_url == url - resp.rethrow() + if image is None: + expected_image = default_image + elif isinstance(allowed_images, (list, dict)): + expected_image = user.spawner._get_allowed_images()[image] + else: + expected_image = image + + assert user.spawner.image == expected_image + obj = await user.spawner.get_object() + assert obj["Config"]["Image"] == expected_image - assert resp.headers['x-jupyterhub-version'].startswith(image) r = await api_request( app, "users",