Skip to content

Commit

Permalink
feat(misconf): port and protocol support for EC2 networks (#7146)
Browse files Browse the repository at this point in the history 
Signed-off-by: nikpivkin <[email protected]>
  • Loading branch information
@nikpivkin
nikpivkin authored Aug 29, 2024
1 parent 9d7264a commit 98e136e
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 28 deletions.
23 changes: 19 additions & 4 deletions pkg/iac/adapters/cloudformation/aws/ec2/adapt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ Resources:
SecurityGroupIngress:
- IpProtocol: tcp
Description: ingress
FromPort: 80
FromPort: "80"
ToPort: 80
CidrIp: 0.0.0.0/0
SecurityGroupEgress:
- IpProtocol: tcp
- IpProtocol: -1
Description: egress
FromPort: 80
ToPort: 80
CidrIp: 0.0.0.0/0
ToPort: "80"
CidrIp: "0.0.0.0/0"
myNetworkAcl:
Type: AWS::EC2::NetworkAcl
Properties:
Expand All @@ -73,6 +73,9 @@ Resources:
Protocol: 6
RuleAction: allow
CidrBlock: 172.16.0.0/24
PortRange:
From: 22
To: "23"
myLaunchConfig:
Type: AWS::AutoScaling::LaunchConfiguration
Properties:
Expand Down Expand Up @@ -137,6 +140,9 @@ Resources:
CIDRs: []types.StringValue{
types.StringTest("0.0.0.0/0"),
},
FromPort: types.IntTest(80),
ToPort: types.IntTest(80),
Protocol: types.StringTest("tcp"),
},
},
EgressRules: []ec2.SecurityGroupRule{
Expand All @@ -145,6 +151,9 @@ Resources:
CIDRs: []types.StringValue{
types.StringTest("0.0.0.0/0"),
},
FromPort: types.IntTest(80),
ToPort: types.IntTest(80),
Protocol: types.StringTest("-1"),
},
},
},
Expand All @@ -159,6 +168,8 @@ Resources:
CIDRs: []types.StringValue{
types.StringTest("172.16.0.0/24"),
},
FromPort: types.IntTest(22),
ToPort: types.IntTest(23),
},
},
},
Expand Down Expand Up @@ -309,6 +320,8 @@ Resources:
CIDRs: []types.StringValue{
types.StringTest("0.0.0.0/0"),
},
FromPort: types.IntTest(-1),
ToPort: types.IntTest(-1),
},
},
EgressRules: []ec2.SecurityGroupRule{
Expand All @@ -317,6 +330,8 @@ Resources:
CIDRs: []types.StringValue{
types.StringTest("0.0.0.0/0"),
},
FromPort: types.IntTest(-1),
ToPort: types.IntTest(-1),
},
},
},
Expand Down
15 changes: 14 additions & 1 deletion pkg/iac/adapters/cloudformation/aws/ec2/nacl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"strconv"

"github.com/aquasecurity/trivy/pkg/iac/providers/aws/ec2"
"github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/cftypes"
"github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/parser"
iacTypes "github.com/aquasecurity/trivy/pkg/iac/types"
)
Expand All @@ -29,7 +30,8 @@ func getRules(id string, ctx parser.FileContext) (rules []ec2.NetworkACLRule) {
Metadata: ruleResource.Metadata(),
Type: iacTypes.StringDefault(ec2.TypeIngress, ruleResource.Metadata()),
Action: iacTypes.StringDefault(ec2.ActionAllow, ruleResource.Metadata()),
Protocol: iacTypes.String("-1", ruleResource.Metadata()),
FromPort: iacTypes.IntDefault(-1, ruleResource.Metadata()),
ToPort: iacTypes.IntDefault(-1, ruleResource.Metadata()),
CIDRs: nil,
}

Expand Down Expand Up @@ -62,6 +64,17 @@ func getRules(id string, ctx parser.FileContext) (rules []ec2.NetworkACLRule) {
rule.CIDRs = append(rule.CIDRs, ipv6Cidr.AsStringValue())
}

portRange := ruleResource.GetProperty("PortRange")
fromPort := portRange.GetProperty("From").ConvertTo(cftypes.Int)
if fromPort.IsInt() {
rule.FromPort = fromPort.AsIntValue()
}

toPort := portRange.GetProperty("To").ConvertTo(cftypes.Int)
if toPort.IsInt() {
rule.ToPort = toPort.AsIntValue()
}

