RSPEED-2651: enforce max header size on x-rh-identity before base64 decode#1352
Merged
tisnik merged 1 commit intolightspeed-core:mainfrom Mar 19, 2026
Merged
Conversation
…ecode Reject x-rh-identity headers exceeding configurable max_header_size (default 8KB) before base64 decoding to prevent DoS from oversized payloads. Resolves: RSPEED-2651 Signed-off-by: Major Hayden <major@redhat.com>
Contributor
WalkthroughThis change adds header size validation to the RH Identity authentication mechanism. A new max_header_size parameter is introduced across the configuration layer, authentication dependency, and constants, enforcing a maximum limit on the x-rh-identity header size before processing, returning a 400 error if exceeded. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/authentication/rh_identity.py (1)
205-218: Add a defensive constructor guard for non-positivemax_header_size.
RHIdentityConfigurationvalidates this already, but direct instantiation ofRHIdentityAuthDependencycan still pass invalid values and cause accidental auth outages.🛡️ Suggested hardening
def __init__( self, required_entitlements: Optional[list[str]] = None, virtual_path: str = DEFAULT_VIRTUAL_PATH, max_header_size: int = DEFAULT_RH_IDENTITY_MAX_HEADER_SIZE, ) -> None: @@ - self.required_entitlements = required_entitlements + if max_header_size <= 0: + raise ValueError("max_header_size must be greater than 0") + + self.required_entitlements = required_entitlements self.virtual_path = virtual_path self.max_header_size = max_header_size self.skip_userid_check = False🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/authentication/rh_identity.py` around lines 205 - 218, Add a defensive check in the RHIdentityAuthDependency.__init__ to guard against non-positive max_header_size: validate the incoming max_header_size parameter (e.g., if max_header_size is None or <= 0) and either raise a clear ValueError or default it to DEFAULT_RH_IDENTITY_MAX_HEADER_SIZE; update the assignment of self.max_header_size to use the validated value. This targets the RHIdentityAuthDependency constructor and references DEFAULT_RH_IDENTITY_MAX_HEADER_SIZE so direct instantiation cannot set an invalid header size.tests/unit/authentication/test_rh_identity.py (1)
573-576: Avoid hardcoded default limit in the parametrized test.Using the shared constant instead of
8192keeps the test resilient if the default changes later.♻️ Suggested update
-from constants import NO_USER_TOKEN +from constants import DEFAULT_RH_IDENTITY_MAX_HEADER_SIZE, NO_USER_TOKEN ... "header_size,max_size", [ - (9000, 8192), # Well over default limit + (9000, DEFAULT_RH_IDENTITY_MAX_HEADER_SIZE), # Well over default limit (101, 100), # One byte over custom limit (200, 100), # Well over custom limit ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/authentication/test_rh_identity.py` around lines 573 - 576, The parametrized test hardcodes 8192 as the default limit; replace that magic number by importing and using the shared default constant from the production module (e.g., DEFAULT_LIMIT or DEFAULT_OIDC_TOKEN_LIMIT) used by the rh identity code and use that constant in the tuple (instead of 8192) so the test follows the real default and stays correct if the default changes; update the import at the top of tests/unit/authentication/test_rh_identity.py and the tuple list where (9000, 8192) appears to use the shared constant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/authentication/rh_identity.py`:
- Around line 205-218: Add a defensive check in the
RHIdentityAuthDependency.__init__ to guard against non-positive max_header_size:
validate the incoming max_header_size parameter (e.g., if max_header_size is
None or <= 0) and either raise a clear ValueError or default it to
DEFAULT_RH_IDENTITY_MAX_HEADER_SIZE; update the assignment of
self.max_header_size to use the validated value. This targets the
RHIdentityAuthDependency constructor and references
DEFAULT_RH_IDENTITY_MAX_HEADER_SIZE so direct instantiation cannot set an
invalid header size.
In `@tests/unit/authentication/test_rh_identity.py`:
- Around line 573-576: The parametrized test hardcodes 8192 as the default
limit; replace that magic number by importing and using the shared default
constant from the production module (e.g., DEFAULT_LIMIT or
DEFAULT_OIDC_TOKEN_LIMIT) used by the rh identity code and use that constant in
the tuple (instead of 8192) so the test follows the real default and stays
correct if the default changes; update the import at the top of
tests/unit/authentication/test_rh_identity.py and the tuple list where (9000,
8192) appears to use the shared constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee70f774-1826-49bd-a7e8-adbe4c869cd9
📒 Files selected for processing (6)
src/authentication/__init__.pysrc/authentication/rh_identity.pysrc/constants.pysrc/models/config.pytests/unit/authentication/test_rh_identity.pytests/unit/models/config/test_authentication_configuration.py
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The rh-identity auth module passes the
x-rh-identityheader value directly tobase64.b64decode()with no size check. An attacker can send a multi-megabyte header and force memory allocation for decoding, JSON parsing, and dict traversal (DoS vector).This PR adds a configurable
max_header_sizefield toRHIdentityConfiguration(default 8KB) and enforces a size check on the raw base64-encoded header before any decoding occurs. Oversized headers are rejected with HTTP 400 and a warning is logged with size details.Changes
src/constants.py: addDEFAULT_RH_IDENTITY_MAX_HEADER_SIZE = 8192src/models/config.py: addmax_header_size: PositiveIntfield toRHIdentityConfigurationsrc/authentication/rh_identity.py: addmax_header_sizeconstructor param and size check beforebase64.b64decode()src/authentication/__init__.py: wiremax_header_sizefrom config into the auth dependencyType of change
Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Testing
uv run make verifypasses all linters (pylint 10/10, pyright 0 errors, ruff clean)uv run make test-unitpasses all 1765 tests with 60%+ coveragerh_identity.pyhas 100% test coveragemax_header_sizeis respectedPositiveIntSummary by CodeRabbit