Skip to content
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions extensions/test/CrtIntegrationTests/V4aSignerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,36 @@ internal static IRequest BuildHeaderRequestToSign(string resourcePath, Dictionar
return request;
}

/// <summary>
/// Dummy request class needed for DefaultRequest constructor
/// </summary>
private class DummyRequest : AmazonWebServiceRequest { }

/// <summary>
/// Creates a test request with mutable collections to verify header modifications
/// during signing.
/// </summary>
internal static IRequest CreateDefaultRequest(string resourcePath)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to create this method instead of using BuildHeaderRequestToSign?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need the real DefaultRequest because CRT modifies the headers directly during signing. The mock version returns an immutable dictionary so the CRT can't add headers to it. The tests check that those headers actually got added to the request

Copy link
Member

Choose a reason for hiding this comment

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

image The dictionary looks like a regular dictionary that the CRT can add to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm still not sure we actually need to change any tests in the extensions folder.

{
var publicRequest = new DummyRequest();
var request = new DefaultRequest(publicRequest, SigningTestService)
{
HttpMethod = "POST",
ResourcePath = resourcePath,
Endpoint = new Uri($"https://{SigningTestHost}/"),
Content = Encoding.ASCII.GetBytes("Param1=value1")
};

request.Headers["Content-Type"] = "application/x-www-form-urlencoded";
request.Headers["Content-Length"] = "13";
// Add SDK tracking headers that should NOT be signed
request.Headers["amz-sdk-request"] = "attempt=1; max=5";
request.Headers["amz-sdk-invocation-id"] = "a7d0c828-1fc1-43e8-9f9e-367a7011fc84";
request.Headers["x-amzn-trace-id"] = "Root=1-63441c4a-abcdef012345678912345678";

return request;
}

internal string GetExpectedCanonicalRequestForHeaderSigningTest(string canonicalizedResourePath)
{
return String.Join('\n',
Expand Down Expand Up @@ -440,5 +470,97 @@ public void TestChunkedRequestWithTrailingHeaders()
Encoding.ASCII.GetBytes(trailerChunkResult), SigningTestEccPubX, SigningTestEccPubY));
}
#endregion

#region Multi-Region SigV4a Signing Tests

/// <summary>
/// Tests multi-region SigV4a signing with different region set configurations.
///
/// The CRT library handles the x-amz-region-set header internally during the signing process.
/// This test verifies that different region configurations produce different signatures
/// and that the region set is actually used in the signing calculation.
/// </summary>
[Fact]
public void TestMultiRegionSigV4a_DifferentRegionSetsProduceDifferentSignatures()
{
var signer = new CrtAWS4aSigner();
var clientConfig = BuildSigningClientConfig(SigningTestService);

// Test with different region configurations
var regionSets = new[] { "us-west-2", "us-west-2,us-east-1", "*", "eu-west-1" };
var signatures = new Dictionary<string, string>();

foreach (var regionSet in regionSets)
{
var request = CreateDefaultRequest("/");
request.SigV4aSigningRegionSet = regionSet;
// The base endpoint resolver would normally set AuthenticationRegion from SigV4aSigningRegionSet
request.AuthenticationRegion = regionSet;

var result = signer.SignRequest(request, clientConfig, null, SigningTestCredentials);

// Verify basic result properties
Assert.NotNull(result);
Assert.NotNull(result.Signature);
Assert.NotEmpty(result.Signature);
Assert.Equal(regionSet, result.RegionSet);

// Store signature for comparison
signatures[regionSet] = result.Signature;
}

// Verify that different region sets produce different signatures
// This ensures the region set is actually being used in the signing calculation
Assert.NotEqual(signatures["us-west-2"], signatures["us-west-2,us-east-1"]);
Assert.NotEqual(signatures["us-west-2"], signatures["*"]);
Assert.NotEqual(signatures["us-west-2"], signatures["eu-west-1"]);
Assert.NotEqual(signatures["us-west-2,us-east-1"], signatures["*"]);

// The x-amz-region-set header is handled internally by CRT for signing.
// Different signatures confirm that multi-region information is being used correctly.
}