rules = append(rules, rule)
}
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/iac/adapters/cloudformation/aws/ec2/security_group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/samber/lo"

"github.com/aquasecurity/trivy/pkg/iac/providers/aws/ec2"
"github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/cftypes"
"github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/parser"
"github.com/aquasecurity/trivy/pkg/iac/types"
)
Expand Down Expand Up @@ -75,7 +76,10 @@ func adaptRule(r interface {
rule := ec2.SecurityGroupRule{
Metadata: r.Metadata(),
Description: r.GetStringProperty("Description"),
FromPort: types.IntDefault(-1, r.Metadata()),
ToPort: types.IntDefault(-1, r.Metadata()),
}

v4Cidr := r.GetProperty("CidrIp")
if v4Cidr.IsString() && v4Cidr.AsStringValue().IsNotEmpty() {
rule.CIDRs = append(rule.CIDRs, types.StringExplicit(v4Cidr.AsString(), v4Cidr.Metadata()))
Expand All @@ -85,5 +89,20 @@ func adaptRule(r interface {
rule.CIDRs = append(rule.CIDRs, types.StringExplicit(v6Cidr.AsString(), v6Cidr.Metadata()))
}

fromPort := r.GetProperty("FromPort").ConvertTo(cftypes.Int)
if fromPort.IsInt() {
rule.FromPort = fromPort.AsIntValue()
}

toPort := r.GetProperty("ToPort").ConvertTo(cftypes.Int)
if toPort.IsInt() {
rule.ToPort = toPort.AsIntValue()
}

protocol := r.GetProperty("IpProtocol").ConvertTo(cftypes.String)
if protocol.IsString() {
rule.Protocol = protocol.AsStringValue()
}

return rule
}
4 changes: 2 additions & 2 deletions pkg/iac/adapters/terraform/aws/ec2/subnet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ func adaptSubnets(modules terraform.Modules) []ec2.Subnet {
var subnets []ec2.Subnet
for _, module := range modules {
for _, resource := range module.GetResourcesByType("aws_subnet") {
subnets = append(subnets, adaptSubnet(resource, module))
subnets = append(subnets, adaptSubnet(resource))
}
}
return subnets
}

func adaptSubnet(resource *terraform.Block, module *terraform.Module) ec2.Subnet {
func adaptSubnet(resource *terraform.Block) ec2.Subnet {
mapPublicIpOnLaunchAttr := resource.GetAttribute("map_public_ip_on_launch")
mapPublicIpOnLaunchVal := mapPublicIpOnLaunchAttr.AsBoolValueOrDefault(false, resource)

Expand Down
2 changes: 1 addition & 1 deletion pkg/iac/adapters/terraform/aws/ec2/subnet_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func Test_adaptSubnet(t *testing.T) {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
modules := tftestutil.CreateModulesFromSource(t, test.terraform, ".tf")
adapted := adaptSubnet(modules.GetBlocks()[0], modules[0])
adapted := adaptSubnet(modules.GetBlocks()[0])
testutil.AssertDefsecEqual(t, test.expected, adapted)
})
}
Expand Down
34 changes: 16 additions & 18 deletions pkg/iac/adapters/terraform/aws/ec2/vpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ func (a *sgAdapter) adaptSecurityGroups(modules terraform.Modules) []ec2.Securit
}
for _, sgRule := range orphanResources {
if sgRule.GetAttribute("type").Equals("ingress") {
orphanage.IngressRules = append(orphanage.IngressRules, adaptSGRule(sgRule, modules))
orphanage.IngressRules = append(orphanage.IngressRules, adaptSGRule(sgRule))
} else if sgRule.GetAttribute("type").Equals("egress") {
orphanage.EgressRules = append(orphanage.EgressRules, adaptSGRule(sgRule, modules))
orphanage.EgressRules = append(orphanage.EgressRules, adaptSGRule(sgRule))
}
}
securityGroups = append(securityGroups, orphanage)
Expand Down Expand Up @@ -116,21 +116,21 @@ func (a *sgAdapter) adaptSecurityGroup(resource *terraform.Block, module terrafo

ingressBlocks := resource.GetBlocks("ingress")
for _, ingressBlock := range ingressBlocks {
ingressRules = append(ingressRules, adaptSGRule(ingressBlock, module))
ingressRules = append(ingressRules, adaptSGRule(ingressBlock))
}

egressBlocks := resource.GetBlocks("egress")
for _, egressBlock := range egressBlocks {
egressRules = append(egressRules, adaptSGRule(egressBlock, module))
egressRules = append(egressRules, adaptSGRule(egressBlock))
}

rulesBlocks := module.GetReferencingResources(resource, "aws_security_group_rule", "security_group_id")
for _, ruleBlock := range rulesBlocks {
a.sgRuleIDs.Resolve(ruleBlock.ID())
if ruleBlock.GetAttribute("type").Equals("ingress") {
ingressRules = append(ingressRules, adaptSGRule(ruleBlock, module))
ingressRules = append(ingressRules, adaptSGRule(ruleBlock))
} else if ruleBlock.GetAttribute("type").Equals("egress") {
egressRules = append(egressRules, adaptSGRule(ruleBlock, module))
egressRules = append(egressRules, adaptSGRule(ruleBlock))
}
}

Expand All @@ -154,24 +154,14 @@ func (a *sgAdapter) adaptSecurityGroup(resource *terraform.Block, module terrafo
}
}

func adaptSGRule(resource *terraform.Block, modules terraform.Modules) ec2.SecurityGroupRule {
func adaptSGRule(resource *terraform.Block) ec2.SecurityGroupRule {
ruleDescAttr := resource.GetAttribute("description")
ruleDescVal := ruleDescAttr.AsStringValueOrDefault("", resource)

var cidrs []iacTypes.StringValue

cidrBlocks := resource.GetAttribute("cidr_blocks")
ipv6cidrBlocks := resource.GetAttribute("ipv6_cidr_blocks")
varBlocks := modules.GetBlocks().OfType("variable")

for _, vb := range varBlocks {
if cidrBlocks.IsNotNil() && cidrBlocks.ReferencesBlock(vb) {
cidrBlocks = vb.GetAttribute("default")
}
if ipv6cidrBlocks.IsNotNil() && ipv6cidrBlocks.ReferencesBlock(vb) {
ipv6cidrBlocks = vb.GetAttribute("default")
}
}

if cidrBlocks.IsNotNil() {
cidrs = cidrBlocks.AsStringValues()
Expand All @@ -185,6 +175,9 @@ func adaptSGRule(resource *terraform.Block, modules terraform.Modules) ec2.Secur
Metadata: resource.GetMetadata(),
Description: ruleDescVal,
CIDRs: cidrs,
FromPort: resource.GetAttribute("from_port").AsIntValueOrDefault(-1, resource),
ToPort: resource.GetAttribute("to_port").AsIntValueOrDefault(-1, resource),
Protocol: resource.GetAttribute("protocol").AsStringValueOrDefault("", resource),
}
}

Expand All @@ -203,6 +196,9 @@ func adaptSingleSGRule(resource *terraform.Block) ec2.SecurityGroupRule {
Metadata: resource.GetMetadata(),
Description: description,
CIDRs: cidrs,
FromPort: resource.GetAttribute("from_port").AsIntValueOrDefault(-1, resource),
ToPort: resource.GetAttribute("to_port").AsIntValueOrDefault(-1, resource),
Protocol: resource.GetAttribute("ip_protocol").AsStringValueOrDefault("", resource),
}
}

Expand Down Expand Up @@ -236,7 +232,7 @@ func adaptNetworkACLRule(resource *terraform.Block) ec2.NetworkACLRule {
actionVal := actionAttr.AsStringValueOrDefault("", resource)

protocolAtrr := resource.GetAttribute("protocol")
protocolVal := protocolAtrr.AsStringValueOrDefault("-1", resource)
protocolVal := protocolAtrr.AsStringValueOrDefault("", resource)

cidrAttr := resource.GetAttribute("cidr_block")
if cidrAttr.IsNotNil() {
Expand All @@ -253,5 +249,7 @@ func adaptNetworkACLRule(resource *terraform.Block) ec2.NetworkACLRule {
Action: actionVal,
Protocol: protocolVal,
CIDRs: cidrs,
FromPort: resource.GetAttribute("from_port").AsIntValueOrDefault(-1, resource),
ToPort: resource.GetAttribute("to_port").AsIntValueOrDefault(-1, resource),
}
}
24 changes: 23 additions & 1 deletion pkg/iac/adapters/terraform/aws/ec2/vpc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ func Test_AdaptVPC(t *testing.T) {
CIDRs: []iacTypes.StringValue{
iacTypes.String("4.5.6.7/32", iacTypes.NewTestMetadata()),
},
FromPort: iacTypes.IntTest(80),
ToPort: iacTypes.IntTest(80),
Protocol: iacTypes.StringTest("tcp"),
},
{
Metadata: iacTypes.NewTestMetadata(),
Expand All @@ -111,6 +114,9 @@ func Test_AdaptVPC(t *testing.T) {
iacTypes.String("1.2.3.4/32", iacTypes.NewTestMetadata()),
iacTypes.String("4.5.6.7/32", iacTypes.NewTestMetadata()),
},
FromPort: iacTypes.IntTest(22),
ToPort: iacTypes.IntTest(22),
Protocol: iacTypes.StringTest("tcp"),
},
},

Expand All @@ -121,6 +127,8 @@ func Test_AdaptVPC(t *testing.T) {
CIDRs: []iacTypes.StringValue{
iacTypes.String("1.2.3.4/32", iacTypes.NewTestMetadata()),
},
FromPort: iacTypes.IntTest(-1),
ToPort: iacTypes.IntTest(-1),
},
},
},
Expand All @@ -137,6 +145,8 @@ func Test_AdaptVPC(t *testing.T) {
CIDRs: []iacTypes.StringValue{
iacTypes.String("10.0.0.0/16", iacTypes.NewTestMetadata()),
},
FromPort: iacTypes.IntTest(22),
ToPort: iacTypes.IntTest(22),
},
},
IsDefaultRule: iacTypes.Bool(false, iacTypes.NewTestMetadata()),
Expand Down Expand Up @@ -169,13 +179,17 @@ func Test_AdaptVPC(t *testing.T) {
{
Metadata: iacTypes.NewTestMetadata(),
Description: iacTypes.String("", iacTypes.NewTestMetadata()),
FromPort: iacTypes.IntTest(-1),
ToPort: iacTypes.IntTest(-1),
},
},

EgressRules: []ec2.SecurityGroupRule{
{
Metadata: iacTypes.NewTestMetadata(),
Description: iacTypes.String("", iacTypes.NewTestMetadata()),
FromPort: iacTypes.IntTest(-1),
ToPort: iacTypes.IntTest(-1),
},
},
},
Expand All @@ -188,7 +202,9 @@ func Test_AdaptVPC(t *testing.T) {
Metadata: iacTypes.NewTestMetadata(),
Type: iacTypes.String("ingress", iacTypes.NewTestMetadata()),
Action: iacTypes.String("", iacTypes.NewTestMetadata()),
Protocol: iacTypes.String("-1", iacTypes.NewTestMetadata()),
Protocol: iacTypes.String("", iacTypes.NewTestMetadata()),
FromPort: iacTypes.IntTest(-1),
ToPort: iacTypes.IntTest(-1),
},
},
IsDefaultRule: iacTypes.Bool(false, iacTypes.NewTestMetadata()),
Expand Down Expand Up @@ -252,13 +268,19 @@ resource "aws_vpc_security_group_ingress_rule" "test" {
CIDRs: []iacTypes.StringValue{
iacTypes.StringTest("0.0.0.0/0"),
},
Protocol: iacTypes.StringTest("tcp"),
FromPort: iacTypes.IntTest(22),
ToPort: iacTypes.IntTest(22),
},
},
EgressRules: []ec2.SecurityGroupRule{
{
CIDRs: []iacTypes.StringValue{
iacTypes.StringTest("0.0.0.0/0"),
},
Protocol: iacTypes.StringTest("-1"),
FromPort: iacTypes.IntTest(-1),
ToPort: iacTypes.IntTest(-1),
},
},
},
Expand Down
5 changes: 5 additions & 0 deletions pkg/iac/providers/aws/ec2/vpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ type SecurityGroupRule struct {
Metadata iacTypes.Metadata
Description iacTypes.StringValue
CIDRs []iacTypes.StringValue
Protocol iacTypes.StringValue
FromPort iacTypes.IntValue
ToPort iacTypes.IntValue
}

type VPC struct {
Expand All @@ -49,4 +52,6 @@ type NetworkACLRule struct {
Action iacTypes.StringValue
Protocol iacTypes.StringValue
CIDRs []iacTypes.StringValue
FromPort iacTypes.IntValue
ToPort iacTypes.IntValue
}
Loading

0 comments on commit 98e136e

Please sign in to comment.