aws / aws-sdk-net

The official AWS SDK for .NET. For more information on the AWS SDK for .NET, see our web site:
http://aws.amazon.com/sdkfornet/
Apache License 2.0
2.06k stars 856 forks source link

no_proxy environment variable is not respected #3198

Closed slinkymanbyday closed 4 months ago

slinkymanbyday commented 8 months ago

Describe the bug

When https_proxy and no_proxy environment variables are set, the SDK will always go through the proxy, event if AWS endpoints are in the no_proxy list.

Bug introduced here: https://github.com/aws/aws-sdk-net/commit/8cf768ce5675271b0fe86516ade9d57c223bc306

For example, we go through proxy for when private endpoints are not supported for a service, and go direct when we have a private endpoint for a service provisioned, our no_proxy looks something like:

PS C:\Windows\system32> $env:no_proxy

169.254.169.254,127.0.0.1,localhost,acm-pca.ap-southeast-2.amazonaws.com,autoscaling.ap-southeast-2.amazonaws.com,cloudformation.ap-southeast-2.amazo
naws.com,codedeploy-commands-secure.ap-southeast-2.amazonaws.com,codedeploy.ap-southeast-2.amazonaws.com,dynamodb.ap-southeast-2.amazonaws.com,ec2.ap-southeast-2.amazonaws.com,ec2messages.ap-southeast-2.amazonaw
s.com,ecs.ap-southeast-2.amazonaws.com,elasticache.ap-southeast-2.amazonaws.com,elasticfilesystem.ap-southeast-2.amazonaws.com,elasticloadbalancing.ap-southeast-2.amazonaws.com,events.ap-southeast-2.amazonaws.co
m,execute-api.ap-southeast-2.amazonaws.com,fsx.ap-southeast-2.amazonaws.com,glue.ap-southeast-2.amazonaws.com,imagebuilder.ap-southeast-2.amazonaws.com,kms.ap-southeast-2.amazonaws.com,lambda.ap-southeast-2.amaz
onaws.com,logs.ap-southeast-2.amazonaws.com,monitoring.ap-southeast-2.amazonaws.com,rds.ap-southeast-2.amazonaws.com,s3-ap-southeast-2.amazonaws.com,s3.ap-southeast-2.amazonaws.com,s3.dualstack.ap-southeast-2.am
azonaws.com,secretsmanager.ap-southeast-2.amazonaws.com,sns.ap-southeast-2.amazonaws.com,sqs.ap-southeast-2.amazonaws.com,ssm.ap-southeast-2.amazonaws.com,ssmmessages.ap-southeast-2.amazonaws.com,sts.ap-southeas
t-2.amazonaws.com

Expected Behavior

endpoints that are specified in the no_proxy list do not use the proxy. The expected behavior is also mentioned in https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-proxy.html

Current Behavior

endpoints that are listed in the no_proxy environment variable still use the proxy set by the HTTP_PROXY environment variable.

Reproduction Steps

setx https_proxy http://proxy.internal:8080 setx no_proxy ssm.ap-southeast-2.amazonaws.com $instance = Get-EC2InstanceMetadata -Category InstanceId $runPSCommand = Send-SSMCommand -DocumentName AWS-ConfigureAWSPackage -InstanceId "$instance"-Parameter @{'action'='Install';'name'='AwsVssComponents'}

should reach endpoint (either IAM failure, or success)

Current behaviour: proxy deny (if endpoint not allowed through proxy), proxy allow, or proxy timeout (if proxy doesn't exist)

Possible Solution

No response

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

AWS Tools for Windows PowerShell Version 4.1.512 Copyright 2012-2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.

Amazon Web Services SDK for .NET Core Runtime Version 3.7.302.7 Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

Targeted .NET Platform

whichever the Powershell tools use

Operating System and version

Windows Server 2016, 2019, 2022

dscpinheiro commented 8 months ago

I believe the answer is yes, but are you using the AWSPowerShell module (it's the one installed by default on all Windows-based AMIs)? The commit you linked didn't include no_proxy, but in newer versions of .NET the HTTP client automatically picks up the environment variable. This is unfortunately not the case for the .NET Framework (which is the target used by AWSPowerShell).

If I try your example using the AWSPowerShell.NetCore module instead (which offers the same functionality as AWSPowerShell), the no_proxy values work as expected. Updating your environment to use that package (or even AWS.Tools, which has a separate module for each service) should unblock you.

normj commented 8 months ago

I have created a PR to no_proxy support for the SDK. We will work on getting this merged and released. https://github.com/aws/aws-sdk-net/pull/3200

slinkymanbyday commented 7 months ago

Thanks for the quick response and PR for this issue. Might not be the right place to ask, but any idea when this will be released and land in Powershell tools?

github-actions[bot] commented 7 months ago

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

normj commented 7 months ago

@slinkymanbyday Version 4.1.525 of the PowerShell modules went out about an hour ago which contains the latest version of the AWS .NET SDK with this fix.

slinkymanbyday commented 6 months ago

Hi @normj and @dscpinheiro

Thought this was good since a couple of my tests seemed to work, but wondering what the implementation of the no_proxy matching here is, since it seems to be failing when chatting to s3 buckets.

https://github.com/aws/aws-sdk-net/blob/main/sdk/src/Core/Amazon.Util/Internal/_bcl/NoProxyFilter.cs#L54 This seems to put together a regex which matches the entire domain, however from my understanding most implemntations do suffix matching. ie, if an entry in the no_proxy list is a suffix match for the requested domain, then don't use the proxy.

For example, i have s3.ap-southeast-2.amazonaws.com in my no_proxy list. This works fine with cli tools when copying a file from a bucket (aws s3 cp ...). However with powershell tools, this doesn't work, as the requested domain is <bucket>.s3.ap-southeast-2.amazonaws.com .

my question is, is there a way I can make my no_proxy list compatible with CLI tools and powershell tools? It doesn't appear so with the current implementation. I'm wondering if NoProxyFilter needs ot be modified to just be a simple suffix match instead?

peterrsongg commented 4 months ago

@slinkymanbyday The fix for this has been released in AWSSDK.Core Version 3.7.304.13. Will close the issue for now. Thanks for reporting the issue!

github-actions[bot] commented 4 months ago

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

slinkymanbyday commented 4 months ago

thanks for the fix @peterrsongg