/// <summary>
/// Tests backward compatibility: when SigV4aSigningRegionSet is not set,
/// the signer should fall back to single-region behavior.
/// </summary>
[Fact]
public void TestSigV4aFallbackToSingleRegion()
{
var signer = new CrtAWS4aSigner();
var request = CreateDefaultRequest("/");
// Explicitly NOT setting request.SigV4aSigningRegionSet

var clientConfig = BuildSigningClientConfig(SigningTestService);
var result = signer.SignRequest(request, clientConfig, null, SigningTestCredentials);

// Should fall back to us-east-1 (from SigningTestRegion)
Assert.Equal(SigningTestRegion, request.DeterminedSigningRegion);
Assert.True(request.Headers.ContainsKey(HeaderKeys.XAmzRegionSetHeader));
Assert.Equal(SigningTestRegion, request.Headers[HeaderKeys.XAmzRegionSetHeader]);

var expectedCanonicalRequest = String.Join('\n',
"POST", "/", "",
"content-length:13",
"content-type:application/x-www-form-urlencoded",
"host:example.amazonaws.com",
"x-amz-content-sha256:9095672bbd1f56dfc5b65f3e153adc8731a4a654192329106275f4c7b24d0b6e",
"x-amz-date:20150830T123600Z",
$"x-amz-region-set:{SigningTestRegion}",
"",
"content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-region-set",
"9095672bbd1f56dfc5b65f3e153adc8731a4a654192329106275f4c7b24d0b6e");

var config = BuildDefaultSigningConfig(SigningTestService);
config.SignatureType = AwsSignatureType.CANONICAL_REQUEST_VIA_HEADERS;
config.Region = SigningTestRegion;

Assert.True(AwsSigner.VerifyV4aCanonicalSigning(
expectedCanonicalRequest, config, result.Signature,
SigningTestEccPubX, SigningTestEccPubY),
"Fallback signature verification failed");
}

#endregion
}
}
16 changes: 16 additions & 0 deletions generator/.DevConfigs/3aa6313d-9526-40ba-b09c-e046e0d4ef2f.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"core": {
"changeLogMessages": [
"Added ability to configure authentication scheme preferences (e.g., prioritize SigV4a over SigV4)",
"Added support for AWS_AUTH_SCHEME_PREFERENCE environment variable and auth_scheme_preference configuration file setting",
"Added support for AWS_SIGV4A_SIGNING_REGION_SET environment variable and sigv4a_signing_region_set profile key to configure SigV4a signing region set"
],
"type": "minor",
"updateMinimum": true,
"backwardIncompatibilitiesToIgnore": [
"Amazon.Runtime.Internal.IRequest/MethodAbstractMethodAdded",
"Amazon.Runtime.IClientConfig/MethodAbstractMethodAdded",
"Amazon.Runtime.IRequestContext/MethodAbstractMethodAdded"
]
}
}
Comment on lines +1 to +16
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

Public interface members were added (IClientConfig, IRequest, IRequestContext) which is a breaking change for any external implementers. Classifying this as a minor version while explicitly ignoring the incompatibilities does not align with the project's API compatibility guideline; either (a) avoid modifying existing public interfaces (e.g., use extension methods, wrapper abstractions, or default interface members with backward-compatible defaults if allowed), or (b) bump the version appropriately (major) instead of suppressing. Recommend reworking to preserve binary/source compatibility or reclassifying the dev config.

Copilot generated this review using guidance from repository custom instructions.
42 changes: 42 additions & 0 deletions sdk/src/Core/Amazon.Runtime/ClientConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public abstract partial class ClientConfig : IClientConfig
private string serviceURL = null;
private string authRegion = null;
private string authServiceName = null;
private string authSchemePreference = null;
private string sigV4aSigningRegionSet = null;
private string clientAppId = null;
private SigningAlgorithm signatureMethod = SigningAlgorithm.HmacSHA256;
private bool logResponse = false;
Expand Down Expand Up @@ -444,6 +446,46 @@ public string AuthenticationServiceName
get { return this.authServiceName; }
set { this.authServiceName = value; }
}

/// <summary>
/// Gets and sets the AuthSchemePreference property.
/// A comma-separated list of authentication scheme names to use in order of preference.
/// For example: "sigv4a,sigv4" to prefer SigV4a over SigV4.
/// </summary>
public string AuthSchemePreference
{
get
{
if (!string.IsNullOrEmpty(this.authSchemePreference))
return this.authSchemePreference;

// Fallback to environment variable or config file:
// 1. Environment variable: AWS_AUTH_SCHEME_PREFERENCE
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments are redundant. I don't expect the variable name to ever change but these comments don't add any value.

// 2. Config file: auth_scheme_preference
return FallbackInternalConfigurationFactory.AuthSchemePreference;
}
set { this.authSchemePreference = value; }
}

/// <summary>
/// Gets and sets the SigV4aSigningRegionSet property.
/// A comma-separated list of regions that a SigV4a signature will be valid for.
/// Use "*" to indicate all regions.
/// </summary>
public string SigV4aSigningRegionSet
{
get
{
if (!string.IsNullOrEmpty(this.sigV4aSigningRegionSet))
return this.sigV4aSigningRegionSet;

// Fallback to environment variable or config file:
// 1. Environment variable: AWS_SIGV4A_SIGNING_REGION_SET
// 2. Config file: sigv4a_signing_region_set
return FallbackInternalConfigurationFactory.SigV4aSigningRegionSet;
}
set { this.sigV4aSigningRegionSet = value; }
}
Comment on lines 502 to 551
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

There is no integration test exercising the environment-variable or profile fallback path for SigV4aSigningRegionSet feeding into actual signed requests (x-amz-region-set header and signature variation). Current tests set the request property directly; add a test that sets AWS_SIGV4A_SIGNING_REGION_SET (and one via profile) and asserts the signer produces expected header/signature.

Copilot generated this review using guidance from repository custom instructions.

/// <summary>
/// The serviceId for the service, which is specified in the metadata in the ServiceModel.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,18 @@ internal Dictionary<string, Dictionary<string, string>> NestedProperties
/// </summary>
public AccountIdEndpointMode? AccountIdEndpointMode { get; set; }

/// <summary>
/// Preference list of authentication schemes to use when multiple schemes are available.
/// This is a comma-separated list of auth scheme names like "sigv4,sigv4a,bearer".
/// </summary>
public string AuthSchemePreference { get; set; }

/// <summary>
/// The region set to use for SigV4a signing. This can be a single region,
/// a comma-separated list of regions, or "*" for all regions.
/// </summary>
public string SigV4aSigningRegionSet { get; set; }

/// <summary>
/// An optional dictionary of name-value pairs stored with the CredentialProfile
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ public class SharedCredentialsFile : ICredentialProfileStore
private const string AccountIdEndpointModeField = "account_id_endpoint_mode";
private const string RequestChecksumCalculationField = "request_checksum_calculation";
private const string ResponseChecksumValidationField = "response_checksum_validation";
private const string AuthSchemePreferenceField = "auth_scheme_preference";
private const string SigV4aSigningRegionSetField = "sigv4a_signing_region_set";
private const string AwsAccountIdField = "aws_account_id";
private readonly Logger _logger = Logger.GetLogger(typeof(SharedCredentialsFile));

Expand Down Expand Up @@ -106,6 +108,8 @@ public class SharedCredentialsFile : ICredentialProfileStore
AccountIdEndpointModeField,
RequestChecksumCalculationField,
ResponseChecksumValidationField,
AuthSchemePreferenceField,
SigV4aSigningRegionSetField,
AwsAccountIdField,
};

Expand Down Expand Up @@ -859,6 +863,19 @@ private bool TryGetProfile(string profileName, bool doRefresh, bool isSsoSession
}
responseChecksumValidation = responseChecksumValidationTemp;
}

string authSchemePreference = null;
if (reservedProperties.TryGetValue(AuthSchemePreferenceField, out var authSchemePrefString))
{
authSchemePreference = authSchemePrefString;
}

string sigV4aSigningRegionSet = null;
if (reservedProperties.TryGetValue(SigV4aSigningRegionSetField, out var sigV4aRegionSetString))
{
sigV4aSigningRegionSet = sigV4aRegionSetString;
}

