search

aws-cloudformation / cfn-lint

CloudFormation Linter
MIT No Attribution
2.4k stars 576 forks source link

v1: possible false-positive for `W3687` #3317

Closed r-heimann closed 2 weeks ago

r-heimann commented 2 weeks ago

CloudFormation Lint Version

cfn-lint 1.3.0

What operating system are you using?

Windows 11

Describe the bug

After updating from v0.87.7 to v1.3.0 i noticed a new finding, which is possibly a false-positive:

  SecurityGroup:
    Type: AWS::EC2::SecurityGroup
    Properties:
      GroupDescription: "Security Group"
      VpcId: !ImportValue VPC1-VPC-ID
      SecurityGroupIngress:
        - Description: "Test"
          IpProtocol: TCP
          FromPort: 443
          ToPort: 443
          CidrIp: 0.0.0.0/0
[cfn-lint] W3687: ['FromPort', 'ToPort'] are ignored when using 'IpProtocol' value 'TCP'

Expected behavior

Not entirely sure what the issue here is, but FromPort and ToPort are not ignored:

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

Reproduction template

AWSTemplateFormatVersion: 2010-09-09
Description: Test

Resources:
  SecurityGroup:
    Type: AWS::EC2::SecurityGroup
    Properties:
      GroupDescription: "Security Group"
      VpcId: !ImportValue VPC1-VPC-ID
      SecurityGroupIngress:
        - Description: "Test"
          IpProtocol: TCP
          FromPort: 443
          ToPort: 443
          CidrIp: 0.0.0.0/0
egut commented 2 weeks ago

TCP should be in lowercase

IpProtocol The IP protocol name (tcp, udp, icmp, icmpv6) or number (see Protocol Numbers).

Use -1 to specify all protocols. When authorizing security group rules, specifying -1 or a protocol number other than tcp, udp, icmp, or icmpv6 allows traffic on all ports, regardless of any port range you specify. For tcp, udp, and icmp, you must specify a port range. For icmpv6, the port range is optional; if you omit the port range, traffic for all types and codes is allowed.

But AWS accept uppercase

r-heimann commented 2 weeks ago

TCP should be in lowercase

IpProtocol The IP protocol name (tcp, udp, icmp, icmpv6) or number (see Protocol Numbers). Use -1 to specify all protocols. When authorizing security group rules, specifying -1 or a protocol number other than tcp, udp, icmp, or icmpv6 allows traffic on all ports, regardless of any port range you specify. For tcp, udp, and icmp, you must specify a port range. For icmpv6, the port range is optional; if you omit the port range, traffic for all types and codes is allowed.

But AWS accept uppercase

Thank you, you are right. cfn-lint v1 doesn't like my uppercase TCP.

egut commented 2 weeks ago

@r-heimann Yes, I notice the same in one of our templates but not all so I got confused first, then I saw it :)