Merge pull request from GHSA-hfgr-h3vc-p6c2

allow only configured image by default

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,

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))
 

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

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

tests/test_dockerspawner.py

@@ -84,62 +84,124 @@ 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,
         "users",
         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",