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

added-ssl-option #300

Closed
wants to merge 3 commits into from
Closed

Conversation

cj-belchez
Copy link

1Password SCIM example - ECS|Fargate deployment

Adding an option to add SSL policy to fix insecure TLS issue

The previous version uses old SSL Policy (ELBSecurityPolicy-2016-08)

@scottisloud scottisloud self-assigned this Apr 12, 2024
@scottisloud
Copy link
Collaborator

Hi @cj-belchez and @scott-doyland-burrows
Thanks for the PR, we will review it as time permits. Appreciate your proposed contribution! :)

@renz-canlas
Copy link

Hi @scottisloud, I would like to follow-up on this PR as our security team has flagged it as an issue. Thanks.

@scottisloud
Copy link
Collaborator

scottisloud commented Jul 5, 2024

Hi @renz-canlas thanks for the nudge. I'll bump this up the todo list to hopefully get this reviewed sooner.

In the meantime, since the manifests in this repo are examples that we expect most people will want or need to modify to suit their specific needs, you're free to modify this terraform plan as desired, without necessarily being beholden to our timeline.

We recognize that this change may be one that should be part of the canonical example and look forward to reviewing it and incorporating it soon. (I realize this PR is only a few lines worth of changes, but we do need to be careful about changes, even ones that are trivial on their face, since they potentially impact existing deployments, common variants of a given deployment, or other aspects of the deployment, that we need to thoroughly understand and document if necessary. We appreciate your patience here).

@plttn
Copy link
Member

plttn commented Sep 4, 2024

Hi there @renz-canlas, thanks for your patience on this. Given the state of TLS today, we decided to make this a change to the current recommended policy, rather than a configurable option. As Scott mentioned, these are examples, and it's very possible to modify it to fit your needs. In any case, thanks for flagging!

@plttn plttn closed this Sep 4, 2024
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

5 participants