mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
fix: reject security group rules without a protocol or tcp/udp ports, and classify rule IDs as AWS does (#1176)
This commit is contained in:
@@ -10,14 +10,14 @@ import (
|
||||
"github.com/nats-io/nats.go"
|
||||
)
|
||||
|
||||
// Nil input is valid and treated as "all rules in the account"; supplied
|
||||
// SecurityGroupRuleIds are validated against the sgr-<17 hex> shape before the
|
||||
// request crosses NATS so a malformed ID never hits the handler.
|
||||
// Nil input is valid and treated as "all rules in the account". A supplied
|
||||
// SecurityGroupRuleId AWS calls malformed is rejected before the request
|
||||
// crosses NATS; whether a well-formed ID exists is the handler's to answer.
|
||||
func DescribeSecurityGroupRules(ctx context.Context, input *ec2.DescribeSecurityGroupRulesInput, natsConn *nats.Conn, accountID string) (ec2.DescribeSecurityGroupRulesOutput, error) {
|
||||
var output ec2.DescribeSecurityGroupRulesOutput
|
||||
if input != nil {
|
||||
for _, id := range input.SecurityGroupRuleIds {
|
||||
if id == nil || !handlers_ec2_vpc.SGRuleIDRegex.MatchString(*id) {
|
||||
if id == nil || handlers_ec2_vpc.SGRuleIDIsMalformed(*id) {
|
||||
return output, errors.New(awserrors.ErrorInvalidSecurityGroupRuleIdMalformed)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,10 +23,9 @@ func TestDescribeSecurityGroupRules_NilInput_PassesValidation(t *testing.T) {
|
||||
func TestDescribeSecurityGroupRules_MalformedRuleID(t *testing.T) {
|
||||
cases := []string{
|
||||
"sgr-toolong0123456789abcdef",
|
||||
"sgr-XYZ",
|
||||
"SGR-0123456789abcdef0",
|
||||
"sg-0123456789abcdef0",
|
||||
"",
|
||||
"sgr-0123456789abcdefX",
|
||||
}
|
||||
for _, bad := range cases {
|
||||
input := &ec2.DescribeSecurityGroupRulesInput{
|
||||
@@ -46,12 +45,14 @@ func TestDescribeSecurityGroupRules_NilRuleIDEntry(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestDescribeSecurityGroupRules_ValidRuleIDPassesValidation(t *testing.T) {
|
||||
// A well-formed sgr- ID passes gateway validation; the call then dives
|
||||
// into the NATS service (nil conn) and fails there.
|
||||
input := &ec2.DescribeSecurityGroupRulesInput{
|
||||
SecurityGroupRuleIds: []*string{aws.String("sgr-0123456789abcdef0")},
|
||||
// An ID AWS answers NotFound for, not Malformed, passes gateway validation;
|
||||
// the call then dives into the NATS service (nil conn) and fails there.
|
||||
for _, id := range []string{"sgr-0123456789abcdef0", "sgr-XYZ", "sgr-0123456789abcdefX"} {
|
||||
input := &ec2.DescribeSecurityGroupRulesInput{
|
||||
SecurityGroupRuleIds: []*string{aws.String(id)},
|
||||
}
|
||||
_, err := DescribeSecurityGroupRules(context.Background(), input, nil, testAccountID)
|
||||
require.Error(t, err)
|
||||
assert.NotEqual(t, awserrors.ErrorInvalidSecurityGroupRuleIdMalformed, err.Error(), "id %q", id)
|
||||
}
|
||||
_, err := DescribeSecurityGroupRules(context.Background(), input, nil, testAccountID)
|
||||
require.Error(t, err)
|
||||
assert.NotEqual(t, awserrors.ErrorInvalidSecurityGroupRuleIdMalformed, err.Error())
|
||||
}
|
||||
|
||||
@@ -10,11 +10,11 @@ import (
|
||||
"github.com/nats-io/nats.go"
|
||||
)
|
||||
|
||||
// validateSGRuleDescriptions checks the sgr-<17 hex> shape before the request
|
||||
// crosses NATS so a malformed ID never reaches the handler.
|
||||
// validateSGRuleDescriptions rejects a rule ID AWS calls malformed before the
|
||||
// request crosses NATS; whether a well-formed ID exists is the handler's to answer.
|
||||
func validateSGRuleDescriptions(descriptions []*ec2.SecurityGroupRuleDescription) error {
|
||||
for _, d := range descriptions {
|
||||
if d == nil || d.SecurityGroupRuleId == nil || !handlers_ec2_vpc.SGRuleIDRegex.MatchString(*d.SecurityGroupRuleId) {
|
||||
if d == nil || d.SecurityGroupRuleId == nil || handlers_ec2_vpc.SGRuleIDIsMalformed(*d.SecurityGroupRuleId) {
|
||||
return errors.New(awserrors.ErrorInvalidSecurityGroupRuleIdMalformed)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,7 +35,7 @@ func TestUpdateSecurityGroupRuleDescriptions_MissingGroupId(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestUpdateSecurityGroupRuleDescriptions_MalformedRuleID(t *testing.T) {
|
||||
for _, bad := range []string{"sgr-toolong0123456789abcdef", "sgr-XYZ", "sg-0123456789abcdef0", ""} {
|
||||
for _, bad := range []string{"sgr-toolong0123456789abcdef", "SGR-0123456789abcdef0", "sg-0123456789abcdef0", ""} {
|
||||
_, err := gateway_ec2_vpc.UpdateSecurityGroupRuleDescriptionsIngress(context.Background(), &ec2.UpdateSecurityGroupRuleDescriptionsIngressInput{
|
||||
GroupId: aws.String("sg-0123456789abcdef0"),
|
||||
SecurityGroupRuleDescriptions: []*ec2.SecurityGroupRuleDescription{
|
||||
|
||||
@@ -26,11 +26,6 @@ import (
|
||||
// sgIDRegex must stay in lockstep with utils.GenerateResourceID("sg").
|
||||
var sgIDRegex = regexp.MustCompile(`^sg-[0-9a-f]{17}$`)
|
||||
|
||||
// SGRuleIDRegex must stay in lockstep with utils.GenerateResourceID("sgr").
|
||||
// Exported so the EC2 gateway can validate SecurityGroupRuleIds without
|
||||
// re-implementing the format check.
|
||||
var SGRuleIDRegex = regexp.MustCompile(`^sgr-[0-9a-f]{17}$`)
|
||||
|
||||
// allProtocols is the IpProtocol value naming every protocol, which is also
|
||||
// the port value AWS reports for such a rule.
|
||||
const allProtocols = "-1"
|
||||
@@ -45,11 +40,13 @@ var canonicalIPProtocols = []string{"tcp", "udp", "icmp", "-1"}
|
||||
var numericIPProtocols = map[string]string{"1": "icmp", "6": "tcp", "17": "udp"}
|
||||
|
||||
// normalizeIPProtocol canonicalises an AWS IpProtocol value the way AWS does,
|
||||
// returning "tcp" for both "tcp" and "6". An empty value means all protocols.
|
||||
// returning "tcp" for both "tcp" and "6". An empty value is rejected as AWS
|
||||
// rejects it: reading it as all protocols would open every port.
|
||||
func normalizeIPProtocol(proto string) (string, error) {
|
||||
proto = strings.ToLower(strings.TrimSpace(proto))
|
||||
if proto == "" {
|
||||
return "-1", nil
|
||||
return "", awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"Invalid value 'null' for protocol. VPC security group rules must specify protocols explicitly.")
|
||||
}
|
||||
if name, ok := numericIPProtocols[proto]; ok {
|
||||
return name, nil
|
||||
@@ -57,7 +54,18 @@ func normalizeIPProtocol(proto string) (string, error) {
|
||||
if slices.Contains(canonicalIPProtocols, proto) {
|
||||
return proto, nil
|
||||
}
|
||||
return "", fmt.Errorf("invalid IpProtocol %q: supported values are tcp, udp, icmp, -1 (or 6, 17, 1)", proto)
|
||||
return "", awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"invalid IpProtocol %q: supported values are tcp, udp, icmp, -1 (or 6, 17, 1)", proto)
|
||||
}
|
||||
|
||||
// requireSGRulePorts rejects a tcp or udp rule missing either port, which would
|
||||
// otherwise be stored as port 0.
|
||||
func requireSGRulePorts(proto string, fromPort, toPort *int64) error {
|
||||
if (proto == "tcp" || proto == "udp") && (fromPort == nil || toPort == nil) {
|
||||
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"Invalid value for portRange. Must specify both from and to ports with TCP/UDP.")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateSGRulePorts rejects a tcp or udp rule whose FromPort exceeds its ToPort.
|
||||
@@ -887,7 +895,7 @@ func (s *VPCServiceImpl) DescribeSecurityGroupRules(ctx context.Context, input *
|
||||
|
||||
for _, id := range input.SecurityGroupRuleIds {
|
||||
if !emitted[*id] {
|
||||
return nil, sgRuleNotFoundError(*id)
|
||||
return nil, sgRuleIDLookupError(*id)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1242,6 +1250,9 @@ func (s *VPCServiceImpl) RevokeSecurityGroupIngress(ctx context.Context, input *
|
||||
revokeRules, err := ipPermissionsToSGRules(input.IpPermissions, sgParseRevoke)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "RevokeSecurityGroupIngress: invalid rule", "groupId", groupId, "err", err)
|
||||
if _, ok := awserrors.ResolveErrorCode(err); ok {
|
||||
return nil, err
|
||||
}
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
}
|
||||
idRules, err := resolveRuleIDsToRemove(record.IngressRules, input.SecurityGroupRuleIds)
|
||||
@@ -1309,6 +1320,9 @@ func (s *VPCServiceImpl) RevokeSecurityGroupEgress(ctx context.Context, input *e
|
||||
revokeRules, err := ipPermissionsToSGRules(input.IpPermissions, sgParseRevoke)
|
||||
if err != nil {
|
||||
slog.WarnContext(ctx, "RevokeSecurityGroupEgress: invalid rule", "groupId", groupId, "err", err)
|
||||
if _, ok := awserrors.ResolveErrorCode(err); ok {
|
||||
return nil, err
|
||||
}
|
||||
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
|
||||
}
|
||||
idRules, err := resolveRuleIDsToRemove(record.EgressRules, input.SecurityGroupRuleIds)
|
||||
@@ -1488,14 +1502,11 @@ func applySGRuleDescriptions(rules []SGRule, descriptions []*ec2.SecurityGroupRu
|
||||
return nil, errors.New(awserrors.ErrorInvalidSecurityGroupRuleIdMalformed)
|
||||
}
|
||||
id := *d.SecurityGroupRuleId
|
||||
if !SGRuleIDRegex.MatchString(id) {
|
||||
return nil, errors.New(awserrors.ErrorInvalidSecurityGroupRuleIdMalformed)
|
||||
}
|
||||
// A rule on the other side, in another group or in another account is
|
||||
// absent from this record and so is not-found, never a silent no-op.
|
||||
i, ok := byID[id]
|
||||
if !ok {
|
||||
return nil, sgRuleNotFoundError(id)
|
||||
return nil, sgRuleIDLookupError(id)
|
||||
}
|
||||
if d.Description != nil {
|
||||
out[i].Description = *d.Description
|
||||
@@ -1769,21 +1780,16 @@ func sgRuleRequestToSGRule(req *ec2.SecurityGroupRuleRequest) (SGRule, error) {
|
||||
return SGRule{}, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "No value specified for securityGroupRule.")
|
||||
}
|
||||
|
||||
if strings.TrimSpace(aws.StringValue(req.IpProtocol)) == "" {
|
||||
return SGRule{}, awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"Invalid value 'null' for protocol. VPC security group rules must specify protocols explicitly.")
|
||||
}
|
||||
proto, err := normalizeIPProtocol(*req.IpProtocol)
|
||||
proto, err := normalizeIPProtocol(aws.StringValue(req.IpProtocol))
|
||||
if err != nil {
|
||||
return SGRule{}, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "%s", err)
|
||||
return SGRule{}, err
|
||||
}
|
||||
|
||||
r := SGRule{IpProtocol: proto, Description: aws.StringValue(req.Description)}
|
||||
switch proto {
|
||||
case "tcp", "udp":
|
||||
if req.FromPort == nil || req.ToPort == nil {
|
||||
return SGRule{}, awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
"Invalid value for portRange. Must specify both from and to ports with TCP/UDP.")
|
||||
if err := requireSGRulePorts(proto, req.FromPort, req.ToPort); err != nil {
|
||||
return SGRule{}, err
|
||||
}
|
||||
r.FromPort, r.ToPort = *req.FromPort, *req.ToPort
|
||||
if err := validateSGRulePorts(proto, r.FromPort, r.ToPort); err != nil {
|
||||
@@ -1850,8 +1856,8 @@ func sgRuleSourceField(r SGRule) string {
|
||||
const sgRuleIDMaxSuffix = 17
|
||||
|
||||
// SGRuleIDIsMalformed reports whether AWS rejects id as malformed: it lacks the
|
||||
// lowercase "sgr-" prefix or has too long a suffix. Looser than SGRuleIDRegex,
|
||||
// because AWS answers NotFound for a short or non-hex suffix such as sgr-xyz.
|
||||
// lowercase "sgr-" prefix or has too long a suffix. AWS answers NotFound, not
|
||||
// Malformed, for a short or non-hex suffix such as sgr-xyz.
|
||||
func SGRuleIDIsMalformed(id string) bool {
|
||||
suffix, ok := strings.CutPrefix(id, "sgr-")
|
||||
return !ok || len(suffix) > sgRuleIDMaxSuffix
|
||||
@@ -1863,10 +1869,6 @@ func sgRuleIDLookupError(id string) error {
|
||||
if SGRuleIDIsMalformed(id) {
|
||||
return awserrors.Errorf(awserrors.ErrorInvalidSecurityGroupRuleIdMalformed, "Invalid id: %q", id)
|
||||
}
|
||||
return sgRuleNotFoundError(id)
|
||||
}
|
||||
|
||||
func sgRuleNotFoundError(id string) error {
|
||||
return awserrors.IDNotFound(awserrors.ErrorInvalidSecurityGroupRuleIdNotFound, "security group rule", id)
|
||||
}
|
||||
|
||||
@@ -1923,10 +1925,18 @@ func ipPermissionsToSGRules(perms []*ec2.IpPermission, mode sgParseMode) ([]SGRu
|
||||
toPort = *perm.ToPort
|
||||
}
|
||||
if mode == sgParseAuthorize {
|
||||
if err := requireSGRulePorts(proto, perm.FromPort, perm.ToPort); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := validateSGRulePorts(proto, fromPort, toPort); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
// AWS ignores the ports of an all-protocol rule. Storing 0/0, as the
|
||||
// default egress rule is, keeps new rules uniform with it.
|
||||
if proto == allProtocols {
|
||||
fromPort, toPort = 0, 0
|
||||
}
|
||||
|
||||
appended := false
|
||||
for _, ipRange := range perm.IpRanges {
|
||||
@@ -2003,6 +2013,10 @@ func sgRulesToIpPermissions(rules []SGRule, accountID string) []*ec2.IpPermissio
|
||||
grouped := make(map[permKey]*ec2.IpPermission)
|
||||
for _, rule := range rules {
|
||||
key := permKey{IpProtocol: rule.IpProtocol, FromPort: rule.FromPort, ToPort: rule.ToPort}
|
||||
// An all-protocol rule stored with -1/-1 is the same permission as 0/0.
|
||||
if rule.IpProtocol == allProtocols {
|
||||
key.FromPort, key.ToPort = 0, 0
|
||||
}
|
||||
perm, exists := grouped[key]
|
||||
if !exists {
|
||||
perm = &ec2.IpPermission{IpProtocol: aws.String(rule.IpProtocol)}
|
||||
@@ -2127,12 +2141,9 @@ func resolveRuleIDsToRemove(existing []SGRule, ruleIDs []*string) ([]SGRule, err
|
||||
if idp == nil || *idp == "" {
|
||||
continue
|
||||
}
|
||||
if !SGRuleIDRegex.MatchString(*idp) {
|
||||
return nil, errors.New(awserrors.ErrorInvalidSecurityGroupRuleIdMalformed)
|
||||
}
|
||||
r, ok := byID[*idp]
|
||||
if !ok {
|
||||
return nil, sgRuleNotFoundError(*idp)
|
||||
return nil, sgRuleIDLookupError(*idp)
|
||||
}
|
||||
out = append(out, r)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,229 @@
|
||||
//test:in-package — builds on the package's unexported SG fixtures
|
||||
//(setupTestVPCService, createTestSG, storedSGRecord) and plants rules in the
|
||||
//stored record that no exported API can create.
|
||||
|
||||
package handlers_ec2_vpc
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws"
|
||||
"github.com/aws/aws-sdk-go/service/ec2"
|
||||
"github.com/mulgadc/spinifex/spinifex/awserrors"
|
||||
"github.com/mulgadc/spinifex/spinifex/utils"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func authorizeIngress(svc *VPCServiceImpl, sgID string, perm *ec2.IpPermission) error {
|
||||
_, err := svc.AuthorizeSecurityGroupIngress(context.Background(), &ec2.AuthorizeSecurityGroupIngressInput{
|
||||
GroupId: aws.String(sgID),
|
||||
IpPermissions: []*ec2.IpPermission{perm},
|
||||
}, testAccountID)
|
||||
return err
|
||||
}
|
||||
|
||||
// plantIngressRules appends rules straight to the stored record, standing in
|
||||
// for rules written before the current validation existed.
|
||||
func plantIngressRules(t *testing.T, svc *VPCServiceImpl, sgID string, rules ...SGRule) {
|
||||
t.Helper()
|
||||
var rec SecurityGroupRecord
|
||||
require.NoError(t, json.Unmarshal(storedSGRecord(t, svc, sgID), &rec))
|
||||
rec.IngressRules = append(rec.IngressRules, rules...)
|
||||
data, err := json.Marshal(rec)
|
||||
require.NoError(t, err)
|
||||
_, err = svc.sgKV.Put(t.Context(), utils.AccountKey(testAccountID, sgID), data)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
// An omitted protocol must not be read as all protocols, which would open
|
||||
// every port to the CIDR.
|
||||
func TestAuthorizeSecurityGroup_RequiresProtocol(t *testing.T) {
|
||||
t.Parallel()
|
||||
svc := setupTestVPCService(t)
|
||||
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
|
||||
sgID := createTestSG(t, svc, vpcID, "no-proto-sg")
|
||||
before := storedSGRecord(t, svc, sgID)
|
||||
const msg = "Invalid value 'null' for protocol. VPC security group rules must specify protocols explicitly."
|
||||
|
||||
for _, proto := range []*string{nil, aws.String(""), aws.String(" ")} {
|
||||
perm := &ec2.IpPermission{
|
||||
IpProtocol: proto, FromPort: aws.Int64(80), ToPort: aws.Int64(80),
|
||||
IpRanges: []*ec2.IpRange{{CidrIp: aws.String("10.0.0.0/24")}},
|
||||
}
|
||||
requireAWSError(t, authorizeIngress(svc, sgID, perm), awserrors.ErrorInvalidParameterValue, msg)
|
||||
|
||||
_, err := svc.AuthorizeSecurityGroupEgress(context.Background(), &ec2.AuthorizeSecurityGroupEgressInput{
|
||||
GroupId: aws.String(sgID), IpPermissions: []*ec2.IpPermission{perm},
|
||||
}, testAccountID)
|
||||
requireAWSError(t, err, awserrors.ErrorInvalidParameterValue, msg)
|
||||
}
|
||||
assert.Equal(t, before, storedSGRecord(t, svc, sgID), "a rejected authorize must store nothing")
|
||||
}
|
||||
|
||||
// A revoke without a protocol must not match the all-traffic rule.
|
||||
func TestRevokeSecurityGroupEgress_RequiresProtocol(t *testing.T) {
|
||||
t.Parallel()
|
||||
svc := setupTestVPCService(t)
|
||||
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
|
||||
sgID := createTestSG(t, svc, vpcID, "revoke-no-proto-sg")
|
||||
before := storedSGRecord(t, svc, sgID)
|
||||
|
||||
_, err := svc.RevokeSecurityGroupEgress(context.Background(), &ec2.RevokeSecurityGroupEgressInput{
|
||||
GroupId: aws.String(sgID),
|
||||
IpPermissions: []*ec2.IpPermission{{
|
||||
IpRanges: []*ec2.IpRange{{CidrIp: aws.String("0.0.0.0/0")}},
|
||||
}},
|
||||
}, testAccountID)
|
||||
requireAWSCode(t, err, awserrors.ErrorInvalidParameterValue)
|
||||
assert.Equal(t, before, storedSGRecord(t, svc, sgID), "the default egress rule must survive")
|
||||
}
|
||||
|
||||
func TestAuthorizeSecurityGroupIngress_TCPUDPRequireBothPorts(t *testing.T) {
|
||||
t.Parallel()
|
||||
svc := setupTestVPCService(t)
|
||||
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
|
||||
sgID := createTestSG(t, svc, vpcID, "no-ports-sg")
|
||||
before := storedSGRecord(t, svc, sgID)
|
||||
|
||||
for _, proto := range []string{"tcp", "6", "udp", "17"} {
|
||||
for _, ports := range [][2]*int64{{nil, nil}, {aws.Int64(80), nil}, {nil, aws.Int64(80)}} {
|
||||
err := authorizeIngress(svc, sgID, &ec2.IpPermission{
|
||||
IpProtocol: aws.String(proto), FromPort: ports[0], ToPort: ports[1],
|
||||
IpRanges: []*ec2.IpRange{{CidrIp: aws.String("10.0.0.0/24")}},
|
||||
})
|
||||
requireAWSError(t, err, awserrors.ErrorInvalidParameterValue,
|
||||
"Invalid value for portRange. Must specify both from and to ports with TCP/UDP.")
|
||||
}
|
||||
}
|
||||
assert.Equal(t, before, storedSGRecord(t, svc, sgID), "a rejected authorize must store nothing")
|
||||
}
|
||||
|
||||
// An all-protocol rule is one rule whatever ports it was sent with, so a
|
||||
// second authorize differing only in ports is a duplicate.
|
||||
func TestAuthorizeSecurityGroupIngress_AllProtocolPortsShareIdentity(t *testing.T) {
|
||||
t.Parallel()
|
||||
svc := setupTestVPCService(t)
|
||||
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
|
||||
sgID := createTestSG(t, svc, vpcID, "all-proto-identity-sg")
|
||||
|
||||
out, err := svc.AuthorizeSecurityGroupIngress(context.Background(), &ec2.AuthorizeSecurityGroupIngressInput{
|
||||
GroupId: aws.String(sgID),
|
||||
IpPermissions: []*ec2.IpPermission{{
|
||||
IpProtocol: aws.String("-1"), FromPort: aws.Int64(-1), ToPort: aws.Int64(-1),
|
||||
IpRanges: []*ec2.IpRange{{CidrIp: aws.String("10.0.0.0/24")}},
|
||||
}},
|
||||
}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, out.SecurityGroupRules, 1)
|
||||
stored := storedSGRule(t, svc, sgID, aws.StringValue(out.SecurityGroupRules[0].SecurityGroupRuleId))
|
||||
assert.Equal(t, [2]int64{0, 0}, [2]int64{stored.FromPort, stored.ToPort}, "stored in the default egress rule's form")
|
||||
|
||||
err = authorizeIngress(svc, sgID, &ec2.IpPermission{
|
||||
IpProtocol: aws.String("-1"),
|
||||
IpRanges: []*ec2.IpRange{{CidrIp: aws.String("10.0.0.0/24")}},
|
||||
})
|
||||
requireAWSCode(t, err, awserrors.ErrorInvalidPermissionDuplicate)
|
||||
}
|
||||
|
||||
// A rule stored with -1/-1 before authorize canonicalised the ports is the
|
||||
// same rule as its 0/0 form for duplicate checks, revoke and reporting.
|
||||
func TestAllProtocolRuleStoredWithMinusOnePorts(t *testing.T) {
|
||||
t.Parallel()
|
||||
svc := setupTestVPCService(t)
|
||||
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
|
||||
sgID := createTestSG(t, svc, vpcID, "legacy-all-proto-sg")
|
||||
plantIngressRules(t, svc, sgID,
|
||||
SGRule{RuleId: "sgr-0000000000000000a", IpProtocol: "-1", FromPort: -1, ToPort: -1, CidrIp: "10.0.0.0/24"},
|
||||
SGRule{RuleId: "sgr-0000000000000000b", IpProtocol: "-1", FromPort: 0, ToPort: 0, CidrIp: "10.0.1.0/24"},
|
||||
)
|
||||
|
||||
err := authorizeIngress(svc, sgID, &ec2.IpPermission{
|
||||
IpProtocol: aws.String("-1"),
|
||||
IpRanges: []*ec2.IpRange{{CidrIp: aws.String("10.0.0.0/24")}},
|
||||
})
|
||||
requireAWSCode(t, err, awserrors.ErrorInvalidPermissionDuplicate)
|
||||
|
||||
desc, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{
|
||||
GroupIds: []*string{aws.String(sgID)},
|
||||
}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, desc.SecurityGroups, 1)
|
||||
require.Len(t, desc.SecurityGroups[0].IpPermissions, 1, "both rules are one all-protocol permission")
|
||||
assert.Len(t, desc.SecurityGroups[0].IpPermissions[0].IpRanges, 2)
|
||||
|
||||
_, err = svc.RevokeSecurityGroupIngress(context.Background(), &ec2.RevokeSecurityGroupIngressInput{
|
||||
GroupId: aws.String(sgID),
|
||||
IpPermissions: []*ec2.IpPermission{{
|
||||
IpProtocol: aws.String("-1"),
|
||||
IpRanges: []*ec2.IpRange{{CidrIp: aws.String("10.0.0.0/24")}},
|
||||
}},
|
||||
}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
var rec SecurityGroupRecord
|
||||
require.NoError(t, json.Unmarshal(storedSGRecord(t, svc, sgID), &rec))
|
||||
require.Len(t, rec.IngressRules, 1)
|
||||
assert.Equal(t, "sgr-0000000000000000b", rec.IngressRules[0].RuleId)
|
||||
}
|
||||
|
||||
// AWS calls a rule ID malformed only without the lowercase sgr- prefix or with
|
||||
// more than 17 characters after it; anything else that matches no rule is
|
||||
// not found. Every operation taking a rule ID must agree.
|
||||
func TestSecurityGroupRuleIDClassification(t *testing.T) {
|
||||
t.Parallel()
|
||||
svc := setupTestVPCService(t)
|
||||
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
|
||||
sgID := createTestSG(t, svc, vpcID, "rule-id-class-sg")
|
||||
|
||||
ops := map[string]func(id string) error{
|
||||
"DescribeSecurityGroupRules": func(id string) error {
|
||||
_, err := svc.DescribeSecurityGroupRules(context.Background(), &ec2.DescribeSecurityGroupRulesInput{
|
||||
SecurityGroupRuleIds: []*string{aws.String(id)},
|
||||
}, testAccountID)
|
||||
return err
|
||||
},
|
||||
"RevokeSecurityGroupIngress": func(id string) error {
|
||||
_, err := svc.RevokeSecurityGroupIngress(context.Background(), &ec2.RevokeSecurityGroupIngressInput{
|
||||
GroupId: aws.String(sgID), SecurityGroupRuleIds: []*string{aws.String(id)},
|
||||
}, testAccountID)
|
||||
return err
|
||||
},
|
||||
"RevokeSecurityGroupEgress": func(id string) error {
|
||||
_, err := svc.RevokeSecurityGroupEgress(context.Background(), &ec2.RevokeSecurityGroupEgressInput{
|
||||
GroupId: aws.String(sgID), SecurityGroupRuleIds: []*string{aws.String(id)},
|
||||
}, testAccountID)
|
||||
return err
|
||||
},
|
||||
"UpdateSecurityGroupRuleDescriptionsIngress": func(id string) error {
|
||||
return updateIngressDescription(svc, sgID, id, "x")
|
||||
},
|
||||
"UpdateSecurityGroupRuleDescriptionsEgress": func(id string) error {
|
||||
_, err := svc.UpdateSecurityGroupRuleDescriptionsEgress(context.Background(), &ec2.UpdateSecurityGroupRuleDescriptionsEgressInput{
|
||||
GroupId: aws.String(sgID),
|
||||
SecurityGroupRuleDescriptions: []*ec2.SecurityGroupRuleDescription{
|
||||
{SecurityGroupRuleId: aws.String(id), Description: aws.String("x")},
|
||||
},
|
||||
}, testAccountID)
|
||||
return err
|
||||
},
|
||||
}
|
||||
ids := map[string]string{
|
||||
"sgr-xyz": awserrors.ErrorInvalidSecurityGroupRuleIdNotFound,
|
||||
"sgr-0123-4567": awserrors.ErrorInvalidSecurityGroupRuleIdNotFound,
|
||||
"sgr-": awserrors.ErrorInvalidSecurityGroupRuleIdNotFound,
|
||||
"sgr-00000000000000000": awserrors.ErrorInvalidSecurityGroupRuleIdNotFound,
|
||||
"SGR-00000000000000000": awserrors.ErrorInvalidSecurityGroupRuleIdMalformed,
|
||||
"sgr-000000000000000000": awserrors.ErrorInvalidSecurityGroupRuleIdMalformed,
|
||||
"not-a-rule-id": awserrors.ErrorInvalidSecurityGroupRuleIdMalformed,
|
||||
}
|
||||
for name, op := range ops {
|
||||
for id, code := range ids {
|
||||
t.Run(name+"/"+id, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
requireAWSCode(t, op(id), code)
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1098,7 +1098,6 @@ func TestValidateSGRule_Protocol(t *testing.T) {
|
||||
func TestNormalizeIPProtocol(t *testing.T) {
|
||||
t.Parallel()
|
||||
cases := map[string]string{
|
||||
"": "-1",
|
||||
"-1": "-1",
|
||||
"tcp": "tcp",
|
||||
"TCP": "tcp",
|
||||
@@ -1114,8 +1113,9 @@ func TestNormalizeIPProtocol(t *testing.T) {
|
||||
assert.Equal(t, want, got, "protocol %q", in)
|
||||
}
|
||||
|
||||
// 58/icmpv6 is rejected with the rest: the ACL builder is IPv4-only.
|
||||
for _, in := range []string{"58", "icmpv6", "47", "50", "0", "256", "banana", "tcp; drop"} {
|
||||
// 58/icmpv6 is rejected with the rest: the ACL builder is IPv4-only. An
|
||||
// empty protocol is rejected rather than read as all protocols.
|
||||
for _, in := range []string{"", "58", "icmpv6", "47", "50", "0", "256", "banana", "tcp; drop"} {
|
||||
_, err := normalizeIPProtocol(in)
|
||||
assert.Error(t, err, "protocol %q", in)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user