Skip to content
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

RevokeSecurityGroupEgress and AuthorizeSecurityGroupEgress requests not working #2559

Open
2 tasks done
awatterson22 opened this issue Mar 15, 2024 · 8 comments · May be fixed by #2630
Open
2 tasks done

RevokeSecurityGroupEgress and AuthorizeSecurityGroupEgress requests not working #2559

awatterson22 opened this issue Mar 15, 2024 · 8 comments · May be fixed by #2630
Labels
bug This issue is a bug. p2 This is a standard priority issue queued This issues is on the AWS team's backlog service-api This issue is due to a problem in a service API, not the SDK implementation.

Comments

@awatterson22
Copy link

awatterson22 commented Mar 15, 2024

Acknowledgements

Describe the bug

When running this file:

Note: sg-fake-id is not what is actually used but a real security group id from the AWS Console is used

package main

import (
	"context"
	"fmt"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/ec2"
	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
)

func main() {
	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"))
	if err != nil {
		panic(err)
	}

	client := ec2.NewFromConfig(cfg)

	output, _ := client.DescribeSecurityGroups(context.TODO(), &ec2.DescribeSecurityGroupsInput{
		Filters: []types.Filter{
			{
				Name:   aws.String("group-id"),
				Values: []string{"sg-fake-id"},
			},
		},
	})

	sg := output.SecurityGroups[0]

	revokeInput := &ec2.RevokeSecurityGroupEgressInput{
		GroupId:       sg.GroupId,
		IpPermissions: sg.IpPermissionsEgress,
	}

	stuff, err := client.RevokeSecurityGroupEgress(context.TODO(), revokeInput)
	if err != nil {
		fmt.Println("Error", err)
		return
	}
	fmt.Println(stuff)

	fmt.Println("Egress successfully revoked")

	authorizeInput := &ec2.AuthorizeSecurityGroupEgressInput{
		GroupId:       sg.GroupId,
		IpPermissions: sg.IpPermissionsEgress,
	}

	_, err = client.AuthorizeSecurityGroupEgress(context.TODO(), authorizeInput)
	if err != nil {
		fmt.Println("Error", err)
		return
	}

	fmt.Println("Egress successfully authorized")
}

I get an error saying

Error operation error EC2: RevokeSecurityGroupEgress, https response error StatusCode: 400, RequestID: *******, api error InvalidRequest: The request received was invalid. 

Or

Error operation error EC2: AuthorizeSecurityGroupEgress, https response error StatusCode: 400, RequestID: *******, api error InvalidRequest: The request received was invalid. 

Expected Behavior

I expected to be able to revoke all the egress rules from the security group after getting them from the describe command. This logic works for aws-sdk-go, but I am trying to upgrade to aws-sdk-go-v2

Current Behavior

I am getting an error saying

Error operation error EC2: RevokeSecurityGroupEgress, https response error StatusCode: 400, RequestID: *******, api error InvalidRequest: The request received was invalid. 

Or

Error operation error EC2: AuthorizeSecurityGroupEgress, https response error StatusCode: 400, RequestID: *******, api error InvalidRequest: The request received was invalid. 

Reproduction Steps

Can be reproduced by running the script above

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

go.mod file:

module utils

go 1.22.1

require (
	github.com/aws/aws-sdk-go-v2 v1.25.3
	github.com/aws/aws-sdk-go-v2/config v1.27.7
	github.com/aws/aws-sdk-go-v2/service/ec2 v1.150.1
)

require (
	github.com/aws/aws-sdk-go-v2/credentials v1.17.7 // indirect
	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.15.3 // indirect
	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.3 // indirect
	github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.3 // indirect
	github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
	github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.1 // indirect
	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.5 // indirect
	github.com/aws/aws-sdk-go-v2/service/sso v1.20.2 // indirect
	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.23.2 // indirect
	github.com/aws/aws-sdk-go-v2/service/sts v1.28.4 // indirect
	github.com/aws/smithy-go v1.20.1 // indirect
	github.com/jmespath/go-jmespath v0.4.0 // indirect
)

Compiler and Version used

go version go1.22.1 darwin/arm64

Operating System and version

macOs Sonoma - Version 14.2.1

@awatterson22 awatterson22 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 15, 2024
@aBurmeseDev
Copy link
Member

Hi @awatterson22 - thanks for reaching out and sorry to hear about the issue.

The errors you're seeing indicate that you might have some params that are invalid for RevokeSecurityGroupEgress and AuthorizeSecurityGroupEgress.

For further investigation, can you please enable request and response logging and share with us here?

	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"), config.WithClientLogMode(aws.LogRequestWithBody|aws.LogResponseWithBody))
	if err != nil {
		panic(err)
	}

Best,
John

@aBurmeseDev aBurmeseDev added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p3 This is a minor priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Mar 22, 2024
@awatterson22
Copy link
Author

