diff --git a/doc/_resource_types/security_group.md b/doc/_resource_types/security_group.md index c3a74f6c8..a3803c443 100644 --- a/doc/_resource_types/security_group.md +++ b/doc/_resource_types/security_group.md @@ -19,13 +19,30 @@ end ```ruby describe security_group('my-security-group-name') do its(:outbound) { should be_opened } + its(:outbound) { should be_opened(443).protocol('tcp').target('203.0.113.1/32') } its(:inbound) { should be_opened(80) } its(:inbound) { should be_opened(80).protocol('tcp').for('203.0.113.1/32') } its(:inbound) { should be_opened(22).protocol('tcp').for('sg-5a6b7cd8') } end ``` -### advanced +#### Defaults for `:inbound` and `:outbound` + +Beware for defaults when not specifying protocol (`protocol()`) and/or the CIDR +(`for()` or `target()`). + +If no protocol is provided, it is assumed to be `-1`. +If no CIDR is provided, it is assumed to be `0.0.0.0/0`. + +Of course, if your current security group being tested does not include those +defaults your spec will fail to match with `should be_opened`. + +In most cases, you will want to at least provide the port and protocol. + +If no port number is provided, the `MissingPortSpecification` exception will be +raised. + +### Advanced `security_group` can use `Aws::EC2::SecurityGroup` resource (see http://docs.aws.amazon.com/sdkforruby/api/Aws/EC2/SecurityGroup.html). diff --git a/doc/resource_types.md b/doc/resource_types.md index d0bd2ba98..31e5f1774 100644 --- a/doc/resource_types.md +++ b/doc/resource_types.md @@ -3547,32 +3547,31 @@ end ```ruby describe security_group('my-security-group-name') do its(:outbound) { should be_opened } + its(:outbound) { should be_opened(443).protocol('tcp').target('203.0.113.1/32') } its(:inbound) { should be_opened(80) } its(:inbound) { should be_opened(80).protocol('tcp').for('203.0.113.1/32') } its(:inbound) { should be_opened(22).protocol('tcp').for('sg-5a6b7cd8') } end ``` +#### Defaults for `:inbound` and `:outbound` -### its(:inbound_rule_count), its(:outbound_rule_count), its(:inbound_permissions_count), its(:outbound_permissions_count), its(:description), its(:group_name), its(:owner_id), its(:group_id), its(:vpc_id) -### :unlock: Advanced use +Beware for defaults when not specifying protocol (`protocol()`) and/or the CIDR +(`for()` or `target()`). -`security_group` can use `Aws::EC2::SecurityGroup` resource (see http://docs.aws.amazon.com/sdkforruby/api/Aws/EC2/SecurityGroup.html). +If no protocol is provided, it is assumed to be `-1`. +If no CIDR is provided, it is assumed to be `0.0.0.0/0`. -```ruby -describe security_group('my-security-group-name') do - its('group_name') { should eq 'my-security-group-name' } -end -``` +Of course, if your current security group being tested does not include those +defaults your spec will fail to match with `should be_opened`. -or +In most cases, you will want to at least provide the port and protocol. + +If no port number is provided, the `MissingPortSpecification` exception will be +raised. -```ruby -describe security_group('my-security-group-name') do - its('resource.group_name') { should eq 'my-security-group-name' } -end -``` +### its(:inbound_rule_count), its(:outbound_rule_count), its(:inbound_permissions_count), its(:outbound_permissions_count), its(:description), its(:group_name), its(:owner_id), its(:group_id), its(:vpc_id) ## ses_identity SesIdentity resource type. diff --git a/lib/awspec/error.rb b/lib/awspec/error.rb index 0187ef2da..80d66fea5 100644 --- a/lib/awspec/error.rb +++ b/lib/awspec/error.rb @@ -33,4 +33,18 @@ def initialize(resource_type, id) # Let the user know the configuration they provided is not known. class UnknownConfiguration < StandardError end + + class MissingPortSpecification < StandardError + def initialize(default_protocol) + msg = "Port must be specificed unless protocol is #{default_protocol}" + super msg + end + end + + class InvalidPortRange < StandardError + def initialize(port, from_port, to_port) + msg = "Found a port range from #{from_port} to #{to_port}, but cannot compare it with '#{port}'" + super msg + end + end end diff --git a/lib/awspec/matcher/be_opened.rb b/lib/awspec/matcher/be_opened.rb index b16a079c1..7d37e0f99 100644 --- a/lib/awspec/matcher/be_opened.rb +++ b/lib/awspec/matcher/be_opened.rb @@ -1,10 +1,19 @@ # frozen_string_literal: true +DEFAULT_PROTOCOL = '-1' +DEFAULT_ROUTE = '0.0.0.0/0' + RSpec::Matchers.define :be_opened do |port| match do |sg| sg.opened?(port, @protocol, @cidr) end + description do + @protocol = DEFAULT_PROTOCOL if @protocol.nil? + @cidr = DEFAULT_ROUTE if @cidr.nil? + "to be opened on port #{port}, protocol \"#{@protocol}\", to/from #{@cidr}" + end + chain :protocol do |protocol| @protocol = protocol end diff --git a/lib/awspec/stub/security_group.rb b/lib/awspec/stub/security_group.rb index 66e1d4f81..decb03226 100644 --- a/lib/awspec/stub/security_group.rb +++ b/lib/awspec/stub/security_group.rb @@ -114,6 +114,16 @@ } ] }, + { + from_port: 443, + to_port: 443, + ip_protocol: 'tcp', + ip_ranges: [ + { + cidr_ip: '0.0.0.0/0' + } + ] + }, { from_port: 8080, to_port: 8080, diff --git a/lib/awspec/type/security_group.rb b/lib/awspec/type/security_group.rb index 6d36551f0..452c91ed2 100644 --- a/lib/awspec/type/security_group.rb +++ b/lib/awspec/type/security_group.rb @@ -6,6 +6,9 @@ class SecurityGroup < ResourceBase aws_resource Aws::EC2::SecurityGroup tags_allowed + DEFAULT_PROTOCOL = '-1' + DEFAULT_ROUTE = '0.0.0.0/0' + def resource_via_client @resource_via_client ||= find_security_group(@display_name) end @@ -26,10 +29,18 @@ def opened_only?(port = nil, protocol = nil, cidr = nil) outbound_opened_only?(port, protocol, cidr) end - def inbound_opened?(port = nil, protocol = nil, cidr = nil) + def inbound_opened?(port, protocol, cidr = DEFAULT_ROUTE) + # https://stackoverflow.com/questions/39021545/how-to-specify-all-ports-in-security-group-cloudformation + raise Awspec::MissingPortSpecification, DEFAULT_PROTOCOL if port.nil? && protocol != DEFAULT_PROTOCOL + # a security group will accept everything from itself + return true if cidr == id + resource_via_client.ip_permissions.find do |permission| - cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?(permission, port) + return true if cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?( + permission, port) end + + false end def inbound_opened_only?(port = nil, protocol = nil, cidr = nil) @@ -44,6 +55,12 @@ def inbound_opened_only?(port = nil, protocol = nil, cidr = nil) end def outbound_opened?(port = nil, protocol = nil, cidr = nil) + # https://stackoverflow.com/questions/39021545/how-to-specify-all-ports-in-security-group-cloudformation + raise Awspec::MissingPortSpecification, DEFAULT_PROTOCOL if port.nil? && protocol != DEFAULT_PROTOCOL + + # a security group will accept everything from itself + return true if cidr == id + resource_via_client.ip_permissions_egress.find do |permission| cidr_opened?(permission, cidr) && protocol_opened?(permission, protocol) && port_opened?(permission, port) end @@ -107,7 +124,7 @@ def outbound_rule_count private def cidr_opened?(permission, cidr) - return true unless cidr + cidr = DEFAULT_ROUTE if cidr.nil? ret = permission.prefix_list_ids.select do |prefix_list_id| prefix_list_id.prefix_list_id == cidr @@ -145,7 +162,6 @@ def cidr_opened?(permission, cidr) end def protocol_opened?(permission, protocol) - return true unless protocol return false if protocol == 'all' && permission.ip_protocol != '-1' return true if permission.ip_protocol == '-1' @@ -153,7 +169,6 @@ def protocol_opened?(permission, protocol) end def port_opened?(permission, port) - return true unless port return true unless permission.from_port return true unless permission.to_port @@ -161,9 +176,13 @@ def port_opened?(permission, port) end def port_between?(port, from_port, to_port) - if port.is_a?(String) && port.include?('-') + if port.is_a?(String) && port.include?('-') && port =~ /^\d+-\d+$/ f, t = port.split('-') from_port == f.to_i && to_port == t.to_i + elsif port.is_a?(String) + raise Awspec::InvalidPortRange.new(port, from_port, to_port) unless port =~ /^\d+$/ + converted = port.to_i + converted.between?(from_port, to_port) else port.between?(from_port, to_port) end diff --git a/spec/generator/spec/security_group_spec.rb b/spec/generator/spec/security_group_spec.rb index 419da0895..bffe22ecf 100644 --- a/spec/generator/spec/security_group_spec.rb +++ b/spec/generator/spec/security_group_spec.rb @@ -22,11 +22,12 @@ its(:inbound) { should be_opened('50000-50009').protocol('tcp').for('123.45.67.89/32') } its(:inbound) { should be_opened.protocol('all').for('sg-3a4b5cd6') } its(:outbound) { should be_opened(50000).protocol('tcp').for('100.45.67.12/32') } + its(:outbound) { should be_opened(443).protocol('tcp').for('0.0.0.0/0') } its(:outbound) { should be_opened(8080).protocol('tcp').for('group-in-other-aws-account-with-vpc-peering') } its(:inbound_rule_count) { should eq 8 } - its(:outbound_rule_count) { should eq 2 } + its(:outbound_rule_count) { should eq 3 } its(:inbound_permissions_count) { should eq 7 } - its(:outbound_permissions_count) { should eq 3 } + its(:outbound_permissions_count) { should eq 4 } it { should belong_to_vpc('my-vpc') } end EOF diff --git a/spec/type/security_group_spec.rb b/spec/type/security_group_spec.rb index 866960a81..549ca7840 100644 --- a/spec/type/security_group_spec.rb +++ b/spec/type/security_group_spec.rb @@ -1,33 +1,67 @@ # frozen_string_literal: true require 'spec_helper' +require 'awspec/error' + Awspec::Stub.load 'security_group' describe security_group('sg-1a2b3cd4') do it { should exist } - its(:inbound) { should be_opened(80) } + it do + # only way found to force and test the exception + subject.inbound + expect { subject.opened?(nil) }.to raise_error(Awspec::MissingPortSpecification, /protocol\sis\s-1$/) + end + its(:inbound) { should_not be_opened(80) } + its(:inbound) { should_not be_opened(80).protocol(-1) } + its(:inbound) { should be_opened(80).protocol(-1).for('sg-3a4b5cd6') } its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.68.89/32') } its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.67.0/25') } its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.67.1/32') } its(:inbound) { should_not be_opened(80).protocol('tcp').for('123.45.0.0/16') } - its(:inbound) { should be_opened(22) } + + its(:inbound) { should be_opened(22).protocol('tcp').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('udp').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('-1').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('icmp').for('sg-1a2b3cd4') } + its(:inbound) { should be_opened(22).protocol('tcp').for('sg-5a6b7cd8') } its(:inbound) { should be_opened('50000-50009').protocol('tcp').for('123.45.67.89/32') } its(:inbound) { should_not be_opened('50010-50019').protocol('tcp').for('123.45.67.89/32') } - its(:outbound) { should be_opened(50_000) } + + its(:outbound) { should be_opened(50_000).protocol('tcp').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(50_000).protocol('udp').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(50_000).protocol('icmp').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(50_000).protocol('-1').for('sg-1a2b3cd4') } + its(:outbound) { should be_opened(8080).protocol('tcp').for('sg-9a8b7c6d') } its(:outbound) { should be_opened(8080).protocol('tcp').for('group-in-other-aws-account-with-vpc-peering') } its(:inbound) { should be_opened_only(60_000).protocol('tcp').for('100.45.67.12/32') } its(:inbound) { should be_opened_only(70_000).protocol('tcp').for(['100.45.67.89/32', '100.45.67.12/32']) } its(:outbound) { should be_opened_only(50_000).protocol('tcp').for('100.45.67.12/32') } - its(:inbound) { should be_opened.protocol('all').for('sg-3a4b5cd6') } - its(:outbound) { should be_opened.protocol('tcp').for('pl-a5321fa3') } + its(:inbound) { should be_opened.protocol('-1').for('sg-3a4b5cd6') } + + it do + # only way found to force and test the exception + subject.outbound + expect { subject.opened?(nil, 'tcp', 'pl-a5321fa3') }.to raise_error( + Awspec::MissingPortSpecification, /protocol\sis\s-1$/) + end + its(:outbound) { should be_opened('443').protocol('tcp').for('pl-a5321fa3') } + its(:outbound) { should be_opened(443).protocol('tcp').for('pl-a5321fa3') } + its(:outbound) { should be_opened('443-443').protocol('tcp').for('pl-a5321fa3') } + it do + # only way found to force and test the exception + subject.outbound + expect { subject.opened?('yada-443-yada', 'tcp', 'pl-a5321fa3') }.to raise_error(Awspec::InvalidPortRange) + end + its(:inbound_permissions_count) { should eq 7 } its(:ip_permissions_count) { should eq 7 } - its(:outbound_permissions_count) { should eq 3 } - its(:ip_permissions_egress_count) { should eq 3 } + its(:outbound_permissions_count) { should eq 4 } + its(:ip_permissions_egress_count) { should eq 4 } its(:inbound_rule_count) { should eq 8 } - its(:outbound_rule_count) { should eq 2 } + its(:outbound_rule_count) { should eq 3 } it { should belong_to_vpc('vpc-ab123cde') } it { should belong_to_vpc('my-vpc') } @@ -96,14 +130,15 @@ describe security_group('my-security-group-name') do it { should exist } - its(:outbound) { should be_opened(50_000) } - its(:inbound) { should be_opened(80) } + its(:outbound) { should_not be_opened(50_000) } + its(:outbound) { should_not be_opened(50_000).protocol('tcp') } + its(:outbound) { should be_opened(50_000).protocol('tcp').target('100.45.67.12/32') } + its(:outbound) { should be_opened(443).protocol('tcp').target('0.0.0.0/0') } + its(:outbound) { should be_opened(443).protocol('tcp') } + its(:inbound) { should_not be_opened(80) } + its(:inbound) { should_not be_opened(80).protocol('tcp') } + its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.67.0/24') } + its(:inbound) { should be_opened(80).protocol('tcp').for('123.45.68.89/32') } it { should belong_to_vpc('my-vpc') } it { should have_tag('env').value('dev') } end - -describe security_group('my-security-tag-name') do - its(:outbound) { should be_opened(50_000) } - its(:inbound) { should be_opened(80) } - it { should belong_to_vpc('my-vpc') } -end