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

Validate aws_security_group for wrong ICMP codes #1175

Closed
stefano-m opened this issue Jul 18, 2017 · 5 comments
Closed

Validate aws_security_group for wrong ICMP codes #1175

stefano-m opened this issue Jul 18, 2017 · 5 comments
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service. stale Old or inactive issues managed by automation, if no further action taken these will get closed.

Comments

@stefano-m
Copy link

stefano-m commented Jul 18, 2017

EDITED: see #1175 (comment)

For details about ICMP types and codes, see https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml

Terraform Version

v0.9.11

Affected Resource(s)

  • aws_security_group

Terraform Configuration Files

resource "aws_security_group" "example" {
  name = "example"
  description = "Example"

  ingress {
    protocol = "tcp"
    from_port = 22
    to_port = 22
    cidr_blocks = ["0.0.0.0/0"]
  }

  ingress {
    protocol = "icmp"
    from_port = 8 # ICMP type: echo request
    to_port = 8  # this a wrong ICMP code, it should be 0
    cidr_blocks = ["0.0.0.0/0"]
  }

  egress {
      from_port = 0
      to_port = 0
      protocol = "-1"
      cidr_blocks = ["0.0.0.0/0"]
  }

  tags {
    Name = "Example"
    Group = "Example Group"
  }
}

Debug Output

For security reasons I cannot provide the entire debug output. Below is what I think is relevant for this issue.

-----------------------------------------------------
aws-provider (internal) 2017/07/18 14:19:17 [DEBUG] [aws-sdk-go] <?xml version="1.0" encoding="UTF-8"?>
<DescribeSecurityGroupsResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/">

    <securityGroupInfo>
        <item>
            <groupName>example</groupName>
            <groupDescription>Example</groupDescription>
            <vpcId>*REDACTED*</vpcId>
            <ipPermissions>
                <item>
                    <ipProtocol>icmp</ipProtocol>
                    <fromPort>8</fromPort>
                    <toPort>8</toPort>
                    <groups/>
                    <ipRanges>
                        <item>
                            <cidrIp>0.0.0.0/0</cidrIp>
                        </item>
                    </ipRanges>
                    <ipv6Ranges/>
                    <prefixListIds/>
                </item>
                <item>
                    <ipProtocol>tcp</ipProtocol>
                    <fromPort>22</fromPort>
                    <toPort>22</toPort>
                    <groups/>
                    <ipRanges>
                        <item>
                            <cidrIp>0.0.0.0/0</cidrIp>
                        </item>
                    </ipRanges>
                    <ipv6Ranges/>
                    <prefixListIds/>
                </item>
            </ipPermissions>

Panic Output

None

Expected Behavior

Terraform rejects the input because the ICMP code is not valid

Actual Behavior

The port range in the security group rule in the AWS console shows 8 (i.e. the ICMP code defined in the configuration)

Custom ICMP Rule - IPv4 | Echo Request | 8 | 0.0.0.0/0

and the EC2 instance connected to this security group is not pingable.

Steps to Reproduce

  1. terraform plan
  2. terraform apply

Important Factoids

I am running this using the latest AWS technology.

References

I did search for related issues but could not find any really relevant. This may be related to #222 though.

@stefano-m
Copy link
Author

I have just realized that I misread the documentation and to_port should be the ICMP code[1].

Using

from_port = 8
to_port = 0

indeed sets up the rule correctly.

One could still argue whether the ICMP code should be validated and the input rejected in case a wrong code is used.

[1] https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml

@stefano-m stefano-m changed the title aws_security_group with ICMP protocol sets "port range" to ICMP code rather than to N/A Validate aws_security_group for wrong ICMP codes Jul 25, 2017
@catsby catsby added the bug Addresses a defect in current functionality. label Jul 25, 2017
@bodgit
Copy link
Contributor

bodgit commented Aug 30, 2017

This seems partly related to #1487 as it turns out that in at least some regions, AWS will accept both 8/0 and 8/-1 for ICMP echo request but internally will always store it as 8/-1 as there are no other valid codes other than 0 for type 8.

It would presumably require Terraform to maintain some sort of map of all of the possible ICMP type/code combinations which I'm not sure is its job.

@stefano-m
Copy link
Author

As far as this issue is concerned, I'd say that invalid ICMP codes should be rejected by Terraform as they are specified by a known standard.

The AWS provider should then be responsible to ensure that the correct code is translated to whatever AWS uses.

For example, let's consider Echo where 0 is the only valid code in the spec, the expected behavior should be:'

  • Invalid ICMP code: to_port = 8 -> refused
  • Invalid ICMP code (but valid value for AWS): to_port = -1 -> refused
  • Valid ICMP code (and also valid for AWS): to_port = 0 -> accepted (and potentially translated to -1 in the API call to AWS, this is an implementation detail)

@bflad bflad added the service/ec2 Issues and PRs that pertain to the ec2 service. label Jan 28, 2018
@github-actions
Copy link

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

@github-actions github-actions bot added the stale Old or inactive issues managed by automation, if no further action taken these will get closed. label Mar 31, 2020
@github-actions github-actions bot closed this as completed May 1, 2020
@ghost
Copy link

ghost commented Jun 2, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Jun 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/ec2 Issues and PRs that pertain to the ec2 service. stale Old or inactive issues managed by automation, if no further action taken these will get closed.
Projects
None yet
Development

No branches or pull requests

4 participants