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

Draft for checking listener ssl policy on ALB #100

Closed
wants to merge 6 commits into from

Conversation

takahashi9854
Copy link

I confirm these files are made available under CC0 1.0 Universal (https://creativecommons.org/publicdomain/zero/1.0/legalcode)

Issue #, if available:

Description of changes:

python/alb_ssl_policy.py Show resolved Hide resolved
listeners = alb_client.describe_listeners( LoadBalancerArns = [alb_arn,], )

for l in listeners['Listeners']:
if ( 'SslPolicy' in l.keys()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

external parenthesis can be removed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed parenthesis.

alb_arn = configuration_item['arn']
alb_client = get_client('elbv2',event);

listeners = alb_client.describe_listeners( LoadBalancerArns = [alb_arn,], )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove comma at the end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed comma.

###############################

alb_arn = configuration_item['arn']
alb_client = get_client('elbv2',event);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove ; at the end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space between comma and variable

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed comma and add space between comma and variable.


for l in listeners['Listeners']:
if ( 'SslPolicy' in l.keys()):
if(l['SslPolicy'] not in COMPLIANT_POLICY):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be

if l['SslPolicy'] not in validated_rule_parameters['ValidPolicies']:

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modified.

Feature:
In order to: apply ssl policy to listers
As: a Security Officer
I want: to chaging configuration of applicatio loac balancer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

several typo in this sections

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected.

Then: Return COMPLIANT
Scenario 2:
Given: SslPolicy is not in COMPLIANT_POLICY
THEN: RETURN NON_COMPLIANT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return NON_COMPLAINT

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected.

As: a Security Officer
I want: to chaging configuration of applicatio loac balancer.
Scenarios:
Scenario 1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a scenario if no CompliantPolicies


listeners = alb_client.describe_listeners( LoadBalancerArns = [alb_arn,], )

for l in listeners['Listeners']:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if no listeners?

python/alb_ssl_policy.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jongogogo jongogogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change the name of the parameter

Then: Return NON_COMPLIANT
Scenario 3:
Given: CompliantPolicies has not been defined.
Then: Return NOT_APPLICABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this should be COMPLIANT is only you have SSL enabled.

Given: CompliantPolicies has not been defined.
Then: Return NOT_APPLICABLE
Scenario 4:
Given: Listner has not been used in a application load balancer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return NON_COMPLIANT

@@ -48,6 +55,8 @@
DEFAULT_RESOURCE_TYPE = ['AWS::LoadBalancingV2::LoadBalancer']
# Pre defined policy name
COMPLIANT_POLICY = ['ELBSecurityPolicy-2016-08','ELBSecurityPolicy-FS-2018-06']
# for Assume Role parametor
ASSUME_ROLE_MODE = ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to be

False

@@ -48,6 +55,8 @@
DEFAULT_RESOURCE_TYPE = ['AWS::LoadBalancingV2::LoadBalancer']
# Pre defined policy name
COMPLIANT_POLICY = ['ELBSecurityPolicy-2016-08','ELBSecurityPolicy-FS-2018-06']
# for Assume Role parametor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

@@ -48,6 +55,8 @@
DEFAULT_RESOURCE_TYPE = ['AWS::LoadBalancingV2::LoadBalancer']
# Pre defined policy name
COMPLIANT_POLICY = ['ELBSecurityPolicy-2016-08','ELBSecurityPolicy-FS-2018-06']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this line

@@ -78,14 +87,20 @@ def evaluate_compliance(event, configuration_item, valid_rule_parameters):
###############################

alb_arn = configuration_item['arn']
alb_client = get_client('elbv2',event);
alb_client = get_client('elbv2' , event)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space only after the comma

if ( 'SslPolicy' in l.keys()):
if(l['SslPolicy'] not in COMPLIANT_POLICY):
return 'NON_COMPLIANT'
if len(COMPLIANT_POLICY) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is a parameter, change the function evaluate_parameter()

Copy link
Contributor

@jongogogo jongogogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting better, keep iterating. Review the gherkin first, please send it to me via chime

I want: to chase configuration of listener ssl policy of an application load balancer.
Scenarios:
Scenario 1:
Given: SslPolicy is in COMPLIANT_POLICY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update the entire gherkin with the new name of the parameter

| ---------------------- | --------- | -------------------------------------------------------------------------------------- |
| Parameter Name | Type | Description |
| ---------------------- | --------- | -------------------------------------------------------------------------------------- |
| ValidPolicies | Constant | The list of ssl policy name which check whether lister configred with comma separated |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type is "Optional"

Given: CompliantPolicies has not been defined.
Then: Return NON_COMPLIANT
Scenario 4:
Given: Listner has not been used in a application load balancer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo

Then: Return COMPLIANT
Scenario 3:
Given: CompliantPolicies has not been defined.
Then: Return NON_COMPLIANT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This need to return compliant. if and only if, there is a SSL listener. The goal is to ensure that SSL is enabled on the ALB.

for l in listeners['Listeners']:
if 'SslPolicy' in l.keys():
if l['SslPolicy'] not in valid_rule_parameters['ValidPolicies']:
return 'NON_COMPLIANT'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use build_evaluation_from_config_item() to add an annotation to the NON_COMPLIANT to explain why to the customer


# for Assume Role parameter
ASSUME_ROLE_MODE = False
DEFAULT_RESOURCE_TYPE = ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEFAULT_RESOURCE_TYPE = 'AWS::LoadBalancingV2::LoadBalancer'

Copy link
Contributor

@jongogogo jongogogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few changes to do, almost there!

In order to: apply ssl policy to listers
As: a Security Officer
I want: to chase configuration of listener ssl policy of an application load balancer.
Scenarios:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add carriage return before

| ---------------------- | --------- | -------------------------------------------------------------------------------------- |

Feature:
In order to: apply ssl policy to listers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to: ensure that the traffic is encrypted in transit with the proper method

Feature:
In order to: apply ssl policy to listers
As: a Security Officer
I want: to chase configuration of listener ssl policy of an application load balancer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want: to verify that the configuration of listener ssl policy of an application load balancer is correct.

return valid_rule_parameters

param_list = rule_parameters['ValidPolicies']
param_list = param_list.split(',')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge this line with 138.

    param_list = rule_parameters['ValidPolicies'].split(',')

#####################################
Rule Name:
alb_ssl_policy_check
Description:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add carriage return before

alb_ssl_policy_check
Description:
Checks that listener of Application Load Balancer has configured applicable SSL policy.
Trigger:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add carriage return before

Checks that listener of Application Load Balancer has configured applicable SSL policy.
Trigger:
Configuration change on AWS::LoadBalancingV2::LoadBalancer.
Resource Type to report on:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add carriage return before

Configuration change on AWS::LoadBalancingV2::LoadBalancer.
Resource Type to report on:
AWS::LoadBalancingV2::LoadBalancer
Rule Parameters:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add carriage return before

@takahashi9854 takahashi9854 deleted the alb_ssl_policy branch September 13, 2018 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants