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

Security Group Ingress failure for tcp/udp/icmp #2999

Closed
1 of 2 tasks
kddejong opened this issue Jan 5, 2024 · 4 comments
Closed
1 of 2 tasks

Security Group Ingress failure for tcp/udp/icmp #2999

kddejong opened this issue Jan 5, 2024 · 4 comments
Labels
new rule New rule v1 v1.X

Comments

@kddejong
Copy link
Contributor

kddejong commented Jan 5, 2024

Is this feature request related to a new rule or cfn-lint capabilities?

rules

Describe the feature you'd like to request

When deploying the provided template you will get the errorInvalid value for portRange. Must specify both from and to ports with ICMP

Describe the solution you'd like

When using tcp, udp, and icmp a port range must be specified.

Additional context

Resources:
  IngressRule:
    Type: AWS::EC2::SecurityGroup
    Properties:
      GroupDescription: 'Security Group Vpc'
      VpcId: "vpc-redacted"
      SecurityGroupIngress:
      -
        IpProtocol: 1
        SourceSecurityGroupName: default

Docs: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-securitygroup-ingress.html#cfn-ec2-securitygroup-ingress-ipprotocol

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request

Would this feature include a breaking change?

  • ⚠️ This feature might incur a breaking change
@kddejong kddejong added the new rule New rule label Jan 5, 2024
@kddejong
Copy link
Contributor Author

IpProtocol of 1, icmp, icmpv6 can use FromPort to -1 and ToPort to -1 if one of the values is -1 the other one has to be -1

@kddejong
Copy link
Contributor Author

In this example to and from ports are ignored. This should be warning. The same holds for IpProtocol when it isn't icmp,icmpv6,tcp,udp

Resources:
  SG1:
    Type: AWS::EC2::SecurityGroup
    Properties:
      GroupDescription: "some_group_desc"
      VpcId: vpc-0a3447fff60767d73
      SecurityGroupIngress:
        - IpProtocol: -1
          CidrIp: 10.0.0.0/8
          FromPort: 1
          ToPort: 65535

@vschurink
Copy link

This addition is causing issues in our automated formatting. IF you only want to allow ICMP echo to a loadbalancer for example, you have to use the FromPort: 8 > Toport: -1, otherwise it does not work. This is also still what official AWS documentation says to enable ICMP.

With this addition an ERROR is always triggered due to the fact that ToPort = -1, and this added rule says that FromPort also should be 8. But I do not want to allow ALL protocols, just ICMP echo.

This should be fixed imho.

@kddejong
Copy link
Contributor Author

kddejong commented Jul 9, 2024

This PR should fix this. It will allow ICMP FromPort to be something other than -1 when ToPort is -1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new rule New rule v1 v1.X
Projects
None yet
Development

No branches or pull requests

2 participants