Hi, thank you for your reply. Here is the logging output with information redacted:

└─ $ go run main.go
SDK 2024/03/21 20:59:29 DEBUG Request
POST / HTTP/1.1
Host: ec2.us-east-1.amazonaws.com
User-Agent: aws-sdk-go-v2/1.25.3 os/macos lang/go#1.22.1 md/GOOS#darwin md/GOARCH#arm64 api/ec2#1.150.1
Content-Length: 109
Amz-Sdk-Invocation-Id: ********
Amz-Sdk-Request: attempt=1; max=3
Authorization: AWS4-HMAC-SHA256 Credential=********/20240322/us-east-1/ec2/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-date;x-amz-security-token, Signature=********
Content-Type: application/x-www-form-urlencoded
X-Amz-Date: 20240322T005929Z
X-Amz-Security-Token: ********
Accept-Encoding: gzip

Action=DescribeSecurityGroups&Filter.1.Name=group-id&Filter.1.Value.1=sg-id&Version=2016-11-15
SDK 2024/03/21 20:59:30 DEBUG Response
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Cache-Control: no-cache, no-store
Content-Type: text/xml;charset=UTF-8
Date: Fri, 22 Mar 2024 00:59:29 GMT
Server: AmazonEC2
Strict-Transport-Security: max-age=31536000; includeSubDomains
Vary: accept-encoding
X-Amzn-Requestid: ********

c49
<?xml version="1.0" encoding="UTF-8"?>
<DescribeSecurityGroupsResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/">
    <requestId>********</requestId>
    <securityGroupInfo>
        <item>
            <ownerId>account-id</ownerId>
            <groupId>sg-id</groupId>
            <groupName>eks-cluster-sg-cluster-name-randomnumber</groupName>
            <groupDescription>EKS created security group applied to ENI that is attached to EKS Control Plane master nodes, as well as any managed workloads.</groupDescription>
            <vpcId>vpc-id</vpcId>
            <ipPermissions>
                <item>
                    <ipProtocol>-1</ipProtocol>
                    <groups>
                        <item>
                            <userId>account-id</userId>
                            <groupId>sg-id</groupId>
                        </item>
                    </groups>
                    <ipRanges/>
                    <ipv6Ranges/>
                    <prefixListIds/>
                </item>
            </ipPermissions>
            <ipPermissionsEgress>
                <item>
                    <ipProtocol>tcp</ipProtocol>
                    <fromPort>1025</fromPort>
                    <toPort>65535</toPort>
                    <groups>
                        <item>
                            <userId>account-id</userId>
                            <groupId>sg-id</groupId>
                        </item>
                    </groups>
                    <ipRanges/>
                    <ipv6Ranges/>
                    <prefixListIds/>
                </item>
                <item>
                    <ipProtocol>tcp</ipProtocol>
                    <fromPort>443</fromPort>
                    <toPort>443</toPort>
                    <groups>
                        <item>
                            <userId>account-id</userId>
                            <groupId>sg-id</groupId>
                        </item>
                    </groups>
                    <ipRanges/>
                    <ipv6Ranges/>
                    <prefixListIds>
                        <item>
                            <prefixListId>pl-id</prefixListId>
                        </item>
                    </prefixListIds>
                </item>
            </ipPermissionsEgress>
            <tagSet>
                <item>
                    <key>Name</key>
                    <value>eks-cluster-sg-cluster-name-randomnumber</value>
                </item>
                <item>
                    <key>aws:eks:cluster-name</key>
                    <value>cluster-name</value>
                </item>
            </tagSet>
        </item>
    </securityGroupInfo>
</DescribeSecurityGroupsResponse>
0

SDK 2024/03/21 20:59:30 DEBUG Request
POST / HTTP/1.1
Host: ec2.us-east-1.amazonaws.com
User-Agent: aws-sdk-go-v2/1.25.3 os/macos lang/go#1.22.1 md/GOOS#darwin md/GOARCH#arm64 api/ec2#1.150.1
Content-Length: 651
Amz-Sdk-Invocation-Id: ********
Amz-Sdk-Request: attempt=1; max=3
Authorization: AWS4-HMAC-SHA256 Credential=********/20240322/us-east-1/ec2/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-date;x-amz-security-token, Signature=********
Content-Type: application/x-www-form-urlencoded
X-Amz-Date: 20240322T005930Z
X-Amz-Security-Token: ********
Accept-Encoding: gzip

Action=RevokeSecurityGroupEgress&GroupId=sg-id&IpPermissions.1.FromPort=1025&IpPermissions.1.Groups.1.GroupId=sg-id&IpPermissions.1.Groups.1.UserId=account-id&IpPermissions.1.IpProtocol=tcp&IpPermissions.1.IpRanges=&IpPermissions.1.Ipv6Ranges=&IpPermissions.1.PrefixListIds=&IpPermissions.1.ToPort=65535&IpPermissions.2.FromPort=443&IpPermissions.2.Groups.1.GroupId=sg-id&IpPermissions.2.Groups.1.UserId=account-id&IpPermissions.2.IpProtocol=tcp&IpPermissions.2.IpRanges=&IpPermissions.2.Ipv6Ranges=&IpPermissions.2.PrefixListIds.1.PrefixListId=pl-id&IpPermissions.2.ToPort=443&Version=2016-11-15
SDK 2024/03/21 20:59:30 DEBUG Response
HTTP/1.1 400 Bad Request
Connection: close
Transfer-Encoding: chunked
Cache-Control: no-cache, no-store
Content-Type: text/xml;charset=UTF-8
Date: Fri, 22 Mar 2024 00:59:29 GMT
Server: AmazonEC2
Strict-Transport-Security: max-age=31536000; includeSubDomains
Vary: accept-encoding
X-Amzn-Requestid: ********

e6
<?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>InvalidRequest</Code><Message>The request received was invalid.</Message></Error></Errors><RequestID>********</RequestID></Response>
0

Error operation error EC2: RevokeSecurityGroupEgress, https response error StatusCode: 400, RequestID: ********, api error InvalidRequest: The request received was invalid.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 23, 2024
@awatterson22
Copy link
Author

@aBurmeseDev - any updates on this issue? Thank you!

@aBurmeseDev
Copy link
Member

HI @awatterson22 - thanks for sharing the logs and apology for the delay.

I've attempted to reproduce by calling RevokeSecurityGroupEgress and AuthorizeSecurityGroupEgress by entering required params to each call and was able to make the requests successfully.

According to the logs and error, it confirms that you have invalid params for both calls (RevokeSecurityGroupEgress & AuthorizeSecurityGroupEgress) and looking at your code, both calls receive their params from DescribeSecurityGroups call.

	output, _ := client.DescribeSecurityGroups(context.TODO(), &ec2.DescribeSecurityGroupsInput{
		Filters: []types.Filter{
			{
				Name:   aws.String("group-id"),
				Values: []string{"sg-fake-id"},
			},
		},
	})

	sg := output.SecurityGroups[0]

	revokeInput := &ec2.RevokeSecurityGroupEgressInput{
		GroupId:       sg.GroupId,
		IpPermissions: sg.IpPermissionsEgress,
	}

Next step is to confirm if you're actually getting your params correctly from DescribeSecurityGroups and your Filter. Also please note that IpPermissions would be an array of IpPermission objects. I would check that to make sure you're passing correct type.
Hope it helps,
John

@aBurmeseDev aBurmeseDev added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 4, 2024
@awatterson22
Copy link
Author

@aBurmeseDev, so I did take a look at what I am getting back from DescribeSecurityGroups. The GroupId is correct. When looking at it I get only 2 items, but in the AWS Console, there are 3 items. Maybe that could be why? This is what the RevokeSecurityGroupEgress is sending:

Action=RevokeSecurityGroupEgress
&GroupId=*******
&IpPermissions.1.FromPort=1025
&IpPermissions.1.Groups.1.GroupId=*******
&IpPermissions.1.Groups.1.UserId=*******
&IpPermissions.1.IpProtocol=tcp
&IpPermissions.1.IpRanges=
&IpPermissions.1.Ipv6Ranges=
&IpPermissions.1.PrefixListIds=
&IpPermissions.1.ToPort=65535
&IpPermissions.2.FromPort=443
&IpPermissions.2.Groups.1.GroupId=*******
&IpPermissions.2.Groups.1.UserId=*******
&IpPermissions.2.IpProtocol=tcp
&IpPermissions.2.IpRanges=
&IpPermissions.2.Ipv6Ranges=
&IpPermissions.2.PrefixListIds.1.PrefixListId=*******
&IpPermissions.2.ToPort=443
&Version=2016-11-15

@awatterson22
Copy link
Author

awatterson22 commented Apr 8, 2024

@aBurmeseDev, so I did take a look at what I am getting back from DescribeSecurityGroups. The GroupId is correct. When looking at it I get only 2 items, but in the AWS Console, there are 3 items. Maybe that could be why? This is what the RevokeSecurityGroupEgress is sending:

Action=RevokeSecurityGroupEgress &GroupId=******* &IpPermissions.1.FromPort=1025 &IpPermissions.1.Groups.1.GroupId=******* &IpPermissions.1.Groups.1.UserId=******* &IpPermissions.1.IpProtocol=tcp &IpPermissions.1.IpRanges= &IpPermissions.1.Ipv6Ranges= &IpPermissions.1.PrefixListIds= &IpPermissions.1.ToPort=65535 &IpPermissions.2.FromPort=443 &IpPermissions.2.Groups.1.GroupId=******* &IpPermissions.2.Groups.1.UserId=******* &IpPermissions.2.IpProtocol=tcp &IpPermissions.2.IpRanges= &IpPermissions.2.Ipv6Ranges= &IpPermissions.2.PrefixListIds.1.PrefixListId=******* &IpPermissions.2.ToPort=443 &Version=2016-11-15

I looped through and changed to this:

package main

import (
	"context"
	"fmt"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/ec2"
	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
)

func main() {
	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"))
	if err != nil {
		panic(err)
	}

	client := ec2.NewFromConfig(cfg)

	output, _ := client.DescribeSecurityGroups(context.TODO(), &ec2.DescribeSecurityGroupsInput{
		Filters: []types.Filter{
			{
				Name:   aws.String("group-id"),
				Values: []string{"********"},
			},
		},
	})

	sg := output.SecurityGroups[0]

	ipPermissions := []types.IpPermission{}
	for _, ipPermission := range sg.IpPermissionsEgress {
		item := types.IpPermission{
			FromPort:   ipPermission.FromPort,
			ToPort:     ipPermission.ToPort,
			IpProtocol: ipPermission.IpProtocol,
		}
		if len(ipPermission.IpRanges) > 0 {
			item.IpRanges = ipPermission.IpRanges
		}
		if len(ipPermission.Ipv6Ranges) > 0 {
			item.Ipv6Ranges = ipPermission.Ipv6Ranges
		}
		if len(ipPermission.PrefixListIds) > 0 {
			item.PrefixListIds = ipPermission.PrefixListIds
		}
		if len(ipPermission.UserIdGroupPairs) > 0 {
			item.UserIdGroupPairs = ipPermission.UserIdGroupPairs
		}
		ipPermissions = append(ipPermissions, item)
	}

	revokeInput := &ec2.RevokeSecurityGroupEgressInput{
		GroupId:       sg.GroupId,
		IpPermissions: ipPermissions,
	}

	_, err = client.RevokeSecurityGroupEgress(context.TODO(), revokeInput)
	if err != nil {
		fmt.Println("Error", err)
		return
	}

	fmt.Println("Egress successfully revoked")

        authorizeInput := &ec2.AuthorizeSecurityGroupEgressInput{
		GroupId:       sg.GroupId,
		IpPermissions: ipPermissions,
	}

	_, err = client.AuthorizeSecurityGroupEgress(context.TODO(), authorizeInput)
	if err != nil {
		fmt.Println("Error", err)
		return
	}

	fmt.Println("Egress successfully authorized")
}

Revoke and Authorize then worked. Is it possible that we could allow empty lists as part of the validation?

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 9, 2024
@lucix-aws
Copy link
Contributor

lucix-aws commented Apr 18, 2024

This breaks down like so:

  1. v1 and v2 serialization behaviors are the same. nil slice doesn't get encoded, nonnil len=0 slice gets encoded as list= (explicitly there, empty string)
  2. v1 and v2 deserialization behaviors are different. v1 does not mutate the list field value (as in, it just leaves it nil) if the xml node that represents the list doesn't have children like we saw in the ec2 response. v2 will set it to an empty slice if the node is present. the slice would only remain nil in v2 if the node for the list is not present

The latter is the root cause here. We can see from the wire log that the DescribeSecurityGroups response encodes several empty lists within the IpPermission structure as self-closing XML nodes (e.g. <ipRanges/>). In v1, the fields that correspond to these elements come back to you the caller left as nil. In v2 they instead come back as []<type>{}. You round-trip that structure into the followup request and for reasons unknown to us, the EC2 service apparently does not accept the latter.

Changing v2's deserialization to match v1's isn't out of the question, but of course that could technically break something else.

v2's deserialization is arguably more correct. Ignoring that the difference doesn't matter in go, v2 correctly handles the distinction between "empty list" and "no list" as it comes back on the wire. As a Go SDK user you won't care though, and since this particular protocol is used only in EC2, I'm including it here as an option.

Upstreaming this to EC2 is the only other way forward. I don't think their software should care about the difference between the two, at least for these particular operations, but it's impossible to know without asking them directly.

@lucix-aws lucix-aws added service-api This issue is due to a problem in a service API, not the SDK implementation. p2 This is a standard priority issue queued This issues is on the AWS team's backlog and removed p3 This is a minor priority issue good-first-issue labels Apr 18, 2024
@aBurmeseDev aBurmeseDev removed their assignment Apr 23, 2024
@lucix-aws
Copy link
Contributor

The change we previously made to serialize empty lists should have only applied to awsQuery services, not ec2Query. Will be resolved by addressing #2627.

@lucix-aws lucix-aws linked a pull request May 6, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue queued This issues is on the AWS team's backlog service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants