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

feat: Security Policy for Listener in Application load balancer #4099

Merged
merged 9 commits into from Oct 22, 2022

Conversation

KollaAdithya
Copy link
Contributor

@KollaAdithya KollaAdithya commented Oct 20, 2022

This PR resolves #1342
AWS Load Balancer listeners can restrict the tls protocol versions clients use to connect through an sslpolicy. The default policy allows for insecure protocol versions for compatibility.
For those with stricter security goals it would be great to expose the ability to only accept current tls protocol versions.

This security policy for a HTTPS listener helps to negotiate SSL connections between a client and the load balancer.

For Example if Environment manifest contains,

In case of Public Application load balancer

http:
  public: 
    certificates:
      - arn:aws:acm: ${AWS_REGION}:${AWS_ACCOUNT_ID}:certificate/13245665-cv8f-adf3-j7gd-adf876af95
    ssl_policy: ELBSecurityPolicy-FS-1-1-2019-08

In case of Internal load balancer

http:
 private: 
    certificates:
      - arn:aws:acm: ${AWS_REGION}:${AWS_ACCOUNT_ID}:certificate/13245665-cv8f-adf3-j7gd-adf876af95
    ssl_policy: ELBSecurityPolicy-FS-1-1-2019-08

By Default ELBSecurityPolicy-2016-08 will be set for a https listener in a public application load balancer and internal load balancer. You can access the available security policies at https://docs.aws.amazon.com/elasticloadbalancing/latest/application/create-https-listener.html#describe-ssl-policies

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

@KollaAdithya KollaAdithya requested a review from a team as a code owner October 20, 2022 17:29
@KollaAdithya KollaAdithya requested review from efekarakus and removed request for a team October 20, 2022 17:29
@KollaAdithya KollaAdithya changed the title Security Policy for Listener feat: Security Policy for Listener in Application load balancer Oct 20, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2022

Codecov Report

Merging #4099 (012a0e4) into mainline (112d9af) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           mainline    #4099   +/-   ##
=========================================
  Coverage     69.13%   69.14%           
=========================================
  Files           249      249           
  Lines         35656    35662    +6     
  Branches        264      264           
=========================================
+ Hits          24652    24658    +6     
  Misses         9808     9808           
  Partials       1196     1196           
Impacted Files Coverage Δ
internal/pkg/template/env.go 49.23% <ø> (ø)
internal/pkg/deploy/cloudformation/stack/env.go 72.52% <100.00%> (+0.47%) ⬆️
internal/pkg/manifest/env.go 77.77% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@paragbhingre paragbhingre left a comment

Choose a reason for hiding this comment

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

Overall looks great, just few nits.

KollaAdithya and others added 3 commits October 21, 2022 12:51
This PR fix multiple problems -
1. This PR fix the env manifest fields that were organized in incorrect manner.
2. Also resolves aws#4032
3. `copilot env init` with private subnets create env manifest with following field
```
http:
  private:
```
This should only be created when there is at least one parameter inside private is enabled. This PR fix this nit as well.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
Fixes second issue in aws#3553.

Here strings (`<<<`) don't work with `sh`. This change makes the pipeline buildspec a bit more universal.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
@huanjani huanjani added the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Oct 21, 2022
Copy link
Contributor

@huanjani huanjani left a comment

Choose a reason for hiding this comment

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

Looks good! 😻 Just a few changes, please, then feel free to remove the do-not-merge label! Thank you!

@KollaAdithya KollaAdithya removed the do-not-merge Pull requests that mergify shouldn't merge until the requester allows it. label Oct 22, 2022
@mergify mergify bot merged commit d7563e3 into aws:mainline Oct 22, 2022
Sprint 🏃‍♀️ automation moved this from In review to Pending release Oct 22, 2022
@KollaAdithya KollaAdithya deleted the sslpolicy branch February 7, 2023 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Sprint 🏃‍♀️
  
Pending release
Development

Successfully merging this pull request may close these issues.

Support listener ssl policy
5 participants