Skip to content

feat(S3): Add transparency logging for signature version downgrades #3965

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

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

alexandrebl
Copy link

Add logging for SigV4 to SigV2 downgrade in presigned URLs

Description

This pull request adds transparent logging to the Amazon S3 client when presigned URLs are automatically downgraded from SigV4 to SigV2 due to expiration time limits. When developers request presigned URLs with expiration times exceeding the SigV4 maximum (7 days / 604,800 seconds), the SDK silently downgrades to SigV2 in supported regions. This change provides visibility into this automatic decision through informative logging.

Key Changes:

  • Added logging in AmazonS3Client.Extensions.cs within the DetermineSignatureVersionToUse method
  • Log message includes specific details: requested expiration time, SigV4 maximum limit, affected bucket name, and region
  • Encourages security best practices by suggesting developers reduce expiration times to maintain SigV4
  • Follows existing SDK logging patterns using Logger.GetLogger() and InfoFormat()
  • Added comprehensive unit tests to verify the logging functionality

Log Message Format:

Presigned URL expiration (691200 seconds) exceeds SigV4 maximum (604800 seconds). Automatically using SigV2 for bucket 'example-bucket' in region 'us-east-1'. Consider reducing expiration time to use SigV4 for better security.

Motivation and Context

Currently, when developers request presigned URLs with long expiration times (> 7 days), the AWS SDK for .NET automatically downgrades from SigV4 to SigV2 without any notification. This silent behavior can lead to:

  1. Security concerns: Developers may unknowingly use less secure SigV2 signatures
  2. Debugging difficulties: No visibility into why signature format changes
  3. Missed optimization opportunities: Developers aren't informed they could use more secure SigV4 with shorter expiration times

This change addresses these issues by providing transparent, actionable logging that:

  • Informs developers when and why the downgrade occurs
  • Provides specific technical details for debugging
  • Encourages security best practices through clear guidance
  • Maintains full backward compatibility

This enhancement improves developer experience and security awareness without introducing any breaking changes.

Testing

Unit Tests Added:

  1. TestSigV4ToSigV2DowngradeLogging: Verifies downgrade behavior in SigV2-supported regions

    • Creates presigned URL with 8-day expiration in us-east-1 region
    • Validates that SigV2 signature format is used (Signature= parameter present)
    • Confirms SigV4 parameters are absent (X-Amz-Signature not present)
    • Indirectly tests that logging code path is executed
  2. TestSigV4ToSigV2DowngradeInUnsupportedRegion: Verifies exception handling in unsupported regions

    • Creates presigned URL with 8-day expiration in eu-north-1 region (no SigV2 support)
    • Validates that appropriate ArgumentException is thrown
    • Confirms error message contains expected content about AWS4 signing limits

Testing Environment:

  • Built and tested against .NET 8.0 and .NET Core 3.1 target frameworks
  • Verified compilation success for all S3 project targets
  • Tests follow existing SDK testing patterns and conventions
  • No breaking changes to existing functionality

Manual Verification:

  • Confirmed logging integration works with existing SDK logging infrastructure
  • Verified log message format and content accuracy
  • Tested both supported and unsupported region scenarios

Screenshots (if appropriate)

N/A - This change involves logging functionality without UI components.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

feat(S3): Add logging for SigV4 to SigV2 downgrade in presigned URLs

When generating presigned URLs with expiration times exceeding the SigV4
maximum (7 days), the SDK automatically downgrades to SigV2 in supported
regions. This change adds transparent logging to inform developers when
this downgrade occurs.

**Changes:**
- Add informative log message in AmazonS3Client.Extensions.cs when SigV4
  downgrades to SigV2 due to expiration time limits
- Log includes specific details: requested expiration time, SigV4 maximum
  limit, bucket name, and region
- Encourages security best practices by suggesting shorter expiration times
- Uses existing SDK logging patterns with Logger.GetLogger() and InfoFormat()
**Testing:**
- Added comprehensive unit tests in SignatureVersionTests.cs
- Test 1: Verifies downgrade behavior in SigV2-supported regions (us-east-1)
- Test 2: Verifies exception handling in unsupported regions (eu-north-1)
- Tests validate signature version changes and URL format correctness

**Benefits:**
- Improves developer experience with transparent security decisions
- Helps developers understand when and why signature version changes occur
- Promotes security best practices through actionable guidance
- Maintains backward compatibility with existing functionality

**Files Modified:**
- sdk/src/Services/S3/Custom/AmazonS3Client.Extensions.cs
- sdk/test/Services/S3/UnitTests/Custom/SignatureVersionTests.cs

Fixes: Lack of visibility into automatic SigV4 to SigV2 downgrades
@Copilot Copilot AI review requested due to automatic review settings August 19, 2025 18:16
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds transparency logging to the Amazon S3 client when presigned URLs are automatically downgraded from SigV4 to SigV2 due to expiration time limits exceeding 7 days. The change provides visibility into this automatic security downgrade that was previously silent.

Key Changes:

  • Added informative logging in the signature version determination logic
  • Added comprehensive unit tests to verify downgrade behavior in both supported and unsupported regions
  • Added manual test projects to validate logging functionality

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
AmazonS3Client.Extensions.cs Adds logging when SigV4 downgrades to SigV2 due to expiration limits
SignatureVersionTests.cs Adds unit tests for downgrade scenarios in supported/unsupported regions
TestS3Logging.csproj Test project configuration for manual logging validation
TestS3Logging.cs Comprehensive manual test with output capture and verification
TestLogging.cs Simple manual test for basic logging functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -0,0 +1,181 @@
using System;
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be a standalone test file that should not be included in a production SDK pull request. Test files for manual verification should be excluded from the main codebase and handled separately.

Copilot uses AI. Check for mistakes.

Console.WriteLine("\nTest completed. Check the log output above for the downgrade message.");
}
}
}
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be a standalone test file that should not be included in a production SDK pull request. Test files for manual verification should be excluded from the main codebase and handled separately.

Suggested change
}

Copilot uses AI. Check for mistakes.

</Reference>
</ItemGroup>

</Project>
Copy link
Preview

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This project file appears to be for manual testing and should not be included in the main SDK repository. Consider moving this to a separate testing directory or excluding it from the pull request.

Suggested change
</Project>

Copilot uses AI. Check for mistakes.

@GarrettBeatty GarrettBeatty self-requested a review August 20, 2025 02:10
@GarrettBeatty
Copy link
Contributor

are you able to add a dev config file similar to https://github.com/aws/aws-sdk-net/blob/50e5ddd9f27a7ce50fd0aa44e9735398ae9a3f50/generator/.DevConfigs/f2b3d288-839a-4d44-8124-efa681fa4b41.json

something like

{
  "services": [
    {
      "serviceName": "S3",
      "type": "patch",
      "changeLogMessages": [
        "Add logging for when the signature version downgrades in presigned urls."
      ]
    }
  ]
}

using Amazon;
using Amazon.S3;
using Amazon.S3.Model;
using Amazon.Runtime.Internal.Util;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove these extra files (testlogging, tests3logging, tests3loggin.csproj)? the rest of the PR seems fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants