fix(vpc): accept security-group rules carrying an IPv6 range (#1060)

AuthorizeSecurityGroupIngress and AuthorizeSecurityGroupEgress rejected any
IpPermission carrying an Ipv6Ranges entry. The security-group module defaults
egress_ipv6_cidr_blocks to ::/0 and the provider revokes the group's own
default egress before authorizing the replacement, so the rejection left the
group with no egress rules at all, which is treated as deny.

SGRule gains CidrIpv6. The rule is stored, deduped, revoked and projected like
any other, and the two policy.Rule adapters drop it before the ACL builder,
which is IPv4-only. No interface here has an IPv6 address, so the rule matches
nothing - which is what an IPv6 rule does on AWS in a VPC with no IPv6 CIDR.

Revoking an IPv6 rule that does not exist now returns InvalidPermission.NotFound
rather than succeeding as a no-op. The no-op existed only because authorize
refused to store the rule, so a caller's state could never match.

Closes mulga-6bu36.
This commit is contained in:
Josh Siviour
2026-09-17 10:33:43 +10:00
committed by GitHub
parent ead2f2fe6c
commit 809ec5f1fe
7 changed files with 308 additions and 85 deletions
+65 -30
View File
@@ -54,13 +54,14 @@ func normalizeIPProtocol(proto string) (string, error) {
return "", fmt.Errorf("invalid IpProtocol %q: supported values are tcp, udp, icmp, -1 (or 6, 17, 1)", proto)
}
// validateSGRule rejects CidrIp values that are non-canonical or IPv6 (OVN ACL
// builder is IPv4-only), IpProtocol values the ACL builder cannot express, and
// SourceSG values not matching the sg-ID format. At least one source must be
// specified.
// validateSGRule rejects CidrIp values that are non-canonical or IPv6, CidrIpv6
// values that are non-canonical or IPv4, IpProtocol values the ACL builder
// cannot express, and SourceSG values not matching the sg-ID format. At least
// one source must be specified. The two CIDR fields stay strictly separated so
// that address family remains part of a rule's identity.
func validateSGRule(r SGRule) error {
if r.CidrIp == "" && r.SourceSG == "" {
return errors.New("rule must specify CidrIp or SourceSG")
if r.CidrIp == "" && r.CidrIpv6 == "" && r.SourceSG == "" {
return errors.New("rule must specify CidrIp, CidrIpv6 or SourceSG")
}
if !slices.Contains(canonicalIPProtocols, r.IpProtocol) {
return fmt.Errorf("invalid IpProtocol %q: must be one of %v", r.IpProtocol, canonicalIPProtocols)
@@ -71,12 +72,24 @@ func validateSGRule(r SGRule) error {
return fmt.Errorf("invalid CidrIp %q: %w", r.CidrIp, err)
}
if ipnet.IP.To4() == nil {
return fmt.Errorf("invalid CidrIp %q: IPv6 not supported", r.CidrIp)
return fmt.Errorf("invalid CidrIp %q: IPv6 belongs in CidrIpv6", r.CidrIp)
}
if ipnet.String() != r.CidrIp {
return fmt.Errorf("invalid CidrIp %q: not canonical (expected %q)", r.CidrIp, ipnet.String())
}
}
if r.CidrIpv6 != "" {
_, ipnet, err := net.ParseCIDR(r.CidrIpv6)
if err != nil {
return fmt.Errorf("invalid CidrIpv6 %q: %w", r.CidrIpv6, err)
}
if ipnet.IP.To4() != nil {
return fmt.Errorf("invalid CidrIpv6 %q: IPv4 belongs in CidrIp", r.CidrIpv6)
}
if ipnet.String() != r.CidrIpv6 {
return fmt.Errorf("invalid CidrIpv6 %q: not canonical (expected %q)", r.CidrIpv6, ipnet.String())
}
}
if r.SourceSG != "" && !sgIDRegex.MatchString(r.SourceSG) {
return fmt.Errorf("invalid SourceSG %q: must match sg-<17 hex chars>", r.SourceSG)
}
@@ -107,7 +120,11 @@ type SGRule struct {
FromPort int64 `json:"from_port"`
ToPort int64 `json:"to_port"`
CidrIp string `json:"cidr_ip,omitempty"`
SourceSG string `json:"source_sg,omitempty"` // Another SG ID for intra-SG rules
// CidrIpv6 is stored and projected but never reaches the ACL builder: no
// interface here has an IPv6 address, so the rule matches nothing, which is
// what an IPv6 rule does on AWS in a VPC with no IPv6 CIDR.
CidrIpv6 string `json:"cidr_ipv6,omitempty"`
SourceSG string `json:"source_sg,omitempty"` // Another SG ID for intra-SG rules
// Description is rule metadata, not identity: it is excluded from sgRuleKey
// so revoke/duplicate matching ignores it. The AWS Load Balancer Controller
// tags the node-SG rules it manages here and revokes them by this value.
@@ -840,6 +857,9 @@ func sgRuleToSecurityGroupRule(record *SecurityGroupRecord, rule SGRule, isEgres
if rule.CidrIp != "" {
out.CidrIpv4 = aws.String(rule.CidrIp)
}
if rule.CidrIpv6 != "" {
out.CidrIpv6 = aws.String(rule.CidrIpv6)
}
if rule.SourceSG != "" {
out.ReferencedGroupInfo = &ec2.ReferencedSecurityGroup{
GroupId: aws.String(rule.SourceSG),
@@ -1348,11 +1368,19 @@ func ipPermissionsToDescriptionTargets(perms []*ec2.IpPermission) ([]sgDescripti
targets = append(targets, sgDescriptionTarget{key: sgRuleKey(r), description: pair.Description})
}
if len(targets) == before {
if len(perm.Ipv6Ranges) > 0 {
return nil, errors.New("IPv6 rules are not supported")
for _, r6 := range perm.Ipv6Ranges {
if r6 == nil || r6.CidrIpv6 == nil {
continue
}
return nil, errors.New("IpPermission must specify at least one IpRange or UserIdGroupPair")
r := SGRule{IpProtocol: proto, FromPort: fromPort, ToPort: toPort, CidrIpv6: *r6.CidrIpv6}
if err := validateSGRule(r); err != nil {
return nil, err
}
targets = append(targets, sgDescriptionTarget{key: sgRuleKey(r), description: r6.Description})
}
if len(targets) == before {
return nil, errors.New("IpPermission must specify at least one IpRange, Ipv6Range or UserIdGroupPair")
}
}
return targets, nil
@@ -1375,9 +1403,8 @@ func (s *VPCServiceImpl) sgRecordToEC2(record *SecurityGroupRecord, accountID st
return sg
}
// sgParseMode selects how ipPermissionsToSGRules handles IPv6 sources.
// Authorize rejects IPv6; Revoke silently skips it (Terraform auto-revokes
// ::/0 IPv6 egress and must not abort on no-op).
// sgParseMode selects whether ipPermissionsToSGRules mints a RuleId for each
// parsed rule. Authorize does; Revoke matches on rule identity instead.
type sgParseMode int
const (
@@ -1387,8 +1414,7 @@ const (
// ipPermissionsToSGRules converts AWS IpPermission slice to SGRule slice,
// normalising IpProtocol to its canonical name and validating every
// tenant-supplied CidrIp/SourceSG. IPv6-only permissions error in Authorize
// mode and are silently skipped in Revoke mode.
// tenant-supplied CidrIp/CidrIpv6/SourceSG.
func ipPermissionsToSGRules(perms []*ec2.IpPermission, mode sgParseMode) ([]SGRule, error) {
var rules []SGRule
for _, perm := range perms {
@@ -1450,24 +1476,26 @@ func ipPermissionsToSGRules(perms []*ec2.IpPermission, mode sgParseMode) ([]SGRu
appended = true
}
hasIPv6 := false
for _, r6 := range perm.Ipv6Ranges {
if r6 != nil && r6.CidrIpv6 != nil {
hasIPv6 = true
break
}
}
if hasIPv6 {
if mode == sgParseAuthorize {
return nil, errors.New("IPv6 rules are not supported")
}
if !appended {
if r6 == nil || r6.CidrIpv6 == nil {
continue
}
r := SGRule{IpProtocol: proto, FromPort: fromPort, ToPort: toPort, CidrIpv6: *r6.CidrIpv6}
if r6.Description != nil {
r.Description = *r6.Description
}
if err := validateSGRule(r); err != nil {
return nil, err
}
if mode == sgParseAuthorize {
r.RuleId = utils.GenerateResourceID("sgr")
}
rules = append(rules, r)
appended = true
}
if !appended {
return nil, errors.New("IpPermission must specify at least one IpRange or UserIdGroupPair")
return nil, errors.New("IpPermission must specify at least one IpRange, Ipv6Range or UserIdGroupPair")
}
}
return rules, nil
@@ -1502,6 +1530,13 @@ func sgRulesToIpPermissions(rules []SGRule) []*ec2.IpPermission {
}
perm.IpRanges = append(perm.IpRanges, ipRange)
}
if rule.CidrIpv6 != "" {
ipRange := &ec2.Ipv6Range{CidrIpv6: aws.String(rule.CidrIpv6)}
if rule.Description != "" {
ipRange.Description = aws.String(rule.Description)
}
perm.Ipv6Ranges = append(perm.Ipv6Ranges, ipRange)
}
if rule.SourceSG != "" {
pair := &ec2.UserIdGroupPair{GroupId: aws.String(rule.SourceSG)}
if rule.Description != "" {
@@ -1555,7 +1590,7 @@ func removeSGRules(existing, toRemove []SGRule) []SGRule {
// sgRuleKey returns a string key for deduplication/matching of SG rules.
func sgRuleKey(r SGRule) string {
return fmt.Sprintf("%s:%d:%d:%s:%s", r.IpProtocol, r.FromPort, r.ToPort, r.CidrIp, r.SourceSG)
return fmt.Sprintf("%s:%d:%d:%s:%s:%s", r.IpProtocol, r.FromPort, r.ToPort, r.CidrIp, r.CidrIpv6, r.SourceSG)
}
// resolveRuleIDsToRemove maps SecurityGroupRuleIds to the matching rules in the
@@ -288,7 +288,7 @@ func TestUpdateSecurityGroupRuleDescriptionsIngress_UnmatchedPermission(t *testi
// TestUpdateSecurityGroupRuleDescriptions_UnresolvablePermission pins that a
// permission naming no rule the store can hold is an error. Authorize rejects
// all three of these, so no matching rule can exist to describe.
// both of these, so no matching rule can exist to describe.
func TestUpdateSecurityGroupRuleDescriptions_UnresolvablePermission(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
@@ -297,12 +297,6 @@ func TestUpdateSecurityGroupRuleDescriptions_UnresolvablePermission(t *testing.T
ruleID := authorizeIngressTCP(t, svc, sgID, 443, "10.0.0.0/24")
cases := map[string]*ec2.IpPermission{
"ipv6 only": {
IpProtocol: aws.String("tcp"),
FromPort: aws.Int64(443),
ToPort: aws.Int64(443),
Ipv6Ranges: []*ec2.Ipv6Range{{CidrIpv6: aws.String("::/0"), Description: aws.String("v6")}},
},
"prefix list only": {
IpProtocol: aws.String("tcp"),
FromPort: aws.Int64(443),
@@ -330,6 +324,45 @@ func TestUpdateSecurityGroupRuleDescriptions_UnresolvablePermission(t *testing.T
}
}
// TestUpdateSecurityGroupRuleDescriptionsIngress_IPv6 pins that the description
// path reaches an IPv6 rule. It used to be unreachable because authorize
// refused to store one, so an Ipv6Ranges permission could only ever be an
// error; now it resolves like any other source.
func TestUpdateSecurityGroupRuleDescriptionsIngress_IPv6(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
sgID := createTestSG(t, svc, vpcID, "desc-ipv6")
perm := &ec2.IpPermission{
IpProtocol: aws.String("tcp"),
FromPort: aws.Int64(443),
ToPort: aws.Int64(443),
Ipv6Ranges: []*ec2.Ipv6Range{{CidrIpv6: aws.String("::/0")}},
}
_, err := svc.AuthorizeSecurityGroupIngress(context.Background(), &ec2.AuthorizeSecurityGroupIngressInput{
GroupId: aws.String(sgID),
IpPermissions: []*ec2.IpPermission{perm},
}, testAccountID)
require.NoError(t, err)
perm.Ipv6Ranges[0].Description = aws.String("from anywhere v6")
_, err = svc.UpdateSecurityGroupRuleDescriptionsIngress(context.Background(), &ec2.UpdateSecurityGroupRuleDescriptionsIngressInput{
GroupId: aws.String(sgID),
IpPermissions: []*ec2.IpPermission{perm},
}, testAccountID)
require.NoError(t, err)
out, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{
GroupIds: []*string{aws.String(sgID)},
}, testAccountID)
require.NoError(t, err)
require.Len(t, out.SecurityGroups[0].IpPermissions, 1)
ranges := out.SecurityGroups[0].IpPermissions[0].Ipv6Ranges
require.Len(t, ranges, 1)
assert.Equal(t, "from anywhere v6", aws.StringValue(ranges[0].Description))
}
// TestUpdateSecurityGroupRuleDescriptions_InvalidPermissionKeepsItsReason pins
// that the underlying validation failure survives into the returned error,
// rather than collapsing to a bare code the logs cannot explain.
+148 -39
View File
@@ -413,56 +413,59 @@ func TestRevokeSecurityGroupEgress_NotFound(t *testing.T) {
assert.Error(t, err)
}
// TestRevokeSecurityGroupEgress_IPv6OnlyIsNoOp asserts that revoking a
// IPv6-only permission succeeds as a no-op; Terraform auto-revokes ::/0 IPv6
// egress and must not abort when spinifex stores no IPv6 rules.
func TestRevokeSecurityGroupEgress_IPv6OnlyIsNoOp(t *testing.T) {
// TestRevokeSecurityGroupEgress_IPv6NotFound: an IPv6 rule that was never
// authorized is missing in the ordinary way. The old silent no-op existed only
// because authorize rejected IPv6, so a caller's state could never match what
// was stored; with authorize working there is nothing left to paper over, and a
// v6 revoke behaves exactly like the v4 revoke beside it.
func TestRevokeSecurityGroupEgress_IPv6NotFound(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
sgID := createTestSG(t, svc, vpcID, "ipv6-revoke-noop")
sgID := createTestSG(t, svc, vpcID, "ipv6-revoke-notfound")
allProto := "-1"
out, err := svc.RevokeSecurityGroupEgress(context.Background(), &ec2.RevokeSecurityGroupEgressInput{
_, err := svc.RevokeSecurityGroupEgress(context.Background(), &ec2.RevokeSecurityGroupEgressInput{
GroupId: aws.String(sgID),
IpPermissions: []*ec2.IpPermission{{
IpProtocol: &allProto,
Ipv6Ranges: []*ec2.Ipv6Range{{CidrIpv6: aws.String("::/0")}},
}},
}, testAccountID)
require.NoError(t, err)
require.NotNil(t, out)
require.True(t, aws.BoolValue(out.Return))
require.Error(t, err)
assert.Contains(t, err.Error(), "InvalidPermission.NotFound")
}
// TestRevokeSecurityGroupIngress_IPv6OnlyIsNoOp mirrors the egress no-op test
// — the ingress path shares the same parsing code and must behave the same.
func TestRevokeSecurityGroupIngress_IPv6OnlyIsNoOp(t *testing.T) {
// TestRevokeSecurityGroupIngress_IPv6NotFound mirrors the egress case — the
// ingress path shares the same parsing code and must behave the same.
func TestRevokeSecurityGroupIngress_IPv6NotFound(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
sgID := createTestSG(t, svc, vpcID, "ipv6-revoke-ingress-noop")
sgID := createTestSG(t, svc, vpcID, "ipv6-revoke-ingress-notfound")
allProto := "-1"
out, err := svc.RevokeSecurityGroupIngress(context.Background(), &ec2.RevokeSecurityGroupIngressInput{
_, err := svc.RevokeSecurityGroupIngress(context.Background(), &ec2.RevokeSecurityGroupIngressInput{
GroupId: aws.String(sgID),
IpPermissions: []*ec2.IpPermission{{
IpProtocol: &allProto,
Ipv6Ranges: []*ec2.Ipv6Range{{CidrIpv6: aws.String("::/0")}},
}},
}, testAccountID)
require.NoError(t, err)
require.NotNil(t, out)
require.True(t, aws.BoolValue(out.Return))
require.Error(t, err)
assert.Contains(t, err.Error(), "InvalidPermission.NotFound")
}
// TestAuthorizeSecurityGroupEgress_IPv6Rejected asserts that authorizing
// IPv6 sources is rejected; spinifex cannot program IPv6 OVN ACLs.
func TestAuthorizeSecurityGroupEgress_IPv6Rejected(t *testing.T) {
// TestAuthorizeSecurityGroupEgress_IPv6Accepted pins the whole of TFC-0001: the
// module's default egress rule carries ::/0, the provider has already revoked
// the group's own default egress by the time it is authorized, and rejecting it
// left the group with no egress at all. The v4 half here is the group's default
// rule, so only the v6 half is authorized.
func TestAuthorizeSecurityGroupEgress_IPv6Accepted(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
sgID := createTestSG(t, svc, vpcID, "ipv6-authorize-rejected")
sgID := createTestSG(t, svc, vpcID, "ipv6-authorize-accepted")
allProto := "-1"
_, err := svc.AuthorizeSecurityGroupEgress(context.Background(), &ec2.AuthorizeSecurityGroupEgressInput{
@@ -472,8 +475,57 @@ func TestAuthorizeSecurityGroupEgress_IPv6Rejected(t *testing.T) {
Ipv6Ranges: []*ec2.Ipv6Range{{CidrIpv6: aws.String("::/0")}},
}},
}, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
require.NoError(t, err)
out, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{
GroupIds: []*string{aws.String(sgID)},
}, testAccountID)
require.NoError(t, err)
require.Len(t, out.SecurityGroups, 1)
require.Len(t, out.SecurityGroups[0].IpPermissionsEgress, 1)
perm := out.SecurityGroups[0].IpPermissionsEgress[0]
require.Len(t, perm.IpRanges, 1)
require.Len(t, perm.Ipv6Ranges, 1)
assert.Equal(t, "0.0.0.0/0", aws.StringValue(perm.IpRanges[0].CidrIp))
assert.Equal(t, "::/0", aws.StringValue(perm.Ipv6Ranges[0].CidrIpv6))
}
// TestRevokeSecurityGroupEgress_IPv6RevokesOnlyIPv6 pins that the two families
// are separately revocable. They share every other field, so a key that ignored
// the family would take both out at once.
func TestRevokeSecurityGroupEgress_IPv6RevokesOnlyIPv6(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
vpcID := createTestVPC(t, svc, "10.0.0.0/16")
sgID := createTestSG(t, svc, vpcID, "ipv6-revoke-scoped")
allProto := "-1"
_, err := svc.AuthorizeSecurityGroupEgress(context.Background(), &ec2.AuthorizeSecurityGroupEgressInput{
GroupId: aws.String(sgID),
IpPermissions: []*ec2.IpPermission{{
IpProtocol: &allProto,
Ipv6Ranges: []*ec2.Ipv6Range{{CidrIpv6: aws.String("::/0")}},
}},
}, testAccountID)
require.NoError(t, err)
_, err = svc.RevokeSecurityGroupEgress(context.Background(), &ec2.RevokeSecurityGroupEgressInput{
GroupId: aws.String(sgID),
IpPermissions: []*ec2.IpPermission{{
IpProtocol: &allProto,
Ipv6Ranges: []*ec2.Ipv6Range{{CidrIpv6: aws.String("::/0")}},
}},
}, testAccountID)
require.NoError(t, err)
out, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{
GroupIds: []*string{aws.String(sgID)},
}, testAccountID)
require.NoError(t, err)
require.Len(t, out.SecurityGroups[0].IpPermissionsEgress, 1)
perm := out.SecurityGroups[0].IpPermissionsEgress[0]
assert.Len(t, perm.IpRanges, 1)
assert.Empty(t, perm.Ipv6Ranges)
}
// TestRevokeSecurityGroupEgress_RuleNotFound: egress counterpart — revoking a
@@ -720,8 +772,9 @@ func TestIpPermissionsToSGRules_SourceSG(t *testing.T) {
assert.Equal(t, srcSG, rules[0].SourceSG)
}
// TestIpPermissionsToSGRules_IPv6OnlyAuthorize rejects authorize calls whose
// only source is Ipv6Ranges (OVN ACL builder is IPv4-only).
// TestIpPermissionsToSGRules_IPv6OnlyAuthorize accepts a permission whose only
// source is Ipv6Ranges. The module's default egress rule is exactly this, and
// rejecting it left the group with no egress at all.
func TestIpPermissionsToSGRules_IPv6OnlyAuthorize(t *testing.T) {
t.Parallel()
allProto := "-1"
@@ -732,14 +785,16 @@ func TestIpPermissionsToSGRules_IPv6OnlyAuthorize(t *testing.T) {
},
}
_, err := ipPermissionsToSGRules(perms, sgParseAuthorize)
require.Error(t, err)
assert.Contains(t, err.Error(), "IPv6")
rules, err := ipPermissionsToSGRules(perms, sgParseAuthorize)
require.NoError(t, err)
require.Len(t, rules, 1)
assert.Equal(t, "::/0", rules[0].CidrIpv6)
assert.Empty(t, rules[0].CidrIp)
assert.NotEmpty(t, rules[0].RuleId)
}
// TestIpPermissionsToSGRules_IPv6OnlyRevoke tolerates IPv6-only permissions on
// revoke (returns zero rules, no error), matching Terraform's auto-revoke of
// the default ::/0 IPv6 egress rule.
// TestIpPermissionsToSGRules_IPv6OnlyRevoke parses an IPv6-only permission on
// revoke into the same rule authorize stored, so the revoke can match it.
func TestIpPermissionsToSGRules_IPv6OnlyRevoke(t *testing.T) {
t.Parallel()
allProto := "-1"
@@ -752,12 +807,14 @@ func TestIpPermissionsToSGRules_IPv6OnlyRevoke(t *testing.T) {
rules, err := ipPermissionsToSGRules(perms, sgParseRevoke)
require.NoError(t, err)
assert.Empty(t, rules)
require.Len(t, rules, 1)
assert.Equal(t, "::/0", rules[0].CidrIpv6)
assert.Empty(t, rules[0].RuleId)
}
// TestIpPermissionsToSGRules_MixedV4V6Revoke keeps the v4 rule and drops the
// v6 part on a mixed-source permission on revoke.
func TestIpPermissionsToSGRules_MixedV4V6Revoke(t *testing.T) {
// TestIpPermissionsToSGRules_MixedV4V6 splits a permission carrying both
// families into one rule each, in both modes.
func TestIpPermissionsToSGRules_MixedV4V6(t *testing.T) {
t.Parallel()
proto := "tcp"
perms := []*ec2.IpPermission{
@@ -770,10 +827,62 @@ func TestIpPermissionsToSGRules_MixedV4V6Revoke(t *testing.T) {
},
}
rules, err := ipPermissionsToSGRules(perms, sgParseRevoke)
require.NoError(t, err)
require.Len(t, rules, 1)
assert.Equal(t, "10.0.0.0/8", rules[0].CidrIp)
for _, mode := range []sgParseMode{sgParseAuthorize, sgParseRevoke} {
rules, err := ipPermissionsToSGRules(perms, mode)
require.NoError(t, err)
require.Len(t, rules, 2)
assert.Equal(t, "10.0.0.0/8", rules[0].CidrIp)
assert.Empty(t, rules[0].CidrIpv6)
assert.Equal(t, "::/0", rules[1].CidrIpv6)
assert.Empty(t, rules[1].CidrIp)
}
}
// TestSGRuleKey_AddressFamilyIsIdentity pins that ::/0 and 0.0.0.0/0 on the
// same protocol and ports are distinct rules. They share every other field, so
// a key omitting CidrIpv6 would collide them and a revoke of one would match
// the other.
func TestSGRuleKey_AddressFamilyIsIdentity(t *testing.T) {
t.Parallel()
v4 := SGRule{IpProtocol: "-1", CidrIp: "0.0.0.0/0"}
v6 := SGRule{IpProtocol: "-1", CidrIpv6: "::/0"}
assert.NotEqual(t, sgRuleKey(v4), sgRuleKey(v6))
}
// TestValidateSGRule_AddressFamiliesDoNotCross keeps each CIDR field to its own
// family, so the family stays part of a rule's identity rather than something
// the caller can smuggle past by choosing a field.
func TestValidateSGRule_AddressFamiliesDoNotCross(t *testing.T) {
t.Parallel()
err := validateSGRule(SGRule{IpProtocol: "-1", CidrIp: "::/0"})
require.Error(t, err)
assert.Contains(t, err.Error(), "CidrIpv6")
err = validateSGRule(SGRule{IpProtocol: "-1", CidrIpv6: "0.0.0.0/0"})
require.Error(t, err)
assert.Contains(t, err.Error(), "CidrIp")
err = validateSGRule(SGRule{IpProtocol: "-1", CidrIpv6: "2001:db8::1/64"})
require.Error(t, err)
assert.Contains(t, err.Error(), "canonical")
require.NoError(t, validateSGRule(SGRule{IpProtocol: "-1", CidrIpv6: "::/0"}))
}
// TestSGRulesToIpPermissions_IPv6RoundTrip pins that a v6 rule comes back in
// Ipv6Ranges rather than IpRanges, sharing the protocol and port grouping with
// the v4 rule beside it.
func TestSGRulesToIpPermissions_IPv6RoundTrip(t *testing.T) {
t.Parallel()
perms := sgRulesToIpPermissions([]SGRule{
{IpProtocol: "-1", CidrIp: "0.0.0.0/0"},
{IpProtocol: "-1", CidrIpv6: "::/0"},
})
require.Len(t, perms, 1)
require.Len(t, perms[0].IpRanges, 1)
require.Len(t, perms[0].Ipv6Ranges, 1)
assert.Equal(t, "0.0.0.0/0", *perms[0].IpRanges[0].CidrIp)
assert.Equal(t, "::/0", *perms[0].Ipv6Ranges[0].CidrIpv6)
}
func TestSGRulesToIpPermissions_Conversion(t *testing.T) {
+11 -5
View File
@@ -699,20 +699,26 @@ func loadNATGWs(
return nil
}
// sgRulesToPolicyRules adapts handler-side SGRule to policy.Rule.
// sgRulesToPolicyRules adapts handler-side SGRule to policy.Rule, dropping any
// rule with no IPv4 source. The ACL builder is IPv4-only, and an IPv6 rule
// reaching it carries an empty CIDR and an empty SourceSG, which is not a
// narrower ACL but an unspecified one.
func sgRulesToPolicyRules(in []handlers_ec2_vpc.SGRule) []policy.Rule {
if len(in) == 0 {
return nil
}
out := make([]policy.Rule, len(in))
for i, r := range in {
out[i] = policy.Rule{
out := make([]policy.Rule, 0, len(in))
for _, r := range in {
if r.CidrIp == "" && r.SourceSG == "" {
continue
}
out = append(out, policy.Rule{
IPProtocol: r.IpProtocol,
FromPort: r.FromPort,
ToPort: r.ToPort,
CIDR: r.CidrIp,
SourceSG: r.SourceSG,
}
})
}
return out
}
+19
View File
@@ -628,3 +628,22 @@ func mustJSON(t *testing.T, v any) []byte {
}
return b
}
// TestSGRulesToPolicyRules_DropsIPv6 pins that an IPv6 rule never reaches the
// ACL builder. It carries no CIDR and no SourceSG, so an ACL built from it
// would be unspecified rather than narrow, and no interface has an IPv6
// address for it to match.
func TestSGRulesToPolicyRules_DropsIPv6(t *testing.T) {
t.Parallel()
out := sgRulesToPolicyRules([]handlers_ec2_vpc.SGRule{
{IpProtocol: "-1", CidrIp: "0.0.0.0/0"},
{IpProtocol: "-1", CidrIpv6: "::/0"},
{IpProtocol: "tcp", FromPort: 443, ToPort: 443, SourceSG: "sg-abc"},
})
if len(out) != 2 {
t.Fatalf("want 2 policy rules, got %d: %+v", len(out), out)
}
if out[0].CIDR != "0.0.0.0/0" || out[1].SourceSG != "sg-abc" {
t.Fatalf("wrong rules survived: %+v", out)
}
}
+10 -4
View File
@@ -24,16 +24,22 @@ func (e SGEvent) toSpec() policy.SGSpec {
}
}
// toPolicyRules drops any rule with no IPv4 source. The ACL builder is
// IPv4-only, and an IPv6 rule reaching it carries an empty CIDR and an empty
// SourceSG, which is not a narrower ACL but an unspecified one.
func toPolicyRules(in []SGRule) []policy.Rule {
out := make([]policy.Rule, len(in))
for i, r := range in {
out[i] = policy.Rule{
out := make([]policy.Rule, 0, len(in))
for _, r := range in {
if r.CidrIp == "" && r.SourceSG == "" {
continue
}
out = append(out, policy.Rule{
IPProtocol: r.IpProtocol,
FromPort: r.FromPort,
ToPort: r.ToPort,
CIDR: r.CidrIp,
SourceSG: r.SourceSG,
}
})
}
return out
}
@@ -686,3 +686,18 @@ func TestHandlePort_NilFlusherIsNoop(t *testing.T) {
sub.handleCreatePort(&nats.Msg{Data: mustJSON(t, evt)})
sub.handleDeletePort(&nats.Msg{Data: mustJSON(t, evt)})
}
// TestToPolicyRules_DropsIPv6 pins that an IPv6 rule never reaches the ACL
// builder. It carries no CIDR and no SourceSG, so an ACL built from it would be
// unspecified rather than narrow.
func TestToPolicyRules_DropsIPv6(t *testing.T) {
t.Parallel()
out := toPolicyRules([]SGRule{
{IpProtocol: "-1", CidrIp: "0.0.0.0/0"},
{IpProtocol: "-1"},
{IpProtocol: "tcp", FromPort: 443, ToPort: 443, SourceSG: "sg-abc"},
})
require.Len(t, out, 2)
assert.Equal(t, "0.0.0.0/0", out[0].CIDR)
assert.Equal(t, "sg-abc", out[1].SourceSG)
}