Skip to content

Commit

Permalink
Limit the size of manifests/signatures during sync
Browse files Browse the repository at this point in the history
Adds limit to the size of manifests and signatures as a safeguard
to avoid DDoS attack during sync operations.
To also prevent this during image upload, this commit configures a
`client_max_body_size` for manifests and signatures Nginx endpoints.

closes: #532
  • Loading branch information
git-hyagi committed Sep 2, 2024
1 parent 5908f47 commit edbaaf1
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 5 deletions.
2 changes: 2 additions & 0 deletions CHANGES/532.feature
Original file line number Diff line number Diff line change
@@ -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.
63 changes: 60 additions & 3 deletions pulp_container/app/downloaders.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import aiohttp
import asyncio
import fnmatch
import json
import re

Expand All @@ -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__)

Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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):
Expand All @@ -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):
"""
Expand Down
3 changes: 1 addition & 2 deletions pulp_container/app/registry_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
EMPTY_BLOB,
SIGNATURE_API_EXTENSION_VERSION,
SIGNATURE_HEADER,
SIGNATURE_PAYLOAD_MAX_SIZE,
SIGNATURE_TYPE,
V2_ACCEPT_HEADERS,
)
Expand Down Expand Up @@ -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:
Expand Down
22 changes: 22 additions & 0 deletions pulp_container/app/webserver_snippets/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
1 change: 1 addition & 0 deletions pulp_container/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit edbaaf1

Please sign in to comment.