diff --git a/CHANGES/532.feature b/CHANGES/532.feature new file mode 100644 index 000000000..e9231dfdb --- /dev/null +++ b/CHANGES/532.feature @@ -0,0 +1,2 @@ +Added a limit of 4mb to the size of manifests and signatures as a safeguard to OOM DoS attack +during sync tasks and updated the Nginx snippet to also limit the size of the body for these endpoints. diff --git a/pulp_container/app/downloaders.py b/pulp_container/app/downloaders.py index d9c6fff1c..0c317da9b 100644 --- a/pulp_container/app/downloaders.py +++ b/pulp_container/app/downloaders.py @@ -1,5 +1,6 @@ import aiohttp import asyncio +import fnmatch import json import re @@ -10,7 +11,14 @@ from pulpcore.plugin.download import DownloaderFactory, HttpDownloader -from pulp_container.constants import V2_ACCEPT_HEADERS +from pulp_container.constants import ( + MANIFEST_MEDIA_TYPES, + MANIFEST_PAYLOAD_MAX_SIZE, + MEGABYTE, + SIGNATURE_PAYLOAD_MAX_SIZE, + V2_ACCEPT_HEADERS, +) +from pulp_container.app.exceptions import InvalidRequest log = getLogger(__name__) @@ -20,7 +28,41 @@ ) -class RegistryAuthHttpDownloader(HttpDownloader): +class ValidateResourceSizeMixin: + async def validate_resource_size(self, response, request_method=None): + """ + Verify if the constrained resources are not exceeding the maximum size allowed. + """ + if request_method == "head": + return + + content_type = response.content_type + max_resource_size = 0 + is_cosign_tag = fnmatch.fnmatch(response.url.name, "sha256-*.sig") + + if isinstance(self, NoAuthSignatureDownloader) or is_cosign_tag: + max_resource_size = SIGNATURE_PAYLOAD_MAX_SIZE + content_type = "Signature" + elif content_type in MANIFEST_MEDIA_TYPES.IMAGE + MANIFEST_MEDIA_TYPES.LIST: + max_resource_size = MANIFEST_PAYLOAD_MAX_SIZE + content_type = "Manifest" + else: + return + + total_size = 0 + buffer = b"" + async for chunk in response.content.iter_chunked(MEGABYTE): + total_size += len(chunk) + buffer += chunk + if total_size > max_resource_size: + raise InvalidRequest( + f"{content_type} size exceeded the {max_resource_size} bytes " + f"limit ({total_size} bytes)." + ) + response.content.unread_data(buffer) + + +class RegistryAuthHttpDownloader(HttpDownloader, ValidateResourceSizeMixin): """ Custom Downloader that automatically handles Token Based and Basic Authentication. @@ -77,6 +119,7 @@ async def _run(self, handle_401=True, extra_data=None): async with session_http_method( self.url, headers=headers, proxy=self.proxy, proxy_auth=self.proxy_auth ) as response: + await self.validate_resource_size(response, http_method) try: response.raise_for_status() except ClientResponseError as e: @@ -193,7 +236,7 @@ async def _handle_head_response(self, response): ) -class NoAuthSignatureDownloader(HttpDownloader): +class NoAuthSignatureDownloader(HttpDownloader, ValidateResourceSizeMixin): """A downloader class suited for signature downloads.""" def raise_for_status(self, response): @@ -208,6 +251,20 @@ def raise_for_status(self, response): else: response.raise_for_status() + async def _run(self, extra_data=None): + if self.download_throttler: + await self.download_throttler.acquire() + async with self.session.get( + self.url, proxy=self.proxy, proxy_auth=self.proxy_auth, auth=self.auth + ) as response: + await self.validate_resource_size(response) + self.raise_for_status(response) + to_return = await self._handle_response(response) + await response.release() + if self._close_session_on_finalize: + await self.session.close() + return to_return + class NoAuthDownloaderFactory(DownloaderFactory): """ diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py index 682c7cb2e..71a81088d 100644 --- a/pulp_container/app/registry_api.py +++ b/pulp_container/app/registry_api.py @@ -91,7 +91,6 @@ EMPTY_BLOB, SIGNATURE_API_EXTENSION_VERSION, SIGNATURE_HEADER, - SIGNATURE_PAYLOAD_MAX_SIZE, SIGNATURE_TYPE, V2_ACCEPT_HEADERS, ) @@ -1426,7 +1425,7 @@ def put(self, request, path, pk): except models.Manifest.DoesNotExist: raise ManifestNotFound(reference=pk) - signature_payload = request.META["wsgi.input"].read(SIGNATURE_PAYLOAD_MAX_SIZE) + signature_payload = request.META["wsgi.input"].read() try: signature_dict = json.loads(signature_payload) except json.decoder.JSONDecodeError: diff --git a/pulp_container/app/webserver_snippets/nginx.conf b/pulp_container/app/webserver_snippets/nginx.conf index a5c27afdd..29cccad71 100644 --- a/pulp_container/app/webserver_snippets/nginx.conf +++ b/pulp_container/app/webserver_snippets/nginx.conf @@ -38,3 +38,25 @@ location /token/ { proxy_redirect off; proxy_pass http://pulp-api; } + +location ~* /v2/.*/manifests/.*$ { + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header Host $http_host; + # we don't want nginx trying to do something clever with + # redirects, we set the Host: header above already. + proxy_redirect off; + proxy_pass http://pulp-api; + client_max_body_size 4m; +} + +location ~* /extensions/v2/.*/signatures/.*$ { + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header Host $http_host; + # we don't want nginx trying to do something clever with + # redirects, we set the Host: header above already. + proxy_redirect off; + proxy_pass http://pulp-api; + client_max_body_size 4m; +} diff --git a/pulp_container/constants.py b/pulp_container/constants.py index 8d6463481..b58431293 100644 --- a/pulp_container/constants.py +++ b/pulp_container/constants.py @@ -69,5 +69,6 @@ MEGABYTE = 1_000_000 SIGNATURE_PAYLOAD_MAX_SIZE = 4 * MEGABYTE +MANIFEST_PAYLOAD_MAX_SIZE = 4 * MEGABYTE SIGNATURE_API_EXTENSION_VERSION = 2