-
Notifications
You must be signed in to change notification settings - Fork 28
Update gold_image_request.py #205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
18ca236
648497c
14fefbf
9404732
864ad14
7d141b2
6b6b266
3a0b015
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,14 +264,18 @@ async def validate_mtls_auth(request: Request) -> bool: | |
| if not xfcc: | ||
| return False | ||
|
|
||
| # Extract SPIFFE ID from URI field | ||
| # Format: Hash=...;URI=spiffe://...;... | ||
| # Extract SPIFFE ID from URI field or use the whole header if it's a SPIFFE ID | ||
| principal = None | ||
| match = re.search(r'URI=(spiffe://[^;,]+)', xfcc) | ||
| if match: | ||
| principal = match.group(1) | ||
| elif xfcc.startswith('spiffe://'): | ||
| principal = xfcc | ||
|
|
||
| if principal: | ||
| if not principal.startswith( | ||
| 'spiffe://cluster.local/ns/client-applications' | ||
| ): | ||
| ) and not principal.startswith('spiffe://cluster.local/ns/gpu-processing'): | ||
| logger.error(f'Invalid mTLS authentication. Principal: {principal}') | ||
| return False | ||
|
|
||
|
|
@@ -290,7 +294,7 @@ async def validate_mtls_auth(request: Request) -> bool: | |
|
|
||
| request_id = getattr(request.state, 'request_id', get_current_request_id()) | ||
| logger.warning( | ||
| f'mTLS header present but no valid URI found: {xfcc} [Request ID: {request_id}]' | ||
| f'mTLS header present but no valid principal found: {xfcc} [Request ID: {request_id}]' | ||
| ) | ||
| return False | ||
|
|
||
|
|
@@ -331,8 +335,9 @@ async def dispatch( | |
| if request.method == 'OPTIONS': | ||
| return await call_next(request) | ||
|
|
||
| authorization = request.headers.get('Authorization') | ||
| # Check if this endpoint requires HMAC validation (skip JWT validation then) | ||
| if request.url.path in required_hmac_apis: | ||
| if request.url.path in required_hmac_apis and not authorization: | ||
| if not await validate_hmac_signature(request, auth_secrets_repository): | ||
| request_id = getattr( | ||
| request.state, 'request_id', get_current_request_id() | ||
|
|
@@ -363,9 +368,8 @@ async def dispatch( | |
| 'Invalid service authentication' | ||
| ), | ||
| ) | ||
| else: # Do the JWT validation or passthrough | ||
| authorization = request.headers.get('Authorization') | ||
|
|
||
| else: | ||
| # Normal auth token flow | ||
| token = None | ||
| if authorization and authorization.startswith('Bearer '): | ||
| token = authorization.split(' ')[1] | ||
|
|
@@ -418,9 +422,21 @@ async def dispatch( | |
| return await call_next(request) | ||
|
|
||
| # Check for mTLS authentication if no token is present | ||
| if request.headers.get('X-Forwarded-Client-Cert'): | ||
| mtls_header = request.headers.get('X-Forwarded-Client-Cert') | ||
| if mtls_header and not token: | ||
| logger.info(f'mTLS authentication by {mtls_header}') | ||
| if await validate_mtls_auth(request): | ||
| return await call_next(request) | ||
| else: | ||
| logger.error( | ||
| f'Invalid mTLS authentication for {request.url.path}' | ||
| ) | ||
| return JSONResponse( | ||
| status_code=status.HTTP_403_FORBIDDEN, | ||
| content=response_formatter.buildErrorResponse( | ||
| error='Invalid mTLS authentication' | ||
| ), | ||
| ) | ||
|
Comment on lines
+425
to
+439
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security concern: Avoid logging sensitive certificate data. Line 427 logs the entire The 🔒 Proposed fix to remove or redact sensitive header loggingOption 1 (Recommended): Remove the redundant log statement Since mtls_header = request.headers.get('X-Forwarded-Client-Cert')
if mtls_header and not token:
- logger.info(f'mTLS authentication by {mtls_header}')
if await validate_mtls_auth(request):Option 2: Log only that mTLS is being attempted If you need logging at the dispatch level, log without exposing the header value: mtls_header = request.headers.get('X-Forwarded-Client-Cert')
if mtls_header and not token:
- logger.info(f'mTLS authentication by {mtls_header}')
+ logger.info('Attempting mTLS authentication')
if await validate_mtls_auth(request):🤖 Prompt for AI Agents |
||
|
|
||
| if not token: | ||
| request_id = getattr( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Avoid logging raw certificate header.
Line 297 logs the full
xfccheader content, which may contain sensitive certificate metadata (serial numbers, subject DNs, issuer info). Consider logging only that no valid principal was found, without exposing the raw header value.🔒 Proposed fix to redact sensitive header data
request_id = getattr(request.state, 'request_id', get_current_request_id()) logger.warning( - f'mTLS header present but no valid principal found: {xfcc} [Request ID: {request_id}]' + f'mTLS header present but no valid SPIFFE principal found [Request ID: {request_id}]' )📝 Committable suggestion
🤖 Prompt for AI Agents