profile = new CredentialProfile(profileName, profileOptions)
{
UniqueKey = toolkitArtifactGuid,
Expand Down Expand Up @@ -886,6 +903,8 @@ private bool TryGetProfile(string profileName, bool doRefresh, bool isSsoSession
AccountIdEndpointMode = accountIdEndpointMode,
RequestChecksumCalculation = requestChecksumCalculation,
ResponseChecksumValidation = responseChecksumValidation,
AuthSchemePreference = authSchemePreference,
SigV4aSigningRegionSet = sigV4aSigningRegionSet,
Services = servicesSection
};

Expand Down
14 changes: 14 additions & 0 deletions sdk/src/Core/Amazon.Runtime/IClientConfig.cs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,20 @@ public partial interface IClientConfig
/// </summary>
string AuthenticationServiceName { get; }

/// <summary>
/// Gets the AuthSchemePreference property.
/// A comma-separated list of authentication scheme names to use in order of preference.
/// For example: "sigv4a,sigv4" to prefer SigV4a over SigV4.
/// </summary>
string AuthSchemePreference { get; }

/// <summary>
/// Gets the SigV4aSigningRegionSet property.
/// A comma-separated list of regions that a SigV4a signature will be valid for.
/// Use "*" to indicate all regions.
/// </summary>
string SigV4aSigningRegionSet { get; }
Comment on lines +166 to +178
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider explicitly documenting the corresponding environment variables (AWS_AUTH_SCHEME_PREFERENCE, AWS_SIGV4A_SIGNING_REGION_SET) and profile keys (auth_scheme_preference, sigv4a_signing_region_set) in these summaries to improve discoverability for users configuring these options outside code.

Copilot uses AI. Check for mistakes.
Comment on lines +166 to +178
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

Adding new members to a public interface is a breaking change for consumers implementing IClientConfig. To maintain backward compatibility per project guidelines, consider: (1) implementing these as extension methods over an internal accessor, (2) introducing a new derived interface while keeping the original stable, or (3) using default interface implementations (only if all supported target frameworks allow it) with safe defaults. Current approach requires a recompilation and will break third-party custom clients.

Copilot generated this review using guidance from repository custom instructions.

/// <summary>
/// Gets the UserAgent property.
/// </summary>
Expand Down
7 changes: 7 additions & 0 deletions sdk/src/Core/Amazon.Runtime/Internal/DefaultRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,13 @@ public string CanonicalResourcePrefix
/// </summary>
public string AuthenticationRegion { get; set; }

/// <summary>
/// The signing region set to use for SigV4a requests.
/// Contains a comma-separated list of regions for multi-region signing.
/// Set from Config.SigV4aSigningRegionSet or endpoints metadata.
/// </summary>
public string SigV4aSigningRegionSet { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed. We should be able to leverage the existing logic for SigV4A without adding new properties here.


/// <summary>
/// The region in which the service request was signed.
/// </summary>
Expand Down
12 changes: 11 additions & 1 deletion sdk/src/Core/Amazon.Runtime/Internal/IRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,17 @@ string CanonicalResourcePrefix
string AuthenticationRegion { get; set; }

/// <summary>
/// The region in which the service request was signed.
/// The signing region set to use for SigV4a requests.
/// Contains a comma-separated list of regions for multi-region signing.
/// Set from Config.SigV4aSigningRegionSet or endpoints metadata.
/// </summary>
string SigV4aSigningRegionSet { get; set; }

/// <summary>
/// The region or region set used for signing the service request.
/// For standard SigV4 signing, this contains a single region (e.g., "us-west-2").
/// For SigV4a multi-region signing, this can be a comma-separated list of regions (e.g., "us-west-2,us-east-1")
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

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

Adding SigV4aSigningRegionSet to IRequest is a breaking interface change. Recommend introducing this via a wrapper (e.g., an associated metadata container), an opt-in capability interface, or an extension method pattern instead of modifying an existing widely-implemented interface to preserve compatibility.

Copilot generated this review using guidance from repository custom instructions.
/// or "*" to indicate the signature is valid for all regions.
/// </summary>
string DeterminedSigningRegion { get; set; }

Expand Down
Loading